Message ID | 20230728-solid-fill-v5-7-053dbefa909c@quicinc.com |
---|---|
State | New |
Headers | show |
Series | Support for Solid Fill Planes | expand |
On 28/07/2023 20:02, Jessica Zhang wrote: > 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> > --- > drivers/gpu/drm/drm_atomic.c | 20 +++++++++++--------- > drivers/gpu/drm/drm_atomic_helper.c | 34 ++++++++++++++++++++-------------- > drivers/gpu/drm/drm_plane.c | 16 ++++++++++++---- > include/drm/drm_atomic_helper.h | 4 ++-- > include/drm/drm_plane.h | 29 +++++++++++++++++++++++++++++ > 5 files changed, 74 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > index 017ce0e6570f..b0c532f3db82 100644 > --- a/drivers/gpu/drm/drm_atomic.c > +++ b/drivers/gpu/drm/drm_atomic.c > @@ -668,14 +668,14 @@ static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state, > 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; > } > > @@ -706,9 +706,11 @@ static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state, > } > > > - ret = drm_atomic_check_fb(new_plane_state); > - if (ret) > - return ret; > + if (new_plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && fb) { > + ret = drm_atomic_check_fb(new_plane_state); > + if (ret) > + return ret; > + } > > if (plane_switching_crtc(old_plane_state, new_plane_state)) { > drm_dbg_atomic(plane->dev, > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 41b8066f61ff..d05ec9ef2b3e 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -864,7 +864,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; > } > @@ -881,25 +881,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); > + if (plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && 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 (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; > + } > } > > if (crtc_state->enable) > drm_mode_get_hv_timing(&crtc_state->mode, &clip.x2, &clip.y2); > > - plane_state->visible = drm_rect_clip_scaled(src, dst, &clip); > - > - drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation); > + if (plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && fb) { You can unify this with the previous if condition, it will be more logical. > + 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) > /* > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index 009d3ebd9b39..8f759f546aae 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -839,6 +839,14 @@ bool drm_any_plane_has_format(struct drm_device *dev, > } > EXPORT_SYMBOL(drm_any_plane_has_format); > > +static bool drm_plane_needs_disable(struct drm_plane_state *state, struct drm_framebuffer *fb) > +{ > + if (state->pixel_source == DRM_PLANE_PIXEL_SOURCE_NONE) > + return true; > + > + return state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && fb == NULL; > +} > + > /* > * __setplane_internal - setplane handler for internal callers > * > @@ -861,8 +869,8 @@ static int __setplane_internal(struct drm_plane *plane, > > WARN_ON(drm_drv_uses_atomic_modeset(plane->dev)); > > - /* No fb means shut it down */ > - if (!fb) { > + /* No visible data means shut it down */ > + if (drm_plane_needs_disable(plane->state, fb)) { > plane->old_fb = plane->fb; > ret = plane->funcs->disable_plane(plane, ctx); > if (!ret) { > @@ -913,8 +921,8 @@ static int __setplane_atomic(struct drm_plane *plane, > > WARN_ON(!drm_drv_uses_atomic_modeset(plane->dev)); > > - /* No fb means shut it down */ > - if (!fb) > + /* No visible data means shut it down */ > + if (drm_plane_needs_disable(plane->state, fb)) > return plane->funcs->disable_plane(plane, ctx); > > /* > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > index 536a0b0091c3..6d97f38ac1f6 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -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; > } > diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h > index 303f01f0588c..9ff2a837ed17 100644 > --- a/include/drm/drm_plane.h > +++ b/include/drm/drm_plane.h > @@ -991,6 +991,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); > >
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 017ce0e6570f..b0c532f3db82 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -668,14 +668,14 @@ static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state, 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; } @@ -706,9 +706,11 @@ static int drm_atomic_plane_check(const struct drm_plane_state *old_plane_state, } - ret = drm_atomic_check_fb(new_plane_state); - if (ret) - return ret; + if (new_plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && fb) { + ret = drm_atomic_check_fb(new_plane_state); + if (ret) + return ret; + } if (plane_switching_crtc(old_plane_state, new_plane_state)) { drm_dbg_atomic(plane->dev, diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 41b8066f61ff..d05ec9ef2b3e 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -864,7 +864,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; } @@ -881,25 +881,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); + if (plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && 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 (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; + } } if (crtc_state->enable) drm_mode_get_hv_timing(&crtc_state->mode, &clip.x2, &clip.y2); - plane_state->visible = drm_rect_clip_scaled(src, dst, &clip); - - drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation); + if (plane_state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && fb) { + 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) /* diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index 009d3ebd9b39..8f759f546aae 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -839,6 +839,14 @@ bool drm_any_plane_has_format(struct drm_device *dev, } EXPORT_SYMBOL(drm_any_plane_has_format); +static bool drm_plane_needs_disable(struct drm_plane_state *state, struct drm_framebuffer *fb) +{ + if (state->pixel_source == DRM_PLANE_PIXEL_SOURCE_NONE) + return true; + + return state->pixel_source == DRM_PLANE_PIXEL_SOURCE_FB && fb == NULL; +} + /* * __setplane_internal - setplane handler for internal callers * @@ -861,8 +869,8 @@ static int __setplane_internal(struct drm_plane *plane, WARN_ON(drm_drv_uses_atomic_modeset(plane->dev)); - /* No fb means shut it down */ - if (!fb) { + /* No visible data means shut it down */ + if (drm_plane_needs_disable(plane->state, fb)) { plane->old_fb = plane->fb; ret = plane->funcs->disable_plane(plane, ctx); if (!ret) { @@ -913,8 +921,8 @@ static int __setplane_atomic(struct drm_plane *plane, WARN_ON(!drm_drv_uses_atomic_modeset(plane->dev)); - /* No fb means shut it down */ - if (!fb) + /* No visible data means shut it down */ + if (drm_plane_needs_disable(plane->state, fb)) return plane->funcs->disable_plane(plane, ctx); /* diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 536a0b0091c3..6d97f38ac1f6 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -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; } diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index 303f01f0588c..9ff2a837ed17 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -991,6 +991,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);
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> --- drivers/gpu/drm/drm_atomic.c | 20 +++++++++++--------- drivers/gpu/drm/drm_atomic_helper.c | 34 ++++++++++++++++++++-------------- drivers/gpu/drm/drm_plane.c | 16 ++++++++++++---- include/drm/drm_atomic_helper.h | 4 ++-- include/drm/drm_plane.h | 29 +++++++++++++++++++++++++++++ 5 files changed, 74 insertions(+), 29 deletions(-)