drm: Nuke modifier[1-3]
authorVille Syrjälä <ville.syrjala@linux.intel.com>
Wed, 16 Nov 2016 11:33:16 +0000 (13:33 +0200)
committerJianxin Pan <jianxin.pan@amlogic.com>
Wed, 15 Aug 2018 02:41:01 +0000 (19:41 -0700)
It has been suggested that having per-plane modifiers is making life
more difficult for userspace, so let's just retire modifier[1-3] and
use modifier[0] to apply to the entire framebuffer.

Obviosuly this means that if individual planes need different tiling
layouts and whatnot we will need a new modifier for each combination
of planes with different tiling layouts.

For a bit of extra backwards compatilbilty the kernel will allow
non-zero modifier[1+] but it require that they will match modifier[0].
This in case there's existing userspace out there that sets
modifier[1+] to something non-zero with planar formats.

Mostly a cocci job, with a bit of manual stuff mixed in.

@@
struct drm_framebuffer *fb;
expression E;
@@
- fb->modifier[E]
+ fb->modifier

@@
struct drm_framebuffer fb;
expression E;
@@
- fb.modifier[E]
+ fb.modifier

Change-Id: Iebf536e81ea538e157d57eebed1ae3f9b3d912ce
Cc: Kristian Høgsberg <hoegsberg@gmail.com>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Tomeu Vizoso <tomeu@tomeuvizoso.net>
Cc: dczaplejewicz@collabora.co.uk
Suggested-by: Kristian Høgsberg <hoegsberg@gmail.com>
Acked-by: Ben Widawsky <ben@bwidawsk.net>
Acked-by: Daniel Stone <daniels@collabora.com>
Acked-by: Rob Clark <robdclark@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1479295996-26246-1-git-send-email-ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_framebuffer.c
drivers/gpu/drm/drm_modeset_helper.c
include/drm/drm_framebuffer.h
include/uapi/drm/drm_mode.h

index 49fd7db..f29659d 100644 (file)
@@ -176,6 +176,13 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
                        return -EINVAL;
                }
 
+               if (r->flags & DRM_MODE_FB_MODIFIERS &&
+                   r->modifier[i] != r->modifier[0]) {
+                       DRM_DEBUG_KMS("bad fb modifier %llu for plane %d\n",
+                                     r->modifier[i], i);
+                       return -EINVAL;
+               }
+
                /* modifier specific checks: */
                switch (r->modifier[i]) {
                case DRM_FORMAT_MOD_SAMSUNG_64_32_TILE:
index 1d45738..7580709 100644 (file)
@@ -77,10 +77,8 @@ void drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
        for (i = 0; i < 4; i++) {
                fb->pitches[i] = mode_cmd->pitches[i];
                fb->offsets[i] = mode_cmd->offsets[i];
-               fb->modifier[i] = mode_cmd->modifier[i];
        }
-       drm_fb_get_bpp_depth(mode_cmd->pixel_format, &fb->depth,
-                                   &fb->bits_per_pixel);
+       fb->modifier = mode_cmd->modifier[0];
        fb->pixel_format = mode_cmd->pixel_format;
        fb->flags = mode_cmd->flags;
 }
index f5ae1f4..b3141a0 100644 (file)
@@ -149,12 +149,12 @@ struct drm_framebuffer {
         */
        unsigned int offsets[4];
        /**
-        * @modifier: Data layout modifier, per buffer. This is used to describe
+        * @modifier: Data layout modifier. This is used to describe
         * tiling, or also special layouts (like compression) of auxiliary
         * buffers. For userspace created object this is copied from
         * drm_mode_fb_cmd2.
         */
-       uint64_t modifier[4];
+       uint64_t modifier;
        /**
         * @width: Logical width of the visible area of the framebuffer, in
         * pixels.
index 254a11c..42981ad 100644 (file)
@@ -397,17 +397,20 @@ struct drm_mode_fb_cmd2 {
         * offsets[1].  Note that offsets[0] will generally
         * be 0 (but this is not required).
         *
-        * To accommodate tiled, compressed, etc formats, a per-plane
+        * To accommodate tiled, compressed, etc formats, a
         * modifier can be specified.  The default value of zero
         * indicates "native" format as specified by the fourcc.
-        * Vendor specific modifier token.  This allows, for example,
-        * different tiling/swizzling pattern on different planes.
-        * See discussion above of DRM_FORMAT_MOD_xxx.
+        * Vendor specific modifier token.  Note that even though
+        * it looks like we have a modifier per-plane, we in fact
+        * do not. The modifier for each plane must be identical.
+        * Thus all combinations of different data layouts for
+        * multi plane formats must be enumerated as separate
+        * modifiers.
         */
        __u32 handles[4];
        __u32 pitches[4]; /* pitch for each plane */
        __u32 offsets[4]; /* offset of each plane */
-       __u64 modifier[4]; /* ie, tiling, compressed (per plane) */
+       __u64 modifier[4]; /* ie, tiling, compress */
 };
 
 #define DRM_MODE_FB_DIRTY_ANNOTATE_COPY 0x01