* [PATCH] drm/i915: Error out when trying to set a y-tiled as a sprite
@ 2012-10-26 17:30 Damien Lespiau
2012-10-26 17:42 ` Jesse Barnes
0 siblings, 1 reply; 6+ messages in thread
From: Damien Lespiau @ 2012-10-26 17:30 UTC (permalink / raw)
To: intel-gfx
From: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
drivers/gpu/drm/i915/intel_sprite.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 3434b6e..aa8d09b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -465,6 +465,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
if (intel_plane->pipe != intel_crtc->pipe)
return -EINVAL;
+ /* Sprite planes can be linear or x-tiled surfaces */
+ if (!(obj->tiling_mode == I915_TILING_NONE ||
+ obj->tiling_mode == I915_TILING_X))
+ return -EINVAL;
+
/*
* 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.
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Error out when trying to set a y-tiled as a sprite
2012-10-26 17:30 [PATCH] drm/i915: Error out when trying to set a y-tiled as a sprite Damien Lespiau
@ 2012-10-26 17:42 ` Jesse Barnes
2012-10-28 10:04 ` Chris Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Jesse Barnes @ 2012-10-26 17:42 UTC (permalink / raw)
To: Damien Lespiau; +Cc: intel-gfx
On Fri, 26 Oct 2012 18:30:50 +0100
Damien Lespiau <damien.lespiau@gmail.com> wrote:
> From: Damien Lespiau <damien.lespiau@intel.com>
>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
> drivers/gpu/drm/i915/intel_sprite.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 3434b6e..aa8d09b 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -465,6 +465,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> if (intel_plane->pipe != intel_crtc->pipe)
> return -EINVAL;
>
> + /* Sprite planes can be linear or x-tiled surfaces */
> + if (!(obj->tiling_mode == I915_TILING_NONE ||
> + obj->tiling_mode == I915_TILING_X))
> + return -EINVAL;
> +
> /*
> * 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.
Do we not catch this when we make the fb? If not we may want to do
this there...
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Error out when trying to set a y-tiled as a sprite
2012-10-26 17:42 ` Jesse Barnes
@ 2012-10-28 10:04 ` Chris Wilson
2012-10-29 15:20 ` Lespiau, Damien
2012-10-29 16:24 ` Ville Syrjälä
0 siblings, 2 replies; 6+ messages in thread
From: Chris Wilson @ 2012-10-28 10:04 UTC (permalink / raw)
To: Jesse Barnes, Damien Lespiau; +Cc: intel-gfx
On Fri, 26 Oct 2012 10:42:54 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Fri, 26 Oct 2012 18:30:50 +0100
> Damien Lespiau <damien.lespiau@gmail.com> wrote:
>
> > From: Damien Lespiau <damien.lespiau@intel.com>
> >
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_sprite.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 3434b6e..aa8d09b 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -465,6 +465,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > if (intel_plane->pipe != intel_crtc->pipe)
> > return -EINVAL;
> >
> > + /* Sprite planes can be linear or x-tiled surfaces */
> > + if (!(obj->tiling_mode == I915_TILING_NONE ||
> > + obj->tiling_mode == I915_TILING_X))
> > + return -EINVAL;
> > +
> > /*
> > * 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.
>
> Do we not catch this when we make the fb? If not we may want to do
> this there...
We do. However, we don't catch userspace changing the tiling on an
existing fb (unless actually pinned)... The extra level of defense is
certainly welcome, and provides documentation at all levels on the known
capabilities of the hw. (The only downside is that we have more guards to
fixup if those ever change, but unlikely ;) The only bikeshed is that
elsewhere we use
switch (obj->tiling_mode) {
case I915_TILING_NONE:
case I915_TILING_X:
break;
default:
return -EINVAL;
}
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: Error out when trying to set a y-tiled as a sprite
2012-10-28 10:04 ` Chris Wilson
@ 2012-10-29 15:20 ` Lespiau, Damien
2012-10-29 16:32 ` Jesse Barnes
2012-10-29 16:24 ` Ville Syrjälä
1 sibling, 1 reply; 6+ messages in thread
From: Lespiau, Damien @ 2012-10-29 15:20 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Sun, Oct 28, 2012 at 10:04 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> We do. However, we don't catch userspace changing the tiling on an
> existing fb (unless actually pinned)... The extra level of defense is
> certainly welcome, and provides documentation at all levels on the known
> capabilities of the hw. (The only downside is that we have more guards to
> fixup if those ever change, but unlikely ;)
I've sent a new version of this one then, turns out it be useful to
catch cases where one changes the tiling mode after the creation of
the fb.
--
Damien
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Error out when trying to set a y-tiled as a sprite
2012-10-29 15:20 ` Lespiau, Damien
@ 2012-10-29 16:32 ` Jesse Barnes
0 siblings, 0 replies; 6+ messages in thread
From: Jesse Barnes @ 2012-10-29 16:32 UTC (permalink / raw)
To: Lespiau, Damien; +Cc: intel-gfx
On Mon, 29 Oct 2012 15:20:15 +0000
"Lespiau, Damien" <damien.lespiau@intel.com> wrote:
> On Sun, Oct 28, 2012 at 10:04 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > We do. However, we don't catch userspace changing the tiling on an
> > existing fb (unless actually pinned)... The extra level of defense is
> > certainly welcome, and provides documentation at all levels on the known
> > capabilities of the hw. (The only downside is that we have more guards to
> > fixup if those ever change, but unlikely ;)
>
> I've sent a new version of this one then, turns out it be useful to
> catch cases where one changes the tiling mode after the creation of
> the fb.
>
Ok sounds good. I'll add my r-b to the latest one.
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Error out when trying to set a y-tiled as a sprite
2012-10-28 10:04 ` Chris Wilson
2012-10-29 15:20 ` Lespiau, Damien
@ 2012-10-29 16:24 ` Ville Syrjälä
1 sibling, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2012-10-29 16:24 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Sun, Oct 28, 2012 at 10:04:54AM +0000, Chris Wilson wrote:
> On Fri, 26 Oct 2012 10:42:54 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > On Fri, 26 Oct 2012 18:30:50 +0100
> > Damien Lespiau <damien.lespiau@gmail.com> wrote:
> >
> > > From: Damien Lespiau <damien.lespiau@intel.com>
> > >
> > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_sprite.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > index 3434b6e..aa8d09b 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -465,6 +465,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > > if (intel_plane->pipe != intel_crtc->pipe)
> > > return -EINVAL;
> > >
> > > + /* Sprite planes can be linear or x-tiled surfaces */
> > > + if (!(obj->tiling_mode == I915_TILING_NONE ||
> > > + obj->tiling_mode == I915_TILING_X))
> > > + return -EINVAL;
> > > +
> > > /*
> > > * 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.
> >
> > Do we not catch this when we make the fb? If not we may want to do
> > this there...
>
> We do. However, we don't catch userspace changing the tiling on an
> existing fb (unless actually pinned)...
Why not simply count how many fbs reference a given bo, and reject
tiling changes when the count > 0?
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-10-29 16:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-26 17:30 [PATCH] drm/i915: Error out when trying to set a y-tiled as a sprite Damien Lespiau
2012-10-26 17:42 ` Jesse Barnes
2012-10-28 10:04 ` Chris Wilson
2012-10-29 15:20 ` Lespiau, Damien
2012-10-29 16:32 ` Jesse Barnes
2012-10-29 16:24 ` Ville Syrjälä
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.