On Wed, Jan 15, 2025 at 03:00:51PM +0530, Renjiang Han wrote: > The Venus driver requires vcodec GDSC to be ON in SW mode for clock > operations and move it back to HW mode to gain power benefits. Earlier, > as there is no interface to switch the GDSC mode from GenPD framework, > the GDSC is moved to HW control mode as part of GDSC enable callback and > venus driver is writing to its POWER_CONTROL register to keep the GDSC ON > from SW whereever required. But the POWER_CONTROL register addresses are > not constant and can vary across the variants. > > Also as per the HW recommendation, the GDSC mode switching needs to be > controlled from respective GDSC register and this is a uniform approach > across all the targets. Hence use dev_pm_genpd_set_hwmode() API which > controls GDSC mode switching using its respective GDSC register. > > If only the clock patch or the venus driver patch is merged, the venus > driver will not work properly. Although it does not affect other system > functions, it is still recommended to wait until both the clock patch > and the venus driver patch are reviewed and passed, and then merge them > into the same release by their respective maintainers. NO! This will not work, as both -media and arm-soc branches will be broken. Please read about the git-bisect before making such suggestions. A proper plan would be: - implement a function which allows one to check that hwmode is supported by the genpd driver - Change Venus to use hwmode for those platforms if it is enabled - Enable HWMODE support in the clock driver. Clearly identify that this patch should be merged together and after Venus changes if all maintainers agree with that - Clean up now-dead code. Doing it in any other way would result in the broken kernels. > > Validated this series on QCS615 and SC7180. > > Signed-off-by: Renjiang Han <quic_renjiang@xxxxxxxxxxx> > --- > Changes in v3: > - 1. Update commit message. > - 2. Add a patch to clean up the dead code for the venus driver. > - 3. Remove vcodec_control_v4() function. > - 4. Directly call dev_pm_genpd_set_hwmode() without vcodec_control_v4(). > - Link to v2: https://lore.kernel.org/r/20241223-switch_gdsc_mode-v2-0-eb5c96aee662@xxxxxxxxxxx > > Changes in v2: > - 1. Add the HW_CTRL_TRIGGER flag for the targets SM7150/SM8150 and SM8450 > video GDSCs supporting movement between HW and SW mode of the GDSC. > (Suggested by Dmitry Baryshkov) > - 2. There is a dependency of the clock driver introducing the new flag > and the video driver adapting to this new API. Missing either the clock > and video driver could potentially break the video driver. > - Link to v1: https://lore.kernel.org/r/20241122-switch_gdsc_mode-v1-0-365f097ecbb0@xxxxxxxxxxx > > --- > Renjiang Han (2): > venus: pm_helpers: Use dev_pm_genpd_set_hwmode to switch GDSC mode on V4 > venus: pm_helpers: Remove dead code and simplify power management > > Taniya Das (1): > clk: qcom: videocc: Use HW_CTRL_TRIGGER flag for video GDSC's > > drivers/clk/qcom/videocc-sc7180.c | 2 +- > drivers/clk/qcom/videocc-sdm845.c | 4 +- > drivers/clk/qcom/videocc-sm7150.c | 4 +- > drivers/clk/qcom/videocc-sm8150.c | 4 +- > drivers/clk/qcom/videocc-sm8450.c | 4 +- > drivers/media/platform/qcom/venus/pm_helpers.c | 73 +++----------------------- > 6 files changed, 17 insertions(+), 74 deletions(-) > --- > base-commit: 63b3ff03d91ae8f875fe8747c781a521f78cde17 > change-id: 20250115-switch_gdsc_mode-a9c14fad9a36 > > Best regards, > -- > Renjiang Han <quic_renjiang@xxxxxxxxxxx> > -- With best wishes Dmitry