From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
Subject: Re: [PATCH] drm/i915: implement HSW display sequences
 for package C8+
Date: Wed, 5 Jun 2013 22:39:03 +0300
Message-ID: <20130605193903.GI5004@intel.com>
References: <1366130864-5471-1-git-send-email-przanoni@gmail.com>
	<20130417074716.GL27612@phenom.ffwll.local>
	<CA+gsUGSVgf-wkHZsTYf_+yK7whC+mFHrkCF5WQo=aaxG8QPF3A@mail.gmail.com>
	<CAKMK7uHcsFMX9LWAZyK-ajAyiBKTj3qr2oRJgU-sBeHuE=qu+w@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
Received: from mga09.intel.com (mga09.intel.com [134.134.136.24])
	by gabe.freedesktop.org (Postfix) with ESMTP id 23E19E5CE2
	for <intel-gfx@lists.freedesktop.org>;
	Wed,  5 Jun 2013 12:39:07 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <CAKMK7uHcsFMX9LWAZyK-ajAyiBKTj3qr2oRJgU-sBeHuE=qu+w@mail.gmail.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=subscribe>
Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>, Paulo Zanoni <paulo.r.zanoni@intel.com>
List-Id: intel-gfx@lists.freedesktop.org

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@gmail.com> wrote:
> >>   So all together I vote for the generic interface to be called someth=
ing
> >>   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.

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.

-- =

Ville Syrj=E4l=E4
Intel OTC