Re: [PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On 18/07/2023 23:44, Jessica Zhang wrote: On 7/17/2023 11:04 PM, Dmitry Baryshkov wrote: On 18/07/2023 03:30, Jessica Zhang wrote: In addition, *_is_widebus_enabled() would only work under the assumption that DSC (if supported) will always be enabled during bootup for DSI. If there ends up being support for a panel that allows for DSC to be enabled on the fly after bootup, the proposed *_is_widebus_enabled() check would break since the assumption that DSC is always enabled on bootup would not be true anymore. Why is that going to break? Currently, the msm_host->dsc is initialized in attach() [1] and we do widebus setup during power_on() [2]. So we can assume that if the panel supports DSC, msm_host->dsc will be not NULL during power_on() and the widebus setup will work as expected. However, if a panel supports enabling DSC after bootup, then msm_host->dsc will be set later within the commit enable() path meaning the necessary widebus setup during power_on() would *not* happen (as *_is_widebus_enabled() would return false on account of msm_host->dsc == NULL during power_on()). Minor corrections: - power_on() happens during pre_enable() stage, - the dynamic msm_host->dsc can also be set during pre_enable() stage. Note: there still exists dsi_mgr_bridge_mode_set(), which can also be used to set msm_host->dsc (however I would prefer to drop mode_set() completely and set the mode from atomic_enable(). I'd say, this is the minor issue. The dsi_timing_setup() is plagued with if (dsc) checks, so if we make DSC dynamic, it will have to be audited anyway. Thanks, Jessica Zhang [1] https://elixir.bootlin.com/linux/v6.5-rc2/source/drivers/gpu/drm/msm/dsi/dsi_host.c#L1587 [2] https://elixir.bootlin.com/linux/v6.5-rc2/source/drivers/gpu/drm/msm/dsi/dsi_host.c#L2359 -- With best wishes Dmitry -- With best wishes Dmitry
Re: [PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On 7/17/2023 11:04 PM, Dmitry Baryshkov wrote: On 18/07/2023 03:30, Jessica Zhang wrote: On 7/14/2023 6:38 PM, Dmitry Baryshkov wrote: On 15/07/2023 03:59, Jessica Zhang wrote: On 7/14/2023 3:30 PM, Dmitry Baryshkov wrote: On Fri, 14 Jul 2023 at 22:03, Jessica Zhang wrote: On 7/13/2023 6:23 PM, Dmitry Baryshkov wrote: On 14/07/2023 03:21, Jessica Zhang wrote: DSI 6G v2.5.x+ and DPU 7.x+ support a data-bus widen mode that allows DSI to send 48 bits of compressed data per pclk instead of 24. For all chipsets that support this mode, enable it whenever DSC is enabled as recommended by the hardware programming guide. Only enable this for command mode as we are currently unable to validate it for video mode. Signed-off-by: Jessica Zhang --- Note: The dsi.xml.h changes were generated using the headergen2 script in envytools [2], but the changes to the copyright and rules-ng-ng source file paths were dropped. Separate commit please, so that it can be replaced by headers sync with Mesa3d. Hi Dmitry, Acked. [1] https://patchwork.freedesktop.org/series/120580/ [2] https://github.com/freedreno/envytools/ -- Changes in v2: - Rebased on top of "drm/msm/dpu: Re-introduce dpu core revision" - Squashed all commits to avoid breaking feature if the series is only partially applied No. Please unsquash it. Please design the series so that the patches work even if it is only partially applied. Acked. - Moved DATABUS_WIDEN bit setting to dsi_ctr_enable() (Marijn) - Have DPU check if wide bus is requested by output driver (Dmitry) - Introduced bytes_per_pclk variable for dsi_timing_setup() hdisplay adjustment (Marijn) - Link to v1: https://lore.kernel.org/r/20230525-add-widebus-support-v1-0-c7069f2ef...@quicinc.com --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++ .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 3 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 1 + drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 23 +- drivers/gpu/drm/msm/msm_drv.h | 6 ++ 9 files changed, 48 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index f0a2a1dca741..6aed63c06c1d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2411,6 +2411,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_encoder *drm_enc = NULL; struct dpu_encoder_virt *dpu_enc = NULL; + int index = disp_info->h_tile_instance[0]; int ret = 0; dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL); @@ -2439,13 +2440,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, timer_setup(_enc->frame_done_timer, dpu_encoder_frame_done_timeout, 0); - if (disp_info->intf_type == INTF_DSI) + if (disp_info->intf_type == INTF_DSI) { timer_setup(_enc->vsync_event_timer, dpu_encoder_vsync_event_handler, While you are touching this part, could you please drop dpu_encoder_vsync_event_handler() and dpu_encoder_vsync_event_work_handler(), they are useless? Since these calls aren't related to widebus, I don't think I'll include it in this series. However, I can post this cleanup as a separate patch and add that as a dependency if that's ok. Sure, that will work for me. Thank you! 0); - else if (disp_info->intf_type == INTF_DP) - dpu_enc->wide_bus_en = msm_dp_wide_bus_available( - priv->dp[disp_info->h_tile_instance[0]]); + dpu_enc->wide_bus_en = msm_dsi_is_widebus_enabled(priv->dsi[index]); + } else if (disp_info->intf_type == INTF_DP) { + dpu_enc->wide_bus_en = msm_dp_wide_bus_available(priv->dp[index]); + } INIT_DELAYED_WORK(_enc->delayed_off_work, dpu_encoder_off_work); 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); + } if (phys_enc->hw_intf->ops.program_intf_cmd_cfg)
Re: [PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On 18/07/2023 03:30, Jessica Zhang wrote: On 7/14/2023 6:38 PM, Dmitry Baryshkov wrote: On 15/07/2023 03:59, Jessica Zhang wrote: On 7/14/2023 3:30 PM, Dmitry Baryshkov wrote: On Fri, 14 Jul 2023 at 22:03, Jessica Zhang wrote: On 7/13/2023 6:23 PM, Dmitry Baryshkov wrote: On 14/07/2023 03:21, Jessica Zhang wrote: DSI 6G v2.5.x+ and DPU 7.x+ support a data-bus widen mode that allows DSI to send 48 bits of compressed data per pclk instead of 24. For all chipsets that support this mode, enable it whenever DSC is enabled as recommended by the hardware programming guide. Only enable this for command mode as we are currently unable to validate it for video mode. Signed-off-by: Jessica Zhang --- Note: The dsi.xml.h changes were generated using the headergen2 script in envytools [2], but the changes to the copyright and rules-ng-ng source file paths were dropped. Separate commit please, so that it can be replaced by headers sync with Mesa3d. Hi Dmitry, Acked. [1] https://patchwork.freedesktop.org/series/120580/ [2] https://github.com/freedreno/envytools/ -- Changes in v2: - Rebased on top of "drm/msm/dpu: Re-introduce dpu core revision" - Squashed all commits to avoid breaking feature if the series is only partially applied No. Please unsquash it. Please design the series so that the patches work even if it is only partially applied. Acked. - Moved DATABUS_WIDEN bit setting to dsi_ctr_enable() (Marijn) - Have DPU check if wide bus is requested by output driver (Dmitry) - Introduced bytes_per_pclk variable for dsi_timing_setup() hdisplay adjustment (Marijn) - Link to v1: https://lore.kernel.org/r/20230525-add-widebus-support-v1-0-c7069f2ef...@quicinc.com --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++ .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 3 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 1 + drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 23 +- drivers/gpu/drm/msm/msm_drv.h | 6 ++ 9 files changed, 48 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index f0a2a1dca741..6aed63c06c1d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2411,6 +2411,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_encoder *drm_enc = NULL; struct dpu_encoder_virt *dpu_enc = NULL; + int index = disp_info->h_tile_instance[0]; int ret = 0; dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL); @@ -2439,13 +2440,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, timer_setup(_enc->frame_done_timer, dpu_encoder_frame_done_timeout, 0); - if (disp_info->intf_type == INTF_DSI) + if (disp_info->intf_type == INTF_DSI) { timer_setup(_enc->vsync_event_timer, dpu_encoder_vsync_event_handler, While you are touching this part, could you please drop dpu_encoder_vsync_event_handler() and dpu_encoder_vsync_event_work_handler(), they are useless? Since these calls aren't related to widebus, I don't think I'll include it in this series. However, I can post this cleanup as a separate patch and add that as a dependency if that's ok. Sure, that will work for me. Thank you! 0); - else if (disp_info->intf_type == INTF_DP) - dpu_enc->wide_bus_en = msm_dp_wide_bus_available( - priv->dp[disp_info->h_tile_instance[0]]); + dpu_enc->wide_bus_en = msm_dsi_is_widebus_enabled(priv->dsi[index]); + } else if (disp_info->intf_type == INTF_DP) { + dpu_enc->wide_bus_en = msm_dp_wide_bus_available(priv->dp[index]); + } INIT_DELAYED_WORK(_enc->delayed_off_work, dpu_encoder_off_work); 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); + } if (phys_enc->hw_intf->ops.program_intf_cmd_cfg) phys_enc->hw_intf->ops.program_intf_cmd_cfg(phys_enc->hw_intf, _mode_cfg);
Re: [PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On 7/14/2023 6:38 PM, Dmitry Baryshkov wrote: On 15/07/2023 03:59, Jessica Zhang wrote: On 7/14/2023 3:30 PM, Dmitry Baryshkov wrote: On Fri, 14 Jul 2023 at 22:03, Jessica Zhang wrote: On 7/13/2023 6:23 PM, Dmitry Baryshkov wrote: On 14/07/2023 03:21, Jessica Zhang wrote: DSI 6G v2.5.x+ and DPU 7.x+ support a data-bus widen mode that allows DSI to send 48 bits of compressed data per pclk instead of 24. For all chipsets that support this mode, enable it whenever DSC is enabled as recommended by the hardware programming guide. Only enable this for command mode as we are currently unable to validate it for video mode. Signed-off-by: Jessica Zhang --- Note: The dsi.xml.h changes were generated using the headergen2 script in envytools [2], but the changes to the copyright and rules-ng-ng source file paths were dropped. Separate commit please, so that it can be replaced by headers sync with Mesa3d. Hi Dmitry, Acked. [1] https://patchwork.freedesktop.org/series/120580/ [2] https://github.com/freedreno/envytools/ -- Changes in v2: - Rebased on top of "drm/msm/dpu: Re-introduce dpu core revision" - Squashed all commits to avoid breaking feature if the series is only partially applied No. Please unsquash it. Please design the series so that the patches work even if it is only partially applied. Acked. - Moved DATABUS_WIDEN bit setting to dsi_ctr_enable() (Marijn) - Have DPU check if wide bus is requested by output driver (Dmitry) - Introduced bytes_per_pclk variable for dsi_timing_setup() hdisplay adjustment (Marijn) - Link to v1: https://lore.kernel.org/r/20230525-add-widebus-support-v1-0-c7069f2ef...@quicinc.com --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++ .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 3 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 1 + drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 23 +- drivers/gpu/drm/msm/msm_drv.h | 6 ++ 9 files changed, 48 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index f0a2a1dca741..6aed63c06c1d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2411,6 +2411,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_encoder *drm_enc = NULL; struct dpu_encoder_virt *dpu_enc = NULL; + int index = disp_info->h_tile_instance[0]; int ret = 0; dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL); @@ -2439,13 +2440,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, timer_setup(_enc->frame_done_timer, dpu_encoder_frame_done_timeout, 0); - if (disp_info->intf_type == INTF_DSI) + if (disp_info->intf_type == INTF_DSI) { timer_setup(_enc->vsync_event_timer, dpu_encoder_vsync_event_handler, While you are touching this part, could you please drop dpu_encoder_vsync_event_handler() and dpu_encoder_vsync_event_work_handler(), they are useless? Since these calls aren't related to widebus, I don't think I'll include it in this series. However, I can post this cleanup as a separate patch and add that as a dependency if that's ok. Sure, that will work for me. Thank you! 0); - else if (disp_info->intf_type == INTF_DP) - dpu_enc->wide_bus_en = msm_dp_wide_bus_available( - priv->dp[disp_info->h_tile_instance[0]]); + dpu_enc->wide_bus_en = msm_dsi_is_widebus_enabled(priv->dsi[index]); + } else if (disp_info->intf_type == INTF_DP) { + dpu_enc->wide_bus_en = msm_dp_wide_bus_available(priv->dp[index]); + } INIT_DELAYED_WORK(_enc->delayed_off_work, dpu_encoder_off_work); 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); + } if (phys_enc->hw_intf->ops.program_intf_cmd_cfg) phys_enc->hw_intf->ops.program_intf_cmd_cfg(phys_enc->hw_intf, _mode_cfg); diff --git
Re: [PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On 15/07/2023 03:59, Jessica Zhang wrote: On 7/14/2023 3:30 PM, Dmitry Baryshkov wrote: On Fri, 14 Jul 2023 at 22:03, Jessica Zhang wrote: On 7/13/2023 6:23 PM, Dmitry Baryshkov wrote: On 14/07/2023 03:21, Jessica Zhang wrote: DSI 6G v2.5.x+ and DPU 7.x+ support a data-bus widen mode that allows DSI to send 48 bits of compressed data per pclk instead of 24. For all chipsets that support this mode, enable it whenever DSC is enabled as recommended by the hardware programming guide. Only enable this for command mode as we are currently unable to validate it for video mode. Signed-off-by: Jessica Zhang --- Note: The dsi.xml.h changes were generated using the headergen2 script in envytools [2], but the changes to the copyright and rules-ng-ng source file paths were dropped. Separate commit please, so that it can be replaced by headers sync with Mesa3d. Hi Dmitry, Acked. [1] https://patchwork.freedesktop.org/series/120580/ [2] https://github.com/freedreno/envytools/ -- Changes in v2: - Rebased on top of "drm/msm/dpu: Re-introduce dpu core revision" - Squashed all commits to avoid breaking feature if the series is only partially applied No. Please unsquash it. Please design the series so that the patches work even if it is only partially applied. Acked. - Moved DATABUS_WIDEN bit setting to dsi_ctr_enable() (Marijn) - Have DPU check if wide bus is requested by output driver (Dmitry) - Introduced bytes_per_pclk variable for dsi_timing_setup() hdisplay adjustment (Marijn) - Link to v1: https://lore.kernel.org/r/20230525-add-widebus-support-v1-0-c7069f2ef...@quicinc.com --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++ .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 3 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 1 + drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 23 +- drivers/gpu/drm/msm/msm_drv.h | 6 ++ 9 files changed, 48 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index f0a2a1dca741..6aed63c06c1d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2411,6 +2411,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_encoder *drm_enc = NULL; struct dpu_encoder_virt *dpu_enc = NULL; + int index = disp_info->h_tile_instance[0]; int ret = 0; dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL); @@ -2439,13 +2440,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, timer_setup(_enc->frame_done_timer, dpu_encoder_frame_done_timeout, 0); - if (disp_info->intf_type == INTF_DSI) + if (disp_info->intf_type == INTF_DSI) { timer_setup(_enc->vsync_event_timer, dpu_encoder_vsync_event_handler, While you are touching this part, could you please drop dpu_encoder_vsync_event_handler() and dpu_encoder_vsync_event_work_handler(), they are useless? Since these calls aren't related to widebus, I don't think I'll include it in this series. However, I can post this cleanup as a separate patch and add that as a dependency if that's ok. Sure, that will work for me. Thank you! 0); - else if (disp_info->intf_type == INTF_DP) - dpu_enc->wide_bus_en = msm_dp_wide_bus_available( - priv->dp[disp_info->h_tile_instance[0]]); + dpu_enc->wide_bus_en = msm_dsi_is_widebus_enabled(priv->dsi[index]); + } else if (disp_info->intf_type == INTF_DP) { + dpu_enc->wide_bus_en = msm_dp_wide_bus_available(priv->dp[index]); + } INIT_DELAYED_WORK(_enc->delayed_off_work, dpu_encoder_off_work); 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); + } if (phys_enc->hw_intf->ops.program_intf_cmd_cfg) phys_enc->hw_intf->ops.program_intf_cmd_cfg(phys_enc->hw_intf, _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
Re: [PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On 7/14/2023 3:30 PM, Dmitry Baryshkov wrote: On Fri, 14 Jul 2023 at 22:03, Jessica Zhang wrote: On 7/13/2023 6:23 PM, Dmitry Baryshkov wrote: On 14/07/2023 03:21, Jessica Zhang wrote: DSI 6G v2.5.x+ and DPU 7.x+ support a data-bus widen mode that allows DSI to send 48 bits of compressed data per pclk instead of 24. For all chipsets that support this mode, enable it whenever DSC is enabled as recommended by the hardware programming guide. Only enable this for command mode as we are currently unable to validate it for video mode. Signed-off-by: Jessica Zhang --- Note: The dsi.xml.h changes were generated using the headergen2 script in envytools [2], but the changes to the copyright and rules-ng-ng source file paths were dropped. Separate commit please, so that it can be replaced by headers sync with Mesa3d. Hi Dmitry, Acked. [1] https://patchwork.freedesktop.org/series/120580/ [2] https://github.com/freedreno/envytools/ -- Changes in v2: - Rebased on top of "drm/msm/dpu: Re-introduce dpu core revision" - Squashed all commits to avoid breaking feature if the series is only partially applied No. Please unsquash it. Please design the series so that the patches work even if it is only partially applied. Acked. - Moved DATABUS_WIDEN bit setting to dsi_ctr_enable() (Marijn) - Have DPU check if wide bus is requested by output driver (Dmitry) - Introduced bytes_per_pclk variable for dsi_timing_setup() hdisplay adjustment (Marijn) - Link to v1: https://lore.kernel.org/r/20230525-add-widebus-support-v1-0-c7069f2ef...@quicinc.com --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 10 ++ .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 3 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 1 + drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 23 +- drivers/gpu/drm/msm/msm_drv.h | 6 ++ 9 files changed, 48 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index f0a2a1dca741..6aed63c06c1d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2411,6 +2411,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_encoder *drm_enc = NULL; struct dpu_encoder_virt *dpu_enc = NULL; +int index = disp_info->h_tile_instance[0]; int ret = 0; dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL); @@ -2439,13 +2440,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, timer_setup(_enc->frame_done_timer, dpu_encoder_frame_done_timeout, 0); -if (disp_info->intf_type == INTF_DSI) +if (disp_info->intf_type == INTF_DSI) { timer_setup(_enc->vsync_event_timer, dpu_encoder_vsync_event_handler, While you are touching this part, could you please drop dpu_encoder_vsync_event_handler() and dpu_encoder_vsync_event_work_handler(), they are useless? Since these calls aren't related to widebus, I don't think I'll include it in this series. However, I can post this cleanup as a separate patch and add that as a dependency if that's ok. Sure, that will work for me. Thank you! 0); -else if (disp_info->intf_type == INTF_DP) -dpu_enc->wide_bus_en = msm_dp_wide_bus_available( -priv->dp[disp_info->h_tile_instance[0]]); +dpu_enc->wide_bus_en = msm_dsi_is_widebus_enabled(priv->dsi[index]); +} else if (disp_info->intf_type == INTF_DP) { +dpu_enc->wide_bus_en = msm_dp_wide_bus_available(priv->dp[index]); +} INIT_DELAYED_WORK(_enc->delayed_off_work, dpu_encoder_off_work); 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); +} if (phys_enc->hw_intf->ops.program_intf_cmd_cfg) phys_enc->hw_intf->ops.program_intf_cmd_cfg(phys_enc->hw_intf, _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 ---
Re: [PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On Fri, 14 Jul 2023 at 22:03, Jessica Zhang wrote: > > > > On 7/13/2023 6:23 PM, Dmitry Baryshkov wrote: > > On 14/07/2023 03:21, Jessica Zhang wrote: > >> DSI 6G v2.5.x+ and DPU 7.x+ support a data-bus widen mode that allows DSI > >> to send 48 bits of compressed data per pclk instead of 24. > >> > >> For all chipsets that support this mode, enable it whenever DSC is > >> enabled as recommended by the hardware programming guide. > >> > >> Only enable this for command mode as we are currently unable to validate > >> it for video mode. > >> > >> Signed-off-by: Jessica Zhang > >> --- > >> Note: The dsi.xml.h changes were generated using the headergen2 script in > >> envytools [2], but the changes to the copyright and rules-ng-ng source > >> file > >> paths were dropped. > > > > Separate commit please, so that it can be replaced by headers sync with > > Mesa3d. > > Hi Dmitry, > > Acked. > > > > >> > >> [1] https://patchwork.freedesktop.org/series/120580/ > >> [2] https://github.com/freedreno/envytools/ > >> > >> -- > >> Changes in v2: > >> - Rebased on top of "drm/msm/dpu: Re-introduce dpu core revision" > >> - Squashed all commits to avoid breaking feature if the series is only > >> partially applied > > > > No. Please unsquash it. Please design the series so that the patches > > work even if it is only partially applied. > > Acked. > > > > >> - Moved DATABUS_WIDEN bit setting to dsi_ctr_enable() (Marijn) > >> - Have DPU check if wide bus is requested by output driver (Dmitry) > >> - Introduced bytes_per_pclk variable for dsi_timing_setup() hdisplay > >> adjustment (Marijn) > >> - Link to v1: > >> https://lore.kernel.org/r/20230525-add-widebus-support-v1-0-c7069f2ef...@quicinc.com > >> --- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 10 ++ > >> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +++- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 3 +++ > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 1 + > >> drivers/gpu/drm/msm/dsi/dsi.c | 5 + > >> drivers/gpu/drm/msm/dsi/dsi.h | 1 + > >> drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 + > >> drivers/gpu/drm/msm/dsi/dsi_host.c | 23 > >> +- > >> drivers/gpu/drm/msm/msm_drv.h | 6 ++ > >> 9 files changed, 48 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> index f0a2a1dca741..6aed63c06c1d 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> @@ -2411,6 +2411,7 @@ struct drm_encoder *dpu_encoder_init(struct > >> drm_device *dev, > >> struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); > >> struct drm_encoder *drm_enc = NULL; > >> struct dpu_encoder_virt *dpu_enc = NULL; > >> +int index = disp_info->h_tile_instance[0]; > >> int ret = 0; > >> dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL); > >> @@ -2439,13 +2440,14 @@ struct drm_encoder *dpu_encoder_init(struct > >> drm_device *dev, > >> timer_setup(_enc->frame_done_timer, > >> dpu_encoder_frame_done_timeout, 0); > >> -if (disp_info->intf_type == INTF_DSI) > >> +if (disp_info->intf_type == INTF_DSI) { > >> timer_setup(_enc->vsync_event_timer, > >> dpu_encoder_vsync_event_handler, > > > > While you are touching this part, could you please drop > > dpu_encoder_vsync_event_handler() and > > dpu_encoder_vsync_event_work_handler(), they are useless? > > Since these calls aren't related to widebus, I don't think I'll include > it in this series. However, I can post this cleanup as a separate patch > and add that as a dependency if that's ok. Sure, that will work for me. Thank you! > > > > >> 0); > >> -else if (disp_info->intf_type == INTF_DP) > >> -dpu_enc->wide_bus_en = msm_dp_wide_bus_available( > >> -priv->dp[disp_info->h_tile_instance[0]]); > >> +dpu_enc->wide_bus_en = > >> msm_dsi_is_widebus_enabled(priv->dsi[index]); > >> +} else if (disp_info->intf_type == INTF_DP) { > >> +dpu_enc->wide_bus_en = > >> msm_dp_wide_bus_available(priv->dp[index]); > >> +} > >> INIT_DELAYED_WORK(_enc->delayed_off_work, > >> dpu_encoder_off_work); > >> 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) { > >>
Re: [PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On Fri, 14 Jul 2023 at 23:09, Jessica Zhang wrote: > > > > On 7/14/2023 12:41 AM, Dmitry Baryshkov wrote: > > On 14/07/2023 03:21, Jessica Zhang wrote: > >> DSI 6G v2.5.x+ and DPU 7.x+ support a data-bus widen mode that allows DSI > > > > sm8250 has widebus support in DP and thus in DPU, according to the > > published DT. Thus the 'DPU 7.x+' is not fully correct. > > Hi Dmitry, > > Acked -- Will change this to say "DSI 6G v2.5+ supports a data-bus widen > mode for DPU 7.x+ that ... " instead. I'd suggest skipping DPU version at all. > > Thanks, > > Jessica Zhang > > > > >> to send 48 bits of compressed data per pclk instead of 24. > >> > >> For all chipsets that support this mode, enable it whenever DSC is > >> enabled as recommended by the hardware programming guide. > >> > >> Only enable this for command mode as we are currently unable to validate > >> it for video mode. > >> > >> Signed-off-by: Jessica Zhang > >> --- > >> Note: The dsi.xml.h changes were generated using the headergen2 script in > >> envytools [2], but the changes to the copyright and rules-ng-ng source > >> file > >> paths were dropped. > >> > >> [1] https://patchwork.freedesktop.org/series/120580/ > >> [2] https://github.com/freedreno/envytools/ > >> > >> -- > >> Changes in v2: > >> - Rebased on top of "drm/msm/dpu: Re-introduce dpu core revision" > >> - Squashed all commits to avoid breaking feature if the series is only > >> partially applied > >> - Moved DATABUS_WIDEN bit setting to dsi_ctr_enable() (Marijn) > >> - Have DPU check if wide bus is requested by output driver (Dmitry) > >> - Introduced bytes_per_pclk variable for dsi_timing_setup() hdisplay > >> adjustment (Marijn) > >> - Link to v1: > >> https://lore.kernel.org/r/20230525-add-widebus-support-v1-0-c7069f2ef...@quicinc.com > >> --- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 10 ++ > >> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +++- > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 3 +++ > >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 1 + > >> drivers/gpu/drm/msm/dsi/dsi.c | 5 + > >> drivers/gpu/drm/msm/dsi/dsi.h | 1 + > >> drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 + > >> drivers/gpu/drm/msm/dsi/dsi_host.c | 23 > >> +- > >> drivers/gpu/drm/msm/msm_drv.h | 6 ++ > >> 9 files changed, 48 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> index f0a2a1dca741..6aed63c06c1d 100644 > >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > >> @@ -2411,6 +2411,7 @@ struct drm_encoder *dpu_encoder_init(struct > >> drm_device *dev, > >> struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); > >> struct drm_encoder *drm_enc = NULL; > >> struct dpu_encoder_virt *dpu_enc = NULL; > >> +int index = disp_info->h_tile_instance[0]; > >> int ret = 0; > >> dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL); > >> @@ -2439,13 +2440,14 @@ struct drm_encoder *dpu_encoder_init(struct > >> drm_device *dev, > >> timer_setup(_enc->frame_done_timer, > >> dpu_encoder_frame_done_timeout, 0); > >> -if (disp_info->intf_type == INTF_DSI) > >> +if (disp_info->intf_type == INTF_DSI) { > >> timer_setup(_enc->vsync_event_timer, > >> dpu_encoder_vsync_event_handler, > >> 0); > >> -else if (disp_info->intf_type == INTF_DP) > >> -dpu_enc->wide_bus_en = msm_dp_wide_bus_available( > >> -priv->dp[disp_info->h_tile_instance[0]]); > >> +dpu_enc->wide_bus_en = > >> msm_dsi_is_widebus_enabled(priv->dsi[index]); > >> +} else if (disp_info->intf_type == INTF_DP) { > >> +dpu_enc->wide_bus_en = > >> msm_dp_wide_bus_available(priv->dp[index]); > >> +} > >> INIT_DELAYED_WORK(_enc->delayed_off_work, > >> dpu_encoder_off_work); > >> 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); > >> +} > >> if (phys_enc->hw_intf->ops.program_intf_cmd_cfg) > >> > >> phys_enc->hw_intf->ops.program_intf_cmd_cfg(phys_enc->hw_intf, > >> _mode_cfg); > >> diff --git
Re: [PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On 7/14/2023 12:41 AM, Dmitry Baryshkov wrote: On 14/07/2023 03:21, Jessica Zhang wrote: DSI 6G v2.5.x+ and DPU 7.x+ support a data-bus widen mode that allows DSI sm8250 has widebus support in DP and thus in DPU, according to the published DT. Thus the 'DPU 7.x+' is not fully correct. Hi Dmitry, Acked -- Will change this to say "DSI 6G v2.5+ supports a data-bus widen mode for DPU 7.x+ that ... " instead. Thanks, Jessica Zhang to send 48 bits of compressed data per pclk instead of 24. For all chipsets that support this mode, enable it whenever DSC is enabled as recommended by the hardware programming guide. Only enable this for command mode as we are currently unable to validate it for video mode. Signed-off-by: Jessica Zhang --- Note: The dsi.xml.h changes were generated using the headergen2 script in envytools [2], but the changes to the copyright and rules-ng-ng source file paths were dropped. [1] https://patchwork.freedesktop.org/series/120580/ [2] https://github.com/freedreno/envytools/ -- Changes in v2: - Rebased on top of "drm/msm/dpu: Re-introduce dpu core revision" - Squashed all commits to avoid breaking feature if the series is only partially applied - Moved DATABUS_WIDEN bit setting to dsi_ctr_enable() (Marijn) - Have DPU check if wide bus is requested by output driver (Dmitry) - Introduced bytes_per_pclk variable for dsi_timing_setup() hdisplay adjustment (Marijn) - Link to v1: https://lore.kernel.org/r/20230525-add-widebus-support-v1-0-c7069f2ef...@quicinc.com --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++ .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 3 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 1 + drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 23 +- drivers/gpu/drm/msm/msm_drv.h | 6 ++ 9 files changed, 48 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index f0a2a1dca741..6aed63c06c1d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2411,6 +2411,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_encoder *drm_enc = NULL; struct dpu_encoder_virt *dpu_enc = NULL; + int index = disp_info->h_tile_instance[0]; int ret = 0; dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL); @@ -2439,13 +2440,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, timer_setup(_enc->frame_done_timer, dpu_encoder_frame_done_timeout, 0); - if (disp_info->intf_type == INTF_DSI) + if (disp_info->intf_type == INTF_DSI) { timer_setup(_enc->vsync_event_timer, dpu_encoder_vsync_event_handler, 0); - else if (disp_info->intf_type == INTF_DP) - dpu_enc->wide_bus_en = msm_dp_wide_bus_available( - priv->dp[disp_info->h_tile_instance[0]]); + dpu_enc->wide_bus_en = msm_dsi_is_widebus_enabled(priv->dsi[index]); + } else if (disp_info->intf_type == INTF_DP) { + dpu_enc->wide_bus_en = msm_dp_wide_bus_available(priv->dp[index]); + } INIT_DELAYED_WORK(_enc->delayed_off_work, dpu_encoder_off_work); 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); + } if (phys_enc->hw_intf->ops.program_intf_cmd_cfg) phys_enc->hw_intf->ops.program_intf_cmd_cfg(phys_enc->hw_intf, _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, 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(>hw, INTF_CONFIG2, intf_cfg2); } diff --git
Re: [PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On 7/13/2023 6:23 PM, Dmitry Baryshkov wrote: On 14/07/2023 03:21, Jessica Zhang wrote: DSI 6G v2.5.x+ and DPU 7.x+ support a data-bus widen mode that allows DSI to send 48 bits of compressed data per pclk instead of 24. For all chipsets that support this mode, enable it whenever DSC is enabled as recommended by the hardware programming guide. Only enable this for command mode as we are currently unable to validate it for video mode. Signed-off-by: Jessica Zhang --- Note: The dsi.xml.h changes were generated using the headergen2 script in envytools [2], but the changes to the copyright and rules-ng-ng source file paths were dropped. Separate commit please, so that it can be replaced by headers sync with Mesa3d. Hi Dmitry, Acked. [1] https://patchwork.freedesktop.org/series/120580/ [2] https://github.com/freedreno/envytools/ -- Changes in v2: - Rebased on top of "drm/msm/dpu: Re-introduce dpu core revision" - Squashed all commits to avoid breaking feature if the series is only partially applied No. Please unsquash it. Please design the series so that the patches work even if it is only partially applied. Acked. - Moved DATABUS_WIDEN bit setting to dsi_ctr_enable() (Marijn) - Have DPU check if wide bus is requested by output driver (Dmitry) - Introduced bytes_per_pclk variable for dsi_timing_setup() hdisplay adjustment (Marijn) - Link to v1: https://lore.kernel.org/r/20230525-add-widebus-support-v1-0-c7069f2ef...@quicinc.com --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++ .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 3 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h | 1 + drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 23 +- drivers/gpu/drm/msm/msm_drv.h | 6 ++ 9 files changed, 48 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index f0a2a1dca741..6aed63c06c1d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2411,6 +2411,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_encoder *drm_enc = NULL; struct dpu_encoder_virt *dpu_enc = NULL; + int index = disp_info->h_tile_instance[0]; int ret = 0; dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL); @@ -2439,13 +2440,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, timer_setup(_enc->frame_done_timer, dpu_encoder_frame_done_timeout, 0); - if (disp_info->intf_type == INTF_DSI) + if (disp_info->intf_type == INTF_DSI) { timer_setup(_enc->vsync_event_timer, dpu_encoder_vsync_event_handler, While you are touching this part, could you please drop dpu_encoder_vsync_event_handler() and dpu_encoder_vsync_event_work_handler(), they are useless? Since these calls aren't related to widebus, I don't think I'll include it in this series. However, I can post this cleanup as a separate patch and add that as a dependency if that's ok. 0); - else if (disp_info->intf_type == INTF_DP) - dpu_enc->wide_bus_en = msm_dp_wide_bus_available( - priv->dp[disp_info->h_tile_instance[0]]); + dpu_enc->wide_bus_en = msm_dsi_is_widebus_enabled(priv->dsi[index]); + } else if (disp_info->intf_type == INTF_DP) { + dpu_enc->wide_bus_en = msm_dp_wide_bus_available(priv->dp[index]); + } INIT_DELAYED_WORK(_enc->delayed_off_work, dpu_encoder_off_work); 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); + } if (phys_enc->hw_intf->ops.program_intf_cmd_cfg) phys_enc->hw_intf->ops.program_intf_cmd_cfg(phys_enc->hw_intf, _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
Re: [PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On 14/07/2023 03:21, Jessica Zhang wrote: DSI 6G v2.5.x+ and DPU 7.x+ support a data-bus widen mode that allows DSI sm8250 has widebus support in DP and thus in DPU, according to the published DT. Thus the 'DPU 7.x+' is not fully correct. to send 48 bits of compressed data per pclk instead of 24. For all chipsets that support this mode, enable it whenever DSC is enabled as recommended by the hardware programming guide. Only enable this for command mode as we are currently unable to validate it for video mode. Signed-off-by: Jessica Zhang --- Note: The dsi.xml.h changes were generated using the headergen2 script in envytools [2], but the changes to the copyright and rules-ng-ng source file paths were dropped. [1] https://patchwork.freedesktop.org/series/120580/ [2] https://github.com/freedreno/envytools/ -- Changes in v2: - Rebased on top of "drm/msm/dpu: Re-introduce dpu core revision" - Squashed all commits to avoid breaking feature if the series is only partially applied - Moved DATABUS_WIDEN bit setting to dsi_ctr_enable() (Marijn) - Have DPU check if wide bus is requested by output driver (Dmitry) - Introduced bytes_per_pclk variable for dsi_timing_setup() hdisplay adjustment (Marijn) - Link to v1: https://lore.kernel.org/r/20230525-add-widebus-support-v1-0-c7069f2ef...@quicinc.com --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 10 ++ .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 3 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 1 + drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 23 +- drivers/gpu/drm/msm/msm_drv.h | 6 ++ 9 files changed, 48 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index f0a2a1dca741..6aed63c06c1d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2411,6 +2411,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_encoder *drm_enc = NULL; struct dpu_encoder_virt *dpu_enc = NULL; + int index = disp_info->h_tile_instance[0]; int ret = 0; dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL); @@ -2439,13 +2440,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, timer_setup(_enc->frame_done_timer, dpu_encoder_frame_done_timeout, 0); - if (disp_info->intf_type == INTF_DSI) + if (disp_info->intf_type == INTF_DSI) { timer_setup(_enc->vsync_event_timer, dpu_encoder_vsync_event_handler, 0); - else if (disp_info->intf_type == INTF_DP) - dpu_enc->wide_bus_en = msm_dp_wide_bus_available( - priv->dp[disp_info->h_tile_instance[0]]); + dpu_enc->wide_bus_en = msm_dsi_is_widebus_enabled(priv->dsi[index]); + } else if (disp_info->intf_type == INTF_DP) { + dpu_enc->wide_bus_en = msm_dp_wide_bus_available(priv->dp[index]); + } INIT_DELAYED_WORK(_enc->delayed_off_work, dpu_encoder_off_work); 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); + } if (phys_enc->hw_intf->ops.program_intf_cmd_cfg) phys_enc->hw_intf->ops.program_intf_cmd_cfg(phys_enc->hw_intf, _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, 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(>hw, INTF_CONFIG2, intf_cfg2); } diff --git
Re: [PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
On 14/07/2023 03:21, Jessica Zhang wrote: DSI 6G v2.5.x+ and DPU 7.x+ support a data-bus widen mode that allows DSI to send 48 bits of compressed data per pclk instead of 24. For all chipsets that support this mode, enable it whenever DSC is enabled as recommended by the hardware programming guide. Only enable this for command mode as we are currently unable to validate it for video mode. Signed-off-by: Jessica Zhang --- Note: The dsi.xml.h changes were generated using the headergen2 script in envytools [2], but the changes to the copyright and rules-ng-ng source file paths were dropped. Separate commit please, so that it can be replaced by headers sync with Mesa3d. [1] https://patchwork.freedesktop.org/series/120580/ [2] https://github.com/freedreno/envytools/ -- Changes in v2: - Rebased on top of "drm/msm/dpu: Re-introduce dpu core revision" - Squashed all commits to avoid breaking feature if the series is only partially applied No. Please unsquash it. Please design the series so that the patches work even if it is only partially applied. - Moved DATABUS_WIDEN bit setting to dsi_ctr_enable() (Marijn) - Have DPU check if wide bus is requested by output driver (Dmitry) - Introduced bytes_per_pclk variable for dsi_timing_setup() hdisplay adjustment (Marijn) - Link to v1: https://lore.kernel.org/r/20230525-add-widebus-support-v1-0-c7069f2ef...@quicinc.com --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 10 ++ .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 3 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 1 + drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 23 +- drivers/gpu/drm/msm/msm_drv.h | 6 ++ 9 files changed, 48 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index f0a2a1dca741..6aed63c06c1d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2411,6 +2411,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_encoder *drm_enc = NULL; struct dpu_encoder_virt *dpu_enc = NULL; + int index = disp_info->h_tile_instance[0]; int ret = 0; dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL); @@ -2439,13 +2440,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, timer_setup(_enc->frame_done_timer, dpu_encoder_frame_done_timeout, 0); - if (disp_info->intf_type == INTF_DSI) + if (disp_info->intf_type == INTF_DSI) { timer_setup(_enc->vsync_event_timer, dpu_encoder_vsync_event_handler, While you are touching this part, could you please drop dpu_encoder_vsync_event_handler() and dpu_encoder_vsync_event_work_handler(), they are useless? 0); - else if (disp_info->intf_type == INTF_DP) - dpu_enc->wide_bus_en = msm_dp_wide_bus_available( - priv->dp[disp_info->h_tile_instance[0]]); + dpu_enc->wide_bus_en = msm_dsi_is_widebus_enabled(priv->dsi[index]); + } else if (disp_info->intf_type == INTF_DP) { + dpu_enc->wide_bus_en = msm_dp_wide_bus_available(priv->dp[index]); + } INIT_DELAYED_WORK(_enc->delayed_off_work, dpu_encoder_off_work); 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); + } if (phys_enc->hw_intf->ops.program_intf_cmd_cfg) phys_enc->hw_intf->ops.program_intf_cmd_cfg(phys_enc->hw_intf, _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, if (cmd_mode_cfg->data_compress) intf_cfg2
[PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode
DSI 6G v2.5.x+ and DPU 7.x+ support a data-bus widen mode that allows DSI to send 48 bits of compressed data per pclk instead of 24. For all chipsets that support this mode, enable it whenever DSC is enabled as recommended by the hardware programming guide. Only enable this for command mode as we are currently unable to validate it for video mode. Signed-off-by: Jessica Zhang --- Note: The dsi.xml.h changes were generated using the headergen2 script in envytools [2], but the changes to the copyright and rules-ng-ng source file paths were dropped. [1] https://patchwork.freedesktop.org/series/120580/ [2] https://github.com/freedreno/envytools/ -- Changes in v2: - Rebased on top of "drm/msm/dpu: Re-introduce dpu core revision" - Squashed all commits to avoid breaking feature if the series is only partially applied - Moved DATABUS_WIDEN bit setting to dsi_ctr_enable() (Marijn) - Have DPU check if wide bus is requested by output driver (Dmitry) - Introduced bytes_per_pclk variable for dsi_timing_setup() hdisplay adjustment (Marijn) - Link to v1: https://lore.kernel.org/r/20230525-add-widebus-support-v1-0-c7069f2ef...@quicinc.com --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 10 ++ .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 4 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c| 3 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h| 1 + drivers/gpu/drm/msm/dsi/dsi.c | 5 + drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c | 23 +- drivers/gpu/drm/msm/msm_drv.h | 6 ++ 9 files changed, 48 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index f0a2a1dca741..6aed63c06c1d 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -2411,6 +2411,7 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, struct dpu_kms *dpu_kms = to_dpu_kms(priv->kms); struct drm_encoder *drm_enc = NULL; struct dpu_encoder_virt *dpu_enc = NULL; + int index = disp_info->h_tile_instance[0]; int ret = 0; dpu_enc = devm_kzalloc(dev->dev, sizeof(*dpu_enc), GFP_KERNEL); @@ -2439,13 +2440,14 @@ struct drm_encoder *dpu_encoder_init(struct drm_device *dev, timer_setup(_enc->frame_done_timer, dpu_encoder_frame_done_timeout, 0); - if (disp_info->intf_type == INTF_DSI) + if (disp_info->intf_type == INTF_DSI) { timer_setup(_enc->vsync_event_timer, dpu_encoder_vsync_event_handler, 0); - else if (disp_info->intf_type == INTF_DP) - dpu_enc->wide_bus_en = msm_dp_wide_bus_available( - priv->dp[disp_info->h_tile_instance[0]]); + dpu_enc->wide_bus_en = msm_dsi_is_widebus_enabled(priv->dsi[index]); + } else if (disp_info->intf_type == INTF_DP) { + dpu_enc->wide_bus_en = msm_dp_wide_bus_available(priv->dp[index]); + } INIT_DELAYED_WORK(_enc->delayed_off_work, dpu_encoder_off_work); 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); + } if (phys_enc->hw_intf->ops.program_intf_cmd_cfg) phys_enc->hw_intf->ops.program_intf_cmd_cfg(phys_enc->hw_intf, _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, 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(>hw, INTF_CONFIG2, intf_cfg2); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h index 77f80531782b..c539025c418b 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.h +++