drm/atomic: Loosen FB atomic checks
authorJessica Zhang <quic_jesszhan@quicinc.com>
Fri, 27 Oct 2023 22:32:57 +0000 (15:32 -0700)
committerDmitry Baryshkov <dmitry.baryshkov@linaro.org>
Fri, 1 Dec 2023 23:56:55 +0000 (01:56 +0200)
Loosen the requirements for atomic and legacy commit so that, in cases
where pixel_source != FB, the commit can still go through.

This includes adding framebuffer NULL checks in other areas to account for
FB being NULL when non-FB pixel sources are enabled.

To disable a plane, the pixel_source must be NONE or the FB must be NULL
if pixel_source == FB.

Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20231027-solid-fill-v7-7-780188bfa7b2@quicinc.com
drivers/gpu/drm/drm_atomic.c
drivers/gpu/drm/drm_atomic_helper.c
include/drm/drm_atomic_helper.h
include/drm/drm_plane.h

index c6f2b86..aed0a69 100644 (file)
@@ -674,17 +674,16 @@ static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state,
 {
        struct drm_plane *plane = new_plane_state->plane;
        struct drm_crtc *crtc = new_plane_state->crtc;
-       const struct drm_framebuffer *fb = new_plane_state->fb;
        int ret;
 
-       /* either *both* CRTC and FB must be set, or neither */
-       if (crtc && !fb) {
-               drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] CRTC set but no FB\n",
+       /* either *both* CRTC and pixel source must be set, or neither */
+       if (crtc && !drm_plane_has_visible_data(new_plane_state)) {
+               drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] CRTC set but no visible data\n",
                               plane->base.id, plane->name);
                return -EINVAL;
-       } else if (fb && !crtc) {
-               drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] FB set but no CRTC\n",
-                              plane->base.id, plane->name);
+       } else if (drm_plane_has_visible_data(new_plane_state) && !crtc) {
+               drm_dbg_atomic(plane->dev, "[PLANE:%d:%s] Source %d has visible data but no CRTC\n",
+                              plane->base.id, plane->name, new_plane_state->pixel_source);
                return -EINVAL;
        }
 
@@ -715,9 +714,11 @@ static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state,
        }
 
 
-       ret = drm_atomic_plane_check_fb(new_plane_state);
-       if (ret)
-               return ret;
+       if (new_plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && new_plane_state->fb) {
+               ret = drm_atomic_plane_check_fb(new_plane_state);
+               if (ret)
+                       return ret;
+       }
 
        if (plane_switching_crtc(old_plane_state, new_plane_state)) {
                drm_dbg_atomic(plane->dev,
index c3f6771..dc04898 100644 (file)
@@ -861,7 +861,6 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
                                        bool can_position,
                                        bool can_update_disabled)
 {
-       struct drm_framebuffer *fb = plane_state->fb;
        struct drm_rect *src = &plane_state->src;
        struct drm_rect *dst = &plane_state->dst;
        unsigned int rotation = plane_state->rotation;
@@ -873,7 +872,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
        *src = drm_plane_state_src(plane_state);
        *dst = drm_plane_state_dest(plane_state);
 
-       if (!fb) {
+       if (!drm_plane_has_visible_data(plane_state)) {
                plane_state->visible = false;
                return 0;
        }
@@ -890,25 +889,31 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
                return -EINVAL;
        }
 
-       drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
+       /* Check that selected pixel source is valid */
+       if (plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && plane_state->fb) {
+               struct drm_framebuffer *fb = plane_state->fb;
+               drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
 
-       /* Check scaling */
-       hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
-       vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
-       if (hscale < 0 || vscale < 0) {
-               drm_dbg_kms(plane_state->plane->dev,
-                           "Invalid scaling of plane\n");
-               drm_rect_debug_print("src: ", &plane_state->src, true);
-               drm_rect_debug_print("dst: ", &plane_state->dst, false);
-               return -ERANGE;
-       }
+               /* Check scaling */
+               hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
+               vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
 
-       if (crtc_state->enable)
-               drm_mode_get_hv_timing(&crtc_state->mode, &clip.x2, &clip.y2);
+               if (hscale < 0 || vscale < 0) {
+                       drm_dbg_kms(plane_state->plane->dev,
+                                       "Invalid scaling of plane\n");
+                       drm_rect_debug_print("src: ", &plane_state->src, true);
+                       drm_rect_debug_print("dst: ", &plane_state->dst, false);
+                       return -ERANGE;
+               }
 
-       plane_state->visible = drm_rect_clip_scaled(src, dst, &clip);
+               if (crtc_state->enable)
+                       drm_mode_get_hv_timing(&crtc_state->mode, &clip.x2, &clip.y2);
 
-       drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
+               plane_state->visible = drm_rect_clip_scaled(src, dst, &clip);
+               drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
+       } else if (drm_plane_solid_fill_enabled(plane_state)) {
+               plane_state->visible = true;
+       }
 
        if (!plane_state->visible)
                /*
index 536a0b0..6d97f38 100644 (file)
@@ -256,8 +256,8 @@ drm_atomic_plane_disabling(struct drm_plane_state *old_plane_state,
         * Anything else should be considered a bug in the atomic core, so we
         * gently warn about it.
         */
-       WARN_ON((new_plane_state->crtc == NULL && new_plane_state->fb != NULL) ||
-               (new_plane_state->crtc != NULL && new_plane_state->fb == NULL));
+       WARN_ON((new_plane_state->crtc == NULL && drm_plane_has_visible_data(new_plane_state)) ||
+               (new_plane_state->crtc != NULL && !drm_plane_has_visible_data(new_plane_state)));
 
        return old_plane_state->crtc && !new_plane_state->crtc;
 }
index d14e2f1..3b187f3 100644 (file)
@@ -1016,6 +1016,35 @@ static inline struct drm_plane *drm_plane_find(struct drm_device *dev,
 #define drm_for_each_plane(plane, dev) \
        list_for_each_entry(plane, &(dev)->mode_config.plane_list, head)
 
+/**
+ * drm_plane_solid_fill_enabled - Check if solid fill is enabled on plane
+ * @state: plane state
+ *
+ * Returns:
+ * Whether the plane has been assigned a solid_fill_blob
+ */
+static inline bool drm_plane_solid_fill_enabled(struct drm_plane_state *state)
+{
+       if (!state)
+               return false;
+       return state->pixel_source == DRM_PLANE_PIXEL_SOURCE_SOLID_FILL && state->solid_fill_blob;
+}
+
+static inline bool drm_plane_has_visible_data(const struct drm_plane_state *state)
+{
+       switch (state->pixel_source) {
+       case DRM_PLANE_PIXEL_SOURCE_NONE:
+               return false;
+       case DRM_PLANE_PIXEL_SOURCE_SOLID_FILL:
+               return state->solid_fill_blob != NULL;
+       case DRM_PLANE_PIXEL_SOURCE_FB:
+       default:
+               WARN_ON(state->pixel_source != DRM_PLANE_PIXEL_SOURCE_FB);
+       }
+
+       return state->fb != NULL;
+}
+
 bool drm_any_plane_has_format(struct drm_device *dev,
                              u32 format, u64 modifier);