Re: [PATCH] drm/i915/skl: Support Y tiling in MMIO flips

Tvrtko Ursulin
2015-04-21T09:05:50Z

On 04/21/2015 05:55 AM, Jindal, Sonika wrote:
> On 4/20/2015 11:14 PM, Daniel Vetter wrote:
>> On Mon, Apr 20, 2015 at 05:38:20PM +0100, Tvrtko Ursulin wrote:
>>>
>>> On 04/20/2015 05:22 PM, Daniel Vetter wrote:
>>>> On Mon, Apr 20, 2015 at 04:22:48PM +0100, Tvrtko Ursulin wrote:
>>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>>
>>>>> Add Y tiling support to skl_do_mmio_flip.
>>>>>
>>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>> Cc: Damien Lespiau <damien.lespiau@intel.com>
>>>>> Cc: Sonika Jindal <sonika.jindal@intel.com>
>>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>> ---
>>>>> Looks like we completely missed this completely parallel instance
>>>>> of plane
>>>>> programming code when working on Y tiling support!
>>>>>
>>>>> Will rotation need supporting here as well?
>>>>
>>>> Oh dear this is a bit ugly. We definitely need either this for 4.1
>>>> or take
>>>> out the Y-tiling support for 4.1. Rotation only made 4.2, so I think
>>>> we're good, but we do indeed need to take rotation into account here
>>>> too.
>>>
>>> What is ugly, this patch or the overall situation?
>>>
>>>> And it seems like we also need a pile of igt testcases to make sure
>>>> Y-tiling, rotation (even 180 probably) works correctly with
>>>> pageflips. Yay
>>>> for kms_flip growing even more.
>>>
>>> I added Y to kms_flip_tiling back then - but both that one and kms_flip
>>> don't do any pipe crc. So not much use in automated fashion. And
>>> testdisplay
>>> apparently does not hit this path.
>>>
>>> I did not have working crc hw back then, maybe that has changed, I'll
>>> check
>>> it and then, depending on the outcome, see if I can add something
>>> automated.
>>>
>>> Either way, adding this fix sounds better than reverting everything.
>>
>> On 2nd thought skl is still tagged as preliminary, so we don't have any
>> issues here at all. Queued for -next, thanks for the patch.
>>
>> While looking at skl_do_mmio_flip I noticed though that rotation support
>> seems to be completely absent. Or did I miss something?
> Yes, it doesn't exist.
> Is it because of it being temporary as per the following comment:
> "/*
> * XXX: This is the temporary way to update the plane registers until
> we get
> * around to using the usual plane update functions for MMIO flips
> */
> "

Question is what are the prerequisites to start using plane update vfunc
from here? Could we just do it now?

Regards,

Tvrtko