On Tue, Mar 06, 2018 at 01:57:26PM +0200, Ville Syrjälä wrote:
On Mon, Mar 05, 2018 at 11:41:02PM +0200, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
To make life easier for drivers, let's have the core check that the requested pixel format is supported by at least one plane when creating a new framebuffer.
v2: Accept anyformat if the driver doesn't do planes (Eric) s/planes_have_format/any_plane_has_format/ (Eric) Check the modifier as well since we already have a function that does both
Cc: Eric Anholt eric@anholt.net Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_framebuffer.c | 43 +++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index c0530a1af5e3..b9a33e3f13e9 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -152,12 +152,37 @@ static int fb_plane_height(int height, return DIV_ROUND_UP(height, format->vsub); }
+static bool any_plane_has_format(struct drm_device *dev,
u32 format, u64 modifier)+{
- struct drm_plane *plane;
- /* TODO: maybe maintain a device level format list? */
- drm_for_each_plane(plane, dev) {
/** In case the driver doesn't really do* planes we have to accept any format here.*/if (plane->format_default)return true;if (drm_plane_check_pixel_format(plane, format, modifier) == 0)return true;- }
- return false;
+}
static int framebuffer_check(struct drm_device *dev, const struct drm_mode_fb_cmd2 *r) { const struct drm_format_info *info;
u64 modifier = 0; int i;
if (r->flags & DRM_MODE_FB_MODIFIERS)
modifier = r->modifier[0];And of course this isn't quite correct. For the !modifiers case we're now checking if any plane supports the format with the linear modifier, but the actual modifier will only be picked by the driver later.
Not quite sure how I would write this in a way that really makes sense. Might have to make it the driver's responsibility to call any_plane_has_format() instead...
Hmm. One idea would be to stick the check into drm_framebuffer_init(), but I'm not comfortable with that since the driver may well be doing things in its .fb_create() hook that expects the format+modifier to have been validated already, and drm_framebuffer_init() should only be called once everything is ready.
I guess I'll just export drm_any_plane_has_format() and add some kerneldoc for .fb_create() to let people know about it. Better ideas are welcome of course.
- /* check if the format is supported at all */ info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN); if (!info) {
@@ -168,6 +193,15 @@ static int framebuffer_check(struct drm_device *dev, return -EINVAL; }
- if (!any_plane_has_format(dev, r->pixel_format, modifier)) {
struct drm_format_name_buf format_name;DRM_DEBUG_KMS("unsupported pixel format %s, modifier 0x%llx\n",drm_get_format_name(r->pixel_format, &format_name),modifier);return -EINVAL;- }
- /* now let the driver pick its own format info */ info = drm_get_format_info(dev, r);
@@ -202,14 +236,7 @@ static int framebuffer_check(struct drm_device *dev, return -EINVAL; }
if (r->modifier[i] && !(r->flags & DRM_MODE_FB_MODIFIERS)) {DRM_DEBUG_KMS("bad fb modifier %llu for plane %d\n",r->modifier[i], i);return -EINVAL;}if (r->flags & DRM_MODE_FB_MODIFIERS &&r->modifier[i] != r->modifier[0]) {
if (r->modifier[i] != modifier) { DRM_DEBUG_KMS("bad fb modifier %llu for plane %d\n", r->modifier[i], i); return -EINVAL;-- 2.16.1
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- Ville Syrjälä Intel OTC