Messages in this thread | | | Date | Thu, 17 Aug 2023 20:07:04 +0300 | Subject | Re: [PATCH v3 2/4] drm/msm/dpu: Enable widebus for DSI INTF | From | Dmitry Baryshkov <> |
| |
On 08/08/2023 00:40, Jessica Zhang wrote: > > > On 8/2/2023 11:20 AM, Dmitry Baryshkov wrote: >> On Wed, 2 Aug 2023 at 21:09, Jessica Zhang <quic_jesszhan@quicinc.com> >> wrote:
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c >>> index df88358e7037..dace6168be2d 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c >>> @@ -69,8 +69,10 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg( >>> phys_enc->hw_intf, >>> phys_enc->hw_pp->idx); >>> >>> - if (intf_cfg.dsc != 0) >>> + if (intf_cfg.dsc != 0) { >>> cmd_mode_cfg.data_compress = true; >>> + cmd_mode_cfg.wide_bus_en = >>> dpu_encoder_is_widebus_enabled(phys_enc->parent); >>> + } >> >> This embeds the knowledge that a wide bus can only be enabled when DSC >> is in use. Please move the wide_bus_en assignment out of conditional >> code. > > Wide bus for DSI will only be enabled if DSC is enabled, so this is > technically not wrong, as DP will use the video mode path. > >> >>> >>> if (phys_enc->hw_intf->ops.program_intf_cmd_cfg) >>> >>> phys_enc->hw_intf->ops.program_intf_cmd_cfg(phys_enc->hw_intf, >>> &cmd_mode_cfg); >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >>> index 8ec6505d9e78..dc6f3febb574 100644 >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c >>> @@ -521,6 +521,9 @@ static void >>> dpu_hw_intf_program_intf_cmd_cfg(struct dpu_hw_intf *ctx, >> >> This function is only enabled for DPU >= 7.0, while IIRC wide bus can >> be enabled even for some of the earlier chipsets. > > The command mode path is only called for DSI, which only supports wide > bus for DPU 7.0+.
After second consideration, let's ignore this part, as wide bus will only be enabled for DSI / CMD after 7.0. If we ever have SoC that has CMD + wide_bus earlier than 5.0, we can reconsider this code pice.
Can you please add a comment that the register itself is present earlier (5.0), but it doesn't have to be programmed since the flags will not be set anyway.
> >> >>> if (cmd_mode_cfg->data_compress) >>> intf_cfg2 |= INTF_CFG2_DCE_DATA_COMPRESS; >>> >>> + if (cmd_mode_cfg->wide_bus_en) >>> + intf_cfg2 |= INTF_CFG2_DATABUS_WIDEN; >>> + >>> DPU_REG_WRITE(&ctx->hw, INTF_CONFIG2, intf_cfg2); >>> } >>>
-- With best wishes Dmitry
| |