The MDP5 plane's atomic_check ops doesn't perform clipping tests.
This didn't hurt us much in the past, but clipping becomes important
with cursor planes.
Use drm_plane_helper_check_state, the way rockchip/intel/mtk drivers
already do. Clipping requires knowledge of the crtc width and height.
This requires us to call drm_atomic_helper_check_modeset before
drm_atomic_helper_check_planes in the driver's atomic_check op,
because check_modetest will populate the mode for the crtc, needed
to populate the clip rectangle.
We also replace the plane_enabled(state) local helper with
state->visible, since they both represent the same thing (i.e, whether
state->fb and state->crtc are non-NULL or not).
One issue with the existing code is that we don't have a way to disable
the plane when it's completely clipped out. Until there isn't an update
on the crtc (which would de-stage the plane), we would still see the
plane in its last 'visible' configuration.
Signed-off-by: Archit Taneja <architt@xxxxxxxxxxxxxx>
---
drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 59 +++++++++++++++++++++++--------
drivers/gpu/drm/msm/msm_atomic.c | 21 +++++++----
2 files changed, 60 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
index 72a9f98..929e58c 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c
@@ -29,10 +29,7 @@ struct mdp5_plane {
static int mdp5_plane_mode_set(struct drm_plane *plane,
struct drm_crtc *crtc, struct drm_framebuffer *fb,
- int crtc_x, int crtc_y,
- unsigned int crtc_w, unsigned int crtc_h,
- uint32_t src_x, uint32_t src_y,
- uint32_t src_w, uint32_t src_h);
+ struct drm_rect *src, struct drm_rect *dest);
static void set_scanout_locked(struct drm_plane *plane,
struct drm_framebuffer *fb);
@@ -275,6 +272,7 @@ static void mdp5_plane_cleanup_fb(struct drm_plane *plane,
msm_framebuffer_cleanup(fb, mdp5_kms->id);
}
+#define FRAC_16_16(mult, div) (((mult) << 16) / (div))
static int mdp5_plane_atomic_check(struct drm_plane *plane,
struct drm_plane_state *state)
{
@@ -283,11 +281,20 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane,
struct mdp5_cfg *config = mdp5_cfg_get_config(get_kms(plane)->cfg);
bool new_hwpipe = false;
uint32_t max_width, max_height;
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
+ struct drm_rect clip;
+ int min_scale, max_scale;
uint32_t caps = 0;
+ int ret;
DBG("%s: check (%d -> %d)", plane->name,
plane_enabled(old_state), plane_enabled(state));
+ crtc = state->crtc ? state->crtc : plane->state->crtc;
+ if (!crtc)
+ return 0;
+
/* We don't allow faster-than-vblank updates.. if we did add this
* some day, we would need to disallow in cases where hwpipe
* changes
@@ -306,7 +313,23 @@ static int mdp5_plane_atomic_check(struct drm_plane *plane,
return -ERANGE;
}
- if (plane_enabled(state)) {
+ crtc_state = drm_atomic_get_existing_crtc_state(state->state, crtc);
+ if (WARN_ON(!crtc_state))
+ return -EINVAL;
+
+ clip.x1 = 0;
+ clip.y1 = 0;
+ clip.x2 = crtc_state->adjusted_mode.hdisplay;
+ clip.y2 = crtc_state->adjusted_mode.vdisplay;
+ min_scale = FRAC_16_16(1, 8);
+ max_scale = FRAC_16_16(8, 1);
+
+ ret = drm_plane_helper_check_state(state, &clip, min_scale,
+ max_scale, true, true);
+ if (ret)
+ return ret;
+
+ if (state->visible) {
unsigned int rotation;
const struct mdp_format *format;
struct mdp5_kms *mdp5_kms = get_kms(plane);
@@ -376,15 +399,12 @@ static void mdp5_plane_atomic_update(struct drm_plane *plane,
mdp5_state->pending = true;
- if (plane_enabled(state)) {
+ if (state->visible) {
int ret;
ret = mdp5_plane_mode_set(plane,
state->crtc, state->fb,
- state->crtc_x, state->crtc_y,
- state->crtc_w, state->crtc_h,
- state->src_x, state->src_y,
- state->src_w, state->src_h);
+ &state->src, &state->dst);
/* atomic_check should have ensured that this doesn't fail */
WARN_ON(ret < 0);
}
@@ -677,10 +697,7 @@ static void mdp5_write_pixel_ext(struct mdp5_kms *mdp5_kms, enum mdp5_pipe pipe,
static int mdp5_plane_mode_set(struct drm_plane *plane,
struct drm_crtc *crtc, struct drm_framebuffer *fb,
- int crtc_x, int crtc_y,
- unsigned int crtc_w, unsigned int crtc_h,
- uint32_t src_x, uint32_t src_y,
- uint32_t src_w, uint32_t src_h)
+ struct drm_rect *src, struct drm_rect *dest)
{
struct drm_plane_state *pstate = plane->state;
struct mdp5_hw_pipe *hwpipe = to_mdp5_plane_state(pstate)->hwpipe;
@@ -696,6 +713,10 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
uint32_t pix_format;
unsigned int rotation;
bool vflip, hflip;
+ int crtc_x, crtc_y;
+ unsigned int crtc_w, crtc_h;
+ uint32_t src_x, src_y;
+ uint32_t src_w, src_h;
unsigned long flags;
int ret;
@@ -708,6 +729,16 @@ static int mdp5_plane_mode_set(struct drm_plane *plane,
format = to_mdp_format(msm_framebuffer_format(fb));
pix_format = format->base.pixel_format;
+ src_x = src->x1;
+ src_y = src->y1;
+ src_w = drm_rect_width(src);
+ src_h = drm_rect_height(src);
+
+ crtc_x = dest->x1;
+ crtc_y = dest->y1;
+ crtc_w = drm_rect_width(dest);
+ crtc_h = drm_rect_height(dest);
+
/* src values are in Q16 fixed point, convert to integer: */
src_x = src_x >> 16;
src_y = src_y >> 16;
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 30b5d23..6924fa2 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -151,20 +151,29 @@ static void commit_worker(struct work_struct *work)
complete_commit(container_of(work, struct msm_commit, work), true);
}
+/*
+ * this func is identical to the drm_atomic_helper_check, but we keep this
+ * because we might eventually need to have a more finegrained check
+ * sequence without using the atomic helpers.
+ *
+ * In the past, we first called drm_atomic_helper_check_planes, and then
+ * drm_atomic_helper_check_modeset. We needed this because the MDP5 plane's
+ * ->atomic_check could update ->mode_changed for pixel format changes.
+ * This, however isn't needed now because if there is a pixel format change,
+ * we just assign a new hwpipe for it with a new SMP allocation. We might
+ * eventually hit a condition where we would need to do a full modeset if
+ * we run out of planes. There, we'd probably need to set mode_changed.
+ */
int msm_atomic_check(struct drm_device *dev,
struct drm_atomic_state *state)
{
int ret;
- /*
- * msm ->atomic_check can update ->mode_changed for pixel format
- * changes, hence must be run before we check the modeset changes.
- */
- ret = drm_atomic_helper_check_planes(dev, state);
+ ret = drm_atomic_helper_check_modeset(dev, state);
if (ret)
return ret;
- ret = drm_atomic_helper_check_modeset(dev, state);
+ ret = drm_atomic_helper_check_planes(dev, state);
if (ret)
return ret;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
