| Message ID | 20181019105752.17741-6-alexandru-cosmin.gheorghe@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add method to describe tile/bit_level_packed formats | expand |
Hi Alex, On Fri, Oct 19, 2018 at 11:57:48AM +0100, Alexandru Gheorghe wrote: >For formats that are supported only with non-linear modifiers it >doesn't make to much sense to define cpp or char_per_block, so that >will be set to 0. > >This patch adds a restriction to force having a modifier attached when >cpp/char_per_block is 0, and to bypass checking the pitch restriction. > >This had been discussed here. >[1] https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2018-09-13&show_html=true > >Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> >--- > drivers/gpu/drm/drm_framebuffer.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > >diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c >index 6aca8a1ccdb6..e346d0ad92e0 100644 >--- a/drivers/gpu/drm/drm_framebuffer.c >+++ b/drivers/gpu/drm/drm_framebuffer.c >@@ -195,8 +195,13 @@ static int framebuffer_check(struct drm_device *dev, > for (i = 0; i < info->num_planes; i++) { > unsigned int width = fb_plane_width(r->width, info, i); > unsigned int height = fb_plane_height(r->height, info, i); >+ unsigned int block_size = info->char_per_block[i]; > u64 min_pitch = drm_format_info_min_pitch(info, i, width); > >+ if (!block_size && (r->modifier[i] == DRM_FORMAT_MOD_LINEAR)) { >+ DRM_DEBUG_KMS("Format requires non-linear modifier for plane %d\n", i); >+ return -EINVAL; >+ } You could probably move that blank like from Patch 2 to here ;-) Otherwise LGTM: Reviewed By: Brian Starkey <brian.starkey@arm.com> > if (!r->handles[i]) { > DRM_DEBUG_KMS("no buffer object handle for plane %d\n", i); > return -EINVAL; >@@ -208,7 +213,7 @@ static int framebuffer_check(struct drm_device *dev, > if ((uint64_t) height * r->pitches[i] + r->offsets[i] > UINT_MAX) > return -ERANGE; > >- if (r->pitches[i] < min_pitch) { >+ if (block_size && r->pitches[i] < min_pitch) { > DRM_DEBUG_KMS("bad pitch %u for plane %d\n", r->pitches[i], i); > return -EINVAL; > } >-- >2.18.0 >
On Fri, Oct 19, 2018 at 02:21:46PM +0100, Brian Starkey wrote: > Hi Alex, > > On Fri, Oct 19, 2018 at 11:57:48AM +0100, Alexandru Gheorghe wrote: > >For formats that are supported only with non-linear modifiers it > >doesn't make to much sense to define cpp or char_per_block, so that > >will be set to 0. > > > >This patch adds a restriction to force having a modifier attached when > >cpp/char_per_block is 0, and to bypass checking the pitch restriction. > > > >This had been discussed here. > >[1] https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2018-09-13&show_html=true > > > >Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> > >--- > >drivers/gpu/drm/drm_framebuffer.c | 7 ++++++- > >1 file changed, 6 insertions(+), 1 deletion(-) > > > >diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c > >index 6aca8a1ccdb6..e346d0ad92e0 100644 > >--- a/drivers/gpu/drm/drm_framebuffer.c > >+++ b/drivers/gpu/drm/drm_framebuffer.c > >@@ -195,8 +195,13 @@ static int framebuffer_check(struct drm_device *dev, > > for (i = 0; i < info->num_planes; i++) { > > unsigned int width = fb_plane_width(r->width, info, i); > > unsigned int height = fb_plane_height(r->height, info, i); > >+ unsigned int block_size = info->char_per_block[i]; > > u64 min_pitch = drm_format_info_min_pitch(info, i, width); > > > >+ if (!block_size && (r->modifier[i] == DRM_FORMAT_MOD_LINEAR)) { > >+ DRM_DEBUG_KMS("Format requires non-linear modifier for plane %d\n", i); > >+ return -EINVAL; > >+ } > > You could probably move that blank like from Patch 2 to here ;-) > Otherwise LGTM: I blame rebase :) > > Reviewed By: Brian Starkey <brian.starkey@arm.com> > > > if (!r->handles[i]) { > > DRM_DEBUG_KMS("no buffer object handle for plane %d\n", i); > > return -EINVAL; > >@@ -208,7 +213,7 @@ static int framebuffer_check(struct drm_device *dev, > > if ((uint64_t) height * r->pitches[i] + r->offsets[i] > UINT_MAX) > > return -ERANGE; > > > >- if (r->pitches[i] < min_pitch) { > >+ if (block_size && r->pitches[i] < min_pitch) { > > DRM_DEBUG_KMS("bad pitch %u for plane %d\n", r->pitches[i], i); > > return -EINVAL; > > } > >-- > >2.18.0 > >
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index 6aca8a1ccdb6..e346d0ad92e0 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -195,8 +195,13 @@ static int framebuffer_check(struct drm_device *dev, for (i = 0; i < info->num_planes; i++) { unsigned int width = fb_plane_width(r->width, info, i); unsigned int height = fb_plane_height(r->height, info, i); + unsigned int block_size = info->char_per_block[i]; u64 min_pitch = drm_format_info_min_pitch(info, i, width); + if (!block_size && (r->modifier[i] == DRM_FORMAT_MOD_LINEAR)) { + DRM_DEBUG_KMS("Format requires non-linear modifier for plane %d\n", i); + return -EINVAL; + } if (!r->handles[i]) { DRM_DEBUG_KMS("no buffer object handle for plane %d\n", i); return -EINVAL; @@ -208,7 +213,7 @@ static int framebuffer_check(struct drm_device *dev, if ((uint64_t) height * r->pitches[i] + r->offsets[i] > UINT_MAX) return -ERANGE; - if (r->pitches[i] < min_pitch) { + if (block_size && r->pitches[i] < min_pitch) { DRM_DEBUG_KMS("bad pitch %u for plane %d\n", r->pitches[i], i); return -EINVAL; }
For formats that are supported only with non-linear modifiers it doesn't make to much sense to define cpp or char_per_block, so that will be set to 0. This patch adds a restriction to force having a modifier attached when cpp/char_per_block is 0, and to bypass checking the pitch restriction. This had been discussed here. [1] https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2018-09-13&show_html=true Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com> --- drivers/gpu/drm/drm_framebuffer.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)