Re: [PATCH] drm/i915/dp: Fallback to DSC for HDR content if needed

["Borah, Chaitanya Kumar" <chaitanya.kumar.borah@intel.com>]

On 7/11/2025 9:57 AM, Kandpal, Suraj wrote:
>
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
>> Chaitanya Kumar Borah
>> Sent: Thursday, July 3, 2025 7:33 PM
>> To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
>> Cc: Deak, Imre <imre.deak@intel.com>; Nautiyal, Ankit K
>> <ankit.k.nautiyal@intel.com>; Borah, Chaitanya Kumar
>> <chaitanya.kumar.borah@intel.com>
>> Subject: [PATCH] drm/i915/dp: Fallback to DSC for HDR content if needed
>>
>> If user-space has enabled HDR by passing HDR metadata and we can't
>> support 10bpc fallback to DSC.
> Do we have this limitation documented anywhere if so a Bspec link would be helpful.

It is a matter of driver policy rather than spec. More on it below.


>> Now that we need the helper to determine the presence of HDR metadata at
>> multiple places make it non static.
>>
>> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dp.c          | 16 ++++++++++++++++
>>   drivers/gpu/drm/i915/display/intel_dp.h          |  1 +
>>   .../drm/i915/display/intel_dp_aux_backlight.c    | 13 -------------
>>   3 files changed, 17 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> b/drivers/gpu/drm/i915/display/intel_dp.c
>> index f48912f308df..abbba159e479 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -1774,6 +1774,9 @@ intel_dp_compute_link_config_wide(struct intel_dp
>> *intel_dp,
>>   	     bpp -= 2 * 3) {
>>   		int link_bpp = intel_dp_output_bpp(pipe_config-
>>> output_format, bpp);
>> +		if (intel_dp_in_hdr_mode(conn_state) && bpp < 30)
>> +			return -EINVAL;
>> +
>  From the commit description it looks like you are trying to say we don't support 10 bpc.
> I think what you want to says is we need at least 10 bpc for HDR to work and if it's not so
> Then fall back to using DSC as we would do in usual use case.

s/don't/can't


>
> I think we should not even get the HDR IOCTL passing id we don't have a minimum bpc of 10
> That way we don't have a need of this code right here.


There can be cases where a panel only supports 10bpc for a lower 
resolution or with DSC.
Panel still reports itself as HDR capable (not sure if it can expose HDR 
capabilties at the granularity of modelines).

In these cases, the "max bpc" property will still show a range (say 
6-10) inclusive of 10bpc.

Userspace can still issue an IOCTL setting HDR metadata because it 
thinks the panel is capable.

This patch adds a DSC fallback in case  we cannot accomodate 10bpc with 
our bandwidth calculations.


Regards

Chaitanya

>
> Regards,
> Suraj Kandpal
>
>>   		mode_rate = intel_dp_link_required(clock, link_bpp);
>>
>>   		for (i = 0; i < intel_dp->num_common_rates; i++) { @@ -
>> 2910,6 +2913,19 @@ static void intel_dp_compute_vsc_sdp(struct intel_dp
>> *intel_dp,
>>   	}
>>   }
>>
>> +bool
>> +intel_dp_in_hdr_mode(const struct drm_connector_state *conn_state) {
>> +	struct hdr_output_metadata *hdr_metadata;
>> +
>> +	if (!conn_state->hdr_output_metadata)
>> +		return false;
>> +
>> +	hdr_metadata = conn_state->hdr_output_metadata->data;
>> +
>> +	return hdr_metadata->hdmi_metadata_type1.eotf ==
>> +HDMI_EOTF_SMPTE_ST2084; }
>> +
>>   static void
>>   intel_dp_compute_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp,
>>   					    struct intel_crtc_state *crtc_state,
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
>> b/drivers/gpu/drm/i915/display/intel_dp.h
>> index 0657f5681196..5def589e3c0e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.h
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
>> @@ -214,5 +214,6 @@ int intel_dp_compute_min_hblank(struct
>> intel_crtc_state *crtc_state,
>>
>>   int intel_dp_dsc_bpp_step_x16(const struct intel_connector *connector);
>> void intel_dp_dpcd_set_probe(struct intel_dp *intel_dp, bool
>> force_on_external);
>> +bool intel_dp_in_hdr_mode(const struct drm_connector_state
>> +*conn_state);
>>
>>   #endif /* __INTEL_DP_H__ */
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> index 41228478b21c..12084a542fc5 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> @@ -225,19 +225,6 @@ intel_dp_aux_hdr_set_aux_backlight(const struct
>> drm_connector_state *conn_state,
>>   			connector->base.base.id, connector->base.name);  }
>>
>> -static bool
>> -intel_dp_in_hdr_mode(const struct drm_connector_state *conn_state) -{
>> -	struct hdr_output_metadata *hdr_metadata;
>> -
>> -	if (!conn_state->hdr_output_metadata)
>> -		return false;
>> -
>> -	hdr_metadata = conn_state->hdr_output_metadata->data;
>> -
>> -	return hdr_metadata->hdmi_metadata_type1.eotf ==
>> HDMI_EOTF_SMPTE_ST2084;
>> -}
>> -
>>   static void
>>   intel_dp_aux_hdr_set_backlight(const struct drm_connector_state
>> *conn_state, u32 level)  {
>> --
>> 2.25.1