Re: [PATCH 5/7] drm/i915/skl: Support secondary (rotated) frame buffer mapping

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 5/7] drm/i915/skl: Support secondary (rotated) frame buffer mapping
Date: Fri, 20 Mar 2015 13:44:36 +0000	[thread overview]
Message-ID: <550C2444.4020908@linux.intel.com> (raw)
In-Reply-To: <1426858290.11755.16.camel@jlahtine-mobl1>

On 03/20/2015 01:31 PM, Joonas Lahtinen wrote:

[snip]

>>>> Hm, rotation info is not considered in comparing views, it is just a
>>>> bucket of data passed around between layers. So I suppose private data
>>>> under your design. Since there is no private union yet, maybe do this later?
>>>
>>> Why not? Isn't a 270 degree rotated view substantially different from a
>>> 90 degree rotated view (even when the difference technically is just
>>> some bit flip somewhere else).
>>>
>>> At least I would be pretty upset if I was returned the address for 90
>>> degree rotated view when I wanted 270 rotated. If multiple rotated views
>>> are not possible, then it is again an implicit thing.
>>>
>>> There are quite a lot of hardware constraints like this that appear in
>>> the code implicitly, which IMHO makes the code hard to follow at times.
>>> So I'd try to make it more explicit that the views are not the same,
>>> there just can be one rotated view at a time (if that is the case).
>>
>> 90 and 270 views are indeed the same page layout - same address for
>> scanout. And there can only be one such VMA for an object at a time.
>>
>> But how this mapping needs to look like is determined by more than the
>> object itself - framebuffer geometry defines it. The private data in the
>> view is used to transfer that meta-data so the GTT core can build the
>> appropriate view.
>>
>
> Right, I think I understand your viewpoint now. I would still prefer it
> to be even more explicit like I915_GGTT_VIEW_Yf_SCANOUT, because what it
> really does is rearranges the pages to layout suitable for rotated
> scanout, not making the view rotated in itself.

Ha, not sure what you mean, but Yf is definitely not the right name for 
it since it is a different thing altogether.

It is rotated in a way that tiles (==pages) are rotated by 90 degrees. 
But content within the tile is not (display engine handles that bit). In 
your mind that would maybe be 
I915_GGTT_VIEW_ROTATED_PAGES_FOR_ROTATED_SCANOUT? :) I am guessing only, 
but I still think I915_GGTT_VIEW_ROTATED is fine.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx