Re: [PATCH 02/17] drm/msm/dpu: move dpu_encoder_helper_phys_setup_cdm to dpu_encoder

2024-01-26 Thread Dmitry Baryshkov
On Sat, 27 Jan 2024 at 02:44, Paloma Arellano  wrote:
>
>
> On 1/25/2024 1:16 PM, Dmitry Baryshkov wrote:
> > On 25/01/2024 21:38, Paloma Arellano wrote:
> >> Move dpu_encoder_helper_phys_setup_cdm to dpu_encoder in preparation for
> >> implementing CDM compatibility for DP.
> >
> > Nit: s/CDM compatibility/YUV support/. It might make sense to spell it
> > out that YUV over DP requires CDM.
> Ack
> >
> >>
> >> Signed-off-by: Paloma Arellano 
> >> ---
> >>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 78 +
> >>   .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |  9 ++
> >>   .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c   | 84 ---
> >>   3 files changed, 87 insertions(+), 84 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> index 83380bc92a00a..6cef98f046ea6 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >> @@ -2114,6 +2114,84 @@ void dpu_encoder_helper_phys_cleanup(struct
> >> dpu_encoder_phys *phys_enc)
> >>   ctl->ops.clear_pending_flush(ctl);
> >>   }
> >>   +void dpu_encoder_helper_phys_setup_cdm(struct dpu_encoder_phys
> >> *phys_enc,
> >> +   const struct dpu_format *dpu_fmt,
> >> +   u32 output_type)
> >
> > My email client suggests that the parameters are not idented properly
> > anymore.
> On my editor it appears to be aligned correctly. Running checkpatch.pl
> doesn't give any warnings either. So perhaps it's the email client.

Checked, you are correct here.

> >

[skipped]

> >>* dpu_encoder_phys_wb_atomic_check - verify and fixup given atomic
> >> states
> >>* @phys_enc:Pointer to physical encoder
> >> @@ -399,7 +316,6 @@ static int dpu_encoder_phys_wb_atomic_check(
> >>   return
> >> drm_atomic_helper_check_wb_connector_state(conn_state->connector,
> >> conn_state->state);
> >>   }
> >>   -
> >
> > irrelevant, please drop.
> Ack

With this fixed:

Reviewed-by: Dmitry Baryshkov 

> >
> >>   /**
> >>* _dpu_encoder_phys_wb_update_flush - flush hardware update
> >>* @phys_enc:Pointer to physical encoder
> >



-- 
With best wishes
Dmitry


Re: [PATCH 02/17] drm/msm/dpu: move dpu_encoder_helper_phys_setup_cdm to dpu_encoder

2024-01-26 Thread Paloma Arellano



On 1/25/2024 1:16 PM, Dmitry Baryshkov wrote:

On 25/01/2024 21:38, Paloma Arellano wrote:

Move dpu_encoder_helper_phys_setup_cdm to dpu_encoder in preparation for
implementing CDM compatibility for DP.


Nit: s/CDM compatibility/YUV support/. It might make sense to spell it 
out that YUV over DP requires CDM.

Ack




Signed-off-by: Paloma Arellano 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 78 +
  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |  9 ++
  .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c   | 84 ---
  3 files changed, 87 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c

index 83380bc92a00a..6cef98f046ea6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2114,6 +2114,84 @@ void dpu_encoder_helper_phys_cleanup(struct 
dpu_encoder_phys *phys_enc)

  ctl->ops.clear_pending_flush(ctl);
  }
  +void dpu_encoder_helper_phys_setup_cdm(struct dpu_encoder_phys 
*phys_enc,

+   const struct dpu_format *dpu_fmt,
+   u32 output_type)


My email client suggests that the parameters are not idented properly 
anymore.
On my editor it appears to be aligned correctly. Running checkpatch.pl 
doesn't give any warnings either. So perhaps it's the email client.



+{
+    struct dpu_hw_cdm *hw_cdm;
+    struct dpu_hw_cdm_cfg *cdm_cfg;
+    struct dpu_hw_pingpong *hw_pp;
+    int ret;
+
+    if (!phys_enc)
+    return;
+
+    cdm_cfg = _enc->cdm_cfg;
+    hw_pp = phys_enc->hw_pp;
+    hw_cdm = phys_enc->hw_cdm;
+
+    if (!hw_cdm)
+    return;
+
+    if (!DPU_FORMAT_IS_YUV(dpu_fmt)) {
+    DPU_DEBUG("[enc:%d] cdm_disable fmt:%x\n", 
DRMID(phys_enc->parent),

+  dpu_fmt->base.pixel_format);
+    if (hw_cdm->ops.bind_pingpong_blk)
+    hw_cdm->ops.bind_pingpong_blk(hw_cdm, PINGPONG_NONE);
+
+    return;
+    }
+
+    memset(cdm_cfg, 0, sizeof(struct dpu_hw_cdm_cfg));
+
+    cdm_cfg->output_width = phys_enc->cached_mode.hdisplay;
+    cdm_cfg->output_height = phys_enc->cached_mode.vdisplay;
+    cdm_cfg->output_fmt = dpu_fmt;
+    cdm_cfg->output_type = output_type;
+    cdm_cfg->output_bit_depth = DPU_FORMAT_IS_DX(dpu_fmt) ?
+    CDM_CDWN_OUTPUT_10BIT : CDM_CDWN_OUTPUT_8BIT;
+    cdm_cfg->csc_cfg = _csc10_rgb2yuv_601l;
+
+    /* enable 10 bit logic */
+    switch (cdm_cfg->output_fmt->chroma_sample) {
+    case DPU_CHROMA_RGB:
+    cdm_cfg->h_cdwn_type = CDM_CDWN_DISABLE;
+    cdm_cfg->v_cdwn_type = CDM_CDWN_DISABLE;
+    break;
+    case DPU_CHROMA_H2V1:
+    cdm_cfg->h_cdwn_type = CDM_CDWN_COSITE;
+    cdm_cfg->v_cdwn_type = CDM_CDWN_DISABLE;
+    break;
+    case DPU_CHROMA_420:
+    cdm_cfg->h_cdwn_type = CDM_CDWN_COSITE;
+    cdm_cfg->v_cdwn_type = CDM_CDWN_OFFSITE;
+    break;
+    case DPU_CHROMA_H1V2:
+    default:
+    DPU_ERROR("[enc:%d] unsupported chroma sampling type\n",
+  DRMID(phys_enc->parent));
+    cdm_cfg->h_cdwn_type = CDM_CDWN_DISABLE;
+    cdm_cfg->v_cdwn_type = CDM_CDWN_DISABLE;
+    break;
+    }
+
+    DPU_DEBUG("[enc:%d] cdm_enable:%d,%d,%X,%d,%d,%d,%d]\n",
+  DRMID(phys_enc->parent), cdm_cfg->output_width,
+  cdm_cfg->output_height, 
cdm_cfg->output_fmt->base.pixel_format,

+  cdm_cfg->output_type, cdm_cfg->output_bit_depth,
+  cdm_cfg->h_cdwn_type, cdm_cfg->v_cdwn_type);
+
+    if (hw_cdm->ops.enable) {
+    cdm_cfg->pp_id = hw_pp->idx;
+    ret = hw_cdm->ops.enable(hw_cdm, cdm_cfg);
+    if (ret < 0) {
+    DPU_ERROR("[enc:%d] failed to enable CDM; ret:%d\n",
+  DRMID(phys_enc->parent), ret);
+    return;
+    }
+    }
+}
+
  #ifdef CONFIG_DEBUG_FS
  static int _dpu_encoder_status_show(struct seq_file *s, void *data)
  {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h

index 37ac385727c3b..310944303a056 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -381,6 +381,15 @@ int dpu_encoder_helper_wait_for_irq(struct 
dpu_encoder_phys *phys_enc,

   */
  void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys 
*phys_enc);

  +/**
+ * dpu_encoder_helper_phys_setup_cdm - setup chroma down sampling block
+ * @phys_enc: Pointer to physical encoder
+ * @output_type: HDMI/WB
+ */
+void dpu_encoder_helper_phys_setup_cdm(struct dpu_encoder_phys 
*phys_enc,

+   const struct dpu_format *dpu_fmt,
+   u32 output_type);


Again, indentation.

See comment above



+
  /**
   * dpu_encoder_vblank_callback - Notify virtual encoder of vblank 
IRQ reception

   * @drm_enc:    Pointer to drm encoder structure
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c 

Re: [PATCH 02/17] drm/msm/dpu: move dpu_encoder_helper_phys_setup_cdm to dpu_encoder

2024-01-25 Thread Dmitry Baryshkov

On 25/01/2024 21:38, Paloma Arellano wrote:

Move dpu_encoder_helper_phys_setup_cdm to dpu_encoder in preparation for
implementing CDM compatibility for DP.


Nit: s/CDM compatibility/YUV support/. It might make sense to spell it 
out that YUV over DP requires CDM.




Signed-off-by: Paloma Arellano 
---
  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 78 +
  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |  9 ++
  .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c   | 84 ---
  3 files changed, 87 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 83380bc92a00a..6cef98f046ea6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2114,6 +2114,84 @@ void dpu_encoder_helper_phys_cleanup(struct 
dpu_encoder_phys *phys_enc)
ctl->ops.clear_pending_flush(ctl);
  }
  
+void dpu_encoder_helper_phys_setup_cdm(struct dpu_encoder_phys *phys_enc,

+  const struct dpu_format *dpu_fmt,
+  u32 output_type)


My email client suggests that the parameters are not idented properly 
anymore.



+{
+   struct dpu_hw_cdm *hw_cdm;
+   struct dpu_hw_cdm_cfg *cdm_cfg;
+   struct dpu_hw_pingpong *hw_pp;
+   int ret;
+
+   if (!phys_enc)
+   return;
+
+   cdm_cfg = _enc->cdm_cfg;
+   hw_pp = phys_enc->hw_pp;
+   hw_cdm = phys_enc->hw_cdm;
+
+   if (!hw_cdm)
+   return;
+
+   if (!DPU_FORMAT_IS_YUV(dpu_fmt)) {
+   DPU_DEBUG("[enc:%d] cdm_disable fmt:%x\n", 
DRMID(phys_enc->parent),
+ dpu_fmt->base.pixel_format);
+   if (hw_cdm->ops.bind_pingpong_blk)
+   hw_cdm->ops.bind_pingpong_blk(hw_cdm, PINGPONG_NONE);
+
+   return;
+   }
+
+   memset(cdm_cfg, 0, sizeof(struct dpu_hw_cdm_cfg));
+
+   cdm_cfg->output_width = phys_enc->cached_mode.hdisplay;
+   cdm_cfg->output_height = phys_enc->cached_mode.vdisplay;
+   cdm_cfg->output_fmt = dpu_fmt;
+   cdm_cfg->output_type = output_type;
+   cdm_cfg->output_bit_depth = DPU_FORMAT_IS_DX(dpu_fmt) ?
+   CDM_CDWN_OUTPUT_10BIT : CDM_CDWN_OUTPUT_8BIT;
+   cdm_cfg->csc_cfg = _csc10_rgb2yuv_601l;
+
+   /* enable 10 bit logic */
+   switch (cdm_cfg->output_fmt->chroma_sample) {
+   case DPU_CHROMA_RGB:
+   cdm_cfg->h_cdwn_type = CDM_CDWN_DISABLE;
+   cdm_cfg->v_cdwn_type = CDM_CDWN_DISABLE;
+   break;
+   case DPU_CHROMA_H2V1:
+   cdm_cfg->h_cdwn_type = CDM_CDWN_COSITE;
+   cdm_cfg->v_cdwn_type = CDM_CDWN_DISABLE;
+   break;
+   case DPU_CHROMA_420:
+   cdm_cfg->h_cdwn_type = CDM_CDWN_COSITE;
+   cdm_cfg->v_cdwn_type = CDM_CDWN_OFFSITE;
+   break;
+   case DPU_CHROMA_H1V2:
+   default:
+   DPU_ERROR("[enc:%d] unsupported chroma sampling type\n",
+ DRMID(phys_enc->parent));
+   cdm_cfg->h_cdwn_type = CDM_CDWN_DISABLE;
+   cdm_cfg->v_cdwn_type = CDM_CDWN_DISABLE;
+   break;
+   }
+
+   DPU_DEBUG("[enc:%d] cdm_enable:%d,%d,%X,%d,%d,%d,%d]\n",
+ DRMID(phys_enc->parent), cdm_cfg->output_width,
+ cdm_cfg->output_height, 
cdm_cfg->output_fmt->base.pixel_format,
+ cdm_cfg->output_type, cdm_cfg->output_bit_depth,
+ cdm_cfg->h_cdwn_type, cdm_cfg->v_cdwn_type);
+
+   if (hw_cdm->ops.enable) {
+   cdm_cfg->pp_id = hw_pp->idx;
+   ret = hw_cdm->ops.enable(hw_cdm, cdm_cfg);
+   if (ret < 0) {
+   DPU_ERROR("[enc:%d] failed to enable CDM; ret:%d\n",
+ DRMID(phys_enc->parent), ret);
+   return;
+   }
+   }
+}
+
  #ifdef CONFIG_DEBUG_FS
  static int _dpu_encoder_status_show(struct seq_file *s, void *data)
  {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index 37ac385727c3b..310944303a056 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -381,6 +381,15 @@ int dpu_encoder_helper_wait_for_irq(struct 
dpu_encoder_phys *phys_enc,
   */
  void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc);
  
+/**

+ * dpu_encoder_helper_phys_setup_cdm - setup chroma down sampling block
+ * @phys_enc: Pointer to physical encoder
+ * @output_type: HDMI/WB
+ */
+void dpu_encoder_helper_phys_setup_cdm(struct dpu_encoder_phys *phys_enc,
+  const struct dpu_format *dpu_fmt,
+  u32 output_type);


Again, indentation.


+
  /**
   * dpu_encoder_vblank_callback - Notify virtual 

[PATCH 02/17] drm/msm/dpu: move dpu_encoder_helper_phys_setup_cdm to dpu_encoder

2024-01-25 Thread Paloma Arellano
Move dpu_encoder_helper_phys_setup_cdm to dpu_encoder in preparation for
implementing CDM compatibility for DP.

Signed-off-by: Paloma Arellano 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 78 +
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  |  9 ++
 .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c   | 84 ---
 3 files changed, 87 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 83380bc92a00a..6cef98f046ea6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2114,6 +2114,84 @@ void dpu_encoder_helper_phys_cleanup(struct 
dpu_encoder_phys *phys_enc)
ctl->ops.clear_pending_flush(ctl);
 }
 
+void dpu_encoder_helper_phys_setup_cdm(struct dpu_encoder_phys *phys_enc,
+  const struct dpu_format *dpu_fmt,
+  u32 output_type)
+{
+   struct dpu_hw_cdm *hw_cdm;
+   struct dpu_hw_cdm_cfg *cdm_cfg;
+   struct dpu_hw_pingpong *hw_pp;
+   int ret;
+
+   if (!phys_enc)
+   return;
+
+   cdm_cfg = _enc->cdm_cfg;
+   hw_pp = phys_enc->hw_pp;
+   hw_cdm = phys_enc->hw_cdm;
+
+   if (!hw_cdm)
+   return;
+
+   if (!DPU_FORMAT_IS_YUV(dpu_fmt)) {
+   DPU_DEBUG("[enc:%d] cdm_disable fmt:%x\n", 
DRMID(phys_enc->parent),
+ dpu_fmt->base.pixel_format);
+   if (hw_cdm->ops.bind_pingpong_blk)
+   hw_cdm->ops.bind_pingpong_blk(hw_cdm, PINGPONG_NONE);
+
+   return;
+   }
+
+   memset(cdm_cfg, 0, sizeof(struct dpu_hw_cdm_cfg));
+
+   cdm_cfg->output_width = phys_enc->cached_mode.hdisplay;
+   cdm_cfg->output_height = phys_enc->cached_mode.vdisplay;
+   cdm_cfg->output_fmt = dpu_fmt;
+   cdm_cfg->output_type = output_type;
+   cdm_cfg->output_bit_depth = DPU_FORMAT_IS_DX(dpu_fmt) ?
+   CDM_CDWN_OUTPUT_10BIT : CDM_CDWN_OUTPUT_8BIT;
+   cdm_cfg->csc_cfg = _csc10_rgb2yuv_601l;
+
+   /* enable 10 bit logic */
+   switch (cdm_cfg->output_fmt->chroma_sample) {
+   case DPU_CHROMA_RGB:
+   cdm_cfg->h_cdwn_type = CDM_CDWN_DISABLE;
+   cdm_cfg->v_cdwn_type = CDM_CDWN_DISABLE;
+   break;
+   case DPU_CHROMA_H2V1:
+   cdm_cfg->h_cdwn_type = CDM_CDWN_COSITE;
+   cdm_cfg->v_cdwn_type = CDM_CDWN_DISABLE;
+   break;
+   case DPU_CHROMA_420:
+   cdm_cfg->h_cdwn_type = CDM_CDWN_COSITE;
+   cdm_cfg->v_cdwn_type = CDM_CDWN_OFFSITE;
+   break;
+   case DPU_CHROMA_H1V2:
+   default:
+   DPU_ERROR("[enc:%d] unsupported chroma sampling type\n",
+ DRMID(phys_enc->parent));
+   cdm_cfg->h_cdwn_type = CDM_CDWN_DISABLE;
+   cdm_cfg->v_cdwn_type = CDM_CDWN_DISABLE;
+   break;
+   }
+
+   DPU_DEBUG("[enc:%d] cdm_enable:%d,%d,%X,%d,%d,%d,%d]\n",
+ DRMID(phys_enc->parent), cdm_cfg->output_width,
+ cdm_cfg->output_height, 
cdm_cfg->output_fmt->base.pixel_format,
+ cdm_cfg->output_type, cdm_cfg->output_bit_depth,
+ cdm_cfg->h_cdwn_type, cdm_cfg->v_cdwn_type);
+
+   if (hw_cdm->ops.enable) {
+   cdm_cfg->pp_id = hw_pp->idx;
+   ret = hw_cdm->ops.enable(hw_cdm, cdm_cfg);
+   if (ret < 0) {
+   DPU_ERROR("[enc:%d] failed to enable CDM; ret:%d\n",
+ DRMID(phys_enc->parent), ret);
+   return;
+   }
+   }
+}
+
 #ifdef CONFIG_DEBUG_FS
 static int _dpu_encoder_status_show(struct seq_file *s, void *data)
 {
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index 37ac385727c3b..310944303a056 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -381,6 +381,15 @@ int dpu_encoder_helper_wait_for_irq(struct 
dpu_encoder_phys *phys_enc,
  */
 void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc);
 
+/**
+ * dpu_encoder_helper_phys_setup_cdm - setup chroma down sampling block
+ * @phys_enc: Pointer to physical encoder
+ * @output_type: HDMI/WB
+ */
+void dpu_encoder_helper_phys_setup_cdm(struct dpu_encoder_phys *phys_enc,
+  const struct dpu_format *dpu_fmt,
+  u32 output_type);
+
 /**
  * dpu_encoder_vblank_callback - Notify virtual encoder of vblank IRQ reception
  * @drm_enc:Pointer to drm encoder structure
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index 072fc6950e496..400580847bde7 100644
---