Re: [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2

From: Daniel Vetter <daniel@ffwll.ch>
To: Rob Clark <robdclark@gmail.com>
Cc: "Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>,
	"DRI Development" <dri-devel@lists.freedesktop.org>,
	"Daniel Vetter" <daniel.vetter@ffwll.ch>,
	"Intel Graphics Development" <intel-gfx@lists.freedesktop.org>,
	"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
	"Daniel Vetter" <daniel.vetter@intel.com>,
	"Michel Dänzer" <michel@daenzer.net>
Subject: Re: [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2
Date: Fri, 30 Jan 2015 16:42:09 +0100	[thread overview]
Message-ID: <20150130154151.GA14009@phenom.ffwll.local> (raw)
In-Reply-To: <CAF6AEGs1_5C7DiP_Q93GH1uQ5xv11jfRRno30RgjKD-ZU=wQzg@mail.gmail.com>

On Fri, Jan 30, 2015 at 09:51:48AM -0500, Rob Clark wrote:
> On Fri, Jan 30, 2015 at 9:35 AM, Tvrtko Ursulin
> <tvrtko.ursulin@linux.intel.com> wrote:
> >
> > On 01/30/2015 01:43 PM, Rob Clark wrote:
> >>
> >> On Fri, Jan 30, 2015 at 5:51 AM, Tvrtko Ursulin
> >> <tvrtko.ursulin@linux.intel.com> wrote:
> >>>>
> >>>> +/*
> >>>> + * Format Modifier tokens:
> >>>> + *
> >>>> + * When adding a new token please document the layout with a code
> >>>> comment,
> >>>> + * similar to the fourcc codes above. drm_fourcc.h is considered the
> >>>> + * authoritative source for all of these.
> >>>> + */
> >>>
> >>>
> >>>
> >>> On one side modifiers are supposed to be opaque, but then this suggest
> >>> they
> >>> are supposed to be added in this file and described. Is that right?
> >>
> >>
> >>
> >> correct..   opaque as in basically enum values.
> >>
> >> We do want a description of the format so when someone comes along and
> >> adds a new value, we have a chance of realizing that it is the same as
> >> an existing value, since there are cases where gpu's from different
> >> vendors can support (for example) the same tiling formats.
> >
> >
> > Opaque kind of suggests it is driver private and from that angle definitions
> > and descriptions wouldn't belong in the core uapi header. So I think that's
> > just confusing and I'd drop opaque from the description and just call it a
> > token.
> 
> hmm, to me, 'opaque' just meant 'do not try to interpret the bits'..
> but if that is confusing, I don't mind to just call it a token

Yeah could be misunderstood, luckily both the commit message and code
comments already just talk about tokens. So I think we're good.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel