[PATCH 14/17] drm/msm/dpu: modify encoder programming for CDM over DP
Paloma Arellano
quic_parellan at quicinc.com
Sun Jan 28 05:48:02 UTC 2024
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 at quicinc.com>
>> ---
>> 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;
>
More information about the dri-devel
mailing list