Re: [PATCH 1/2] drm/i915: make backlight functions take a connector v3

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Fri, 11 Oct 2013 23:31:10 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Fri, Oct 11, 2013 at 11:31:53AM -0700, Jesse Barnes wrote:
> 
> [snip]
> 
> >  static int intel_panel_get_brightness(struct backlight_device *bd)
> >  {
> > -	struct drm_device *dev = bl_get_data(bd);
> > -	return intel_panel_get_backlight(dev);
> > +	struct intel_connector *connector = bl_get_data(bd);
> > +	struct drm_device *dev = connector->base.dev;
> > +	enum pipe pipe;
> > +	int ret = 0;
> > +
> > +	mutex_lock(&dev->mode_config.mutex);
> > +	pipe = intel_get_pipe_from_connector(connector);
> > +	mutex_unlock(&dev->mode_config.mutex);
> > +	if (pipe == INVALID_PIPE) {
> > +		ret = 0;
> > +		goto out_unlock;
> > +	}
> > +
> > +	ret = intel_panel_get_backlight(connector->base.dev, pipe);
> > +out_unlock:
> > +	mutex_unlock(&dev->mode_config.mutex);
> 
> I see mutex_unlock(&dev->mode_config.mutex) twice here. I think you've
> just volunteered yourselve for some testcases ;-)
> 
> Ideas:
> - Make sure all lvds/edp connectors are enabled and bash on all backlight
>   interfaces (with igt_fork it's easy to do that concurrently).
> - Race the above with output changes: dpms on/off and changing the crtc
>   around.
> - Race the above with system suspend for bonus points (can be completely
>   stitched together from igt helpers).

Sorry can't volunteer for that now, but those sound like good tests to
write.

I'll fix up the patch, we don't need to make double sure the lock is
unlocked do we? and test locally and re-send.

-- 
Jesse Barnes, Intel Open Source Technology Center