Re: [PATCH v2] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux