Unfortunately old userspace didn't clear this properly, but since we've added fb modifiers that's fixed. Checking properly that unused fields is important for abi extensions, and just right now there's a bunch of discussions going on about how exactly the additional aux planes for render compression should be specified.
So let's first make sure that the values in those additional fields can be indeed used.
Cc: Thierry Reding thierry.reding@gmail.com Testcase: igt/kms_addfb/unused-* Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/drm_crtc.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 2e26988a9762..8ad582f4026b 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3308,6 +3308,32 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) } }
+ for (i = num_planes; i < 4; i++) { + if (r->modifier[i]) { + DRM_DEBUG_KMS("non-zero modifier for unused plane %d\n", i); + return -EINVAL; + } + + /* Pre-FB_MODIFIERS userspace didn't clear the structs properly. */ + if (!(r->flags & DRM_MODE_FB_MODIFIERS)) + continue; + + if (r->handles[i]) { + DRM_DEBUG_KMS("buffer object handle for unused plane %d\n", i); + return -EINVAL; + } + + if (r->pitches[i]) { + DRM_DEBUG_KMS("non-zero pitch for unused plane %d\n", i); + return -EINVAL; + } + + if (r->offsets[i]) { + DRM_DEBUG_KMS("non-zero offset for unused plane %d\n", i); + return -EINVAL; + } + } + return 0; }
On Wed, May 20, 2015 at 04:53:53PM +0200, Daniel Vetter wrote:
Unfortunately old userspace didn't clear this properly, but since we've added fb modifiers that's fixed. Checking properly that unused fields is important for abi extensions, and just right now there's a bunch of discussions going on about how exactly the additional aux planes for render compression should be specified.
So let's first make sure that the values in those additional fields can be indeed used.
Cc: Thierry Reding thierry.reding@gmail.com Testcase: igt/kms_addfb/unused-* Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/drm_crtc.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
I think this is fine for now. There's still some unclarity about how exactly to pass in auxiliary planes, but in order to do so we need to prevent userspace from passing in garbage, so:
Reviewed-by: Thierry Reding treding@nvidia.com
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 2e26988a9762..8ad582f4026b 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3308,6 +3308,32 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) } }
- for (i = num_planes; i < 4; i++) {
if (r->modifier[i]) {DRM_DEBUG_KMS("non-zero modifier for unused plane %d\n", i);return -EINVAL;}/* Pre-FB_MODIFIERS userspace didn't clear the structs properly. */if (!(r->flags & DRM_MODE_FB_MODIFIERS))continue;if (r->handles[i]) {DRM_DEBUG_KMS("buffer object handle for unused plane %d\n", i);return -EINVAL;}if (r->pitches[i]) {DRM_DEBUG_KMS("non-zero pitch for unused plane %d\n", i);return -EINVAL;}if (r->offsets[i]) {DRM_DEBUG_KMS("non-zero offset for unused plane %d\n", i);return -EINVAL;}- }
- return 0;
}
-- 2.1.4
On Tue, May 26, 2015 at 02:38:44PM +0200, Thierry Reding wrote:
On Wed, May 20, 2015 at 04:53:53PM +0200, Daniel Vetter wrote:
Unfortunately old userspace didn't clear this properly, but since we've added fb modifiers that's fixed. Checking properly that unused fields is important for abi extensions, and just right now there's a bunch of discussions going on about how exactly the additional aux planes for render compression should be specified.
So let's first make sure that the values in those additional fields can be indeed used.
Cc: Thierry Reding thierry.reding@gmail.com Testcase: igt/kms_addfb/unused-* Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/drm_crtc.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
I think this is fine for now. There's still some unclarity about how exactly to pass in auxiliary planes, but in order to do so we need to prevent userspace from passing in garbage, so:
Reviewed-by: Thierry Reding treding@nvidia.com
Thanks for the review, patch merged. -Daniel
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 2e26988a9762..8ad582f4026b 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3308,6 +3308,32 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r) } }
- for (i = num_planes; i < 4; i++) {
if (r->modifier[i]) {DRM_DEBUG_KMS("non-zero modifier for unused plane %d\n", i);return -EINVAL;}/* Pre-FB_MODIFIERS userspace didn't clear the structs properly. */if (!(r->flags & DRM_MODE_FB_MODIFIERS))continue;if (r->handles[i]) {DRM_DEBUG_KMS("buffer object handle for unused plane %d\n", i);return -EINVAL;}if (r->pitches[i]) {DRM_DEBUG_KMS("non-zero pitch for unused plane %d\n", i);return -EINVAL;}if (r->offsets[i]) {DRM_DEBUG_KMS("non-zero offset for unused plane %d\n", i);return -EINVAL;}- }
- return 0;
}
-- 2.1.4
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Daniel Vetter -
Thierry Reding