On Mon, Apr 7, 2014 at 6:32 AM, Grazvydas Ignotas <notasas@xxxxxxxxx> wrote:
> On Sun, Apr 6, 2014 at 12:45 AM, Rob Clark <robdclark@xxxxxxxxx> wrote:
>> On Sat, Apr 5, 2014 at 2:33 PM, Grazvydas Ignotas <notasas@xxxxxxxxx> 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@xxxxxxxxx>
>>> Signed-off-by: Grazvydas Ignotas <notasas@xxxxxxxxx>
>>> ---
>>> 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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
