On Thu, Dec 05, 2024 at 08:31:53PM -0800, Abhinav Kumar wrote:
> VC payload fill sequence is inserted by the DP controller in the
> absence of stream symbols that is before stream is disabled. Add
> support to send the VCPF sequence for msm dp controller.
>
> Signed-off-by: Abhinav Kumar <quic_abhin...@quicinc.com>
> ---
> drivers/gpu/drm/msm/dp/dp_catalog.c | 25 +++++++++++++++++++++++
> drivers/gpu/drm/msm/dp/dp_catalog.h | 4 ++++
> drivers/gpu/drm/msm/dp/dp_ctrl.c | 40
> +++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/msm/dp/dp_ctrl.h | 1 +
> drivers/gpu/drm/msm/dp/dp_display.c | 5 ++++-
> drivers/gpu/drm/msm/dp/dp_reg.h | 3 ++-
> 6 files changed, 76 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
> b/drivers/gpu/drm/msm/dp/dp_catalog.c
> index
> f9d21444d7891bcd043d282b31ae75711add4817..4826a698979ce7c37112812299879411c5743fa9
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> @@ -50,6 +50,11 @@
> (PSR_UPDATE_INT | PSR_CAPTURE_INT | PSR_EXIT_INT | \
> PSR_UPDATE_ERROR_INT | PSR_WAKE_ERROR_INT)
>
> +#define DP_INTERRUPT_STATUS5 \
> + (DP_INTR_DP0_VCPF_SENT | DP_INTR_DP1_VCPF_SENT)
> +#define DP_INTERRUPT_STATUS5_MASK \
> + (DP_INTERRUPT_STATUS5 << DP_INTERRUPT_STATUS_MASK_SHIFT)
> +
> #define DP_INTERRUPT_MASK4 \
> (PSR_UPDATE_MASK | PSR_CAPTURE_MASK | PSR_EXIT_MASK | \
> PSR_UPDATE_ERROR_MASK | PSR_WAKE_ERROR_MASK)
> @@ -694,9 +699,12 @@ void msm_dp_catalog_ctrl_enable_irq(struct
> msm_dp_catalog *msm_dp_catalog,
> DP_INTERRUPT_STATUS1_MASK);
> msm_dp_write_ahb(catalog, REG_DP_INTR_STATUS2,
> DP_INTERRUPT_STATUS2_MASK);
> + msm_dp_write_ahb(catalog, REG_DP_INTR_STATUS5,
> + DP_INTERRUPT_STATUS5_MASK);
> } else {
> msm_dp_write_ahb(catalog, REG_DP_INTR_STATUS, 0x00);
> msm_dp_write_ahb(catalog, REG_DP_INTR_STATUS2, 0x00);
> + msm_dp_write_ahb(catalog, REG_DP_INTR_STATUS5, 0x00);
> }
> }
>
> @@ -850,6 +858,23 @@ int msm_dp_catalog_ctrl_get_interrupt(struct
> msm_dp_catalog *msm_dp_catalog)
> return intr;
> }
>
> +int msm_dp_catalog_ctrl_get_interrupt_5(struct msm_dp_catalog
> *msm_dp_catalog)
> +{
> + struct msm_dp_catalog_private *catalog = container_of(msm_dp_catalog,
> + struct
> msm_dp_catalog_private,
> + msm_dp_catalog);
> + u32 intr, intr_ack;
> +
> + intr = msm_dp_read_ahb(catalog, REG_DP_INTR_STATUS5);
> + intr &= ~DP_INTERRUPT_STATUS5_MASK;
> + intr_ack = (intr & DP_INTERRUPT_STATUS5)
> + << DP_INTERRUPT_STATUS_ACK_SHIFT;
> + msm_dp_write_ahb(catalog, REG_DP_INTR_STATUS5,
> + intr_ack | DP_INTERRUPT_STATUS5_MASK);
> +
> + return intr;
> +}
> +
> void msm_dp_catalog_ctrl_phy_reset(struct msm_dp_catalog *msm_dp_catalog)
> {
> struct msm_dp_catalog_private *catalog = container_of(msm_dp_catalog,
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h
> b/drivers/gpu/drm/msm/dp/dp_catalog.h
> index
> 560016e2f929d4b92d6ea764d81a099c09c0e668..323858c587f85996d296156c7b8b201cdb7b7eb4
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.h
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
> @@ -28,6 +28,9 @@
> #define DP_INTR_FRAME_END BIT(6)
> #define DP_INTR_CRC_UPDATED BIT(9)
>
> +#define DP_INTR_DP0_VCPF_SENT BIT(0)
> +#define DP_INTR_DP1_VCPF_SENT BIT(3)
> +
> #define DP_HW_VERSION_1_0 0x10000000
> #define DP_HW_VERSION_1_2 0x10020000
>
> @@ -103,6 +106,7 @@ u32 msm_dp_catalog_link_is_connected(struct
> msm_dp_catalog *msm_dp_catalog);
> u32 msm_dp_catalog_hpd_get_intr_status(struct msm_dp_catalog
> *msm_dp_catalog);
> void msm_dp_catalog_ctrl_phy_reset(struct msm_dp_catalog *msm_dp_catalog);
> int msm_dp_catalog_ctrl_get_interrupt(struct msm_dp_catalog *msm_dp_catalog);
> +int msm_dp_catalog_ctrl_get_interrupt_5(struct msm_dp_catalog
> *msm_dp_catalog);
> u32 msm_dp_catalog_ctrl_read_psr_interrupt_status(struct msm_dp_catalog
> *msm_dp_catalog);
I'd prefer some uniformity here. read_psr_interrupt_status() returns
REG_DP_INTR_STATUS4, get_interrupt() returns REG_DP_INTR_STATUS2 value,
Now you are adding get_interrupt_5(). Could you please make that similar
and logical?
> void msm_dp_catalog_ctrl_update_transfer_unit(struct msm_dp_catalog
> *msm_dp_catalog,
> u32 msm_dp_tu, u32 valid_boundary,
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index
> 14562def1e70b769434243d1ce72661a7b4d4c6b..2288c379283c721a01c81302f8d307d0b3c76527
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -27,6 +27,11 @@
>
> #define DP_CTRL_INTR_READY_FOR_VIDEO BIT(0)
> #define DP_CTRL_INTR_IDLE_PATTERN_SENT BIT(3)
> +#define DP_CTRL_INTR_DP0_VCPF_SENT BIT(0)
> +#define DP_CTRL_INTR_DP1_VCPF_SENT BIT(3)
> +
> +#define MST_DP0_PUSH_VCPF BIT(12)
> +#define MST_DP1_PUSH_VCPF BIT(14)
>
> #define MR_LINK_TRAINING1 0x8
> #define MR_LINK_SYMBOL_ERM 0x80
> @@ -144,6 +149,34 @@ void msm_dp_ctrl_push_idle(struct msm_dp_ctrl
> *msm_dp_ctrl)
> drm_dbg_dp(ctrl->drm_dev, "mainlink off\n");
> }
>
> +void msm_dp_ctrl_push_vcpf(struct msm_dp_ctrl *msm_dp_ctrl, struct
> msm_dp_panel *msm_dp_panel)
> +{
> + u32 state = 0x0;
> + struct msm_dp_ctrl_private *ctrl;
> +
> + ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private,
> msm_dp_ctrl);
> +
> + if (msm_dp_panel->stream_id >= DP_STREAM_MAX) {
> + DRM_ERROR("invalid input\n");
> + return;
> + }
> +
> + if (msm_dp_panel->stream_id == DP_STREAM_0)
> + state |= MST_DP0_PUSH_VCPF;
> + else
> + state |= MST_DP1_PUSH_VCPF;
> +
> + reinit_completion(&ctrl->idle_comp);
> +
> + msm_dp_catalog_ctrl_state_ctrl(ctrl->catalog, state);
> +
> + if (!wait_for_completion_timeout(&ctrl->idle_comp,
> +
> IDLE_PATTERN_COMPLETION_TIMEOUT_JIFFIES))
> + pr_warn("PUSH_VCPF pattern timedout\n");
> +
> + drm_dbg_dp(ctrl->drm_dev, "mainlink off\n");
??
The comment seems misplaced or wrong to me. The msm_dp_ctrl_push_vcpf()
doesn't turn link off.
> +}
> +
> static void msm_dp_ctrl_config_ctrl(struct msm_dp_ctrl_private *ctrl,
> struct msm_dp_panel *msm_dp_panel)
> {
> @@ -2332,6 +2365,13 @@ irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl
> *msm_dp_ctrl)
> ret = IRQ_HANDLED;
> }
>
> + isr = msm_dp_catalog_ctrl_get_interrupt_5(ctrl->catalog);
> + if (isr & (DP_INTR_DP0_VCPF_SENT | DP_INTR_DP1_VCPF_SENT)) {
> + drm_dbg_dp(ctrl->drm_dev, "vcpf sent\n");
> + complete(&ctrl->idle_comp);
> + ret = IRQ_HANDLED;
> + }
> +
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> index
> b126651da24b3abdaf540268758b37dca9fe1291..9ad7022d6217572395d69294c3cc4d4dbaddf0ac
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
> @@ -52,5 +52,6 @@ void msm_dp_ctrl_mst_stream_channel_slot_setup(struct
> msm_dp_ctrl *msm_dp_ctrl,
> void msm_dp_ctrl_set_mst_channel_info(struct msm_dp_ctrl *msm_dp_ctrl,
> enum msm_dp_stream_id strm,
> u32 start_slot, u32 tot_slots);
> +void msm_dp_ctrl_push_vcpf(struct msm_dp_ctrl *dp_ctrl, struct msm_dp_panel
> *msm_dp_panel);
>
> #endif /* _DP_CTRL_H_ */
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index
> 2a4a79317153817cb24537ea95fad07c9bc20715..1dfc82211c50bb4ed239f9730b91c33c4897c78f
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1646,7 +1646,10 @@ void msm_dp_display_atomic_disable(struct msm_dp *dp)
>
> msm_dp_display = container_of(dp, struct msm_dp_display_private,
> msm_dp_display);
>
> - msm_dp_ctrl_push_idle(msm_dp_display->ctrl);
> + if (msm_dp_display->max_stream > DEFAULT_STREAM_COUNT)
Same comment regarding default vs SST
> + msm_dp_ctrl_push_vcpf(msm_dp_display->ctrl,
> msm_dp_display->panel);
> + else
> + msm_dp_ctrl_push_idle(msm_dp_display->ctrl);
>
> if (msm_dp_display->max_stream > DEFAULT_STREAM_COUNT) {
> msm_dp_ctrl_mst_stream_channel_slot_setup(msm_dp_display->ctrl,
> diff --git a/drivers/gpu/drm/msm/dp/dp_reg.h b/drivers/gpu/drm/msm/dp/dp_reg.h
> index
> 8bc2a431462fc1fb45b1fe8e43a0a0ec7f75e5b1..c7532217b369c6235b2fe5fe9c86642d5c2712cb
> 100644
> --- a/drivers/gpu/drm/msm/dp/dp_reg.h
> +++ b/drivers/gpu/drm/msm/dp/dp_reg.h
> @@ -24,8 +24,9 @@
> #define REG_DP_INTR_STATUS (0x00000020)
> #define REG_DP_INTR_STATUS2 (0x00000024)
> #define REG_DP_INTR_STATUS3 (0x00000028)
> -
> #define REG_DP_INTR_STATUS4 (0x0000002C)
> +#define REG_DP_INTR_STATUS5 (0x00000034)
> +
> #define PSR_UPDATE_INT (0x00000001)
> #define PSR_CAPTURE_INT (0x00000004)
> #define PSR_EXIT_INT (0x00000010)
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
