[Intel-gfx] [PATCH 1/2] drm/i915: make backlight functions take a connector

[Ville Syrjälä]

On Mon, Sep 30, 2013 at 11:18:28AM +0300, Jani Nikula wrote:
> On Sat, 28 Sep 2013, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> > On VLV/BYT, backlight controls a per-pipe, so when adjusting the
> > backlight we need to pass the correct info.  So make the externally
> > visible backlight functions take a connector argument, which can be used
> > internally to figure out the pipe backlight to adjust.
> >
> > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c  |  8 +++++
> >  drivers/gpu/drm/i915/intel_dp.c       |  5 ++-
> >  drivers/gpu/drm/i915/intel_drv.h      |  8 +++--
> >  drivers/gpu/drm/i915/intel_lvds.c     |  9 +++--
> >  drivers/gpu/drm/i915/intel_opregion.c | 28 ++++++++++++++-
> >  drivers/gpu/drm/i915/intel_panel.c    | 66 +++++++++++++++++++++--------------
> >  6 files changed, 88 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 9a7136c..d6a1868 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9716,6 +9716,14 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
> >  	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
> >  }
> >  
> > +enum pipe intel_get_pipe_from_connector(struct intel_connector *connector)
> > +{
> > +	struct intel_encoder *encoder = connector->encoder;
> > +	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> > +
> > +	return crtc->pipe;
> 
> What if crtc == NULL?
> 
> I guess this shouldn't happen on the backlight enable/disable call paths
> through encoder callbacks, but what about backlight sysfs and opregion?

I was discussing things a bit with Jani, and I'll repeat my thoughts
here for others to see. Feel free to kick me if I get things totally 
wrong as I don't know the backlight code, nor do I really want to.

We may need to handle backlight configuration requests when there's no
pipe attached to the port, and we also want to be able to change the
pipe<->port mapping, so my idea would be to shadow the backlight
registers in the connector, and when we assign a pipe to the port,
we write all the regs from the shadow copies to the real pipe blc
registers. Otherwise I'm worried about all the rmw stuff we do to the
registers, and leaving bits around in the registers from when the pipe
was still used with some other port.

-- 
Ville Syrjälä
Intel OTC