| Message ID | 20240404032931.380887-8-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enable Aux Based EDP HDR | expand |
On Thu, Apr 04, 2024 at 08:59:30AM +0530, Suraj Kandpal wrote: > Write panel override luminance values which helps the TCON decide > if tone mapping needs to be enabled or not. > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > .../drm/i915/display/intel_dp_aux_backlight.c | 25 +++++++++++++++++++ > 1 file changed, 25 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 7af876e2d210..20dd5a6a0f3f 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > @@ -381,6 +381,29 @@ static const char *dpcd_vs_pwm_str(bool aux) > return aux ? "DPCD" : "PWM"; > } > > +static void > +intel_dp_aux_write_panel_luminance_override(struct intel_connector *connector) > +{ > + struct drm_i915_private *i915 = to_i915(connector->base.dev); > + struct intel_panel *panel = &connector->panel; > + struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); > + int ret; > + u8 buf[4] = {}; > + > + buf[0] = panel->backlight.min & 0xFF; > + buf[1] = (panel->backlight.min & 0xFF00) >> 8; > + buf[2] = panel->backlight.max & 0xFF; > + buf[3] = (panel->backlight.max & 0xFF00) >> 8; > + > + ret = drm_dp_dpcd_write(&intel_dp->aux, > + INTEL_EDP_HDR_PANEL_LUMINANCE_OVERRIDE, > + buf, sizeof(buf)); > + if (ret < 0) > + drm_dbg_kms(&i915->drm, > + "Panel Luminance DPCD reg write failed, err:-%d\n", > + ret); > +} > + > static int > intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pipe) > { > @@ -412,6 +435,8 @@ intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pi > panel->backlight.min = 0; > } > > + intel_dp_aux_write_panel_luminance_override(connector); > + Should this really always be set? It says override. Doesn't the TCON have some values already that we're overriding? > drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Using AUX HDR interface for backlight control (range %d..%d)\n", > connector->base.base.id, connector->base.name, > panel->backlight.min, panel->backlight.max);
> -----Original Message----- > From: Sebastian Wick <sebastian.wick@redhat.com> > Sent: Friday, April 5, 2024 11:02 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com> > Cc: intel-gfx@lists.freedesktop.org; Borah, Chaitanya Kumar > <chaitanya.kumar.borah@intel.com>; Shankar, Uma > <uma.shankar@intel.com>; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; > Murthy, Arun R <arun.r.murthy@intel.com>; Syrjala, Ville > <ville.syrjala@intel.com>; Kumar, Naveen1 <naveen1.kumar@intel.com> > Subject: Re: [6/7] drm/i915/dp: Write panel override luminance values > > On Thu, Apr 04, 2024 at 08:59:30AM +0530, Suraj Kandpal wrote: > > Write panel override luminance values which helps the TCON decide if > > tone mapping needs to be enabled or not. > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > .../drm/i915/display/intel_dp_aux_backlight.c | 25 > > +++++++++++++++++++ > > 1 file changed, 25 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 7af876e2d210..20dd5a6a0f3f 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c > > @@ -381,6 +381,29 @@ static const char *dpcd_vs_pwm_str(bool aux) > > return aux ? "DPCD" : "PWM"; > > } > > > > +static void > > +intel_dp_aux_write_panel_luminance_override(struct intel_connector > > +*connector) { > > + struct drm_i915_private *i915 = to_i915(connector->base.dev); > > + struct intel_panel *panel = &connector->panel; > > + struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); > > + int ret; > > + u8 buf[4] = {}; > > + > > + buf[0] = panel->backlight.min & 0xFF; > > + buf[1] = (panel->backlight.min & 0xFF00) >> 8; > > + buf[2] = panel->backlight.max & 0xFF; > > + buf[3] = (panel->backlight.max & 0xFF00) >> 8; > > + > > + ret = drm_dp_dpcd_write(&intel_dp->aux, > > + > INTEL_EDP_HDR_PANEL_LUMINANCE_OVERRIDE, > > + buf, sizeof(buf)); > > + if (ret < 0) > > + drm_dbg_kms(&i915->drm, > > + "Panel Luminance DPCD reg write failed, err:- > %d\n", > > + ret); > > +} > > + > > static int > > intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, > > enum pipe pipe) { @@ -412,6 +435,8 @@ > > intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum > pipe pi > > panel->backlight.min = 0; > > } > > > > + intel_dp_aux_write_panel_luminance_override(connector); > > + > > Should this really always be set? It says override. Doesn't the TCON have > some values already that we're overriding? Yes we calculate our own min and max panel luminance these values always need to be sent to the TCON as it takes these values into account before it takes the decision to enable or disable tone mapping. Note: TM is enablement is a TCON decision ICL onwards and before that its always disable (According to internal specs) Regards, Suraj Kandpal > > > drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Using AUX HDR > interface for backlight control (range %d..%d)\n", > > connector->base.base.id, connector->base.name, > > panel->backlight.min, panel->backlight.max);
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 7af876e2d210..20dd5a6a0f3f 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c @@ -381,6 +381,29 @@ static const char *dpcd_vs_pwm_str(bool aux) return aux ? "DPCD" : "PWM"; } +static void +intel_dp_aux_write_panel_luminance_override(struct intel_connector *connector) +{ + struct drm_i915_private *i915 = to_i915(connector->base.dev); + struct intel_panel *panel = &connector->panel; + struct intel_dp *intel_dp = enc_to_intel_dp(connector->encoder); + int ret; + u8 buf[4] = {}; + + buf[0] = panel->backlight.min & 0xFF; + buf[1] = (panel->backlight.min & 0xFF00) >> 8; + buf[2] = panel->backlight.max & 0xFF; + buf[3] = (panel->backlight.max & 0xFF00) >> 8; + + ret = drm_dp_dpcd_write(&intel_dp->aux, + INTEL_EDP_HDR_PANEL_LUMINANCE_OVERRIDE, + buf, sizeof(buf)); + if (ret < 0) + drm_dbg_kms(&i915->drm, + "Panel Luminance DPCD reg write failed, err:-%d\n", + ret); +} + static int intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pipe) { @@ -412,6 +435,8 @@ intel_dp_aux_hdr_setup_backlight(struct intel_connector *connector, enum pipe pi panel->backlight.min = 0; } + intel_dp_aux_write_panel_luminance_override(connector); + drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] Using AUX HDR interface for backlight control (range %d..%d)\n", connector->base.base.id, connector->base.name, panel->backlight.min, panel->backlight.max);
Write panel override luminance values which helps the TCON decide if tone mapping needs to be enabled or not. Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- .../drm/i915/display/intel_dp_aux_backlight.c | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+)