diff mbox series

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

Message ID 20240125193834.7065-3-quic_parellan@quicinc.com (mailing list archive)
State New, archived
Headers show
Series Add support for CDM over DP | expand

Commit Message

Paloma Arellano Jan. 25, 2024, 7:38 p.m. UTC
Move dpu_encoder_helper_phys_setup_cdm to dpu_encoder in preparation for
implementing CDM compatibility for DP.

Signed-off-by: Paloma Arellano <quic_parellan@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(-)

Comments

Dmitry Baryshkov Jan. 25, 2024, 9:16 p.m. UTC | #1
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 <quic_parellan@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.

> +{
> +	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.

> +
>   /**
>    * 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.

>   /**
>    * _dpu_encoder_phys_wb_update_flush - flush hardware update
>    * @phys_enc:	Pointer to physical encoder
Paloma Arellano Jan. 27, 2024, 12:43 a.m. UTC | #2
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@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
>
Dmitry Baryshkov Jan. 27, 2024, 2:26 a.m. UTC | #3
On Sat, 27 Jan 2024 at 02:44, Paloma Arellano <quic_parellan@quicinc.com> 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 <quic_parellan@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.

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 <dmitry.baryshkov@linaro.org>

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

Patch

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 = &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);
+
 /**
  * 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);
 }
 
-
 /**
  * _dpu_encoder_phys_wb_update_flush - flush hardware update
  * @phys_enc:	Pointer to physical encoder