[PATCH] drm/omap: fix plane rotation

[Rob Clark]

On Mon, Apr 7, 2014 at 6:32 AM, Grazvydas Ignotas <notasas at gmail.com> wrote:
> On Sun, Apr 6, 2014 at 12:45 AM, Rob Clark <robdclark at gmail.com> wrote:
>> On Sat, Apr 5, 2014 at 2:33 PM, Grazvydas Ignotas <notasas at gmail.com> wrote:
>>> Plane rotation with omapdrm is currently broken.
>>> It seems omap_plane_mode_set() expects width and height in screen
>>> coordinates, so pass it like that.
>>>
>>> Cc: Rob Clark <robdclark at gmail.com>
>>> Signed-off-by: Grazvydas Ignotas <notasas at gmail.com>
>>> ---
>>>  drivers/gpu/drm/omapdrm/omap_plane.c |    8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
>>> index 370580c..5611f15 100644
>>> --- a/drivers/gpu/drm/omapdrm/omap_plane.c
>>> +++ b/drivers/gpu/drm/omapdrm/omap_plane.c
>>> @@ -253,6 +253,14 @@ static int omap_plane_update(struct drm_plane *plane,
>>>
>>>         drm_framebuffer_reference(fb);
>>>
>>> +       /* omap_plane_mode_set() takes adjusted src */
>>> +       switch (omap_plane->win.rotation & 0xf) {
>>> +       case BIT(DRM_ROTATE_90):
>>> +       case BIT(DRM_ROTATE_270):
>>> +               swap(src_w, src_h);
>>> +               break;
>>> +       }
>>> +
>>
>> hmm, I think that the better thing would be to do this in
>> omap_framebuffer_update_scanout().  In fact we do already swap
>> src_w/src_h there.  Only we don't swap the width/height parameters we
>> pass down to omapdss.  Not quite sure if something changed in omapdss
>> with regards to rotation_type, but keeping it with the rest of the
>> rotation related maths in _update_scanout() seems cleaner.
>
> But then it has to know somehow if apply was triggered from
> omap_crtc_mode_set() or omap_plane_update(). The problem is
> omap_crtc_mode_set() passes rotated width/height (and crtc rotation
> works correctly), but omap_plane_update() uses unrotated width/height
> (so plane rotation is currently broken).


Something still seems a bit suspicious..  drm core is not swapping
width/height for crtc (other than for validating the configuration...
which might also be needed for planes).  I guess it is getting
already-rotated width/height from userspace.  So probably we want to
do the same for planes, so it might be a good idea to move
crtc->invert_dimensions into plane.


BR,
-R.

>
> Gražvydas