mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] drm/amd/display: fix PSR-SU/DSC interoperability support
@ 2023-01-05 17:38 Hamza Mahfooz
  2023-01-05 18:29 ` Harry Wentland
  0 siblings, 1 reply; 5+ messages in thread
From: Hamza Mahfooz @ 2023-01-05 17:38 UTC (permalink / raw)
  To: amd-gfx
  Cc: Hamza Mahfooz, Harry Wentland, Leo Li, Rodrigo Siqueira,
	Alex Deucher, Christian König, Pan, Xinhui, David Airlie,
	Daniel Vetter, David Zhang, Nicholas Kazlauskas, Alex Hung,
	Po Ting Chen, Shirish S, Robin Chen, Brian Chang, Camille Cho,
	Tom Chung, dri-devel, linux-kernel

Currently, there are issues with enabling PSR-SU + DSC. This stems from
the fact that DSC imposes a slice height on transmitted video data and
we are not conforming to that slice height in PSR-SU regions. So, pass
slice_height into su_y_granularity to feed the DSC slice height into
PSR-SU code.

Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
---
v2: move code to modules/power.
---
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c |  3 ++
 .../amd/display/modules/power/power_helpers.c | 35 +++++++++++++++++++
 .../amd/display/modules/power/power_helpers.h |  3 ++
 3 files changed, 41 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
index 26291db0a3cf..872d06fe1436 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
@@ -122,6 +122,9 @@ bool amdgpu_dm_link_setup_psr(struct dc_stream_state *stream)
 		psr_config.allow_multi_disp_optimizations =
 			(amdgpu_dc_feature_mask & DC_PSR_ALLOW_MULTI_DISP_OPT);
 
+		if (!psr_su_set_y_granularity(dc, link, stream, &psr_config))
+			return false;
+
 		ret = dc_link_setup_psr(link, stream, &psr_config, &psr_context);
 
 	}
diff --git a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
index 9b5d9b2c9a6a..4d27ad9f7370 100644
--- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
+++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
@@ -916,3 +916,38 @@ bool mod_power_only_edp(const struct dc_state *context, const struct dc_stream_s
 {
 	return context && context->stream_count == 1 && dc_is_embedded_signal(stream->signal);
 }
+
+bool psr_su_set_y_granularity(struct dc *dc, struct dc_link *link,
+			      struct dc_stream_state *stream,
+			      struct psr_config *config)
+{
+	uint16_t pic_height;
+	uint8_t slice_height;
+
+	if (!dc->caps.edp_dsc_support ||
+	    link->panel_config.dsc.disable_dsc_edp ||
+	    !link->dpcd_caps.dsc_caps.dsc_basic_caps.fields.dsc_support.DSC_SUPPORT ||
+	    !stream->timing.dsc_cfg.num_slices_v)
+		return true;
+
+	pic_height = stream->timing.v_addressable +
+		stream->timing.v_border_top + stream->timing.v_border_bottom;
+	slice_height = pic_height / stream->timing.dsc_cfg.num_slices_v;
+
+	if (slice_height) {
+		if (config->su_y_granularity &&
+		    (slice_height % config->su_y_granularity)) {
+			WARN(1,
+			     "%s: dsc: %d, slice_height: %d, num_slices_v: %d\n",
+			     __func__,
+			     stream->sink->dsc_caps.dsc_dec_caps.is_dsc_supported,
+			     slice_height,
+			     stream->timing.dsc_cfg.num_slices_v);
+			return false;
+		}
+
+		config->su_y_granularity = slice_height;
+	}
+
+	return true;
+}
diff --git a/drivers/gpu/drm/amd/display/modules/power/power_helpers.h b/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
index 316452e9dbc9..bb16b37b83da 100644
--- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
+++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
@@ -59,4 +59,7 @@ void mod_power_calc_psr_configs(struct psr_config *psr_config,
 		const struct dc_stream_state *stream);
 bool mod_power_only_edp(const struct dc_state *context,
 		const struct dc_stream_state *stream);
+bool psr_su_set_y_granularity(struct dc *dc, struct dc_link *link,
+			      struct dc_stream_state *stream,
+			      struct psr_config *config);
 #endif /* MODULES_POWER_POWER_HELPERS_H_ */
-- 
2.38.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] drm/amd/display: fix PSR-SU/DSC interoperability support
  2023-01-05 17:38 [PATCH v2] drm/amd/display: fix PSR-SU/DSC interoperability support Hamza Mahfooz
@ 2023-01-05 18:29 ` Harry Wentland
  2023-01-05 20:07   ` Hamza Mahfooz
  0 siblings, 1 reply; 5+ messages in thread
From: Harry Wentland @ 2023-01-05 18:29 UTC (permalink / raw)
  To: Hamza Mahfooz, amd-gfx
  Cc: Leo Li, Rodrigo Siqueira, Alex Deucher, Christian König,
	Pan, Xinhui, David Airlie, Daniel Vetter, David Zhang,
	Nicholas Kazlauskas, Alex Hung, Po Ting Chen, Shirish S,
	Robin Chen, Brian Chang, Camille Cho, Tom Chung, dri-devel,
	linux-kernel



On 1/5/23 12:38, Hamza Mahfooz wrote:
> Currently, there are issues with enabling PSR-SU + DSC. This stems from
> the fact that DSC imposes a slice height on transmitted video data and
> we are not conforming to that slice height in PSR-SU regions. So, pass
> slice_height into su_y_granularity to feed the DSC slice height into
> PSR-SU code.
> 
> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
> ---
> v2: move code to modules/power.
> ---
>  .../drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c |  3 ++
>  .../amd/display/modules/power/power_helpers.c | 35 +++++++++++++++++++
>  .../amd/display/modules/power/power_helpers.h |  3 ++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
> index 26291db0a3cf..872d06fe1436 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
> @@ -122,6 +122,9 @@ bool amdgpu_dm_link_setup_psr(struct dc_stream_state *stream)
>  		psr_config.allow_multi_disp_optimizations =
>  			(amdgpu_dc_feature_mask & DC_PSR_ALLOW_MULTI_DISP_OPT);
>  
> +		if (!psr_su_set_y_granularity(dc, link, stream, &psr_config))
> +			return false;
> +
>  		ret = dc_link_setup_psr(link, stream, &psr_config, &psr_context);
>  
>  	}
> diff --git a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
> index 9b5d9b2c9a6a..4d27ad9f7370 100644
> --- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
> +++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
> @@ -916,3 +916,38 @@ bool mod_power_only_edp(const struct dc_state *context, const struct dc_stream_s
>  {
>  	return context && context->stream_count == 1 && dc_is_embedded_signal(stream->signal);
>  }
> +
> +bool psr_su_set_y_granularity(struct dc *dc, struct dc_link *link,
> +			      struct dc_stream_state *stream,
> +			      struct psr_config *config)
> +{
> +	uint16_t pic_height;
> +	uint8_t slice_height;
> +
> +	if (!dc->caps.edp_dsc_support ||
> +	    link->panel_config.dsc.disable_dsc_edp ||
> +	    !link->dpcd_caps.dsc_caps.dsc_basic_caps.fields.dsc_support.DSC_SUPPORT ||
> +	    !stream->timing.dsc_cfg.num_slices_v)

I'm not sure this condition is correct. We can have DSC but not eDP DSC
support.

> +		return true;
> +
> +	pic_height = stream->timing.v_addressable +
> +		stream->timing.v_border_top + stream->timing.v_border_bottom;
> +	slice_height = pic_height / stream->timing.dsc_cfg.num_slices_v;
> +
> +	if (slice_height) {
> +		if (config->su_y_granularity &&
> +		    (slice_height % config->su_y_granularity)) {
> +			WARN(1,

We don't use WARN in display/dc or display/modules. DC_LOG_WARNING
might be better, or log it in the caller.

Harry

> +			     "%s: dsc: %d, slice_height: %d, num_slices_v: %d\n",
> +			     __func__,
> +			     stream->sink->dsc_caps.dsc_dec_caps.is_dsc_supported,
> +			     slice_height,
> +			     stream->timing.dsc_cfg.num_slices_v);
> +			return false;
> +		}
> +
> +		config->su_y_granularity = slice_height;
> +	}
> +
> +	return true;
> +}
> diff --git a/drivers/gpu/drm/amd/display/modules/power/power_helpers.h b/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
> index 316452e9dbc9..bb16b37b83da 100644
> --- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
> +++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
> @@ -59,4 +59,7 @@ void mod_power_calc_psr_configs(struct psr_config *psr_config,
>  		const struct dc_stream_state *stream);
>  bool mod_power_only_edp(const struct dc_state *context,
>  		const struct dc_stream_state *stream);
> +bool psr_su_set_y_granularity(struct dc *dc, struct dc_link *link,
> +			      struct dc_stream_state *stream,
> +			      struct psr_config *config);
>  #endif /* MODULES_POWER_POWER_HELPERS_H_ */


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] drm/amd/display: fix PSR-SU/DSC interoperability support
  2023-01-05 18:29 ` Harry Wentland
@ 2023-01-05 20:07   ` Hamza Mahfooz
  2023-01-05 20:12     ` Leo Li
  0 siblings, 1 reply; 5+ messages in thread
From: Hamza Mahfooz @ 2023-01-05 20:07 UTC (permalink / raw)
  To: Harry Wentland, amd-gfx
  Cc: Leo Li, Rodrigo Siqueira, Alex Deucher, Christian König,
	Pan, Xinhui, David Airlie, Daniel Vetter, David Zhang,
	Nicholas Kazlauskas, Alex Hung, Po Ting Chen, Shirish S,
	Robin Chen, Brian Chang, Camille Cho, Tom Chung, dri-devel,
	linux-kernel

On 1/5/23 13:29, Harry Wentland wrote:
> 
> 
> On 1/5/23 12:38, Hamza Mahfooz wrote:
>> Currently, there are issues with enabling PSR-SU + DSC. This stems from
>> the fact that DSC imposes a slice height on transmitted video data and
>> we are not conforming to that slice height in PSR-SU regions. So, pass
>> slice_height into su_y_granularity to feed the DSC slice height into
>> PSR-SU code.
>>
>> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
>> ---
>> v2: move code to modules/power.
>> ---
>>   .../drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c |  3 ++
>>   .../amd/display/modules/power/power_helpers.c | 35 +++++++++++++++++++
>>   .../amd/display/modules/power/power_helpers.h |  3 ++
>>   3 files changed, 41 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
>> index 26291db0a3cf..872d06fe1436 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
>> @@ -122,6 +122,9 @@ bool amdgpu_dm_link_setup_psr(struct dc_stream_state *stream)
>>   		psr_config.allow_multi_disp_optimizations =
>>   			(amdgpu_dc_feature_mask & DC_PSR_ALLOW_MULTI_DISP_OPT);
>>   
>> +		if (!psr_su_set_y_granularity(dc, link, stream, &psr_config))
>> +			return false;
>> +
>>   		ret = dc_link_setup_psr(link, stream, &psr_config, &psr_context);
>>   
>>   	}
>> diff --git a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
>> index 9b5d9b2c9a6a..4d27ad9f7370 100644
>> --- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
>> +++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
>> @@ -916,3 +916,38 @@ bool mod_power_only_edp(const struct dc_state *context, const struct dc_stream_s
>>   {
>>   	return context && context->stream_count == 1 && dc_is_embedded_signal(stream->signal);
>>   }
>> +
>> +bool psr_su_set_y_granularity(struct dc *dc, struct dc_link *link,
>> +			      struct dc_stream_state *stream,
>> +			      struct psr_config *config)
>> +{
>> +	uint16_t pic_height;
>> +	uint8_t slice_height;
>> +
>> +	if (!dc->caps.edp_dsc_support ||
>> +	    link->panel_config.dsc.disable_dsc_edp ||
>> +	    !link->dpcd_caps.dsc_caps.dsc_basic_caps.fields.dsc_support.DSC_SUPPORT ||
>> +	    !stream->timing.dsc_cfg.num_slices_v)
> 
> I'm not sure this condition is correct. We can have DSC but not eDP DSC
> support.
> 

AFAIK PSR-SU displays use eDP exclusively, so we shouldn't have to worry 
about this case.

>> +		return true;
>> +
>> +	pic_height = stream->timing.v_addressable +
>> +		stream->timing.v_border_top + stream->timing.v_border_bottom;
>> +	slice_height = pic_height / stream->timing.dsc_cfg.num_slices_v;
>> +
>> +	if (slice_height) {
>> +		if (config->su_y_granularity &&
>> +		    (slice_height % config->su_y_granularity)) {
>> +			WARN(1,
> 
> We don't use WARN in display/dc or display/modules. DC_LOG_WARNING
> might be better, or log it in the caller.
> 
> Harry
> 
>> +			     "%s: dsc: %d, slice_height: %d, num_slices_v: %d\n",
>> +			     __func__,
>> +			     stream->sink->dsc_caps.dsc_dec_caps.is_dsc_supported,
>> +			     slice_height,
>> +			     stream->timing.dsc_cfg.num_slices_v);
>> +			return false;
>> +		}
>> +
>> +		config->su_y_granularity = slice_height;
>> +	}
>> +
>> +	return true;
>> +}
>> diff --git a/drivers/gpu/drm/amd/display/modules/power/power_helpers.h b/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
>> index 316452e9dbc9..bb16b37b83da 100644
>> --- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
>> +++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
>> @@ -59,4 +59,7 @@ void mod_power_calc_psr_configs(struct psr_config *psr_config,
>>   		const struct dc_stream_state *stream);
>>   bool mod_power_only_edp(const struct dc_state *context,
>>   		const struct dc_stream_state *stream);
>> +bool psr_su_set_y_granularity(struct dc *dc, struct dc_link *link,
>> +			      struct dc_stream_state *stream,
>> +			      struct psr_config *config);
>>   #endif /* MODULES_POWER_POWER_HELPERS_H_ */
> 

-- 
Hamza


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] drm/amd/display: fix PSR-SU/DSC interoperability support
  2023-01-05 20:07   ` Hamza Mahfooz
@ 2023-01-05 20:12     ` Leo Li
  2023-01-05 21:40       ` Harry Wentland
  0 siblings, 1 reply; 5+ messages in thread
From: Leo Li @ 2023-01-05 20:12 UTC (permalink / raw)
  To: Hamza Mahfooz, Harry Wentland, amd-gfx
  Cc: Rodrigo Siqueira, Alex Deucher, Christian König, Pan,
	Xinhui, David Airlie, Daniel Vetter, David Zhang,
	Nicholas Kazlauskas, Alex Hung, Po Ting Chen, Shirish S,
	Robin Chen, Brian Chang, Camille Cho, Tom Chung, dri-devel,
	linux-kernel




On 1/5/23 15:07, Hamza Mahfooz wrote:
> On 1/5/23 13:29, Harry Wentland wrote:
>>
>>
>> On 1/5/23 12:38, Hamza Mahfooz wrote:
>>> Currently, there are issues with enabling PSR-SU + DSC. This stems from
>>> the fact that DSC imposes a slice height on transmitted video data and
>>> we are not conforming to that slice height in PSR-SU regions. So, pass
>>> slice_height into su_y_granularity to feed the DSC slice height into
>>> PSR-SU code.
>>>
>>> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
>>> ---
>>> v2: move code to modules/power.
>>> ---
>>>   .../drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c |  3 ++
>>>   .../amd/display/modules/power/power_helpers.c | 35 +++++++++++++++++++
>>>   .../amd/display/modules/power/power_helpers.h |  3 ++
>>>   3 files changed, 41 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c 
>>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
>>> index 26291db0a3cf..872d06fe1436 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
>>> @@ -122,6 +122,9 @@ bool amdgpu_dm_link_setup_psr(struct 
>>> dc_stream_state *stream)
>>>           psr_config.allow_multi_disp_optimizations =
>>>               (amdgpu_dc_feature_mask & DC_PSR_ALLOW_MULTI_DISP_OPT);
>>> +        if (!psr_su_set_y_granularity(dc, link, stream, &psr_config))
>>> +            return false;
>>> +
>>>           ret = dc_link_setup_psr(link, stream, &psr_config, 
>>> &psr_context);
>>>       }
>>> diff --git 
>>> a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c 
>>> b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
>>> index 9b5d9b2c9a6a..4d27ad9f7370 100644
>>> --- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
>>> +++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
>>> @@ -916,3 +916,38 @@ bool mod_power_only_edp(const struct dc_state 
>>> *context, const struct dc_stream_s
>>>   {
>>>       return context && context->stream_count == 1 && 
>>> dc_is_embedded_signal(stream->signal);
>>>   }
>>> +
>>> +bool psr_su_set_y_granularity(struct dc *dc, struct dc_link *link,
>>> +                  struct dc_stream_state *stream,
>>> +                  struct psr_config *config)
>>> +{
>>> +    uint16_t pic_height;
>>> +    uint8_t slice_height;
>>> +
>>> +    if (!dc->caps.edp_dsc_support ||
>>> +        link->panel_config.dsc.disable_dsc_edp ||
>>> +        
>>> !link->dpcd_caps.dsc_caps.dsc_basic_caps.fields.dsc_support.DSC_SUPPORT ||
>>> +        !stream->timing.dsc_cfg.num_slices_v)
>>
>> I'm not sure this condition is correct. We can have DSC but not eDP DSC
>> support.
>>
> 
> AFAIK PSR-SU displays use eDP exclusively, so we shouldn't have to worry 
> about this case.

Right, the dc_link here should only be eDP. I suppose that isn't quite
clear.

Maybe add this as part of the condition?

if (!(link->connector_signal & SIGNAL_TYPE_EDP))
     return true;

Thanks,
Leo

> 
>>> +        return true;
>>> +
>>> +    pic_height = stream->timing.v_addressable +
>>> +        stream->timing.v_border_top + stream->timing.v_border_bottom;
>>> +    slice_height = pic_height / stream->timing.dsc_cfg.num_slices_v;
>>> +
>>> +    if (slice_height) {
>>> +        if (config->su_y_granularity &&
>>> +            (slice_height % config->su_y_granularity)) {
>>> +            WARN(1,
>>
>> We don't use WARN in display/dc or display/modules. DC_LOG_WARNING
>> might be better, or log it in the caller.
>>
>> Harry
>>
>>> +                 "%s: dsc: %d, slice_height: %d, num_slices_v: %d\n",
>>> +                 __func__,
>>> +                 stream->sink->dsc_caps.dsc_dec_caps.is_dsc_supported,
>>> +                 slice_height,
>>> +                 stream->timing.dsc_cfg.num_slices_v);
>>> +            return false;
>>> +        }
>>> +
>>> +        config->su_y_granularity = slice_height;
>>> +    }
>>> +
>>> +    return true;
>>> +}
>>> diff --git 
>>> a/drivers/gpu/drm/amd/display/modules/power/power_helpers.h 
>>> b/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
>>> index 316452e9dbc9..bb16b37b83da 100644
>>> --- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
>>> +++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
>>> @@ -59,4 +59,7 @@ void mod_power_calc_psr_configs(struct psr_config 
>>> *psr_config,
>>>           const struct dc_stream_state *stream);
>>>   bool mod_power_only_edp(const struct dc_state *context,
>>>           const struct dc_stream_state *stream);
>>> +bool psr_su_set_y_granularity(struct dc *dc, struct dc_link *link,
>>> +                  struct dc_stream_state *stream,
>>> +                  struct psr_config *config);
>>>   #endif /* MODULES_POWER_POWER_HELPERS_H_ */
>>
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] drm/amd/display: fix PSR-SU/DSC interoperability support
  2023-01-05 20:12     ` Leo Li
@ 2023-01-05 21:40       ` Harry Wentland
  0 siblings, 0 replies; 5+ messages in thread
From: Harry Wentland @ 2023-01-05 21:40 UTC (permalink / raw)
  To: Leo Li, Hamza Mahfooz, amd-gfx
  Cc: Rodrigo Siqueira, Alex Deucher, Christian König, Pan,
	Xinhui, David Airlie, Daniel Vetter, David Zhang,
	Nicholas Kazlauskas, Alex Hung, Po Ting Chen, Shirish S,
	Robin Chen, Brian Chang, Camille Cho, Tom Chung, dri-devel,
	linux-kernel



On 1/5/23 15:12, Leo Li wrote:
> 
> 
> 
> On 1/5/23 15:07, Hamza Mahfooz wrote:
>> On 1/5/23 13:29, Harry Wentland wrote:
>>>
>>>
>>> On 1/5/23 12:38, Hamza Mahfooz wrote:
>>>> Currently, there are issues with enabling PSR-SU + DSC. This stems from
>>>> the fact that DSC imposes a slice height on transmitted video data and
>>>> we are not conforming to that slice height in PSR-SU regions. So, pass
>>>> slice_height into su_y_granularity to feed the DSC slice height into
>>>> PSR-SU code.
>>>>
>>>> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
>>>> ---
>>>> v2: move code to modules/power.
>>>> ---
>>>>   .../drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c |  3 ++
>>>>   .../amd/display/modules/power/power_helpers.c | 35 +++++++++++++++++++
>>>>   .../amd/display/modules/power/power_helpers.h |  3 ++
>>>>   3 files changed, 41 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
>>>> index 26291db0a3cf..872d06fe1436 100644
>>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
>>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_psr.c
>>>> @@ -122,6 +122,9 @@ bool amdgpu_dm_link_setup_psr(struct dc_stream_state *stream)
>>>>           psr_config.allow_multi_disp_optimizations =
>>>>               (amdgpu_dc_feature_mask & DC_PSR_ALLOW_MULTI_DISP_OPT);
>>>> +        if (!psr_su_set_y_granularity(dc, link, stream, &psr_config))
>>>> +            return false;
>>>> +
>>>>           ret = dc_link_setup_psr(link, stream, &psr_config, &psr_context);
>>>>       }
>>>> diff --git a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
>>>> index 9b5d9b2c9a6a..4d27ad9f7370 100644
>>>> --- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
>>>> +++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.c
>>>> @@ -916,3 +916,38 @@ bool mod_power_only_edp(const struct dc_state *context, const struct dc_stream_s
>>>>   {
>>>>       return context && context->stream_count == 1 && dc_is_embedded_signal(stream->signal);
>>>>   }
>>>> +
>>>> +bool psr_su_set_y_granularity(struct dc *dc, struct dc_link *link,
>>>> +                  struct dc_stream_state *stream,
>>>> +                  struct psr_config *config)
>>>> +{
>>>> +    uint16_t pic_height;
>>>> +    uint8_t slice_height;
>>>> +
>>>> +    if (!dc->caps.edp_dsc_support ||
>>>> +        link->panel_config.dsc.disable_dsc_edp ||
>>>> +        !link->dpcd_caps.dsc_caps.dsc_basic_caps.fields.dsc_support.DSC_SUPPORT ||
>>>> +        !stream->timing.dsc_cfg.num_slices_v)
>>>
>>> I'm not sure this condition is correct. We can have DSC but not eDP DSC
>>> support.
>>>
>>
>> AFAIK PSR-SU displays use eDP exclusively, so we shouldn't have to worry about this case.
> 
> Right, the dc_link here should only be eDP. I suppose that isn't quite
> clear.
>

Right, I was thinking of DSC but PSR-SU is eDP only.

Harry
 
> Maybe add this as part of the condition?
> 
> if (!(link->connector_signal & SIGNAL_TYPE_EDP))
>     return true;
> 
> Thanks,
> Leo
> 
>>
>>>> +        return true;
>>>> +
>>>> +    pic_height = stream->timing.v_addressable +
>>>> +        stream->timing.v_border_top + stream->timing.v_border_bottom;
>>>> +    slice_height = pic_height / stream->timing.dsc_cfg.num_slices_v;
>>>> +
>>>> +    if (slice_height) {
>>>> +        if (config->su_y_granularity &&
>>>> +            (slice_height % config->su_y_granularity)) {
>>>> +            WARN(1,
>>>
>>> We don't use WARN in display/dc or display/modules. DC_LOG_WARNING
>>> might be better, or log it in the caller.
>>>
>>> Harry
>>>
>>>> +                 "%s: dsc: %d, slice_height: %d, num_slices_v: %d\n",
>>>> +                 __func__,
>>>> +                 stream->sink->dsc_caps.dsc_dec_caps.is_dsc_supported,
>>>> +                 slice_height,
>>>> +                 stream->timing.dsc_cfg.num_slices_v);
>>>> +            return false;
>>>> +        }
>>>> +
>>>> +        config->su_y_granularity = slice_height;
>>>> +    }
>>>> +
>>>> +    return true;
>>>> +}
>>>> diff --git a/drivers/gpu/drm/amd/display/modules/power/power_helpers.h b/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
>>>> index 316452e9dbc9..bb16b37b83da 100644
>>>> --- a/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
>>>> +++ b/drivers/gpu/drm/amd/display/modules/power/power_helpers.h
>>>> @@ -59,4 +59,7 @@ void mod_power_calc_psr_configs(struct psr_config *psr_config,
>>>>           const struct dc_stream_state *stream);
>>>>   bool mod_power_only_edp(const struct dc_state *context,
>>>>           const struct dc_stream_state *stream);
>>>> +bool psr_su_set_y_granularity(struct dc *dc, struct dc_link *link,
>>>> +                  struct dc_stream_state *stream,
>>>> +                  struct psr_config *config);
>>>>   #endif /* MODULES_POWER_POWER_HELPERS_H_ */
>>>
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-01-05 21:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-05 17:38 [PATCH v2] drm/amd/display: fix PSR-SU/DSC interoperability support Hamza Mahfooz
2023-01-05 18:29 ` Harry Wentland
2023-01-05 20:07   ` Hamza Mahfooz
2023-01-05 20:12     ` Leo Li
2023-01-05 21:40       ` Harry Wentland

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®