[PATCH 10/10] drm/imx: ipuv3-plane: use drm_plane_helper_check_state, clipped coordinates

[Ying Liu]

On Thu, Oct 20, 2016 at 9:29 PM, Philipp Zabel <p.zabel at pengutronix.de> wrote:
> Am Donnerstag, den 20.10.2016, 16:51 +0800 schrieb Ying Liu:
>> >> Does the clip thing potentially change the user's request by force?
>> >> For example, the user request an unreasonable big resolution.
>> >
>> > The user is allowed to ask for destination coordinates extending outside
>> > the crtc dimensions. This will chop off the parts that aren't visible,
>> > and it will chop off the corresponding areas of the source as well.
>>
>> How about returning -EINVAL in this case which stands for
>> an atomic check failure?
>
> Say the user requests to display a 640x480+0,0 source framebuffer at
> destination offset -320,0 on a 320x240 screen, unscaled. The expectation
> would be to see the upper right quarter of the framebuffer on the
> screen, at least if the hardware was actually able to position overlays
> partially offscreen.
> If we can also fulfill that expectation by clipping the source rectangle
> to 320,240+320,0 and changing the destination rectangle to 320x240+0,0,
> why should -EINVAL be returned?

Well, IIUC, there are two kinds of clipping.
1) Clipping a rectangle from a fb according to src_x/y and src_w/h.
2) Clipping done by drm_plane_helper_check_state(), which potentially
    changes src/dst->x1/2 and src/dst->y1/2(in other words, src_x/y,
    src_w/h and crtc_x/y/w/h, though not directly).

1) is fine, no problem.
I doubt 2) is wrong as the users' original request could be changed.
That's why I mentioned returning -EINVAL.

Moreover, before and after applying the patch, I think the
->atomic_check behavior consistency is broken. For example,
negative crtc_x or crtc_y for overlay are changed from unacceptable
to potentially acceptable just because 2) may change their equivalent
dst_>x/y1.

Regards,
Liu Ying

>
> regards
> Philipp
>