On 9/13/2023 10:06 PM, Dmitry Baryshkov wrote:
The helper drm_atomic_helper_check_plane_state() runs several checks on
plane src and dst rectangles, including the check whether required
scaling fits into the required margins. The msm driver would benefit
from having a function that does all these checks except the scaling
one. Split them into a new helper called
drm_atomic_helper_check_plane_noscale().
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
drivers/gpu/drm/drm_atomic_helper.c | 110 ++++++++++++++++++++++------
include/drm/drm_atomic_helper.h | 7 ++
2 files changed, 96 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 292e38eb6218..2d7dd66181c9 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -825,11 +825,9 @@ drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
/**
- * drm_atomic_helper_check_plane_state() - Check plane state for validity
+ * drm_atomic_helper_check_plane_noscale() - Check plane state for validity
* @plane_state: plane state to check
* @crtc_state: CRTC state to check
- * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
- * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
* @can_position: is it legal to position the plane such that it
* doesn't cover the entire CRTC? This will generally
* only be false for primary planes.
@@ -845,19 +843,16 @@ EXPORT_SYMBOL(drm_atomic_helper_check_wb_encoder_state);
* RETURNS:
* Zero if update appears valid, error code on failure
*/
-int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
- const struct drm_crtc_state *crtc_state,
- int min_scale,
- int max_scale,
- bool can_position,
- bool can_update_disabled)
+int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
+ const struct drm_crtc_state *crtc_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;
struct drm_rect clip = {};
- int hscale, vscale;
WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc);
@@ -883,17 +878,6 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
Do we need to do the rotation even for noscale before validation?
- /* 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);
@@ -921,6 +905,90 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
return 0;
}
+EXPORT_SYMBOL(drm_atomic_helper_check_plane_noscale);
+
+/**
+ * drm_atomic_helper_check_plane_scale() - Check whether plane can be scaled
+ * @plane_state: plane state to check
+ * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
+ * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
+ *
+ * Checks that a desired plane scale fits into the min_scale..max_scale
+ * boundaries.
+ * Drivers that provide their own plane handling rather than helper-provided
+ * implementations may still wish to call this function to avoid duplication of
+ * error checking code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
+ int min_scale,
+ int max_scale)
+{
+ struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_rect src;
+ struct drm_rect dst;
+ int hscale, vscale;
+
+ if (!plane_state->visible)
+ return 0;
+
+ src = drm_plane_state_src(plane_state);
+ dst = drm_plane_state_dest(plane_state);
+
+ drm_rect_rotate(&src, fb->width << 16, fb->height << 16, plane_state->rotation);
+
Does this need to be accompanied by a drm_rect_rotate_inv()?
+ 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;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_check_plane_scale);
+
+/**
+ * drm_atomic_helper_check_plane_state() - Check plane state for validity
+ * @plane_state: plane state to check
+ * @crtc_state: CRTC state to check
+ * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
+ * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
+ * @can_position: is it legal to position the plane such that it
+ * doesn't cover the entire CRTC? This will generally
+ * only be false for primary planes.
+ * @can_update_disabled: can the plane be updated while the CRTC
+ * is disabled?
+ *
+ * Checks that a desired plane update is valid, and updates various
+ * bits of derived state (clipped coordinates etc.). Drivers that provide
+ * their own plane handling rather than helper-provided implementations may
+ * still wish to call this function to avoid duplication of error checking
+ * code.
+ *
+ * RETURNS:
+ * Zero if update appears valid, error code on failure
+ */
+int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
+ const struct drm_crtc_state *crtc_state,
+ int min_scale,
+ int max_scale,
+ bool can_position,
+ bool can_update_disabled)
+{
+ int ret;
+
+ ret = drm_atomic_helper_check_plane_noscale(plane_state, crtc_state, can_position, can_update_disabled);
+ if (ret < 0)
+ return ret;
+
+ return drm_atomic_helper_check_plane_scale(plane_state, min_scale, max_scale);
+}
EXPORT_SYMBOL(drm_atomic_helper_check_plane_state);
/**
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 536a0b0091c3..32ac55aea94e 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -52,6 +52,13 @@ int drm_atomic_helper_check_modeset(struct drm_device *dev,
int
drm_atomic_helper_check_wb_encoder_state(struct drm_encoder *encoder,
struct drm_connector_state *conn_state);
+int drm_atomic_helper_check_plane_noscale(struct drm_plane_state *plane_state,
+ const struct drm_crtc_state *crtc_state,
+ bool can_position,
+ bool can_update_disabled);
+int drm_atomic_helper_check_plane_scale(struct drm_plane_state *plane_state,
+ int min_scale,
+ int max_scale);
int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
const struct drm_crtc_state *crtc_state,
int min_scale,
