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@xxxxxxxxxxxxxxx> > > Signed-off-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> > > --- > > 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel