diff mbox series

[v3,16/19] drm/msm/dpu: modify encoder programming for CDM over DP

Message ID 20240214180347.1399-17-quic_parellan@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Add support for CDM over DP | expand

Commit Message

Paloma Arellano Feb. 14, 2024, 6:03 p.m. UTC
Adjust the encoder format programming in the case of video mode for DP
to accommodate CDM related changes.

Changes in v2:
	- Move timing engine programming to a separate patch from this
	  one
	- Move update_pending_flush_periph() invocation completely to
	  this patch
	- Change the logic of dpu_encoder_get_drm_fmt() so that it only
	  calls drm_mode_is_420_only() instead of doing additional
	  unnecessary checks
	- Create new functions msm_dp_needs_periph_flush() and it's
	  supporting function dpu_encoder_needs_periph_flush() to check
	  if the mode is YUV420 and VSC SDP is enabled before doing a
	  peripheral flush

Signed-off-by: Paloma Arellano <quic_parellan@quicinc.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 35 +++++++++++++++++++
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  | 13 +++++++
 .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  | 19 ++++++++++
 drivers/gpu/drm/msm/dp/dp_display.c           | 18 ++++++++++
 drivers/gpu/drm/msm/msm_drv.h                 | 17 ++++++++-
 5 files changed, 101 insertions(+), 1 deletion(-)

Comments

Dmitry Baryshkov Feb. 15, 2024, 8:45 a.m. UTC | #1
On Wed, 14 Feb 2024 at 20:04, Paloma Arellano <quic_parellan@quicinc.com> wrote:
>
> Adjust the encoder format programming in the case of video mode for DP
> to accommodate CDM related changes.
>
> Changes in v2:
>         - Move timing engine programming to a separate patch from this
>           one
>         - Move update_pending_flush_periph() invocation completely to
>           this patch
>         - Change the logic of dpu_encoder_get_drm_fmt() so that it only
>           calls drm_mode_is_420_only() instead of doing additional
>           unnecessary checks
>         - Create new functions msm_dp_needs_periph_flush() and it's
>           supporting function dpu_encoder_needs_periph_flush() to check
>           if the mode is YUV420 and VSC SDP is enabled before doing a
>           peripheral flush
>
> Signed-off-by: Paloma Arellano <quic_parellan@quicinc.com>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 35 +++++++++++++++++++
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  | 13 +++++++
>  .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  | 19 ++++++++++
>  drivers/gpu/drm/msm/dp/dp_display.c           | 18 ++++++++++
>  drivers/gpu/drm/msm/msm_drv.h                 | 17 ++++++++-
>  5 files changed, 101 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 7e7796561009a..6280c6be6dca9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -222,6 +222,41 @@ 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(struct dpu_encoder_phys *phys_enc)
> +{
> +       struct drm_encoder *drm_enc;
> +       struct dpu_encoder_virt *dpu_enc;
> +       struct drm_display_info *info;
> +       struct drm_display_mode *mode;
> +
> +       drm_enc = phys_enc->parent;
> +       dpu_enc = to_dpu_encoder_virt(drm_enc);
> +       info = &dpu_enc->connector->display_info;
> +       mode = &phys_enc->cached_mode;
> +
> +       if (drm_mode_is_420_only(info, mode))
> +               return DRM_FORMAT_YUV420;
> +
> +       return DRM_FORMAT_RGB888;
> +}
> +
> +bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc)
> +{
> +       struct drm_encoder *drm_enc;
> +       struct dpu_encoder_virt *dpu_enc;
> +       struct msm_display_info *disp_info;
> +       struct msm_drm_private *priv;
> +       struct drm_display_mode *mode;
> +
> +       drm_enc = phys_enc->parent;
> +       dpu_enc = to_dpu_encoder_virt(drm_enc);
> +       disp_info = &dpu_enc->disp_info;
> +       priv = drm_enc->dev->dev_private;
> +       mode = &phys_enc->cached_mode;
> +
> +       return phys_enc->hw_intf->cap->type == INTF_DP && phys_enc->hw_cdm &&
> +              msm_dp_needs_periph_flush(priv->dp[disp_info->h_tile_instance[0]], mode);
> +}
>
>  bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
>  {
> 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 f43d57d9c74e1..211a3d90eb690 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -341,6 +341,19 @@ static inline enum dpu_3d_blend_mode dpu_encoder_helper_get_3d_blend_mode(
>   */
>  unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys *phys_enc);
>
> +/**
> + * dpu_encoder_get_drm_fmt - return DRM fourcc format
> + * @phys_enc: Pointer to physical encoder structure
> + */
> +u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc);
> +
> +/**
> + * dpu_encoder_needs_periph_flush - return true if physical encoder requires
> + *     peripheral flush
> + * @phys_enc: Pointer to physical encoder structure
> + */
> +bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc);
> +
>  /**
>   * dpu_encoder_helper_split_config - split display configuration helper function
>   *     This helper function may be used by physical encoders to configure
> 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 f02411b062c4c..e29bc4bd39208 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
> @@ -415,8 +415,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)

I thought that Abhinav proposed to drop the if(hw_cdm) condition here.
LGTM otherwise.

> +               fmt_fourcc = dpu_encoder_get_drm_fmt(phys_enc);
> +       fmt = dpu_get_dpu_format(fmt_fourcc);
>
>         DPU_DEBUG_VIDENC(phys_enc, "\n");
>
> @@ -425,6 +432,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);
> +
>         dpu_encoder_phys_vid_setup_timing_engine(phys_enc);
>
>         /*
> @@ -440,6 +449,16 @@ 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_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 && dpu_encoder_needs_periph_flush(phys_enc))
> +               ctl->ops.update_pending_flush_periph(ctl, phys_enc->hw_intf->idx);
> +
>  skip_flush:
>         DPU_DEBUG_VIDENC(phys_enc,
>                 "update pending flush ctl %d intf %d\n",
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 4b04388719363..ebcc76ef1d590 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1397,6 +1397,24 @@ 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 && drm_mode_is_420_only(info, mode);
> +}
> +
> +bool msm_dp_needs_periph_flush(const struct msm_dp *dp_display,
> +                              const struct drm_display_mode *mode)
> +{
> +       return msm_dp_is_yuv_420_enabled(dp_display, mode);
> +}
> +
>  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..b876ebd48effe 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -387,7 +387,10 @@ 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_needs_periph_flush(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 +412,18 @@ 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_needs_periph_flush(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;
> --
> 2.39.2
>
Abhinav Kumar Feb. 15, 2024, 3:47 p.m. UTC | #2
On 2/15/2024 12:45 AM, Dmitry Baryshkov wrote:
> On Wed, 14 Feb 2024 at 20:04, Paloma Arellano <quic_parellan@quicinc.com> wrote:
>>
>> Adjust the encoder format programming in the case of video mode for DP
>> to accommodate CDM related changes.
>>
>> Changes in v2:
>>          - Move timing engine programming to a separate patch from this
>>            one
>>          - Move update_pending_flush_periph() invocation completely to
>>            this patch
>>          - Change the logic of dpu_encoder_get_drm_fmt() so that it only
>>            calls drm_mode_is_420_only() instead of doing additional
>>            unnecessary checks
>>          - Create new functions msm_dp_needs_periph_flush() and it's
>>            supporting function dpu_encoder_needs_periph_flush() to check
>>            if the mode is YUV420 and VSC SDP is enabled before doing a
>>            peripheral flush
>>
>> Signed-off-by: Paloma Arellano <quic_parellan@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 35 +++++++++++++++++++
>>   .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  | 13 +++++++
>>   .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  | 19 ++++++++++
>>   drivers/gpu/drm/msm/dp/dp_display.c           | 18 ++++++++++
>>   drivers/gpu/drm/msm/msm_drv.h                 | 17 ++++++++-
>>   5 files changed, 101 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index 7e7796561009a..6280c6be6dca9 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -222,6 +222,41 @@ 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(struct dpu_encoder_phys *phys_enc)
>> +{
>> +       struct drm_encoder *drm_enc;
>> +       struct dpu_encoder_virt *dpu_enc;
>> +       struct drm_display_info *info;
>> +       struct drm_display_mode *mode;
>> +
>> +       drm_enc = phys_enc->parent;
>> +       dpu_enc = to_dpu_encoder_virt(drm_enc);
>> +       info = &dpu_enc->connector->display_info;
>> +       mode = &phys_enc->cached_mode;
>> +
>> +       if (drm_mode_is_420_only(info, mode))
>> +               return DRM_FORMAT_YUV420;
>> +
>> +       return DRM_FORMAT_RGB888;
>> +}
>> +
>> +bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc)
>> +{
>> +       struct drm_encoder *drm_enc;
>> +       struct dpu_encoder_virt *dpu_enc;
>> +       struct msm_display_info *disp_info;
>> +       struct msm_drm_private *priv;
>> +       struct drm_display_mode *mode;
>> +
>> +       drm_enc = phys_enc->parent;
>> +       dpu_enc = to_dpu_encoder_virt(drm_enc);
>> +       disp_info = &dpu_enc->disp_info;
>> +       priv = drm_enc->dev->dev_private;
>> +       mode = &phys_enc->cached_mode;
>> +
>> +       return phys_enc->hw_intf->cap->type == INTF_DP && phys_enc->hw_cdm &&
>> +              msm_dp_needs_periph_flush(priv->dp[disp_info->h_tile_instance[0]], mode);
>> +}
>>
>>   bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
>>   {
>> 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 f43d57d9c74e1..211a3d90eb690 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
>> @@ -341,6 +341,19 @@ static inline enum dpu_3d_blend_mode dpu_encoder_helper_get_3d_blend_mode(
>>    */
>>   unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys *phys_enc);
>>
>> +/**
>> + * dpu_encoder_get_drm_fmt - return DRM fourcc format
>> + * @phys_enc: Pointer to physical encoder structure
>> + */
>> +u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc);
>> +
>> +/**
>> + * dpu_encoder_needs_periph_flush - return true if physical encoder requires
>> + *     peripheral flush
>> + * @phys_enc: Pointer to physical encoder structure
>> + */
>> +bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc);
>> +
>>   /**
>>    * dpu_encoder_helper_split_config - split display configuration helper function
>>    *     This helper function may be used by physical encoders to configure
>> 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 f02411b062c4c..e29bc4bd39208 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
>> @@ -415,8 +415,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)
> 
> I thought that Abhinav proposed to drop the if(hw_cdm) condition here.
> LGTM otherwise.
> 

Yes I did.

This needs to be fixed in v4.

>> +               fmt_fourcc = dpu_encoder_get_drm_fmt(phys_enc);
>> +       fmt = dpu_get_dpu_format(fmt_fourcc);
>>
>>          DPU_DEBUG_VIDENC(phys_enc, "\n");
>>
>> @@ -425,6 +432,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);
>> +
>>          dpu_encoder_phys_vid_setup_timing_engine(phys_enc);
>>
>>          /*
>> @@ -440,6 +449,16 @@ 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_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 && dpu_encoder_needs_periph_flush(phys_enc))
>> +               ctl->ops.update_pending_flush_periph(ctl, phys_enc->hw_intf->idx);
>> +
>>   skip_flush:
>>          DPU_DEBUG_VIDENC(phys_enc,
>>                  "update pending flush ctl %d intf %d\n",
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
>> index 4b04388719363..ebcc76ef1d590 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -1397,6 +1397,24 @@ 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 && drm_mode_is_420_only(info, mode);
>> +}
>> +
>> +bool msm_dp_needs_periph_flush(const struct msm_dp *dp_display,
>> +                              const struct drm_display_mode *mode)
>> +{
>> +       return msm_dp_is_yuv_420_enabled(dp_display, mode);
>> +}
>> +
>>   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..b876ebd48effe 100644
>> --- a/drivers/gpu/drm/msm/msm_drv.h
>> +++ b/drivers/gpu/drm/msm/msm_drv.h
>> @@ -387,7 +387,10 @@ 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_needs_periph_flush(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 +412,18 @@ 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_needs_periph_flush(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;
>> --
>> 2.39.2
>>
> 
>
Paloma Arellano Feb. 15, 2024, 6:37 p.m. UTC | #3
On 2/15/2024 7:47 AM, Abhinav Kumar wrote:
>
>
> On 2/15/2024 12:45 AM, Dmitry Baryshkov wrote:
>> On Wed, 14 Feb 2024 at 20:04, Paloma Arellano 
>> <quic_parellan@quicinc.com> wrote:
>>>
>>> Adjust the encoder format programming in the case of video mode for DP
>>> to accommodate CDM related changes.
>>>
>>> Changes in v2:
>>>          - Move timing engine programming to a separate patch from this
>>>            one
>>>          - Move update_pending_flush_periph() invocation completely to
>>>            this patch
>>>          - Change the logic of dpu_encoder_get_drm_fmt() so that it 
>>> only
>>>            calls drm_mode_is_420_only() instead of doing additional
>>>            unnecessary checks
>>>          - Create new functions msm_dp_needs_periph_flush() and it's
>>>            supporting function dpu_encoder_needs_periph_flush() to 
>>> check
>>>            if the mode is YUV420 and VSC SDP is enabled before doing a
>>>            peripheral flush
>>>
>>> Signed-off-by: Paloma Arellano <quic_parellan@quicinc.com>
>>> ---
>>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c   | 35 
>>> +++++++++++++++++++
>>>   .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h  | 13 +++++++
>>>   .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  | 19 ++++++++++
>>>   drivers/gpu/drm/msm/dp/dp_display.c           | 18 ++++++++++
>>>   drivers/gpu/drm/msm/msm_drv.h                 | 17 ++++++++-
>>>   5 files changed, 101 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> index 7e7796561009a..6280c6be6dca9 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> @@ -222,6 +222,41 @@ 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(struct dpu_encoder_phys *phys_enc)
>>> +{
>>> +       struct drm_encoder *drm_enc;
>>> +       struct dpu_encoder_virt *dpu_enc;
>>> +       struct drm_display_info *info;
>>> +       struct drm_display_mode *mode;
>>> +
>>> +       drm_enc = phys_enc->parent;
>>> +       dpu_enc = to_dpu_encoder_virt(drm_enc);
>>> +       info = &dpu_enc->connector->display_info;
>>> +       mode = &phys_enc->cached_mode;
>>> +
>>> +       if (drm_mode_is_420_only(info, mode))
>>> +               return DRM_FORMAT_YUV420;
>>> +
>>> +       return DRM_FORMAT_RGB888;
>>> +}
>>> +
>>> +bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc)
>>> +{
>>> +       struct drm_encoder *drm_enc;
>>> +       struct dpu_encoder_virt *dpu_enc;
>>> +       struct msm_display_info *disp_info;
>>> +       struct msm_drm_private *priv;
>>> +       struct drm_display_mode *mode;
>>> +
>>> +       drm_enc = phys_enc->parent;
>>> +       dpu_enc = to_dpu_encoder_virt(drm_enc);
>>> +       disp_info = &dpu_enc->disp_info;
>>> +       priv = drm_enc->dev->dev_private;
>>> +       mode = &phys_enc->cached_mode;
>>> +
>>> +       return phys_enc->hw_intf->cap->type == INTF_DP && 
>>> phys_enc->hw_cdm &&
>>> + msm_dp_needs_periph_flush(priv->dp[disp_info->h_tile_instance[0]], 
>>> mode);
>>> +}
>>>
>>>   bool dpu_encoder_is_widebus_enabled(const struct drm_encoder 
>>> *drm_enc)
>>>   {
>>> 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 f43d57d9c74e1..211a3d90eb690 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
>>> @@ -341,6 +341,19 @@ static inline enum dpu_3d_blend_mode 
>>> dpu_encoder_helper_get_3d_blend_mode(
>>>    */
>>>   unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys 
>>> *phys_enc);
>>>
>>> +/**
>>> + * dpu_encoder_get_drm_fmt - return DRM fourcc format
>>> + * @phys_enc: Pointer to physical encoder structure
>>> + */
>>> +u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc);
>>> +
>>> +/**
>>> + * dpu_encoder_needs_periph_flush - return true if physical encoder 
>>> requires
>>> + *     peripheral flush
>>> + * @phys_enc: Pointer to physical encoder structure
>>> + */
>>> +bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys 
>>> *phys_enc);
>>> +
>>>   /**
>>>    * dpu_encoder_helper_split_config - split display configuration 
>>> helper function
>>>    *     This helper function may be used by physical encoders to 
>>> configure
>>> 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 f02411b062c4c..e29bc4bd39208 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
>>> @@ -415,8 +415,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)
>>
>> I thought that Abhinav proposed to drop the if(hw_cdm) condition here.
>> LGTM otherwise.
>>
>
> Yes I did.
>
> This needs to be fixed in v4.


Ack, I must have forgotten to drop it, but I'll do it in the v4

>
>>> +               fmt_fourcc = dpu_encoder_get_drm_fmt(phys_enc);
>>> +       fmt = dpu_get_dpu_format(fmt_fourcc);
>>>
>>>          DPU_DEBUG_VIDENC(phys_enc, "\n");
>>>
>>> @@ -425,6 +432,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);
>>> +
>>>          dpu_encoder_phys_vid_setup_timing_engine(phys_enc);
>>>
>>>          /*
>>> @@ -440,6 +449,16 @@ 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_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 && 
>>> dpu_encoder_needs_periph_flush(phys_enc))
>>> +               ctl->ops.update_pending_flush_periph(ctl, 
>>> phys_enc->hw_intf->idx);
>>> +
>>>   skip_flush:
>>>          DPU_DEBUG_VIDENC(phys_enc,
>>>                  "update pending flush ctl %d intf %d\n",
>>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c 
>>> b/drivers/gpu/drm/msm/dp/dp_display.c
>>> index 4b04388719363..ebcc76ef1d590 100644
>>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>>> @@ -1397,6 +1397,24 @@ 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 && 
>>> drm_mode_is_420_only(info, mode);
>>> +}
>>> +
>>> +bool msm_dp_needs_periph_flush(const struct msm_dp *dp_display,
>>> +                              const struct drm_display_mode *mode)
>>> +{
>>> +       return msm_dp_is_yuv_420_enabled(dp_display, mode);
>>> +}
>>> +
>>>   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..b876ebd48effe 100644
>>> --- a/drivers/gpu/drm/msm/msm_drv.h
>>> +++ b/drivers/gpu/drm/msm/msm_drv.h
>>> @@ -387,7 +387,10 @@ 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_needs_periph_flush(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 +412,18 @@ 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_needs_periph_flush(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;
>>> -- 
>>> 2.39.2
>>>
>>
>>
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 7e7796561009a..6280c6be6dca9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -222,6 +222,41 @@  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(struct dpu_encoder_phys *phys_enc)
+{
+	struct drm_encoder *drm_enc;
+	struct dpu_encoder_virt *dpu_enc;
+	struct drm_display_info *info;
+	struct drm_display_mode *mode;
+
+	drm_enc = phys_enc->parent;
+	dpu_enc = to_dpu_encoder_virt(drm_enc);
+	info = &dpu_enc->connector->display_info;
+	mode = &phys_enc->cached_mode;
+
+	if (drm_mode_is_420_only(info, mode))
+		return DRM_FORMAT_YUV420;
+
+	return DRM_FORMAT_RGB888;
+}
+
+bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc)
+{
+	struct drm_encoder *drm_enc;
+	struct dpu_encoder_virt *dpu_enc;
+	struct msm_display_info *disp_info;
+	struct msm_drm_private *priv;
+	struct drm_display_mode *mode;
+
+	drm_enc = phys_enc->parent;
+	dpu_enc = to_dpu_encoder_virt(drm_enc);
+	disp_info = &dpu_enc->disp_info;
+	priv = drm_enc->dev->dev_private;
+	mode = &phys_enc->cached_mode;
+
+	return phys_enc->hw_intf->cap->type == INTF_DP && phys_enc->hw_cdm &&
+	       msm_dp_needs_periph_flush(priv->dp[disp_info->h_tile_instance[0]], mode);
+}
 
 bool dpu_encoder_is_widebus_enabled(const struct drm_encoder *drm_enc)
 {
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 f43d57d9c74e1..211a3d90eb690 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -341,6 +341,19 @@  static inline enum dpu_3d_blend_mode dpu_encoder_helper_get_3d_blend_mode(
  */
 unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys *phys_enc);
 
+/**
+ * dpu_encoder_get_drm_fmt - return DRM fourcc format
+ * @phys_enc: Pointer to physical encoder structure
+ */
+u32 dpu_encoder_get_drm_fmt(struct dpu_encoder_phys *phys_enc);
+
+/**
+ * dpu_encoder_needs_periph_flush - return true if physical encoder requires
+ *	peripheral flush
+ * @phys_enc: Pointer to physical encoder structure
+ */
+bool dpu_encoder_needs_periph_flush(struct dpu_encoder_phys *phys_enc);
+
 /**
  * dpu_encoder_helper_split_config - split display configuration helper function
  *	This helper function may be used by physical encoders to configure
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 f02411b062c4c..e29bc4bd39208 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
@@ -415,8 +415,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);
+	fmt = dpu_get_dpu_format(fmt_fourcc);
 
 	DPU_DEBUG_VIDENC(phys_enc, "\n");
 
@@ -425,6 +432,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);
+
 	dpu_encoder_phys_vid_setup_timing_engine(phys_enc);
 
 	/*
@@ -440,6 +449,16 @@  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_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 && dpu_encoder_needs_periph_flush(phys_enc))
+		ctl->ops.update_pending_flush_periph(ctl, phys_enc->hw_intf->idx);
+
 skip_flush:
 	DPU_DEBUG_VIDENC(phys_enc,
 		"update pending flush ctl %d intf %d\n",
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 4b04388719363..ebcc76ef1d590 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1397,6 +1397,24 @@  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 && drm_mode_is_420_only(info, mode);
+}
+
+bool msm_dp_needs_periph_flush(const struct msm_dp *dp_display,
+			       const struct drm_display_mode *mode)
+{
+	return msm_dp_is_yuv_420_enabled(dp_display, mode);
+}
+
 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..b876ebd48effe 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -387,7 +387,10 @@  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_needs_periph_flush(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 +412,18 @@  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_needs_periph_flush(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;