Re: [PATCH 2/2] drm/mediatek: Bypass atomic helpers for cursor updates
Hi, Bibby:
On Fri, 2019-08-30 at 15:38 +0800, Bibby Hsieh wrote:
> Moving the driver to atomic helpers regressed cursor responsiveness,
> because cursor updates need their own atomic commits, which have to be
> serialized with other commits, that might include fence waits. To avoid
> this, in certain conditions, we can bypass the atomic helpers for legacy
> cursor update IOCTLs. Currently the conditions are:
> - no asynchronous mode setting commit pending,
> - no asynchronous commit that updates the cursor plane is pending.
> With the above two conditions met, we know that the manual cursor state
> update will not conflict with any scheduled update.
>
> Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
>
> Signed-off-by: Bibby Hsieh
> Signed-off-by: Daniel Kurtz
> ---
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 41 -
> drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 2 +
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 34 ++-
> drivers/gpu/drm/mediatek/mtk_drm_drv.h | 3 +
> drivers/gpu/drm/mediatek/mtk_drm_plane.c | 73 +++-
> 5 files changed, 148 insertions(+), 5 deletions(-)
>
[snip]
> +
> +static int mtk_plane_update(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_modeset_acquire_ctx *ctx)
> +{
> + struct mtk_drm_private *private = plane->dev->dev_private;
> + uint32_t crtc_mask = (1 << drm_crtc_index(crtc));
> +
> + if (crtc && plane == crtc->cursor &&
> + plane->state->crtc == crtc &&
> + !(private->commit.flush_for_cursor & crtc_mask))
> + return mtk_plane_cursor_update(plane, crtc, fb,
> + crtc_x, crtc_y, crtc_w, crtc_h,
> + src_x, src_y, src_w, src_h);
> +
> + return drm_atomic_helper_update_plane(plane, crtc, fb,
> + crtc_x, crtc_y, crtc_w, crtc_h,
> + src_x, src_y, src_w, src_h, ctx);
> +}
> +
> static const struct drm_plane_funcs mtk_plane_funcs = {
> - .update_plane = drm_atomic_helper_update_plane,
> + .update_plane = mtk_plane_update,
I think drm core has already process cursor async problem. In [1], you
could search 'legacy_cursor_update' and it need driver to implement
atomic_async_check() and atomic_async_update() callback function. You
could refer to [2] for the implementation.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_atomic_helper.c?h=v5.3-rc6
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/rockchip_drm_vop.c?h=v5.3-rc6#n955
Regards,
CK
> .disable_plane = drm_atomic_helper_disable_plane,
> .destroy = drm_plane_cleanup,
> .reset = mtk_plane_reset,
> @@ -90,7 +154,12 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
> if (!state->crtc)
> return 0;
>
> - crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
> + if (state->state)
> + crtc_state = drm_atomic_get_existing_crtc_state(state->state,
> + state->crtc);
> + else /* Special case for asynchronous cursor updates. */
> + crtc_state = state->crtc->state;
> +
> if (IS_ERR(crtc_state))
> return PTR_ERR(crtc_state);
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/mediatek: Bypass atomic helpers for cursor updates
Moving the driver to atomic helpers regressed cursor responsiveness,
because cursor updates need their own atomic commits, which have to be
serialized with other commits, that might include fence waits. To avoid
this, in certain conditions, we can bypass the atomic helpers for legacy
cursor update IOCTLs. Currently the conditions are:
- no asynchronous mode setting commit pending,
- no asynchronous commit that updates the cursor plane is pending.
With the above two conditions met, we know that the manual cursor state
update will not conflict with any scheduled update.
Fixes: 119f5173628a ("drm/mediatek: Add DRM Driver for Mediatek SoC MT8173.")
Signed-off-by: Bibby Hsieh
Signed-off-by: Daniel Kurtz
---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 41 -
drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 2 +
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 34 ++-
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 3 +
drivers/gpu/drm/mediatek/mtk_drm_plane.c | 73 +++-
5 files changed, 148 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 7697b40baac0..092e502ed27b 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -40,7 +40,7 @@ struct mtk_drm_crtc {
struct drm_plane*planes;
unsigned intlayer_nr;
boolpending_planes;
-
+ boolcursor_update;
void __iomem*config_regs;
const struct mtk_mmsys_reg_data *mmsys_reg_data;
struct mtk_disp_mutex *mutex;
@@ -386,8 +386,45 @@ static void mtk_crtc_ddp_config(struct drm_crtc *crtc)
}
}
mtk_crtc->pending_planes = false;
- mtk_atomic_state_put_queue(atomic_state);
+ if (!mtk_crtc->cursor_update)
+ mtk_atomic_state_put_queue(atomic_state);
+ mtk_crtc->cursor_update = false;
+ }
+}
+
+void mtk_drm_crtc_cursor_update(struct drm_crtc *crtc, struct drm_plane *plane,
+ struct drm_plane_state *plane_state)
+{
+ struct mtk_drm_private *priv = crtc->dev->dev_private;
+ struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+ const struct drm_plane_helper_funcs *plane_helper_funcs =
+ plane->helper_private;
+ int i;
+
+ if (!mtk_crtc->enabled)
+ return;
+
+ mutex_lock(>hw_lock);
+ plane_helper_funcs->atomic_update(plane, plane_state);
+ for (i = 0; i < mtk_crtc->layer_nr; i++) {
+ struct drm_plane *plane = _crtc->planes[i];
+ struct mtk_plane_state *plane_state;
+
+ plane_state = to_mtk_plane_state(plane->state);
+ if (plane_state->pending.dirty) {
+ plane_state->pending.config = true;
+ plane_state->pending.dirty = false;
+ }
+ }
+ mtk_crtc->pending_planes = true;
+ mtk_crtc->cursor_update = true;
+
+ if (priv->data->shadow_register) {
+ mtk_disp_mutex_acquire(mtk_crtc->mutex);
+ mtk_crtc_ddp_config(crtc);
+ mtk_disp_mutex_release(mtk_crtc->mutex);
}
+ mutex_unlock(>hw_lock);
}
static void mtk_drm_crtc_atomic_enable(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
index fcc134eb00c9..46e903be68ec 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
@@ -19,5 +19,7 @@ void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct
mtk_ddp_comp *comp);
int mtk_drm_crtc_create(struct drm_device *drm_dev,
const enum mtk_ddp_comp_id *path,
unsigned int path_len);
+void mtk_drm_crtc_cursor_update(struct drm_crtc *crtc, struct drm_plane *plane,
+ struct drm_plane_state *plane_state);
#endif /* MTK_DRM_CRTC_H */
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index b0308a3a7483..ca754b954c7c 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -80,11 +80,36 @@ static int mtk_atomic_get_crtcs(struct drm_device *drm,
struct drm_atomic_state *state)
{
struct mtk_drm_private *private = drm->dev_private;
- uint32_t crtc_mask;
+ uint32_t crtc_mask, needs_modeset, has_cursor_plane;
+ struct drm_plane *plane;
+ struct drm_plane_state *plane_state;
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
+ int i;
int ret;
crtc_mask = mtk_atomic_crtc_mask(drm, state);
+ /*
+* Allow cursor updates unless there is a pending modeset or cursor
+*
