| Message ID | 20230926-topic-a643-v1-1-7af6937ac0a3@linaro.org |
|---|---|
| State | Accepted |
| Commit | 75cb60d4f5f762b12643b67cbefefcf05ecfd7eb |
| Headers | show |
| Series | [1/7] drm/msm/a6xx: Fix unknown speedbin case | expand |
On Tue, Sep 26, 2023 at 08:24:36PM +0200, Konrad Dybcio wrote: > > When opp-supported-hw is present under an OPP node, but no form of > opp_set_supported_hw() has been called, that OPP is ignored by the API > and marked as unsupported. > > Before Commit c928a05e4415 ("drm/msm/adreno: Move speedbin mapping to > device table"), an unknown speedbin would result in marking all OPPs > as available, but it's better to avoid potentially overclocking the > silicon - the GMU will simply refuse to power up the chip. > > Currently, the Adreno speedbin code does just that (AND returns an > invalid error, (int)UINT_MAX). Fix that by defaulting to speedbin 0 > (which is conveniently always bound to fuseval == 0). Wish we documented somewhere that we should reserve BIT(0) for fuse val=0 always and assume that would be the super SKU. Reviewed-by: Akhil P Oommen <quic_akhilpo@quicinc.com> -Akhil > > Fixes: c928a05e4415 ("drm/msm/adreno: Move speedbin mapping to device table") > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index d4e85e24002f..522ca7fe6762 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -2237,7 +2237,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); > - return UINT_MAX; > + supp_hw = BIT(0); /* Default */ > } > > ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1); > > -- > 2.42.0 >
On Tue, Oct 17, 2023 at 01:22:27AM +0530, Akhil P Oommen wrote: > > On Tue, Sep 26, 2023 at 08:24:36PM +0200, Konrad Dybcio wrote: > > > > When opp-supported-hw is present under an OPP node, but no form of > > opp_set_supported_hw() has been called, that OPP is ignored by the API > > and marked as unsupported. > > > > Before Commit c928a05e4415 ("drm/msm/adreno: Move speedbin mapping to > > device table"), an unknown speedbin would result in marking all OPPs > > as available, but it's better to avoid potentially overclocking the > > silicon - the GMU will simply refuse to power up the chip. > > > > Currently, the Adreno speedbin code does just that (AND returns an > > invalid error, (int)UINT_MAX). Fix that by defaulting to speedbin 0 > > (which is conveniently always bound to fuseval == 0). > > Wish we documented somewhere that we should reserve BIT(0) for fuse > val=0 always and assume that would be the super SKU. Aah! I got this backward. Fuseval=0 is the supersku and it is not safe to fallback to that blindly. Ideally, we should fallback to the lowest denominator SKU, but it is difficult to predict that upfront and assign BIT(0). Anyway, I can't see a better way to handle this. -Akhil > > Reviewed-by: Akhil P Oommen <quic_akhilpo@quicinc.com> > > -Akhil > > > > > Fixes: c928a05e4415 ("drm/msm/adreno: Move speedbin mapping to device table") > > Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> > > --- > > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > > index d4e85e24002f..522ca7fe6762 100644 > > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > > @@ -2237,7 +2237,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); > > - return UINT_MAX; > > + supp_hw = BIT(0); /* Default */ > > } > > > > ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1); > > > > -- > > 2.42.0 > >
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index d4e85e24002f..522ca7fe6762 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -2237,7 +2237,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); - return UINT_MAX; + supp_hw = BIT(0); /* Default */ } ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1);
When opp-supported-hw is present under an OPP node, but no form of opp_set_supported_hw() has been called, that OPP is ignored by the API and marked as unsupported. Before Commit c928a05e4415 ("drm/msm/adreno: Move speedbin mapping to device table"), an unknown speedbin would result in marking all OPPs as available, but it's better to avoid potentially overclocking the silicon - the GMU will simply refuse to power up the chip. Currently, the Adreno speedbin code does just that (AND returns an invalid error, (int)UINT_MAX). Fix that by defaulting to speedbin 0 (which is conveniently always bound to fuseval == 0). Fixes: c928a05e4415 ("drm/msm/adreno: Move speedbin mapping to device table") Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org> --- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)