| Message ID | 20240105-topic-venus_reset-v1-9-981c7a624855@linaro.org (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
| Series | Qualcomm GCC/VIDEOCC reset overhaul for Venus | expand |
On 08/01/2024 12:32, Konrad Dybcio wrote: > Some Venus resets may require more time when toggling. Describe that. May or does ? I'd prefer a strong declaration of where this value came from and why its being added. May is ambiguous. "Downstream has a 150 us delay for this. My own testing shows this to be necessary in upstream" Later commits want to add a 1000 us delay. Have all of these delays been tested ? If not please describe where the values come. > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > drivers/clk/qcom/gcc-sm8250.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/qcom/gcc-sm8250.c b/drivers/clk/qcom/gcc-sm8250.c > index c6c5261264f1..61d01d4c379b 100644 > --- a/drivers/clk/qcom/gcc-sm8250.c > +++ b/drivers/clk/qcom/gcc-sm8250.c > @@ -3576,8 +3576,8 @@ static const struct qcom_reset_map gcc_sm8250_resets[] = { > [GCC_USB3PHY_PHY_PRIM_BCR] = { 0x50004 }, > [GCC_USB3PHY_PHY_SEC_BCR] = { 0x50010 }, > [GCC_USB_PHY_CFG_AHB2PHY_BCR] = { 0x6a000 }, > - [GCC_VIDEO_AXI0_CLK_ARES] = { 0xb024, 2 }, > - [GCC_VIDEO_AXI1_CLK_ARES] = { 0xb028, 2 }, > + [GCC_VIDEO_AXI0_CLK_ARES] = { 0xb024, .bit = 2, .udelay = 150 }, > + [GCC_VIDEO_AXI1_CLK_ARES] = { 0xb028, .bit = 2, .udelay = 150 }, > }; > > static const struct clk_rcg_dfs_data gcc_dfs_clocks[] = { >
On 1/9/24 01:34, Bryan O'Donoghue wrote: > On 08/01/2024 12:32, Konrad Dybcio wrote: >> Some Venus resets may require more time when toggling. Describe that. > > May or does ? > > I'd prefer a strong declaration of where this value came from and why its being added. > > May is ambiguous. > > "Downstream has a 150 us delay for this. My own testing shows this to be necessary in upstream" Alright > > Later commits want to add a 1000 us delay. Have all of these delays been tested ? No, we don't support Venus on many of the newer SoCs.. > > If not please describe where the values come. They come from the downstream Venus driver as you mentioned. I checked a couple different downstream SoC kernel trees and tried to assign the values based on what I found in a kernel for that platform. Some are fairly educated guesses. Konrad
On Tue, Jan 09, 2024 at 10:33:39AM +0100, Konrad Dybcio wrote: > > > On 1/9/24 01:34, Bryan O'Donoghue wrote: > > On 08/01/2024 12:32, Konrad Dybcio wrote: > > > Some Venus resets may require more time when toggling. Describe that. > > > > May or does ? > > > > I'd prefer a strong declaration of where this value came from and why its being added. > > > > May is ambiguous. > > > > "Downstream has a 150 us delay for this. My own testing shows this to be necessary in upstream" > > Alright > > > > > Later commits want to add a 1000 us delay. Have all of these delays been tested ? > > No, we don't support Venus on many of the newer SoCs.. > > > > > > If not please describe where the values come. > > They come from the downstream Venus driver as you mentioned. > I checked a couple different downstream SoC kernel trees and > tried to assign the values based on what I found in a kernel > for that platform. Some are fairly educated guesses. > It would be nice to have documented for which cases you guessed (and in which downstream kernel you found other values?), so that if anyone is coming to the tree later with conflicting information they have a better chance to reason about the discrepancy. Thanks, Bjorn
diff --git a/drivers/clk/qcom/gcc-sm8250.c b/drivers/clk/qcom/gcc-sm8250.c index c6c5261264f1..61d01d4c379b 100644 --- a/drivers/clk/qcom/gcc-sm8250.c +++ b/drivers/clk/qcom/gcc-sm8250.c @@ -3576,8 +3576,8 @@ static const struct qcom_reset_map gcc_sm8250_resets[] = { [GCC_USB3PHY_PHY_PRIM_BCR] = { 0x50004 }, [GCC_USB3PHY_PHY_SEC_BCR] = { 0x50010 }, [GCC_USB_PHY_CFG_AHB2PHY_BCR] = { 0x6a000 }, - [GCC_VIDEO_AXI0_CLK_ARES] = { 0xb024, 2 }, - [GCC_VIDEO_AXI1_CLK_ARES] = { 0xb028, 2 }, + [GCC_VIDEO_AXI0_CLK_ARES] = { 0xb024, .bit = 2, .udelay = 150 }, + [GCC_VIDEO_AXI1_CLK_ARES] = { 0xb028, .bit = 2, .udelay = 150 }, }; static const struct clk_rcg_dfs_data gcc_dfs_clocks[] = {
Some Venus resets may require more time when toggling. Describe that. Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- drivers/clk/qcom/gcc-sm8250.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)