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

[Jesse Barnes]

On Sat, 5 Oct 2013 13:04:08 +0200
Daniel Vetter <daniel at ffwll.ch> wrote:

> On Fri, Oct 4, 2013 at 9:42 PM, 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.
> >
> > v2: make connector pipe lookup check for NULL crtc (Jani)
> >     fixup connector check in ASLE code (Jani)
> >
> > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> 
> To high-level things that we need to figure out first:
> 
> 1. You're connector->pipe chasing is buggy. You shouldn't chase
> intel_connector->encoder since for shared encoders this doesn't
> reflect whether the connector is actually connected to that encoder.
> Instead check intel_connector->base.encoder for non-NULL, which
> guarantees you that you also have a valid crtc. Then getting at the
> pipe is simple:
> 
> pipe = to_intel_crtc(intel_connector->base.encoder->crtc);
> 
> so I think we can also ditch your little helper and PIPE_INVALID
> (which is good, since otherwise I'll volunteer your for a follow-up
> patch to use PIPE_INVALID at a bunch more places ...).
> 
> 2. You can only chase the above pointers if you have the big modeset
> lock locked. Which at least for the functions implementing the
> backlight sysfs interface seems to not be the case.

Ah ok thanks.  I actually coded it a couple of times and I think I used
the base.encoder first, but then lost that somehow in a rebase.  I'll
go back to that.

Good catch on the locking, I'll see about that too.

As for putting this in the connector, yes it really only applies to
LVDS and eDP panels (though it could apply to monitors that support the
VESA backlight control extensions too, but I don't think we have code
for that yet).  Anyway all of that is just icing on the cake after the
panel bits handle multiple pipes correctly.

-- 
Jesse Barnes, Intel Open Source Technology Center