linux-kernel - Re: [PATCH 0/4] drm/atmel-hlcdc: fix plane clipping/rotation issues

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 11 Jan 2019 09:16:53 +0000
From:   Peter Rosin <peda@...ntia.se>
To:     Sam Ravnborg <sam@...nborg.org>,
        Boris Brezillon <bbrezillon@...nel.org>
CC:     Alexandre Belloni <alexandre.belloni@...tlin.com>,
        David Airlie <airlied@...ux.ie>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Boris Brezillon <boris.brezillon@...tlin.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 0/4] drm/atmel-hlcdc: fix plane clipping/rotation issues

On 2019-01-10 21:16, Sam Ravnborg wrote:
> Hi Peter.
> 
> (Hijacking this thread as I lost the orginal mails)

Assuming you wanted to reply to this patch?
https://patchwork.kernel.org/patch/10753571/

>>> I found an unfortunate issue while recoding plane handling to use
>>> drm_atomic_helper_check_plane_state(). The driver rotates clockwise,
>>> which is not correct. I simply fixed it (patch 1/4), but maybe that
>>> will cause regressions for unsuspecting users who simply assumed
>>> that the clockwise rotation was correct? I don't know what to do
>>> about that? Adding an option to get the old broken behavior seems
>>> useless, wouldn't it be just as easy to just fix whatever app to
>>> rotate the other way instead of adding an option somewhere?
>>
>> Hm, rotation support has been added before the standard rotation
>> property was created, and at that time I assumed rotation was clockwise
>> (which apparently was an unwise choice). Anyway, I don't have a
>> solution for this problem, so I'll let Nicolas decide if it's
>> acceptable to change the rotation behavior.
>>
>>> I have only tested this series on sama5d3, but I did check the docs
>>> for various other chips (sama5d2, sama5d4, sam9n12, sam9g15, sam9g35
>>> and sam9x35) supported by the driver (relevant to patch 4/4).
> 
> I wonder if, when this code path is anyway touched, could benefit
> from drm_rect_rotate().
> 
> It is obviously not a simple replacement, but could it be used then
> I hope the resulting code is simpler.

What are you referring to that goes beyond what is done in patch 3/4
in this series? After setting up the strides, the only use of src_[xywh]
are to calculate the scaling factors, and for that the position is
irrelevant. I.e. src_x and src_y are not used. Sure, in some theoretical
sense it might be good if src_[xy] are also transformed into the rotated
coordinate system along with src_[wh], but it seems a bit backwards to
switch over to struct drm_rect when the interesting properties are the
width and height, not the coordinates of the corners. No strong feelings
on the issue though...

Cheers,
Peter