Re: [Intel-gfx] [PATCH 4/4] drm/vc4: Validate framebuffer pixel format/modifier
On Fri, Mar 09, 2018 at 12:54:43PM -0800, Eric Anholt wrote: > Ville Syrjalawrites: > > > From: Ville Syrj??l?? > > > > Only create framebuffers with supported format/modifier combinations by > > checking that at least one plane supports the requested combination. > > > > Using drm_any_plane_has_format() is somewhat suboptimal for vc4 since > > the planes have (mostly) uniform capabilities. But I was lazy and > > didn't feel like exporting drm_plane_format_check() and hand rolling > > anything better. Also I really just wanted to come up with another > > user for drm_any_plane_has_format() ;) > > I'm not excited about vc4 having error-return behavior that other > drivers don't have. Actually, I don't really see why we should be doing > this check in fb create at all, given that you have to do so again at > atomic_check time with the specific plane. > > Could you just delete the i915 fb format check code? Yeah I thought we agreed that any driver not using the compat primary plane helper will get this checking by default? That seems simplest, and safest too. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/vc4: Validate framebuffer pixel format/modifier
On Fri, Mar 09, 2018 at 12:54:43PM -0800, Eric Anholt wrote: > Ville Syrjalawrites: > > > From: Ville Syrjälä > > > > Only create framebuffers with supported format/modifier combinations by > > checking that at least one plane supports the requested combination. > > > > Using drm_any_plane_has_format() is somewhat suboptimal for vc4 since > > the planes have (mostly) uniform capabilities. But I was lazy and > > didn't feel like exporting drm_plane_format_check() and hand rolling > > anything better. Also I really just wanted to come up with another > > user for drm_any_plane_has_format() ;) > > I'm not excited about vc4 having error-return behavior that other > drivers don't have. Actually, I don't really see why we should be doing > this check in fb create at all, given that you have to do so again at > atomic_check time with the specific plane. > > Could you just delete the i915 fb format check code? I don't want unsupported formats to get anywhere near the driver code. Makes life much less stressful when you know what can and can't get in. Also I see no good reason to allow userspace to create fbs it can't actually use later on. Much better to reject early and tell userspace to pick another format. I imagine pre-blobifier userspace could even use this as a way to probe what's supported by the driver. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/vc4: Validate framebuffer pixel format/modifier
Ville Syrjalawrites: > From: Ville Syrjälä > > Only create framebuffers with supported format/modifier combinations by > checking that at least one plane supports the requested combination. > > Using drm_any_plane_has_format() is somewhat suboptimal for vc4 since > the planes have (mostly) uniform capabilities. But I was lazy and > didn't feel like exporting drm_plane_format_check() and hand rolling > anything better. Also I really just wanted to come up with another > user for drm_any_plane_has_format() ;) I'm not excited about vc4 having error-return behavior that other drivers don't have. Actually, I don't really see why we should be doing this check in fb create at all, given that you have to do so again at atomic_check time with the specific plane. Could you just delete the i915 fb format check code? signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/vc4: Validate framebuffer pixel format/modifier
From: Ville SyrjäläOnly create framebuffers with supported format/modifier combinations by checking that at least one plane supports the requested combination. Using drm_any_plane_has_format() is somewhat suboptimal for vc4 since the planes have (mostly) uniform capabilities. But I was lazy and didn't feel like exporting drm_plane_format_check() and hand rolling anything better. Also I really just wanted to come up with another user for drm_any_plane_has_format() ;) Compile tested only. Cc: Eric Anholt Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/vc4/vc4_kms.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c index ba60153dddb5..b6f15102dda0 100644 --- a/drivers/gpu/drm/vc4/vc4_kms.c +++ b/drivers/gpu/drm/vc4/vc4_kms.c @@ -184,6 +184,17 @@ static struct drm_framebuffer *vc4_fb_create(struct drm_device *dev, mode_cmd = _cmd_local; } + if (!drm_any_plane_has_format(dev, mode_cmd->pixel_format, + mode_cmd->modifier[0])) { + struct drm_format_name_buf format_name; + + DRM_DEBUG_KMS("unsupported pixel format %s / modifier 0x%llx\n", + drm_get_format_name(mode_cmd->pixel_format, + _name), + mode_cmd->modifier[0]); + return ERR_PTR(-EINVAL); + } + return drm_gem_fb_create(dev, file_priv, mode_cmd); } -- 2.16.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx