[Intel-gfx] [PATCH v2] drm/i915: Check EDID for HDR static metadata when choosing blc

[Ville Syrjälä]

On Thu, Apr 14, 2022 at 04:44:13PM -0400, Lyude Paul wrote:
> Reviewed-by: Lyude Paul <lyude at redhat.com>

Thanks for the patch and review. Pushed to drm-intel-next.

> 
> On Wed, 2022-04-13 at 11:28 +0300, Jouni Högander wrote:
> > We have now seen panel (XMG Core 15 e21 laptop) advertizing 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
> > Intel proprietary eDP backlight control is used only if that exists.
> > Missing HDR static metadata is ignored if user specifically asks for
> > Intel proprietary eDP backlight control via enable_dpcd_backlight
> > parameter.
> > 
> > v2 :
> > - Ignore missing HDR static metadata if Intel proprietary eDP
> >   backlight control is forced via i915.enable_dpcd_backlight
> > - Printout info message if panel is missing HDR static metadata and
> >   support for Intel proprietary eDP backlight control is detected
> > 
> > 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 at redhat.com>
> > Cc: Mika Kahola <mika.kahola at intel.com>
> > Cc: Jani Nikula <jani.nikula at intel.com>
> > Cc: Filippo Falezza <filippo.falezza at outlook.it>
> > Cc: stable at vger.kernel.org
> > Signed-off-by: Jouni Högander <jouni.hogander at intel.com>
> > ---
> >  .../drm/i915/display/intel_dp_aux_backlight.c | 34 ++++++++++++++-----
> >  1 file changed, 26 insertions(+), 8 deletions(-)
> > 
> > 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..fb6cf30ee628 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> > @@ -97,6 +97,14 @@
> >  
> >  #define INTEL_EDP_BRIGHTNESS_OPTIMIZATION_1                           
> > 0x359
> >  
> > +enum intel_dp_aux_backlight_modparam {
> > +       INTEL_DP_AUX_BACKLIGHT_AUTO = -1,
> > +       INTEL_DP_AUX_BACKLIGHT_OFF = 0,
> > +       INTEL_DP_AUX_BACKLIGHT_ON = 1,
> > +       INTEL_DP_AUX_BACKLIGHT_FORCE_VESA = 2,
> > +       INTEL_DP_AUX_BACKLIGHT_FORCE_INTEL = 3,
> > +};
> > +
> >  /* Intel EDP backlight callbacks */
> >  static bool
> >  intel_dp_aux_supports_hdr_backlight(struct intel_connector *connector)
> > @@ -126,6 +134,24 @@ intel_dp_aux_supports_hdr_backlight(struct
> > intel_connector *connector)
> >                 return false;
> >         }
> >  
> > +       /*
> > +        * 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 (i915->params.enable_dpcd_backlight !=
> > INTEL_DP_AUX_BACKLIGHT_FORCE_INTEL &&
> > +           !(connector->base.hdr_sink_metadata.hdmi_type1.metadata_type &
> > +             BIT(HDMI_STATIC_METADATA_TYPE1))) {
> > +               drm_info(&i915->drm,
> > +                        "Panel is missing HDR static metadata. Possible
> > support for Intel HDR backlight interface is not used. If your backlight
> > controls don't work try booting with i915.enable_dpcd_backlight=%d. needs
> > this, please file a _new_ bug report on drm/i915, see " FDO_BUG_URL " for
> > details.\n",
> > +                        INTEL_DP_AUX_BACKLIGHT_FORCE_INTEL);
> > +               return false;
> > +       }
> > +
> >         panel->backlight.edp.intel.sdr_uses_aux =
> >                 tcon_cap[2] & INTEL_EDP_SDR_TCON_BRIGHTNESS_AUX_CAP;
> >  
> > @@ -413,14 +439,6 @@ static const struct intel_panel_bl_funcs
> > intel_dp_vesa_bl_funcs = {
> >         .get = intel_dp_aux_vesa_get_backlight,
> >  };
> >  
> > -enum intel_dp_aux_backlight_modparam {
> > -       INTEL_DP_AUX_BACKLIGHT_AUTO = -1,
> > -       INTEL_DP_AUX_BACKLIGHT_OFF = 0,
> > -       INTEL_DP_AUX_BACKLIGHT_ON = 1,
> > -       INTEL_DP_AUX_BACKLIGHT_FORCE_VESA = 2,
> > -       INTEL_DP_AUX_BACKLIGHT_FORCE_INTEL = 3,
> > -};
> > -
> >  int intel_dp_aux_init_backlight_funcs(struct intel_connector *connector)
> >  {
> >         struct drm_device *dev = connector->base.dev;
> 
> -- 
> Cheers,
>  Lyude Paul (she/her)
>  Software Engineer at Red Hat

-- 
Ville Syrjälä
Intel