[PATCH 03/13] drm/dp: Add argument for luminance range info in drm_edp_backlight_init

[Kandpal, Suraj]

> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy at intel.com>
> Sent: Thursday, June 12, 2025 4:43 PM
> To: Kandpal, Suraj <suraj.kandpal at intel.com>;
> nouveau at lists.freedesktop.org; dri-devel at lists.freedesktop.org; intel-
> xe at lists.freedesktop.org; intel-gfx at lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>
> Subject: RE: [PATCH 03/13] drm/dp: Add argument for luminance range info in
> drm_edp_backlight_init
> 
> > > > -----Original Message-----
> > > > From: Kandpal, Suraj <suraj.kandpal at intel.com>
> > > > Sent: Monday, April 14, 2025 9:46 AM
> > > > To: nouveau at lists.freedesktop.org;
> > > > dri-devel at lists.freedesktop.org;
> > > > intel- xe at lists.freedesktop.org; intel-gfx at lists.freedesktop.org
> > > > Cc: Nautiyal, Ankit K <ankit.k.nautiyal at intel.com>; Murthy, Arun R
> > > > <arun.r.murthy at intel.com>; Kandpal, Suraj
> > > > <suraj.kandpal at intel.com>
> > > > Subject: [PATCH 03/13] drm/dp: Add argument for luminance range
> > > > info in drm_edp_backlight_init
> > > >
> > > > Add new argument to drm_edp_backlight_init which gives the
> > > > drm_luminance_range_info struct which will be needed to set the
> > > > min and max values for backlight.
> > > >
> > > > Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/display/drm_dp_helper.c               | 5 ++++-
> > > >  drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 5 +++--
> > > >  drivers/gpu/drm/nouveau/nouveau_backlight.c           | 5 ++++-
> > > >  include/drm/display/drm_dp_helper.h                   | 1 +
> > > >  4 files changed, 12 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c
> > > > b/drivers/gpu/drm/display/drm_dp_helper.c
> > > > index 99b27e5e3365..3b309ac5190b 100644
> > > > --- a/drivers/gpu/drm/display/drm_dp_helper.c
> > > > +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> > > > @@ -4227,6 +4227,8 @@ drm_edp_backlight_probe_state(struct
> > > drm_dp_aux
> > > > *aux, struct drm_edp_backlight_i
> > > >   * interface.
> > > >   * @aux: The DP aux device to use for probing
> > > >   * @bl: The &drm_edp_backlight_info struct to fill out with
> > > > information on the backlight
> > > > + * @lr: The &drm_luminance_range_info struct which is used to get
> > > > + the min max when using *luminance override
> > > >   * @driver_pwm_freq_hz: Optional PWM frequency from the driver in
> hz
> > > >   * @edp_dpcd: A cached copy of the eDP DPCD
> > > >   * @current_level: Where to store the probed brightness level, if
> > > > any @@ -
> > > > 4243,6 +4245,7 @@ drm_edp_backlight_probe_state(struct
> drm_dp_aux
> > > > *aux, struct drm_edp_backlight_i
> > > >   */
> > > >  int
> > > >  drm_edp_backlight_init(struct drm_dp_aux *aux, struct
> > > > drm_edp_backlight_info *bl,
> > > > +		       struct drm_luminance_range_info *lr,
> > > Would it be better to have this drm_luminance_range_info inside the
> > > drm_edp_backlight_info?
> >
> > The thing is we fill drm_edp_backlight_info struct in
> > drm_edp_backlight_init Which means we would have to pass it anyways.
> > So having a reference of this in drm_edp_backlight_info didn't make sense.
> >
> The main intention for this ask is two xx_info struct passed as argument.
> Moreover luminance is part of backlight and this new element is _info and
> there already exists backlight_info. So wondering is luminance can be put
> inside backlight_info. The caller of this function can fill the luminance part
> and then make a call.
> 

I see you point but the thing is luminance range is not something we will be using later and is
only used the set the max level of brightness that can be set.
That being said I do get your point on sending two xx_info struct here, I was thinking we send only the
U32 max luminance here since that's the only one we actually use. Drivers can send the max luminance they like.
What do you think?

Regards,
Suraj Kandpal

> Thanks and Regards,
> Arun R Murthy
> --------------------