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

Reply via email to