Newer (SM8550+) SoCs don't seem to have a nice speedbin fuse anymore,
but instead rely on a set of combinations of "feature code" (FC) and
"product code" (PC) identifiers to match the bins. This series adds
support for that.
I suppose a qcom/for-soc immutable branch would be in order if we want
to land this in the upcoming cycle.
FWIW I preferred the fuses myself..
Signed-off-by: Konrad Dybcio <[email protected]>
---
Konrad Dybcio (5):
soc: qcom: Move some socinfo defines to the header, expand them
soc: qcom: smem: Add pcode/fcode getters
drm/msm/adreno: Implement SMEM-based speed bin
drm/msm/adreno: Add speedbin data for SM8550 / A740
arm64: dts: qcom: sm8550: Wire up GPU speed bin & more OPPs
Neil Armstrong (1):
drm/msm/adreno: Allow specifying default speedbin value
arch/arm64/boot/dts/qcom/sm8550.dtsi | 21 +++++++++-
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 10 +++--
drivers/gpu/drm/msm/adreno/adreno_device.c | 16 ++++++++
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 39 ++++++++++++++++--
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 13 ++++--
drivers/soc/qcom/smem.c | 66 ++++++++++++++++++++++++++++++
drivers/soc/qcom/socinfo.c | 8 ----
include/linux/soc/qcom/smem.h | 2 +
include/linux/soc/qcom/socinfo.h | 36 ++++++++++++++++
9 files changed, 191 insertions(+), 20 deletions(-)
---
base-commit: 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd
change-id: 20240404-topic-smem_speedbin-8deecd0bef0e
Best regards,
--
Konrad Dybcio <[email protected]>
From: Neil Armstrong <[email protected]>
Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
the highest. Falling back to it when things go wrong is largely
suboptimal, as more often than not, the top frequencies are not
supposed to work on other bins.
Let the developer specify the intended "lowest common denominator" bin
in struct adreno_info. If not specified, partial struct initialization
will ensure it's set to zero, retaining previous behavior.
Signed-off-by: Neil Armstrong <[email protected]>
[Konrad: clean up, add commit message]
Signed-off-by: Konrad Dybcio <[email protected]>
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 0674aca0f8a3..4cbdfabbcee5 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -2915,7 +2915,7 @@ static int a6xx_set_supported_hw(struct device *dev, const struct adreno_info *i
DRM_DEV_ERROR(dev,
"missing support for speed-bin: %u. Some OPPs may not be supported by hardware\n",
speedbin);
- supp_hw = BIT(0); /* Default */
+ supp_hw = BIT(info->default_speedbin); /* Default */
}
ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 77526892eb8c..460b399be37b 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -110,6 +110,7 @@ struct adreno_info {
* {SHRT_MAX, 0} sentinal.
*/
struct adreno_speedbin *speedbins;
+ unsigned int default_speedbin;
};
#define ADRENO_CHIP_IDS(tbl...) (uint32_t[]) { tbl, 0 }
--
2.40.1
Add the speedbin masks to ensure only the desired OPPs are available on
chips of a given bin.
Using this, add the binned 719 MHz OPP and the non-binned 124.8 MHz.
Signed-off-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/sm8550.dtsi | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 5cae8d773cec..2f6842f6a5b7 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -2087,48 +2087,67 @@ zap-shader {
memory-region = <&gpu_micro_code_mem>;
};
- /* Speedbin needs more work on A740+, keep only lower freqs */
gpu_opp_table: opp-table {
compatible = "operating-points-v2";
+ opp-719000000 {
+ opp-hz = /bits/ 64 <719000000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_SVS_L2>;
+ opp-supported-hw = <0x1>;
+ };
+
opp-680000000 {
opp-hz = /bits/ 64 <680000000>;
opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
+ opp-supported-hw = <0x3>;
};
opp-615000000 {
opp-hz = /bits/ 64 <615000000>;
opp-level = <RPMH_REGULATOR_LEVEL_SVS_L0>;
+ opp-supported-hw = <0x3>;
};
opp-550000000 {
opp-hz = /bits/ 64 <550000000>;
opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
+ opp-supported-hw = <0x3>;
};
opp-475000000 {
opp-hz = /bits/ 64 <475000000>;
opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_L1>;
+ opp-supported-hw = <0x3>;
};
opp-401000000 {
opp-hz = /bits/ 64 <401000000>;
opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+ opp-supported-hw = <0x3>;
};
opp-348000000 {
opp-hz = /bits/ 64 <348000000>;
opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D0>;
+ opp-supported-hw = <0x3>;
};
opp-295000000 {
opp-hz = /bits/ 64 <295000000>;
opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D1>;
+ opp-supported-hw = <0x3>;
};
opp-220000000 {
opp-hz = /bits/ 64 <220000000>;
opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D2>;
+ opp-supported-hw = <0x3>;
+ };
+
+ opp-124800000 {
+ opp-hz = /bits/ 64 <124800000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D2>;
+ opp-supported-hw = <0x3>;
};
};
};
--
2.40.1
On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
> From: Neil Armstrong <[email protected]>
>
> Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
> the highest. Falling back to it when things go wrong is largely
> suboptimal, as more often than not, the top frequencies are not
> supposed to work on other bins.
Isn't it better to just return an error here instead of trying to guess
which speedbin to use?
If that's not the case, I think the commit should be expanded with
actually setting default_speedbin for the existing GPUs.
>
> Let the developer specify the intended "lowest common denominator" bin
> in struct adreno_info. If not specified, partial struct initialization
> will ensure it's set to zero, retaining previous behavior.
>
> Signed-off-by: Neil Armstrong <[email protected]>
> [Konrad: clean up, add commit message]
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 0674aca0f8a3..4cbdfabbcee5 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -2915,7 +2915,7 @@ static int a6xx_set_supported_hw(struct device *dev, const struct adreno_info *i
> DRM_DEV_ERROR(dev,
> "missing support for speed-bin: %u. Some OPPs may not be supported by hardware\n",
> speedbin);
> - supp_hw = BIT(0); /* Default */
> + supp_hw = BIT(info->default_speedbin); /* Default */
> }
>
> ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1);
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 77526892eb8c..460b399be37b 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -110,6 +110,7 @@ struct adreno_info {
> * {SHRT_MAX, 0} sentinal.
> */
> struct adreno_speedbin *speedbins;
> + unsigned int default_speedbin;
> };
>
> #define ADRENO_CHIP_IDS(tbl...) (uint32_t[]) { tbl, 0 }
>
> --
> 2.40.1
>
--
With best wishes
Dmitry
On Fri, Apr 05, 2024 at 10:41:28AM +0200, Konrad Dybcio wrote:
> Newer (SM8550+) SoCs don't seem to have a nice speedbin fuse anymore,
> but instead rely on a set of combinations of "feature code" (FC) and
> "product code" (PC) identifiers to match the bins. This series adds
> support for that.
>
> I suppose a qcom/for-soc immutable branch would be in order if we want
> to land this in the upcoming cycle.
>
> FWIW I preferred the fuses myself..
>
> Signed-off-by: Konrad Dybcio <[email protected]>
> ---
> Konrad Dybcio (5):
> soc: qcom: Move some socinfo defines to the header, expand them
> soc: qcom: smem: Add pcode/fcode getters
> drm/msm/adreno: Implement SMEM-based speed bin
> drm/msm/adreno: Add speedbin data for SM8550 / A740
> arm64: dts: qcom: sm8550: Wire up GPU speed bin & more OPPs
>
> Neil Armstrong (1):
> drm/msm/adreno: Allow specifying default speedbin value
Generic comment: as you are reworking speed bins implementaiton, could
you please take a broader look. A5xx just reads nvmem manually. A6xx
uses adreno_read_speedbin(). And then we call adreno_read_speedbin
second time from from adreno_gpu_init(). Can we get to the point where
the function is called only once for all the platforms which implements
speed binning?
--
With best wishes
Dmitry
On 4/6/24 04:56, Dmitry Baryshkov wrote:
> On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
>> From: Neil Armstrong <[email protected]>
>>
>> Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
>> the highest. Falling back to it when things go wrong is largely
>> suboptimal, as more often than not, the top frequencies are not
>> supposed to work on other bins.
>
> Isn't it better to just return an error here instead of trying to guess
> which speedbin to use?
Not sure. I'd rather better compatibility for e.g. booting up a new
laptop with just dt.
>
> If that's not the case, I think the commit should be expanded with
> actually setting default_speedbin for the existing GPUs.
I think that should be addressed, although separately.
Konrad
On Tue, Apr 09, 2024 at 05:12:46PM +0200, Konrad Dybcio wrote:
>
>
> On 4/6/24 04:56, Dmitry Baryshkov wrote:
> > On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
> > > From: Neil Armstrong <[email protected]>
> > >
> > > Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
> > > the highest. Falling back to it when things go wrong is largely
> > > suboptimal, as more often than not, the top frequencies are not
> > > supposed to work on other bins.
> >
> > Isn't it better to just return an error here instead of trying to guess
> > which speedbin to use?
>
> Not sure. I'd rather better compatibility for e.g. booting up a new
> laptop with just dt.
New speedbin can have lower max speed, so by attempting to run it at
higher freq you might be breaking it.
>
> >
> > If that's not the case, I think the commit should be expanded with
> > actually setting default_speedbin for the existing GPUs.
>
> I think that should be addressed, although separately.
I'd prefer to have it as a part of this patch, but I'd not NAK it just
for this reason.
--
With best wishes
Dmitry
On Tue, Apr 9, 2024 at 8:23 AM Dmitry Baryshkov
<[email protected]> wrote:
>
> On Tue, Apr 09, 2024 at 05:12:46PM +0200, Konrad Dybcio wrote:
> >
> >
> > On 4/6/24 04:56, Dmitry Baryshkov wrote:
> > > On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
> > > > From: Neil Armstrong <[email protected]>
> > > >
> > > > Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
> > > > the highest. Falling back to it when things go wrong is largely
> > > > suboptimal, as more often than not, the top frequencies are not
> > > > supposed to work on other bins.
> > >
> > > Isn't it better to just return an error here instead of trying to guess
> > > which speedbin to use?
> >
> > Not sure. I'd rather better compatibility for e.g. booting up a new
> > laptop with just dt.
>
> New speedbin can have lower max speed, so by attempting to run it at
> higher freq you might be breaking it.
Usually there are some OPPs in common to all speedbins, so picking a
freq from that set would seem like the safe thing to do
BR,
-R
>
> >
> > >
> > > If that's not the case, I think the commit should be expanded with
> > > actually setting default_speedbin for the existing GPUs.
> >
> > I think that should be addressed, although separately.
>
> I'd prefer to have it as a part of this patch, but I'd not NAK it just
> for this reason.
>
> --
> With best wishes
> Dmitry
On Tue, Apr 09, 2024 at 10:12:00AM -0700, Rob Clark wrote:
> On Tue, Apr 9, 2024 at 8:23 AM Dmitry Baryshkov
> <[email protected]> wrote:
> >
> > On Tue, Apr 09, 2024 at 05:12:46PM +0200, Konrad Dybcio wrote:
> > >
> > >
> > > On 4/6/24 04:56, Dmitry Baryshkov wrote:
> > > > On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
> > > > > From: Neil Armstrong <[email protected]>
> > > > >
> > > > > Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
> > > > > the highest. Falling back to it when things go wrong is largely
> > > > > suboptimal, as more often than not, the top frequencies are not
> > > > > supposed to work on other bins.
> > > >
> > > > Isn't it better to just return an error here instead of trying to guess
> > > > which speedbin to use?
> > >
> > > Not sure. I'd rather better compatibility for e.g. booting up a new
> > > laptop with just dt.
> >
> > New speedbin can have lower max speed, so by attempting to run it at
> > higher freq you might be breaking it.
>
> Usually there are some OPPs in common to all speedbins, so picking a
> freq from that set would seem like the safe thing to do
Well, the issue is about an uknown speed bin. So in theory we know
nothing about the set of speeds itsupports. My point is that we should
simplfy fail in such case.
>
> BR,
> -R
>
> >
> > >
> > > >
> > > > If that's not the case, I think the commit should be expanded with
> > > > actually setting default_speedbin for the existing GPUs.
> > >
> > > I think that should be addressed, although separately.
> >
> > I'd prefer to have it as a part of this patch, but I'd not NAK it just
> > for this reason.
> >
> > --
> > With best wishes
> > Dmitry
--
With best wishes
Dmitry
On 4/9/24 20:04, Dmitry Baryshkov wrote:
> On Tue, Apr 09, 2024 at 10:12:00AM -0700, Rob Clark wrote:
>> On Tue, Apr 9, 2024 at 8:23 AM Dmitry Baryshkov
>> <[email protected]> wrote:
>>>
>>> On Tue, Apr 09, 2024 at 05:12:46PM +0200, Konrad Dybcio wrote:
>>>>
>>>>
>>>> On 4/6/24 04:56, Dmitry Baryshkov wrote:
>>>>> On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
>>>>>> From: Neil Armstrong <[email protected]>
>>>>>>
>>>>>> Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
>>>>>> the highest. Falling back to it when things go wrong is largely
>>>>>> suboptimal, as more often than not, the top frequencies are not
>>>>>> supposed to work on other bins.
>>>>>
>>>>> Isn't it better to just return an error here instead of trying to guess
>>>>> which speedbin to use?
>>>>
>>>> Not sure. I'd rather better compatibility for e.g. booting up a new
>>>> laptop with just dt.
>>>
>>> New speedbin can have lower max speed, so by attempting to run it at
>>> higher freq you might be breaking it.
>>
>> Usually there are some OPPs in common to all speedbins, so picking a
>> freq from that set would seem like the safe thing to do
>
> Well, the issue is about an uknown speed bin. So in theory we know
> nothing about the set of speeds itsupports. My point is that we should
> simplfy fail in such case.
Or we could allow e.g. the lowest frequency (or 2) which if often shared
across the board to work, giving a compromise between OOBE and sanity
Konrad
On Tue, Apr 09, 2024 at 08:07:56PM +0200, Konrad Dybcio wrote:
>
>
> On 4/9/24 20:04, Dmitry Baryshkov wrote:
> > On Tue, Apr 09, 2024 at 10:12:00AM -0700, Rob Clark wrote:
> > > On Tue, Apr 9, 2024 at 8:23 AM Dmitry Baryshkov
> > > <[email protected]> wrote:
> > > >
> > > > On Tue, Apr 09, 2024 at 05:12:46PM +0200, Konrad Dybcio wrote:
> > > > >
> > > > >
> > > > > On 4/6/24 04:56, Dmitry Baryshkov wrote:
> > > > > > On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
> > > > > > > From: Neil Armstrong <[email protected]>
> > > > > > >
> > > > > > > Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
> > > > > > > the highest. Falling back to it when things go wrong is largely
> > > > > > > suboptimal, as more often than not, the top frequencies are not
> > > > > > > supposed to work on other bins.
> > > > > >
> > > > > > Isn't it better to just return an error here instead of trying to guess
> > > > > > which speedbin to use?
> > > > >
> > > > > Not sure. I'd rather better compatibility for e.g. booting up a new
> > > > > laptop with just dt.
> > > >
> > > > New speedbin can have lower max speed, so by attempting to run it at
> > > > higher freq you might be breaking it.
> > >
> > > Usually there are some OPPs in common to all speedbins, so picking a
> > > freq from that set would seem like the safe thing to do
> >
> > Well, the issue is about an uknown speed bin. So in theory we know
> > nothing about the set of speeds itsupports. My point is that we should
> > simplfy fail in such case.
>
> Or we could allow e.g. the lowest frequency (or 2) which if often shared
> across the board to work, giving a compromise between OOBE and sanity
That's also an option. But we should not be using existing speed table for
the unknown bin.
--
With best wishes
Dmitry
On 4/9/24 20:15, Dmitry Baryshkov wrote:
> On Tue, Apr 09, 2024 at 08:07:56PM +0200, Konrad Dybcio wrote:
>>
>>
>> On 4/9/24 20:04, Dmitry Baryshkov wrote:
>>> On Tue, Apr 09, 2024 at 10:12:00AM -0700, Rob Clark wrote:
>>>> On Tue, Apr 9, 2024 at 8:23 AM Dmitry Baryshkov
>>>> <[email protected]> wrote:
>>>>>
>>>>> On Tue, Apr 09, 2024 at 05:12:46PM +0200, Konrad Dybcio wrote:
>>>>>>
>>>>>>
>>>>>> On 4/6/24 04:56, Dmitry Baryshkov wrote:
>>>>>>> On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
>>>>>>>> From: Neil Armstrong <[email protected]>
>>>>>>>>
>>>>>>>> Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
>>>>>>>> the highest. Falling back to it when things go wrong is largely
>>>>>>>> suboptimal, as more often than not, the top frequencies are not
>>>>>>>> supposed to work on other bins.
>>>>>>>
>>>>>>> Isn't it better to just return an error here instead of trying to guess
>>>>>>> which speedbin to use?
>>>>>>
>>>>>> Not sure. I'd rather better compatibility for e.g. booting up a new
>>>>>> laptop with just dt.
>>>>>
>>>>> New speedbin can have lower max speed, so by attempting to run it at
>>>>> higher freq you might be breaking it.
>>>>
>>>> Usually there are some OPPs in common to all speedbins, so picking a
>>>> freq from that set would seem like the safe thing to do
>>>
>>> Well, the issue is about an uknown speed bin. So in theory we know
>>> nothing about the set of speeds itsupports. My point is that we should
>>> simplfy fail in such case.
>>
>> Or we could allow e.g. the lowest frequency (or 2) which if often shared
>> across the board to work, giving a compromise between OOBE and sanity
>
> That's also an option. But we should not be using existing speed table for
> the unknown bin.
I derived this logic from msm-5.15 where it's "intended behavior".. I
suppose we can do better as you said though
There have been cases in the past where the default speed bin ended up
having a higher max freq than subsequent ones, and I don't think I can
trust this product/feature code approach to guarantee this never
happening again.
So. I think sticking to a single lowest freq and printing a big red line
in dmesg makes sense here
Konrad
On Tue, 9 Apr 2024 at 21:27, Konrad Dybcio <[email protected]> wrote:
>
>
>
> On 4/9/24 20:15, Dmitry Baryshkov wrote:
> > On Tue, Apr 09, 2024 at 08:07:56PM +0200, Konrad Dybcio wrote:
> >>
> >>
> >> On 4/9/24 20:04, Dmitry Baryshkov wrote:
> >>> On Tue, Apr 09, 2024 at 10:12:00AM -0700, Rob Clark wrote:
> >>>> On Tue, Apr 9, 2024 at 8:23 AM Dmitry Baryshkov
> >>>> <[email protected]> wrote:
> >>>>>
> >>>>> On Tue, Apr 09, 2024 at 05:12:46PM +0200, Konrad Dybcio wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 4/6/24 04:56, Dmitry Baryshkov wrote:
> >>>>>>> On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
> >>>>>>>> From: Neil Armstrong <[email protected]>
> >>>>>>>>
> >>>>>>>> Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
> >>>>>>>> the highest. Falling back to it when things go wrong is largely
> >>>>>>>> suboptimal, as more often than not, the top frequencies are not
> >>>>>>>> supposed to work on other bins.
> >>>>>>>
> >>>>>>> Isn't it better to just return an error here instead of trying to guess
> >>>>>>> which speedbin to use?
> >>>>>>
> >>>>>> Not sure. I'd rather better compatibility for e.g. booting up a new
> >>>>>> laptop with just dt.
> >>>>>
> >>>>> New speedbin can have lower max speed, so by attempting to run it at
> >>>>> higher freq you might be breaking it.
> >>>>
> >>>> Usually there are some OPPs in common to all speedbins, so picking a
> >>>> freq from that set would seem like the safe thing to do
> >>>
> >>> Well, the issue is about an uknown speed bin. So in theory we know
> >>> nothing about the set of speeds itsupports. My point is that we should
> >>> simplfy fail in such case.
> >>
> >> Or we could allow e.g. the lowest frequency (or 2) which if often shared
> >> across the board to work, giving a compromise between OOBE and sanity
> >
> > That's also an option. But we should not be using existing speed table for
> > the unknown bin.
>
> I derived this logic from msm-5.15 where it's "intended behavior".. I
> suppose we can do better as you said though
>
> There have been cases in the past where the default speed bin ended up
> having a higher max freq than subsequent ones, and I don't think I can
> trust this product/feature code approach to guarantee this never
> happening again.
>
> So. I think sticking to a single lowest freq and printing a big red line
> in dmesg makes sense here
Make 0x80 the default supported-hw, make sure that the lowest freq has
0xff. Plus big-red-line.
And hope that we'll never see 16 speed bins for the hardware.
--
With best wishes
Dmitry
On 4/9/24 20:31, Dmitry Baryshkov wrote:
> On Tue, 9 Apr 2024 at 21:27, Konrad Dybcio <[email protected]> wrote:
>>
>>
>>
>> On 4/9/24 20:15, Dmitry Baryshkov wrote:
>>> On Tue, Apr 09, 2024 at 08:07:56PM +0200, Konrad Dybcio wrote:
>>>>
>>>>
>>>> On 4/9/24 20:04, Dmitry Baryshkov wrote:
>>>>> On Tue, Apr 09, 2024 at 10:12:00AM -0700, Rob Clark wrote:
>>>>>> On Tue, Apr 9, 2024 at 8:23 AM Dmitry Baryshkov
>>>>>> <[email protected]> wrote:
>>>>>>>
>>>>>>> On Tue, Apr 09, 2024 at 05:12:46PM +0200, Konrad Dybcio wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 4/6/24 04:56, Dmitry Baryshkov wrote:
>>>>>>>>> On Fri, Apr 05, 2024 at 10:41:31AM +0200, Konrad Dybcio wrote:
>>>>>>>>>> From: Neil Armstrong <[email protected]>
>>>>>>>>>>
>>>>>>>>>> Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
>>>>>>>>>> the highest. Falling back to it when things go wrong is largely
>>>>>>>>>> suboptimal, as more often than not, the top frequencies are not
>>>>>>>>>> supposed to work on other bins.
>>>>>>>>>
>>>>>>>>> Isn't it better to just return an error here instead of trying to guess
>>>>>>>>> which speedbin to use?
>>>>>>>>
>>>>>>>> Not sure. I'd rather better compatibility for e.g. booting up a new
>>>>>>>> laptop with just dt.
>>>>>>>
>>>>>>> New speedbin can have lower max speed, so by attempting to run it at
>>>>>>> higher freq you might be breaking it.
>>>>>>
>>>>>> Usually there are some OPPs in common to all speedbins, so picking a
>>>>>> freq from that set would seem like the safe thing to do
>>>>>
>>>>> Well, the issue is about an uknown speed bin. So in theory we know
>>>>> nothing about the set of speeds itsupports. My point is that we should
>>>>> simplfy fail in such case.
>>>>
>>>> Or we could allow e.g. the lowest frequency (or 2) which if often shared
>>>> across the board to work, giving a compromise between OOBE and sanity
>>>
>>> That's also an option. But we should not be using existing speed table for
>>> the unknown bin.
>>
>> I derived this logic from msm-5.15 where it's "intended behavior".. I
>> suppose we can do better as you said though
>>
>> There have been cases in the past where the default speed bin ended up
>> having a higher max freq than subsequent ones, and I don't think I can
>> trust this product/feature code approach to guarantee this never
>> happening again.
>>
>> So. I think sticking to a single lowest freq and printing a big red line
>> in dmesg makes sense here
>
> Make 0x80 the default supported-hw, make sure that the lowest freq has
> 0xff. Plus big-red-line.
> And hope that we'll never see 16 speed bins for the hardware.
opp-supported-hw is a u32 bitmask fwiw
I was thinking, either 0xffffffff or some driver-side hackery
(dev_pm_opp_enable).
Perhaps I'd be more in favor of the latter, as putting meaningless gobblygoo
in dt is not good
Konrad