Re: [Intel-gfx] [PATCH 06/15] drm/i915: Extract intel_edp_backlight_setup()

[Luca Coelho <luca@coelho.fi>]

On Mon, 2022-09-26 at 14:33 +0300, Luca Coelho wrote:
> On Mon, 2022-09-26 at 14:16 +0300, Ville Syrjälä wrote:
> > On Mon, Sep 26, 2022 at 01:58:42PM +0300, Luca Coelho wrote:
> > > On Mon, 2022-09-12 at 14:18 +0300, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Pull the eDP backlight setup ino its own function. No
> > > > reason to pollute intel_edp_init_connector() with all
> > > > the mundane details.
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_dp.c | 51 +++++++++++++++----------
> > > >  1 file changed, 30 insertions(+), 21 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > index a5eca5396fed..de5a4d2df78e 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > @@ -5217,6 +5217,35 @@ intel_edp_add_properties(struct intel_dp *intel_dp)
> > > >  						       fixed_mode->vdisplay);
> > > >  }
> > > >  
> > > > +static void intel_edp_backlight_setup(struct intel_dp *intel_dp,
> > > > +				      struct intel_connector *connector)
> > > > +{
> > > > +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > > > +	enum pipe pipe = INVALID_PIPE;
> > > > +
> > > > +	if (IS_VALLEYVIEW(i915) || IS_CHERRYVIEW(i915)) {
> > > 
> > > Isn't this too restrictive? Isn't there another way to check whether
> > > the hardware supports backlight?
> > 
> > That's not what we're checking.
> > 
> > Only VLV/CHV have per-pipe backlight registers, whereas
> > other platforms have less insane design. So we only need
> > to figure out the pipe on VLV/CHV.

BTW, this is exactly the kind of code that I think deserves some
comments. 😉 Very hard to figure out where this comes from...

--
Cheers,
Luca.