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

Paloma Arellano quic_parellan at quicinc.com
Sat Jan 27 00:43:56 UTC 2024


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 <quic_parellan at quicinc.com>
>> ---
>>   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 = &phys_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 = &dpu_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 
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
>> index 072fc6950e496..400580847bde7 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
>> @@ -264,89 +264,6 @@ static void dpu_encoder_phys_wb_setup_ctl(struct 
>> dpu_encoder_phys *phys_enc)
>>       }
>>   }
>>   -/**
>> - * dpu_encoder_helper_phys_setup_cdm - setup chroma down sampling block
>> - *                                     This API does not handle 
>> DPU_CHROMA_H1V2.
>> - * @phys_enc:Pointer to physical encoder
>> - */
>> -static 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 = &phys_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 = &dpu_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;
>> -        }
>> -    }
>> -}
>> -
>>   /**
>>    * 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
>
>>   /**
>>    * _dpu_encoder_phys_wb_update_flush - flush hardware update
>>    * @phys_enc:    Pointer to physical encoder
>


More information about the dri-devel mailing list