Re: [PATCH 14/17] drm/msm/dpu: modify encoder programming for CDM over DP

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 1/25/2024 1:57 PM, Dmitry Baryshkov wrote:
On 25/01/2024 21:38, Paloma Arellano wrote:
Adjust the encoder format programming in the case of video mode for DP
to accommodate CDM related changes.

Signed-off-by: Paloma Arellano <quic_parellan@xxxxxxxxxxx>
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 16 +++++++++
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h   |  8 +++++
  .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  | 35 ++++++++++++++++---
  drivers/gpu/drm/msm/dp/dp_display.c           | 12 +++++++
  drivers/gpu/drm/msm/msm_drv.h                 |  9 ++++-
  5 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index b0896814c1562..99ec53446ad21 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -222,6 +222,22 @@ static u32 dither_matrix[DITHER_MATRIX_SZ] = {
      15, 7, 13, 5, 3, 11, 1, 9, 12, 4, 14, 6, 0, 8, 2, 10
  };
  +u32 dpu_encoder_get_drm_fmt(const struct drm_encoder *drm_enc, const struct drm_display_mode *mode)
+{
+    const struct dpu_encoder_virt *dpu_enc;
+    const struct msm_display_info *disp_info;
+    struct msm_drm_private *priv;
+
+    dpu_enc = to_dpu_encoder_virt(drm_enc);
+    disp_info = &dpu_enc->disp_info;
+    priv = drm_enc->dev->dev_private;
+
+    if (disp_info->intf_type == INTF_DP &&
+ msm_dp_is_yuv_420_enabled(priv->dp[disp_info->h_tile_instance[0]], mode))

This should not require interacting with DP. If we got here, we must be sure that 4:2:0 is supported and can be configured.
Ack. Will drop this function and only check for if the mode is YUV420.

+        return DRM_FORMAT_YUV420;
+
+    return DRM_FORMAT_RGB888;
+}
    bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
  {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 7b4afa71f1f96..62255d0aa4487 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -162,6 +162,14 @@ int dpu_encoder_get_vsync_count(struct drm_encoder *drm_enc);
   */
  bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc);
  +/**
+ * dpu_encoder_get_drm_fmt - return DRM fourcc format
+ * @drm_enc:    Pointer to previously created drm encoder structure
+ * @mode:    Corresponding drm_display_mode for dpu encoder
+ */
+u32 dpu_encoder_get_drm_fmt(const struct drm_encoder *drm_enc,
+                const struct drm_display_mode *mode);
+
  /**
   * dpu_encoder_get_crc_values_cnt - get number of physical encoders contained
   *    in virtual encoder that can collect CRC values
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index e284bf448bdda..a1dde0ff35dc8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -234,6 +234,7 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
  {
      struct drm_display_mode mode;
      struct dpu_hw_intf_timing_params timing_params = { 0 };
+    struct dpu_hw_cdm *hw_cdm;
      const struct dpu_format *fmt = NULL;
      u32 fmt_fourcc = DRM_FORMAT_RGB888;
      unsigned long lock_flags;
@@ -254,17 +255,26 @@ static void dpu_encoder_phys_vid_setup_timing_engine(
      DPU_DEBUG_VIDENC(phys_enc, "enabling mode:\n");
      drm_mode_debug_printmodeline(&mode);
  -    if (phys_enc->split_role != ENC_ROLE_SOLO) {
+    hw_cdm = phys_enc->hw_cdm;
+    if (hw_cdm) {
+        intf_cfg.cdm = hw_cdm->idx;
+        fmt_fourcc = dpu_encoder_get_drm_fmt(phys_enc->parent, &mode);
+    }
+
+    if (phys_enc->split_role != ENC_ROLE_SOLO ||
+        dpu_encoder_get_drm_fmt(phys_enc->parent, &mode) == DRM_FORMAT_YUV420) {
          mode.hdisplay >>= 1;
          mode.htotal >>= 1;
          mode.hsync_start >>= 1;
          mode.hsync_end >>= 1;
+        mode.hskew >>= 1;

Separate patch.
Ack.

            DPU_DEBUG_VIDENC(phys_enc,
-            "split_role %d, halve horizontal %d %d %d %d\n",
+            "split_role %d, halve horizontal %d %d %d %d %d\n",
              phys_enc->split_role,
              mode.hdisplay, mode.htotal,
-            mode.hsync_start, mode.hsync_end);
+            mode.hsync_start, mode.hsync_end,
+            mode.hskew);
      }
        drm_mode_to_intf_timing_params(phys_enc, &mode, &timing_params);
@@ -412,8 +422,15 @@ static int dpu_encoder_phys_vid_control_vblank_irq(
  static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)
  {
      struct dpu_hw_ctl *ctl;
+    struct dpu_hw_cdm *hw_cdm;
+    const struct dpu_format *fmt = NULL;
+    u32 fmt_fourcc = DRM_FORMAT_RGB888;
        ctl = phys_enc->hw_ctl;
+    hw_cdm = phys_enc->hw_cdm;
+    if (hw_cdm)
+        fmt_fourcc = dpu_encoder_get_drm_fmt(phys_enc->parent, &phys_enc->cached_mode);
+    fmt = dpu_get_dpu_format(fmt_fourcc);
        DPU_DEBUG_VIDENC(phys_enc, "\n");
  @@ -422,6 +439,8 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)         dpu_encoder_helper_split_config(phys_enc, phys_enc->hw_intf->idx);   +    dpu_encoder_helper_phys_setup_cdm(phys_enc, fmt, CDM_CDWN_OUTPUT_HDMI);

If there is no CDM, why do we need to call this?
Inside of dpu_encoder_helper_phys_setup_cdm(), there's a check to see if there is a hw_cdm. If there is not, then it immediately exits the function.

+
      dpu_encoder_phys_vid_setup_timing_engine(phys_enc);
        /*
@@ -437,7 +456,15 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc)       if (ctl->ops.update_pending_flush_merge_3d && phys_enc->hw_pp->merge_3d)           ctl->ops.update_pending_flush_merge_3d(ctl, phys_enc->hw_pp->merge_3d->idx);   -    if (ctl->ops.update_pending_flush_periph && phys_enc->hw_intf->cap->type == INTF_DP)
+    if (ctl->ops.update_pending_flush_cdm && phys_enc->hw_cdm)
+        ctl->ops.update_pending_flush_cdm(ctl, hw_cdm->idx);
+
+    /*
+     * Peripheral flush must be updated whenever flushing SDP packets is needed. +     * SDP packets are required for any YUV format (YUV420, YUV422, YUV444).
+     */
+    if (ctl->ops.update_pending_flush_periph && phys_enc->hw_intf->cap->type == INTF_DP &&
+        phys_enc->hw_cdm)
          ctl->ops.update_pending_flush_periph(ctl, phys_enc->hw_intf->idx);

Should there be a flush if we are switching from YUV 420 to RGB mode?
We only need to flush for the sdp packet, but for msa we do not need to flush.

Also, I'd say, we should move update_pending_flush_periph invocation to this patch.
Ack

    skip_flush:
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 6d764f5b08727..4329435518351 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1399,6 +1399,18 @@ void __exit msm_dp_unregister(void)
      platform_driver_unregister(&dp_display_driver);
  }
  +bool msm_dp_is_yuv_420_enabled(const struct msm_dp *dp_display,
+                   const struct drm_display_mode *mode)
+{
+    struct dp_display_private *dp;
+    const struct drm_display_info *info;
+
+    dp = container_of(dp_display, struct dp_display_private, dp_display);
+    info = &dp_display->connector->display_info;
+
+    return dp_panel_vsc_sdp_supported(dp->panel) && drm_mode_is_420_only(info, mode);

YUV 420 modes should be filtered out in mode_valid if VSC SDP is not supported.
Ack. Will change

+}
+
  bool msm_dp_wide_bus_available(const struct msm_dp *dp_display)
  {
      struct dp_display_private *dp;
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 16a7cbc0b7dd8..b9581bd934e9e 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -387,7 +387,8 @@ void __exit msm_dp_unregister(void);
  int msm_dp_modeset_init(struct msm_dp *dp_display, struct drm_device *dev,
               struct drm_encoder *encoder);
  void msm_dp_snapshot(struct msm_disp_state *disp_state, struct msm_dp *dp_display);
-
+bool msm_dp_is_yuv_420_enabled(const struct msm_dp *dp_display,
+                   const struct drm_display_mode *mode);
  bool msm_dp_wide_bus_available(const struct msm_dp *dp_display);
    #else
@@ -409,6 +410,12 @@ static inline void msm_dp_snapshot(struct msm_disp_state *disp_state, struct msm
  {
  }
  +static inline bool msm_dp_is_yuv_420_enabled(const struct msm_dp *dp_display,
+                         const struct drm_display_mode *mode)
+{
+    return false;
+}
+
  static inline bool msm_dp_wide_bus_available(const struct msm_dp *dp_display)
  {
      return false;




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux