| Message ID | 20220413082826.120634-1-jouni.hogander@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915: Check EDID for HDR static metadata when choosing blc | expand |
Reviewed-by: Lyude Paul <lyude@redhat.com> 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@redhat.com> > Cc: Mika Kahola <mika.kahola@intel.com> > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Filippo Falezza <filippo.falezza@outlook.it> > Cc: stable@vger.kernel.org > Signed-off-by: Jouni Högander <jouni.hogander@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;
On Thu, Apr 14, 2022 at 04:44:13PM -0400, Lyude Paul wrote: > Reviewed-by: Lyude Paul <lyude@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@redhat.com> > > Cc: Mika Kahola <mika.kahola@intel.com> > > Cc: Jani Nikula <jani.nikula@intel.com> > > Cc: Filippo Falezza <filippo.falezza@outlook.it> > > Cc: stable@vger.kernel.org > > Signed-off-by: Jouni Högander <jouni.hogander@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
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;
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@redhat.com> Cc: Mika Kahola <mika.kahola@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Cc: Filippo Falezza <filippo.falezza@outlook.it> Cc: stable@vger.kernel.org Signed-off-by: Jouni Högander <jouni.hogander@intel.com> --- .../drm/i915/display/intel_dp_aux_backlight.c | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-)