| Message ID | 1351698624-26626-7-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 31 Oct 2012 17:50:19 +0200 ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > 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ä <ville.syrjala@linux.intel.com> > --- > > 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...
On Wed, Oct 31, 2012 at 01:26:12PM -0700, Jesse Barnes wrote: > On Wed, 31 Oct 2012 17:50:19 +0200 > ville.syrjala@linux.intel.com wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > 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ä <ville.syrjala@linux.intel.com> > > --- > > > > 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.
On Thu, Nov 1, 2012 at 3:09 PM, Ville Syrjälä <ville.syrjala@linux.intel.com> 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
On Thu, 1 Nov 2012 15:18:04 +0100 Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Nov 1, 2012 at 3:09 PM, Ville Syrjälä > <ville.syrjala@linux.intel.com> 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.
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ä > <ville.syrjala@linux.intel.com> 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.
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);