| Message ID | 1355869108-4917-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 18, 2012 at 10:18:28PM +0000, Chris Wilson wrote: > We make no attempt to fixup the source as we adjust the destination > rectangle, expect the caller to have already fixed that up. This makes > no sense, asking userspace to compensate for the kernel adjusting the > one rectangle not the other, so simply reject any attempt to show an > partially visible box. Userspace should know better. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> I guess this makes sense for now, given that the only user we have (ddx) needs to copy things around anyway ... Still, some common users and testcases across drivers would be nice for this </wishful-thinking>. Applied to fixes. -Daniel > --- > drivers/gpu/drm/i915/intel_sprite.c | 31 ++++++------------------------- > 1 file changed, 6 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 22d65f7..97866c2 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -498,31 +498,13 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > } > > /* > - * Clamp the width & height into the visible area. Note we don't > - * try to scale the source if part of the visible region is offscreen. > - * The caller must handle that by adjusting source offset and size. > + * Reject any attempt to display video outside the visible area. > + * The caller must handle this by adjusting source offset and size. > */ > - if ((crtc_x < 0) && ((crtc_x + crtc_w) > 0)) { > - crtc_w += crtc_x; > - crtc_x = 0; > - } > - if ((crtc_x + crtc_w) <= 0) /* Nothing to display */ > - goto out; > - if ((crtc_x + crtc_w) > primary_w) > - crtc_w = primary_w - crtc_x; > - > - if ((crtc_y < 0) && ((crtc_y + crtc_h) > 0)) { > - crtc_h += crtc_y; > - crtc_y = 0; > - } > - if ((crtc_y + crtc_h) <= 0) /* Nothing to display */ > - goto out; > - if (crtc_y + crtc_h > primary_h) > - crtc_h = primary_h - crtc_y; > - > - if (!crtc_w || !crtc_h) /* Again, nothing to display */ > - goto out; > - > + if (crtc_x < 0 || crtc_x + crtc_w > primary_w) > + return -EINVAL; > + if (crtc_y < 0 || crtc_y + crtc_h > primary_h) > + return -EINVAL; > /* > * We may not have a scaler, eg. HSW does not have it any more > */ > @@ -571,7 +553,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > > out_unlock: > mutex_unlock(&dev->struct_mutex); > -out: > return ret; > } > > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Jan 08, 2013 at 12:11:38PM +0100, Daniel Vetter wrote: > On Tue, Dec 18, 2012 at 10:18:28PM +0000, Chris Wilson wrote: > > We make no attempt to fixup the source as we adjust the destination > > rectangle, expect the caller to have already fixed that up. This makes > > no sense, asking userspace to compensate for the kernel adjusting the > > one rectangle not the other, so simply reject any attempt to show an > > partially visible box. Userspace should know better. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > I guess this makes sense for now, given that the only user we have (ddx) > needs to copy things around anyway ... Still, some common users and > testcases across drivers would be nice for this </wishful-thinking>. > > Applied to fixes. Dropped again, since Ville has a more complete solution to this in his queue. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index 22d65f7..97866c2 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -498,31 +498,13 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, } /* - * Clamp the width & height into the visible area. Note we don't - * try to scale the source if part of the visible region is offscreen. - * The caller must handle that by adjusting source offset and size. + * Reject any attempt to display video outside the visible area. + * The caller must handle this by adjusting source offset and size. */ - if ((crtc_x < 0) && ((crtc_x + crtc_w) > 0)) { - crtc_w += crtc_x; - crtc_x = 0; - } - if ((crtc_x + crtc_w) <= 0) /* Nothing to display */ - goto out; - if ((crtc_x + crtc_w) > primary_w) - crtc_w = primary_w - crtc_x; - - if ((crtc_y < 0) && ((crtc_y + crtc_h) > 0)) { - crtc_h += crtc_y; - crtc_y = 0; - } - if ((crtc_y + crtc_h) <= 0) /* Nothing to display */ - goto out; - if (crtc_y + crtc_h > primary_h) - crtc_h = primary_h - crtc_y; - - if (!crtc_w || !crtc_h) /* Again, nothing to display */ - goto out; - + if (crtc_x < 0 || crtc_x + crtc_w > primary_w) + return -EINVAL; + if (crtc_y < 0 || crtc_y + crtc_h > primary_h) + return -EINVAL; /* * We may not have a scaler, eg. HSW does not have it any more */ @@ -571,7 +553,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, out_unlock: mutex_unlock(&dev->struct_mutex); -out: return ret; }
We make no attempt to fixup the source as we adjust the destination rectangle, expect the caller to have already fixed that up. This makes no sense, asking userspace to compensate for the kernel adjusting the one rectangle not the other, so simply reject any attempt to show an partially visible box. Userspace should know better. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_sprite.c | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-)