Re: [PATCH 09/16] drm/irq: Use unsigned int pipe in public API

From: Thierry Reding <thierry.reding@gmail.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
Cc: "Thomas Hellstrom" <thellstrom@vmware.com>,
	"Russell King - ARM Linux" <linux@arm.linux.org.uk>,
	"Alison Wang" <alison.wang@freescale.com>,
	dri-devel@lists.freedesktop.org,
	"Tomi Valkeinen" <tomi.valkeinen@ti.com>,
	"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
	"Benjamin Gaignard" <benjamin.gaignard@linaro.org>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Daniel Vetter" <daniel.vetter@intel.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Ben Skeggs" <bskeggs@redhat.com>
Subject: Re: [PATCH 09/16] drm/irq: Use unsigned int pipe in public API
Date: Fri, 25 Sep 2015 14:26:36 +0200	[thread overview]
Message-ID: <20150925122636.GB30567@ulmo> (raw)
In-Reply-To: <20150924202054.GI26517@intel.com>

[-- Attachment #1.1: Type: text/plain, Size: 3051 bytes --]

On Thu, Sep 24, 2015 at 11:20:54PM +0300, Ville Syrjälä wrote:
> On Thu, Sep 24, 2015 at 08:21:53PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Sep 24, 2015 at 06:35:31PM +0200, Thierry Reding wrote:
> > > This continues the pattern started in commit cc1ef118fc09 ("drm/irq:
> > > Make pipe unsigned and name consistent"). This is applied to the public
> > > APIs and driver callbacks, so pretty much all drivers need to be updated
> > > to match the new prototypes.
> > 
> > I don't like being forced to use the "pipe" naming in all drivers; I
> > found that really confusing when I first looked at DRM, and it took
> > a long time to work out that "pipe" basically means "crtc" or "gpu".
> 
> crtc mean "cathode ray tube controller". If you weren't used to it,
> it would be pretty much impossible to guess what piece of modern
> hardware that means. So I think it's more a case of "a lot of this
> junk used to be in what people called the crtc, so we'll call
> this modern thing a crtc too". Although I think most of the
> hardware people moved on from using that name quite a long time ago.
> 
> "gpu" can mean either just the part that does the rendering and 
> whatnot, or it can mean the entire graphics "card". But calling
> a display pipeline a "gpu" is not done. Now _that_ would be
> confusing.
> 
> "pipe" is fairly nice term, meaning "pipeline". That describes the
> thing quite nicely. Stuff comes in from one end, flows through the
> pipe, and goes out the other end.
> 
> At least both Intel and OMAP used the term pipe or pipeline in the
> hardware docs. Can't really say aything about other brands, at least
> if I discount the more ancient junk I'm familiar with.
> 
> > What's wrong with keeping "crtc" as the index terminology for crtc
> > things?  Surely that's more descriptive of what's going on here?
> 
> I can't speak for Thierry, but I assume the real motivation for this
> renaming was to make it clear which is the "index", and which is the
> crtc object. "pipe" for one, "crtc" for the other. Avoids having
> to call the object "dcrtc", "drm_crtc" or something else entirely.
> And since the object is called "crtc" everywhere else, it's nice not
> to have to make an exception in the vblank code.

Exactly this. I find it to be very confusing to read through drivers and
see completely inconsistent naming for what's really the same thing. The
list isn't limited to "pipe", "crtc" or "index". There are others that
use "head" or "crtc_id". The latter in particular is very confusing
because, at least in most of the cases I've seen, it refers to the pipe
rather than the CRTC object ID.

That said this is all only temporary, though it may take us a while to
get rid of it. The ultimate goal is to convert all drivers over to using
struct drm_crtc * everywhere, at which point we don't have to bother
with what to call the CRTC index. As a preliminary step, having drivers
use consistent naming is hopefully going to make it easier to replace.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel