linux-kernel - Re: [PATCH v3 0/3] Use APIs in gdsc genpd to switch gdsc mode for venus v4 core

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bkjthw5osu5dkwpr46zanwyfqquo45gaivlojqp6n7ixwbdnua@3t3ap2yor5z4>
Date: Wed, 15 Jan 2025 11:59:48 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Renjiang Han <quic_renjiang@...cinc.com>
Cc: Bjorn Andersson <andersson@...nel.org>, 
	Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, 
	Stanimir Varbanov <stanimir.k.varbanov@...il.com>, Vikash Garodia <quic_vgarodia@...cinc.com>, 
	Bryan O'Donoghue <bryan.odonoghue@...aro.org>, Mauro Carvalho Chehab <mchehab@...nel.org>, 
	linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-media@...r.kernel.org, Taniya Das <quic_tdas@...cinc.com>
Subject: Re: [PATCH v3 0/3] Use APIs in gdsc genpd to switch gdsc mode for
 venus v4 core

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

-- 
With best wishes
Dmitry