Re: [Intel-gfx] [PATCH] drm/i915: Check EDID before dpcd for possible HDR aux bl support

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 12 Apr 2022, Jouni Högander <jouni.hogander@intel.com> wrote:
> We have now seen panel (XMG Core 15 e21 laptop) avertizing support
> for Intel proprietary eDP backlight control via DPCD registers, but
> actually working only with legacy pwm control.
>
> This patch adds panel EDID check for possible HDR static metadata and
> does detection from DPCD registers only if this data block exists.
>
> Fixes: 4a8d79901d5b ("drm/i915/dp: Enable Intel's HDR backlight interface (only SDR for now)")
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5284
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Tested-by: Filippo Falezza <filippo.falezza@outlook.it>
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  .../gpu/drm/i915/display/intel_dp_aux_backlight.c   | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> 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 97cf3cac0105..f69e185b58c1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -108,6 +108,19 @@ intel_dp_aux_supports_hdr_backlight(struct intel_connector *connector)
>  	int ret;
>  	u8 tcon_cap[4];
>  
> +	/*
> +	 * If we don't have HDR static metadata there is no way to
> +	 * runtime detect used range for nits based control. For now
> +	 * do not use Intel proprietary eDP backlight control if we
> +	 * don't have this data in panel EDID. In case we find panel
> +	 * which supports only nits based control, but doesn't provide
> +	 * HDR static metadata we need to start maintaining table of
> +	 * ranges for such panels.
> +	 */
> +	if (!(connector->base.hdr_sink_metadata.hdmi_type1.metadata_type &
> +	      BIT(HDMI_STATIC_METADATA_TYPE1)))
> +		return false;

Considering the complexities around this, I'd probably start gathering
the info in variables, then debug log all of it, with the conclusion the
driver makes. It's makes future debugging much easier.

Other than that, I guess

Acked-by: Jani Nikula <jani.nikula@intel.com>

because I don't really know what's going on with these...

BR,
Jani.

> +
>  	intel_dp_wait_source_oui(intel_dp);
>  
>  	ret = drm_dp_dpcd_read(aux, INTEL_EDP_HDR_TCON_CAP0, tcon_cap, sizeof(tcon_cap));

-- 
Jani Nikula, Intel Open Source Graphics Center