On Tue, 2017-02-07 at 13:19 +0200, Ville Syrjälä wrote: > On Tue, Feb 07, 2017 at 09:51:50AM +0100, Philipp Zabel wrote: > > 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. > > I don't really agree. Userspace has to be prepared for anything > to fail really. Whether it fails every time or sometimes is not that > much different IMO. Given the simple return value userspace can't > really draw any real conclusions about the reason for the failure. So you say we should allow negative crtc_x, even if the valid range with something on the screen ist [ - plane width ... crtc width ] with the [- plane width ... 0] range in steps of 2, 4, or 8 pixels depending on the pixel format and the [0 ... crtc width] range in steps of 1 pixels? > A related question is how strict we should really be. In the past we've > been very lax in fudging the coordinates to make something appear on the > screen. Probably I went a bit too far in the laxness actually, and with > atomic we want to tighten things up a little. Just no one has managed to > really answer how tight we should make things. Is it OK to round things > to pixel boundaries perhaps? In my opinion: yes. > Maybe even macropixel boundaries? Maybe even memory burst size / line start address boundaries? regards Philipp _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
