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

[Philipp Zabel]

On Fri, 2017-02-03 at 15:20 +0200, Ville Syrjälä wrote:
> On Fri, Feb 03, 2017 at 10:31:39AM +0100, Philipp Zabel wrote:
> > Use drm_plane_helper_check_state to clip raw user coordinates to crtc
> > bounds. This checks for full plane coverage and scaling already, so
> > we can drop some custom checks. Use the clipped coordinates everywhere.
> > 
> > Suggested-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Signed-off-by: Philipp Zabel <p.zabel at pengutronix.de>
> > ---
> > Changes since v3:
> >  - Disallow target frames that start offscreen (state->crtc_x/y < 0), to avoid
> >    confusing userspace: due to the necessary line start address alignment, we
> >    could only support very specific negative values for crtc_x/y, depending on
> >    the pixel format.
> 
> That's really no different to the user specifying non-zero src
> coordinates. So I'm wondering what's the point of special casing
> crtc coordinates this way because you'll need to check the src
> coordinates anyway.

User expectations.

For the src_x/y / src.x1/y1 source coordinates we have format specific
alignment requirements due to limitations of the DMA unit, there is no
way around it.
For the crtc_x/y plane coordinates there are no alignment requirements
at all, the partial plane can be positioned at any integer x/y position.
If we allow to simulate a partially offscreen plane at negative
positions by clipping and setting src.x1/y1, suddenly the crtc_x/y have
alignment requirements, but only if the values are negative.
I assume that would rather confuse any user space application that tries
to reason about which crtc_x/y values are valid, so it's probably better
to disallow it altogether.

regards
Philipp