[Intel-gfx] [PATCH] drm/i915: implement HSW display sequences for package C8+

[Daniel Vetter]

On Wed, Jun 05, 2013 at 10:39:03PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 05, 2013 at 08:04:48PM +0200, Daniel Vetter wrote:
> > On Wed, Jun 5, 2013 at 7:20 PM, Paulo Zanoni <przanoni at gmail.com> wrote:
> > >>   So all together I vote for the generic interface to be called something
> > >>   like
> > >>
> > >>   intel_aux_display_runtime_forbid/allow
> > >
> > > I changed to intel_aux_display_shutdown_forbid/allow, because we
> > > forbid the shutdown, not the "runtime".
> > 
> > Just one quick comment: The idea was to be 100% consisten with the
> > runtime pm api (I've dropped the pm_ part unfortunately), which is:
> > 
> > extern void pm_runtime_allow(struct device *dev);
> > extern void pm_runtime_forbid(struct device *dev);
> 
> I've not seen those guys before, but they don't seem like something
> we ought to be using. In fact it seems they're there solely for the
> purpose of sysfs. They frob the runtime_auto boolean which has no ref
> count protecting it, so someone writing to the sysfs "control" file
> could undo what we just did from the driver, and vice versa. There's
> still the usage_count though so the device should remain alive, but
> then that's what _get()/_put() also modify.

Yeah, I've been confused here. _get/_put seem to be the ones we actually
want to emulate.

> I do see quite a few pm_runtime_allow/forbid calls from drivers though,
> which is confusing to say the least. Either I'm confused about the
> purpose of these functions, or there has been a serious problem with 
> driver review.

I think it's to prevent runtime pm in driver init code, e.g. usb devices
which crap out when autosuspended. Since presumable the driver will do the
allow/forbid only once it should all work out. And root is allowed to
break stuff by touching randomg things in sysfs ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch