[6/7] drm/i915/dp: Write panel override luminance values

[Kandpal, Suraj]

> -----Original Message-----
> From: Sebastian Wick <sebastian.wick at redhat.com>
> Sent: Friday, April 5, 2024 11:02 PM
> To: Kandpal, Suraj <suraj.kandpal at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; Borah, Chaitanya Kumar
> <chaitanya.kumar.borah at intel.com>; Shankar, Uma
> <uma.shankar at intel.com>; Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>;
> Murthy, Arun R <arun.r.murthy at intel.com>; Syrjala, Ville
> <ville.syrjala at intel.com>; Kumar, Naveen1 <naveen1.kumar at 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 at 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);