[PATCH] drm/omap: fix plane rotation

[Grazvydas Ignotas]

Gražvydas


On Mon, Apr 7, 2014 at 8:17 PM, Rob Clark <robdclark at gmail.com> wrote:
> 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.

OK I guess I said it wrong..
The display I have is 1080x1920 portrait panel.
For omap_crtc_mode_set(), hdisplay is always 1080 and vdisplay, is
1920, regardless of rotation. That is passed over to
omap_plane_mode_set() and everything works.
But in omap_plane_update(), src_w and src_h are passed instead, which
correspond to framebuffer size and not display? At least
drmModeSetPlane(), where those values originate, seems to expect
framebuffer dimensions, but passing 1920, 1080 there (with 90 deg.
rotation set) results in corruption on screen caused by bad base
address for hardware overlay without my patch.


Gražvydas