On Tue, 2022-04-12 at 08:25 +0300, Jouni Högander 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 <[email protected]>
> Cc: Mika Kahola <[email protected]>
> Cc: Jani Nikula <[email protected]>
> Tested-by: Filippo Falezza <[email protected]>
> Signed-off-by: Jouni Högander <[email protected]>
> ---
>  .../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;

The block used for this is actually for HDMI?? How bizarre…

Anyway yeah - patch looks good to me, but I think we should print a debugging
message of some sort when we determine that there's no HDR backlight because
of the EDID - along with printing instructions for how the user can override
it if we've made the wrong choice along with reporting a bug. Also - we should
have the

Cc: [email protected]

tag from dim added here using `dim fixes $commit`.

With that fixed:

Reviewed-by: Lyude Paul <[email protected]>

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

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat

Reply via email to