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

[Jesse Barnes]

On Mon, 30 Sep 2013 15:15:12 +0300
Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:

> 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.

Yeah it's definitely a gap with this hw design.  Tracking those bits in
the connector makes more sense than having the backlight level change
if the panel gets moved between pipes.

Jesse