Re: [Intel-gfx] [PATCH 06/11] drm/i915: Check the framebuffer offset

2012-11-01 Thread Ville Syrjälä
On Thu, Nov 01, 2012 at 03:18:04PM +0100, Daniel Vetter wrote:
> On Thu, Nov 1, 2012 at 3:09 PM, Ville Syrjälä
>  wrote:
> >> Userspace doesn't use this today at all even in the panning case?
> >
> > If it does, then the user is going to be upset when nothing happens.
> > Only the x/y offsets are effective with the current code.
> >
> >> I
> >> know it worked at one point at least, but that may have been back in
> >> the UMS days...
> >
> > Before my time.
> 
> Afaik the fb->offsets have been added to support planar formats with
> all planes smashed into the same buffer object (where the fourcc alone
> doesn't specify anything about inter-plane offsets). We don't support
> planar buffers, so enforcing an offset of 0 is imo totally ok.

There's also another use case for it. That is, if you specify a source
rectangle for a plane, which is smaller than the full fb, then it's
perfectly legal for the plane to access the pixels outside the source
rectangle to produce good looking filtered edges. But when the source
rectangle edge matches the fb edge, then it's clearly not OK to do
that. So if you want to cut the edges of your source rectangle sharply,
then you can do it through fb->offsets[].

It's similar to texturing w/ GL_CLAMP_TO_EDGE. The texture coordinates
are clamped so that nothing outside the texture is sampled, but
when sampling inside the texture, the filter can blend in texels that
are not inside the area specified by the texture coordinates.

But I supose doing this could be easier if we just added a property
to the plane which specifies how the filtering is done at the edges
of the source rectangle. Then you at least wouldn't need to create
a new fb every time the source rectangle changes.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 06/11] drm/i915: Check the framebuffer offset

2012-11-01 Thread Jesse Barnes
On Thu, 1 Nov 2012 15:18:04 +0100
Daniel Vetter  wrote:

> On Thu, Nov 1, 2012 at 3:09 PM, Ville Syrjälä
>  wrote:
> >> Userspace doesn't use this today at all even in the panning case?
> >
> > If it does, then the user is going to be upset when nothing happens.
> > Only the x/y offsets are effective with the current code.
> >
> >> I
> >> know it worked at one point at least, but that may have been back in
> >> the UMS days...
> >
> > Before my time.
> 
> Afaik the fb->offsets have been added to support planar formats with
> all planes smashed into the same buffer object (where the fourcc alone
> doesn't specify anything about inter-plane offsets). We don't support
> planar buffers, so enforcing an offset of 0 is imo totally ok.

Ok yeah was confusing it with the x/y offsets.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 06/11] drm/i915: Check the framebuffer offset

2012-11-01 Thread Daniel Vetter
On Thu, Nov 1, 2012 at 3:09 PM, Ville Syrjälä
 wrote:
>> Userspace doesn't use this today at all even in the panning case?
>
> If it does, then the user is going to be upset when nothing happens.
> Only the x/y offsets are effective with the current code.
>
>> I
>> know it worked at one point at least, but that may have been back in
>> the UMS days...
>
> Before my time.

Afaik the fb->offsets have been added to support planar formats with
all planes smashed into the same buffer object (where the fourcc alone
doesn't specify anything about inter-plane offsets). We don't support
planar buffers, so enforcing an offset of 0 is imo totally ok.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 06/11] drm/i915: Check the framebuffer offset

2012-11-01 Thread Ville Syrjälä
On Wed, Oct 31, 2012 at 01:26:12PM -0700, Jesse Barnes wrote:
> On Wed, 31 Oct 2012 17:50:19 +0200
> ville.syrj...@linux.intel.com wrote:
> 
> > From: Ville Syrjälä 
> > 
> > The current code can't deal with framebuffers with an offset. Return an
> > error when trying to create such a framebuffer until the rest of the
> > code is fixed to handle them.
> > 
> > Signed-off-by: Ville Syrjälä 
> > ---
> > 
> > I had an earlier version that actually added the handling for the offsets,
> > but as I still haven't managed to write test cases for that, I decided
> > that just refusing any offset is a good enough solution for now.
> > 
> >  drivers/gpu/drm/i915/intel_display.c |4 
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index f431f2a..a3496f5 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8276,6 +8276,10 @@ int intel_framebuffer_init(struct drm_device *dev,
> > return -EINVAL;
> > }
> >  
> > +   /* FIXME need to adjust LINOFF/TILEOFF accordingly. */
> > +   if (mode_cmd->offsets[0] != 0)
> > +   return -EINVAL;
> > +
> > ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
> > if (ret) {
> > DRM_ERROR("framebuffer init failed %d\n", ret);
> 
> Userspace doesn't use this today at all even in the panning case?

If it does, then the user is going to be upset when nothing happens.
Only the x/y offsets are effective with the current code.

> I
> know it worked at one point at least, but that may have been back in
> the UMS days...

Before my time.

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 06/11] drm/i915: Check the framebuffer offset

2012-10-31 Thread Jesse Barnes
On Wed, 31 Oct 2012 17:50:19 +0200
ville.syrj...@linux.intel.com wrote:

> From: Ville Syrjälä 
> 
> The current code can't deal with framebuffers with an offset. Return an
> error when trying to create such a framebuffer until the rest of the
> code is fixed to handle them.
> 
> Signed-off-by: Ville Syrjälä 
> ---
> 
> I had an earlier version that actually added the handling for the offsets,
> but as I still haven't managed to write test cases for that, I decided
> that just refusing any offset is a good enough solution for now.
> 
>  drivers/gpu/drm/i915/intel_display.c |4 
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index f431f2a..a3496f5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8276,6 +8276,10 @@ int intel_framebuffer_init(struct drm_device *dev,
>   return -EINVAL;
>   }
>  
> + /* FIXME need to adjust LINOFF/TILEOFF accordingly. */
> + if (mode_cmd->offsets[0] != 0)
> + return -EINVAL;
> +
>   ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
>   if (ret) {
>   DRM_ERROR("framebuffer init failed %d\n", ret);

Userspace doesn't use this today at all even in the panning case?  I
know it worked at one point at least, but that may have been back in
the UMS days...

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 06/11] drm/i915: Check the framebuffer offset

2012-10-31 Thread ville . syrjala
From: Ville Syrjälä 

The current code can't deal with framebuffers with an offset. Return an
error when trying to create such a framebuffer until the rest of the
code is fixed to handle them.

Signed-off-by: Ville Syrjälä 
---

I had an earlier version that actually added the handling for the offsets,
but as I still haven't managed to write test cases for that, I decided
that just refusing any offset is a good enough solution for now.

 drivers/gpu/drm/i915/intel_display.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f431f2a..a3496f5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8276,6 +8276,10 @@ int intel_framebuffer_init(struct drm_device *dev,
return -EINVAL;
}
 
+   /* FIXME need to adjust LINOFF/TILEOFF accordingly. */
+   if (mode_cmd->offsets[0] != 0)
+   return -EINVAL;
+
ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs);
if (ret) {
DRM_ERROR("framebuffer init failed %d\n", ret);
-- 
1.7.8.6

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx