mbox series

[v3,0/3] Use APIs in gdsc genpd to switch gdsc mode for venus v4 core

Message ID 20250115-switch_gdsc_mode-v3-0-9a24d2fd724c@quicinc.com (mailing list archive)
Headers show
Series Use APIs in gdsc genpd to switch gdsc mode for venus v4 core | expand

Message

Renjiang Han Jan. 15, 2025, 9:30 a.m. UTC
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.

Validated this series on QCS615 and SC7180.

Signed-off-by: Renjiang Han <quic_renjiang@quicinc.com>
---
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@quicinc.com

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@quicinc.com

---
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,

Comments

Dmitry Baryshkov Jan. 15, 2025, 9:59 a.m. UTC | #1
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@quicinc.com>
> ---
> 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@quicinc.com
> 
> 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@quicinc.com
> 
> ---
> 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@quicinc.com>
>