[PATCH 2/2] drm/rockchip: analogix_dp: move PSR entry wait to VOP CRTC handling
Andy Yan
andyshrk at 163.com
Sat Feb 15 00:59:18 PST 2025
Hi Lucas，
At 2025-02-08 02:22:47, "Lucas Stach" <l.stach at pengutronix.de> wrote:
>Instead of calling from the Analogix DP encoder into the VOP crtc
>handling, move the wait for PSR entry to vop_crtc_atomic_disable().
>
>This untangles the Analogix DP code from the VOP, so it can safely
>be used with VOP2.
>
>Signed-off-by: Lucas Stach <l.stach at pengutronix.de>
>---
>Note: I don't have any Rockchip system with a panel capable of PSR,
>so while I am pretty sure that this doesn't change the flow of
>operations at all, it should be tested by someone who has the
>necessary hardware before applying.
>---
> .../gpu/drm/rockchip/analogix_dp-rockchip.c | 26 -----
> drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 1 -
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 102 +++++++++---------
> 3 files changed, 53 insertions(+), 76 deletions(-)
>
>diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>index 0844175c37c5..6fec687d7db1 100644
>--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>@@ -39,8 +39,6 @@
>
> #define HIWORD_UPDATE(val, mask) (val | (mask) << 16)
>
>-#define PSR_WAIT_LINE_FLAG_TIMEOUT_MS 100
>-
> /**
> * struct rockchip_dp_chip_data - splite the grf setting of kind of chips
> * @lcdsel_grf_reg: grf register offset of lcdc select
>@@ -216,29 +214,6 @@ static void rockchip_dp_drm_encoder_enable(struct drm_encoder *encoder,
> clk_disable_unprepare(dp->grfclk);
> }
>
>-static void rockchip_dp_drm_encoder_disable(struct drm_encoder *encoder,
>- struct drm_atomic_state *state)
>-{
>- struct rockchip_dp_device *dp = encoder_to_dp(encoder);
>- struct drm_crtc *crtc;
>- struct drm_crtc_state *new_crtc_state = NULL;
>- int ret;
>-
>- crtc = rockchip_dp_drm_get_new_crtc(encoder, state);
>- /* No crtc means we're doing a full shutdown */
>- if (!crtc)
>- return;
>-
>- new_crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
>- /* If we're not entering self-refresh, no need to wait for vact */
>- if (!new_crtc_state || !new_crtc_state->self_refresh_active)
>- return;
>-
>- ret = rockchip_drm_wait_vact_end(crtc, PSR_WAIT_LINE_FLAG_TIMEOUT_MS);
>- if (ret)
>- DRM_DEV_ERROR(dp->dev, "line flag irq timed out\n");
>-}
>-
> static int
> rockchip_dp_drm_encoder_atomic_check(struct drm_encoder *encoder,
> struct drm_crtc_state *crtc_state,
>@@ -266,7 +241,6 @@ static const struct drm_encoder_helper_funcs rockchip_dp_encoder_helper_funcs =
> .mode_fixup = rockchip_dp_drm_encoder_mode_fixup,
> .mode_set = rockchip_dp_drm_encoder_mode_set,
> .atomic_enable = rockchip_dp_drm_encoder_enable,
>- .atomic_disable = rockchip_dp_drm_encoder_disable,
> .atomic_check = rockchip_dp_drm_encoder_atomic_check,
> };
>
>diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>index c183e82a42a5..d1333f432b4e 100644
>--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h
>@@ -82,7 +82,6 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev,
> struct device *dev);
> void rockchip_drm_dma_init_device(struct drm_device *drm_dev,
> struct device *dev);
>-int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout);
> int rockchip_drm_encoder_set_crtc_endpoint_id(struct rockchip_encoder *rencoder,
> struct device_node *np, int port, int reg);
> int rockchip_drm_endpoint_is_subdriver(struct device_node *ep);
>diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>index 7f5fbea34951..77778df3c225 100644
>--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
>@@ -726,14 +726,67 @@ static void rockchip_drm_set_win_enabled(struct drm_crtc *crtc, bool enabled)
> spin_unlock(&vop->reg_lock);
> }
>
>+/**
>+ * rockchip_drm_wait_vact_end
>+ * @crtc: CRTC to enable line flag
>+ * @mstimeout: millisecond for timeout
>+ *
>+ * Wait for vact_end line flag irq or timeout.
>+ *
>+ * Returns:
>+ * Zero on success, negative errno on failure.
>+ */
>+static int
>+rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout)
As now this is a function used internally within the vop driver. I suggest rename it
to vop2_wait_vact_end, This will maintain consistency with other functions.
And also make the first argument as "struct vop *vop"
>+{
>+ struct vop *vop = to_vop(crtc);
>+ unsigned long jiffies_left;
>+ int ret = 0;
>+
>+ if (!crtc || !vop->is_enabled)
>+ return -ENODEV;
>+
>+ mutex_lock(&vop->vop_lock);
>+ if (mstimeout <= 0) {
>+ ret = -EINVAL;
>+ goto out;
>+ }
>+
>+ if (vop_line_flag_irq_is_enabled(vop)) {
>+ ret = -EBUSY;
>+ goto out;
>+ }
>+
>+ reinit_completion(&vop->line_flag_completion);
>+ vop_line_flag_irq_enable(vop);
>+
>+ jiffies_left = wait_for_completion_timeout(&vop->line_flag_completion,
>+ msecs_to_jiffies(mstimeout));
>+ vop_line_flag_irq_disable(vop);
>+
>+ if (jiffies_left == 0) {
>+ DRM_DEV_ERROR(vop->dev, "Timeout waiting for IRQ\n");
>+ ret = -ETIMEDOUT;
>+ goto out;
>+ }
>+
>+out:
>+ mutex_unlock(&vop->vop_lock);
>+ return ret;
>+}
>+
> static void vop_crtc_atomic_disable(struct drm_crtc *crtc,
> struct drm_atomic_state *state)
> {
> struct vop *vop = to_vop(crtc);
>+ int ret;
>
> WARN_ON(vop->event);
>
> if (crtc->state->self_refresh_active) {
>+ ret = rockchip_drm_wait_vact_end(crtc, 100);
>+ if (ret)
>+ DRM_DEV_ERROR(vop->dev, "line flag irq timed out\n");
> rockchip_drm_set_win_enabled(crtc, false);
> goto out;
> }
>@@ -2131,55 +2184,6 @@ static void vop_win_init(struct vop *vop)
> }
> }
>
>-/**
>- * rockchip_drm_wait_vact_end
>- * @crtc: CRTC to enable line flag
>- * @mstimeout: millisecond for timeout
>- *
>- * Wait for vact_end line flag irq or timeout.
>- *
>- * Returns:
>- * Zero on success, negative errno on failure.
>- */
>-int rockchip_drm_wait_vact_end(struct drm_crtc *crtc, unsigned int mstimeout)
>-{
>- struct vop *vop = to_vop(crtc);
>- unsigned long jiffies_left;
>- int ret = 0;
>-
>- if (!crtc || !vop->is_enabled)
>- return -ENODEV;
>-
>- mutex_lock(&vop->vop_lock);
>- if (mstimeout <= 0) {
>- ret = -EINVAL;
>- goto out;
>- }
>-
>- if (vop_line_flag_irq_is_enabled(vop)) {
>- ret = -EBUSY;
>- goto out;
>- }
>-
>- reinit_completion(&vop->line_flag_completion);
>- vop_line_flag_irq_enable(vop);
>-
>- jiffies_left = wait_for_completion_timeout(&vop->line_flag_completion,
>- msecs_to_jiffies(mstimeout));
>- vop_line_flag_irq_disable(vop);
>-
>- if (jiffies_left == 0) {
>- DRM_DEV_ERROR(vop->dev, "Timeout waiting for IRQ\n");
>- ret = -ETIMEDOUT;
>- goto out;
>- }
>-
>-out:
>- mutex_unlock(&vop->vop_lock);
>- return ret;
>-}
>-EXPORT_SYMBOL(rockchip_drm_wait_vact_end);
>-
> static int vop_bind(struct device *dev, struct device *master, void *data)
> {
> struct platform_device *pdev = to_platform_device(dev);
>--
>2.48.1
>
>
>_______________________________________________
>Linux-rockchip mailing list
>Linux-rockchip at lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-rockchip
More information about the Linux-rockchip
mailing list