Re: [Freedreno] [PATCH v2 10/14] drm/msm/a6xx: Fix up A6XX protected registers
On Tue, Feb 14, 2023 at 4:38 PM Konrad Dybcio wrote:
>
>
>
> On 15.02.2023 01:10, Dmitry Baryshkov wrote:
> > On 14/02/2023 23:56, Rob Clark wrote:
> >> On Tue, Feb 14, 2023 at 9:32 AM Konrad Dybcio
> >> wrote:
> >>>
> >>> One of the protected ranges was too small (compared to the data we
> >>> have downstream). Fix it.
> >>>
> >>> Fixes: 408434036958 ("drm/msm/a6xx: update/fix CP_PROTECT initialization")
> >>> Signed-off-by: Konrad Dybcio
> >>> ---
> >>> 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 503c750216e6..d6b38bfdb3b4 100644
> >>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> >>> @@ -690,7 +690,7 @@ static const u32 a6xx_protect[] = {
> >>> A6XX_PROTECT_NORDWR(0x00800, 0x0082),
> >>> A6XX_PROTECT_NORDWR(0x008a0, 0x0008),
> >>> A6XX_PROTECT_NORDWR(0x008ab, 0x0024),
> >>> - A6XX_PROTECT_RDONLY(0x008de, 0x00ae),
> >>> + A6XX_PROTECT_RDONLY(0x008d0, 0x00bc),
> >>
> >> Nak, this is intentional, we need userspace to be able to configure
> >> the CP counters. Otherwise this would break fdperf, perfetto, etc
> >>
> >> (although maybe we should comment where we diverge from downstream)
> >
> > Yes, please. Otherwise it is extremely hard to understand the reason for
> > diversion between the vendor driver and our one.
> +1
>
> I am content with dropping this patch from this series, so long
> as you leave a clue for others to not scratch their heads on this!
Yeah, I admit it is kinda a trap as-is. And makes things less obvious
what to do when porting from downstream. When I get a few minutes
I'll double check that there weren't any other exceptions (I don't
think they were but it has been a while) and add some comments.
BR,
-R
> Konrad
> >
> >>
> >> BR,
> >> -R
> >>
> >>> A6XX_PROTECT_NORDWR(0x00900, 0x004d),
> >>> A6XX_PROTECT_NORDWR(0x0098d, 0x0272),
> >>> A6XX_PROTECT_NORDWR(0x00e00, 0x0001),
> >>> --
> >>> 2.39.1
> >>>
> >
Re: [Freedreno] [PATCH v2 10/14] drm/msm/a6xx: Fix up A6XX protected registers
On 15.02.2023 01:10, Dmitry Baryshkov wrote:
> On 14/02/2023 23:56, Rob Clark wrote:
>> On Tue, Feb 14, 2023 at 9:32 AM Konrad Dybcio
>> wrote:
>>>
>>> One of the protected ranges was too small (compared to the data we
>>> have downstream). Fix it.
>>>
>>> Fixes: 408434036958 ("drm/msm/a6xx: update/fix CP_PROTECT initialization")
>>> Signed-off-by: Konrad Dybcio
>>> ---
>>> 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 503c750216e6..d6b38bfdb3b4 100644
>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>>> @@ -690,7 +690,7 @@ static const u32 a6xx_protect[] = {
>>> A6XX_PROTECT_NORDWR(0x00800, 0x0082),
>>> A6XX_PROTECT_NORDWR(0x008a0, 0x0008),
>>> A6XX_PROTECT_NORDWR(0x008ab, 0x0024),
>>> - A6XX_PROTECT_RDONLY(0x008de, 0x00ae),
>>> + A6XX_PROTECT_RDONLY(0x008d0, 0x00bc),
>>
>> Nak, this is intentional, we need userspace to be able to configure
>> the CP counters. Otherwise this would break fdperf, perfetto, etc
>>
>> (although maybe we should comment where we diverge from downstream)
>
> Yes, please. Otherwise it is extremely hard to understand the reason for
> diversion between the vendor driver and our one.
+1
I am content with dropping this patch from this series, so long
as you leave a clue for others to not scratch their heads on this!
Konrad
>
>>
>> BR,
>> -R
>>
>>> A6XX_PROTECT_NORDWR(0x00900, 0x004d),
>>> A6XX_PROTECT_NORDWR(0x0098d, 0x0272),
>>> A6XX_PROTECT_NORDWR(0x00e00, 0x0001),
>>> --
>>> 2.39.1
>>>
>
Re: [Freedreno] [PATCH v2 10/14] drm/msm/a6xx: Fix up A6XX protected registers
On 14/02/2023 23:56, Rob Clark wrote:
On Tue, Feb 14, 2023 at 9:32 AM Konrad Dybcio wrote:
One of the protected ranges was too small (compared to the data we
have downstream). Fix it.
Fixes: 408434036958 ("drm/msm/a6xx: update/fix CP_PROTECT initialization")
Signed-off-by: Konrad Dybcio
---
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 503c750216e6..d6b38bfdb3b4 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -690,7 +690,7 @@ static const u32 a6xx_protect[] = {
A6XX_PROTECT_NORDWR(0x00800, 0x0082),
A6XX_PROTECT_NORDWR(0x008a0, 0x0008),
A6XX_PROTECT_NORDWR(0x008ab, 0x0024),
- A6XX_PROTECT_RDONLY(0x008de, 0x00ae),
+ A6XX_PROTECT_RDONLY(0x008d0, 0x00bc),
Nak, this is intentional, we need userspace to be able to configure
the CP counters. Otherwise this would break fdperf, perfetto, etc
(although maybe we should comment where we diverge from downstream)
Yes, please. Otherwise it is extremely hard to understand the reason for
diversion between the vendor driver and our one.
BR,
-R
A6XX_PROTECT_NORDWR(0x00900, 0x004d),
A6XX_PROTECT_NORDWR(0x0098d, 0x0272),
A6XX_PROTECT_NORDWR(0x00e00, 0x0001),
--
2.39.1
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH v3 15/27] drm/msm/dpu: move the rest of plane checks to dpu_plane_atomic_check()
On 15/02/2023 01:25, Abhinav Kumar wrote:
Hi Dmitry
Sorry for the late response on this one.
On 2/3/2023 2:55 PM, Dmitry Baryshkov wrote:
On 04/02/2023 00:44, Abhinav Kumar wrote:
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
Move plane state updates from dpu_crtc_atomic_check() to the function
where they belong: to dpu_plane_atomic_check().
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 18 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 18 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 6 --
3 files changed, 11 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index b485234eefb2..bd09bb319a58 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1129,7 +1129,6 @@ static int dpu_crtc_atomic_check(struct
drm_crtc *crtc,
crtc);
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
- struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
const struct drm_plane_state *pstate;
struct drm_plane *plane;
@@ -1161,11 +1160,10 @@ static int dpu_crtc_atomic_check(struct
drm_crtc *crtc,
crtc_rect.x2 = mode->hdisplay;
crtc_rect.y2 = mode->vdisplay;
- /* get plane state for all drm planes associated with crtc
state */
+ /* FIXME: move this to dpu_plane_atomic_check? */
drm_atomic_crtc_state_for_each_plane_state(plane, pstate,
crtc_state) {
struct dpu_plane_state *dpu_pstate =
to_dpu_plane_state(pstate);
struct drm_rect dst, clip = crtc_rect;
- int stage;
if (IS_ERR_OR_NULL(pstate)) {
rc = PTR_ERR(pstate);
@@ -1179,8 +1177,6 @@ static int dpu_crtc_atomic_check(struct
drm_crtc *crtc,
dpu_pstate->needs_dirtyfb = needs_dirtyfb;
- dpu_plane_clear_multirect(pstate);
-
dst = drm_plane_state_dest(pstate);
if (!drm_rect_intersect(, )) {
DPU_ERROR("invalid vertical/horizontal destination\n");
@@ -1189,18 +1185,6 @@ static int dpu_crtc_atomic_check(struct
drm_crtc *crtc,
DRM_RECT_ARG());
return -E2BIG;
}
-
- /* verify stage setting before using it */
- stage = DPU_STAGE_0 + pstate->normalized_zpos;
- if (stage >= dpu_kms->catalog->caps->max_mixer_blendstages) {
- DPU_ERROR("> %d plane stages assigned\n",
- dpu_kms->catalog->caps->max_mixer_blendstages -
DPU_STAGE_0);
- return -EINVAL;
- }
-
- to_dpu_plane_state(pstate)->stage = stage;
- DRM_DEBUG_ATOMIC("%s: stage %d\n", dpu_crtc->name, stage);
-
}
atomic_inc(&_dpu_crtc_get_kms(crtc)->bandwidth_ref);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 1b3033b15bfa..5aabf9694a53 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -733,14 +733,6 @@ static int _dpu_plane_color_fill(struct
dpu_plane *pdpu,
return 0;
}
-void dpu_plane_clear_multirect(const struct drm_plane_state
*drm_state)
-{
- struct dpu_plane_state *pstate = to_dpu_plane_state(drm_state);
-
- pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
- pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-}
-
int dpu_plane_validate_multirect_v2(struct
dpu_multirect_plane_states *plane)
{
struct dpu_plane_state *pstate[R_MAX];
@@ -994,6 +986,16 @@ static int dpu_plane_atomic_check(struct
drm_plane *plane,
if (!new_plane_state->visible)
return 0;
+ pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
+ pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+
But I am not sure if clearing the multirect belongs here and now I
want to clarify one thing about
https://patchwork.freedesktop.org/patch/521354/?series=99909=4
which was R-bed in the v1 and carried fwd since then.
So prior to that change, we were only clearing the multirects of the
planes that were staged to the crtc and we were getting those from
the crtc state. But now we are clearing the multirect of all the planes.
Dont we have to keep that in the crtc_atomic_check() since we do that
on all the planes attached to a certain CRTC.
In that case shouldnt we keep this in the crtc_atomic_check() and
bring back pipe_staged[] without the multirect and source split cases
ofcourse.
What for? In other words, what would be the difference?
So, please correct my understanding here. drm_plane's atomic_check()
will be called for all the planes which are getting updated in this
atomic commit using for_each_oldnew_plane_in_state() and drm_crtc's
atomic_check() will be called for all the CRTC's in this atomic update
using for_each_new_crtc_in_state() >
If the plane is not connected to any
Re: [Freedreno] [PATCH 02/10] dt-bindings: display/msm: dsi-controller-main: Add SM6375
On Sat, 11 Feb 2023 13:26:48 +0100, Konrad Dybcio wrote: > Add the DSI host found on SM6375. > > Signed-off-by: Konrad Dybcio > --- > .../devicetree/bindings/display/msm/dsi-controller-main.yaml| 2 ++ > 1 file changed, 2 insertions(+) > Acked-by: Rob Herring
Re: [Freedreno] [PATCH 01/10] dt-bindings: display/msm: dsi-controller-main: Add SM6350
On Sat, 11 Feb 2023 13:26:47 +0100, Konrad Dybcio wrote: > Add the DSI host found on SM6350. > > Signed-off-by: Konrad Dybcio > --- > .../devicetree/bindings/display/msm/dsi-controller-main.yaml| 2 ++ > 1 file changed, 2 insertions(+) > Acked-by: Rob Herring
Re: [Freedreno] [PATCH v3 15/27] drm/msm/dpu: move the rest of plane checks to dpu_plane_atomic_check()
Hi Dmitry
Sorry for the late response on this one.
On 2/3/2023 2:55 PM, Dmitry Baryshkov wrote:
On 04/02/2023 00:44, Abhinav Kumar wrote:
On 2/3/2023 10:21 AM, Dmitry Baryshkov wrote:
Move plane state updates from dpu_crtc_atomic_check() to the function
where they belong: to dpu_plane_atomic_check().
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 18 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 18 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 6 --
3 files changed, 11 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index b485234eefb2..bd09bb319a58 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1129,7 +1129,6 @@ static int dpu_crtc_atomic_check(struct
drm_crtc *crtc,
crtc);
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
- struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
const struct drm_plane_state *pstate;
struct drm_plane *plane;
@@ -1161,11 +1160,10 @@ static int dpu_crtc_atomic_check(struct
drm_crtc *crtc,
crtc_rect.x2 = mode->hdisplay;
crtc_rect.y2 = mode->vdisplay;
- /* get plane state for all drm planes associated with crtc
state */
+ /* FIXME: move this to dpu_plane_atomic_check? */
drm_atomic_crtc_state_for_each_plane_state(plane, pstate,
crtc_state) {
struct dpu_plane_state *dpu_pstate =
to_dpu_plane_state(pstate);
struct drm_rect dst, clip = crtc_rect;
- int stage;
if (IS_ERR_OR_NULL(pstate)) {
rc = PTR_ERR(pstate);
@@ -1179,8 +1177,6 @@ static int dpu_crtc_atomic_check(struct
drm_crtc *crtc,
dpu_pstate->needs_dirtyfb = needs_dirtyfb;
- dpu_plane_clear_multirect(pstate);
-
dst = drm_plane_state_dest(pstate);
if (!drm_rect_intersect(, )) {
DPU_ERROR("invalid vertical/horizontal destination\n");
@@ -1189,18 +1185,6 @@ static int dpu_crtc_atomic_check(struct
drm_crtc *crtc,
DRM_RECT_ARG());
return -E2BIG;
}
-
- /* verify stage setting before using it */
- stage = DPU_STAGE_0 + pstate->normalized_zpos;
- if (stage >= dpu_kms->catalog->caps->max_mixer_blendstages) {
- DPU_ERROR("> %d plane stages assigned\n",
- dpu_kms->catalog->caps->max_mixer_blendstages -
DPU_STAGE_0);
- return -EINVAL;
- }
-
- to_dpu_plane_state(pstate)->stage = stage;
- DRM_DEBUG_ATOMIC("%s: stage %d\n", dpu_crtc->name, stage);
-
}
atomic_inc(&_dpu_crtc_get_kms(crtc)->bandwidth_ref);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 1b3033b15bfa..5aabf9694a53 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -733,14 +733,6 @@ static int _dpu_plane_color_fill(struct
dpu_plane *pdpu,
return 0;
}
-void dpu_plane_clear_multirect(const struct drm_plane_state *drm_state)
-{
- struct dpu_plane_state *pstate = to_dpu_plane_state(drm_state);
-
- pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
- pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-}
-
int dpu_plane_validate_multirect_v2(struct
dpu_multirect_plane_states *plane)
{
struct dpu_plane_state *pstate[R_MAX];
@@ -994,6 +986,16 @@ static int dpu_plane_atomic_check(struct
drm_plane *plane,
if (!new_plane_state->visible)
return 0;
+ pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
+ pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
+
But I am not sure if clearing the multirect belongs here and now I
want to clarify one thing about
https://patchwork.freedesktop.org/patch/521354/?series=99909=4
which was R-bed in the v1 and carried fwd since then.
So prior to that change, we were only clearing the multirects of the
planes that were staged to the crtc and we were getting those from the
crtc state. But now we are clearing the multirect of all the planes.
Dont we have to keep that in the crtc_atomic_check() since we do that
on all the planes attached to a certain CRTC.
In that case shouldnt we keep this in the crtc_atomic_check() and
bring back pipe_staged[] without the multirect and source split cases
ofcourse.
What for? In other words, what would be the difference?
So, please correct my understanding here. drm_plane's atomic_check()
will be called for all the planes which are getting updated in this
atomic commit using for_each_oldnew_plane_in_state() and drm_crtc's
atomic_check() will be called for all the CRTC's in this atomic update
using for_each_new_crtc_in_state().
If the plane is not connected to any CRTC, why do we need to clear the
multirect
Re: [Freedreno] [PATCH v2 00/14] GMU-less A6xx support (A610, A619_holi)
+ Akhil On Tue, Feb 14, 2023 at 10:03 AM Konrad Dybcio wrote: > > > v1 -> v2: > - Fix A630 values in [2/14] > - Fix [6/14] for GMU-equipped GPUs > > Link to v1: > https://lore.kernel.org/linux-arm-msm/20230126151618.225127-1-konrad.dyb...@linaro.org/ > > This series concludes my couple-weeks-long suffering of figuring out > the ins and outs of the "non-standard" A6xx GPUs which feature no GMU. > > The GMU functionality is essentially emulated by parting out a > "GMU wrapper" region, which is essentially just a register space > within the GPU. It's modeled to be as similar to the actual GMU > as possible while staying as unnecessary as we can make it - there's > no IRQs, communicating with a microcontroller, no RPMh communication > etc. etc. I tried to reuse as much code as possible without making > a mess where every even line is used for GMU and every odd line is > used for GMU wrapper.. > > This series contains: > - plumbing for non-GMU operation, if-ing out GMU calls based on > GMU presence > - GMU wrapper support > - A610 support (w/ speedbin) > - A619 support (w/ speedbin) > - couple of minor fixes and improvements > - VDDCX/VDDGX scaling fix for non-GMU GPUs (concerns more than just > A6xx) > - Enablement of opp interconnect properties > > A619_holi works perfectly fine using the already-present A619 support > in mesa. A610 needs more work on that front, but can already replay > command traces captures on downstream. > > NOTE: the "drm/msm/a6xx: Add support for A619_holi" patch contains > two occurences of 0x18 used in place of a register #define, as it's > supposed to be RBBM_GPR0_CNTL, but that will only be present after > mesa-side changes are merged and headers are synced from there. > > Speedbin patches depend on: > https://lore.kernel.org/linux-arm-msm/20230120172233.1905761-1-konrad.dyb...@linaro.org/ > > > Konrad Dybcio (14): > drm/msm/a6xx: De-staticize sptprac en/disable functions > drm/msm/a6xx: Extend UBWC config > drm/msm/a6xx: Introduce GMU wrapper support > drm/msm/a6xx: Remove both GBIF and RBBM GBIF halt on hw init > drm/msm/adreno: Disable has_cached_coherent for A610/A619_holi > drm/msm/gpu: Use dev_pm_opp_set_rate for non-GMU GPUs > drm/msm/a6xx: Add support for A619_holi > drm/msm/a6xx: Add A610 support > drm/msm/a6xx: Fix some A619 tunables > drm/msm/a6xx: Fix up A6XX protected registers > drm/msm/a6xx: Enable optional icc voting from OPP tables > drm/msm/a6xx: Use "else if" in GPU speedbin rev matching > drm/msm/a6xx: Add A619_holi speedbin support > drm/msm/a6xx: Add A610 speedbin support > > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 55 ++- > drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 2 + > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 427 +--- > drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + > drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 14 +- > drivers/gpu/drm/msm/adreno/adreno_device.c | 34 +- > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 4 + > drivers/gpu/drm/msm/adreno/adreno_gpu.h | 19 +- > drivers/gpu/drm/msm/msm_gpu_devfreq.c | 2 +- > 9 files changed, 492 insertions(+), 66 deletions(-) > > -- > 2.39.1 >
Re: [Freedreno] [PATCH v2 10/14] drm/msm/a6xx: Fix up A6XX protected registers
On Tue, Feb 14, 2023 at 9:32 AM Konrad Dybcio wrote:
>
> One of the protected ranges was too small (compared to the data we
> have downstream). Fix it.
>
> Fixes: 408434036958 ("drm/msm/a6xx: update/fix CP_PROTECT initialization")
> Signed-off-by: Konrad Dybcio
> ---
> 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 503c750216e6..d6b38bfdb3b4 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -690,7 +690,7 @@ static const u32 a6xx_protect[] = {
> A6XX_PROTECT_NORDWR(0x00800, 0x0082),
> A6XX_PROTECT_NORDWR(0x008a0, 0x0008),
> A6XX_PROTECT_NORDWR(0x008ab, 0x0024),
> - A6XX_PROTECT_RDONLY(0x008de, 0x00ae),
> + A6XX_PROTECT_RDONLY(0x008d0, 0x00bc),
Nak, this is intentional, we need userspace to be able to configure
the CP counters. Otherwise this would break fdperf, perfetto, etc
(although maybe we should comment where we diverge from downstream)
BR,
-R
> A6XX_PROTECT_NORDWR(0x00900, 0x004d),
> A6XX_PROTECT_NORDWR(0x0098d, 0x0272),
> A6XX_PROTECT_NORDWR(0x00e00, 0x0001),
> --
> 2.39.1
>
[Freedreno] [PATCH v2 00/14] GMU-less A6xx support (A610, A619_holi)
v1 -> v2: - Fix A630 values in [2/14] - Fix [6/14] for GMU-equipped GPUs Link to v1: https://lore.kernel.org/linux-arm-msm/20230126151618.225127-1-konrad.dyb...@linaro.org/ This series concludes my couple-weeks-long suffering of figuring out the ins and outs of the "non-standard" A6xx GPUs which feature no GMU. The GMU functionality is essentially emulated by parting out a "GMU wrapper" region, which is essentially just a register space within the GPU. It's modeled to be as similar to the actual GMU as possible while staying as unnecessary as we can make it - there's no IRQs, communicating with a microcontroller, no RPMh communication etc. etc. I tried to reuse as much code as possible without making a mess where every even line is used for GMU and every odd line is used for GMU wrapper.. This series contains: - plumbing for non-GMU operation, if-ing out GMU calls based on GMU presence - GMU wrapper support - A610 support (w/ speedbin) - A619 support (w/ speedbin) - couple of minor fixes and improvements - VDDCX/VDDGX scaling fix for non-GMU GPUs (concerns more than just A6xx) - Enablement of opp interconnect properties A619_holi works perfectly fine using the already-present A619 support in mesa. A610 needs more work on that front, but can already replay command traces captures on downstream. NOTE: the "drm/msm/a6xx: Add support for A619_holi" patch contains two occurences of 0x18 used in place of a register #define, as it's supposed to be RBBM_GPR0_CNTL, but that will only be present after mesa-side changes are merged and headers are synced from there. Speedbin patches depend on: https://lore.kernel.org/linux-arm-msm/20230120172233.1905761-1-konrad.dyb...@linaro.org/ Konrad Dybcio (14): drm/msm/a6xx: De-staticize sptprac en/disable functions drm/msm/a6xx: Extend UBWC config drm/msm/a6xx: Introduce GMU wrapper support drm/msm/a6xx: Remove both GBIF and RBBM GBIF halt on hw init drm/msm/adreno: Disable has_cached_coherent for A610/A619_holi drm/msm/gpu: Use dev_pm_opp_set_rate for non-GMU GPUs drm/msm/a6xx: Add support for A619_holi drm/msm/a6xx: Add A610 support drm/msm/a6xx: Fix some A619 tunables drm/msm/a6xx: Fix up A6XX protected registers drm/msm/a6xx: Enable optional icc voting from OPP tables drm/msm/a6xx: Use "else if" in GPU speedbin rev matching drm/msm/a6xx: Add A619_holi speedbin support drm/msm/a6xx: Add A610 speedbin support drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 55 ++- drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 2 + drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 427 +--- drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 14 +- drivers/gpu/drm/msm/adreno/adreno_device.c | 34 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 4 + drivers/gpu/drm/msm/adreno/adreno_gpu.h | 19 +- drivers/gpu/drm/msm/msm_gpu_devfreq.c | 2 +- 9 files changed, 492 insertions(+), 66 deletions(-) -- 2.39.1
Re: [Freedreno] [RFC PATCH 5/7] drm/msm/dpu: Document and enable TEAR interrupts on DSI interfaces
On 2/14/2023 5:06 AM, Marijn Suijten wrote: On 2023-02-13 19:09:32, Abhinav Kumar wrote: On 2/13/2023 1:46 PM, Dmitry Baryshkov wrote: On 13/02/2023 21:37, Jessica Zhang wrote: On 12/31/2022 1:50 PM, Marijn Suijten wrote: All SoCs since DPU 5.0.0 (and seemingly up until and including 6.0.0, but excluding 7.x.x) have the tear interrupt and control registers moved out of the PINGPONG block and into the INTF block. Wire up the necessary interrupts and IRQ masks on all supported hardware. Hi Marijn, Thanks for the patch. I saw that in your commit msg, you mentioned that 7.x doesn't have tearcheck in the INTF block -- can you double check that this is correct? It wasn't correct and has already been removed for v2 [1] after rebasing on top of SM8[345]50 support, where the registers reside at a different (named 7 downstream) offset. [1] https://github.com/SoMainline/linux/commit/886d3fb9eed925e7e9c8d6ca63d2439eaec1c702 I'm working on SM8350 (DPU v7) and I'm seeing that it does have tearcheck in INTF block. I confirm, according to the vendor drivers INTF TE should be used for all DPU >= 5.0, including 7.x and 8.x However I think I know what Marijn meant here. For 5.x and 6.x these IRQs are handled at the address MDSS + 0x6e800 / + 0x6e900 (which means offset here should 0x6d800 and 0x6d900) for INTF_1 and INTF_2. Since DPU 7.x these IRQ registers were moved close to the main INTF block (0x36800 and 0x37800 = INTF + 0x800). That might have been the case. Got it, then the commit text should remove "control" and just say tear interrupt registers. It got a bit confusing. The wording here points to both the interrupt (MDP_INTFx_TEAR_INTR) registers and control (INTF_TEAR_xxx) registers separately. Feel free to bikeshed the wording in preliminary v2 [1]; should I drop the mention of the control registers being "moved" from PP to INTF entirely, leaving just the wording about the interrupt registers moving from MDP_SSPP_TOP0_INTR to a dedicated MDP_INTFx_TEAR_INTR region? Yes, that makes more sense to me. Drop the mention on control registers. We will add the 7xxx intf tear check support on top of this series. No need, that is already taken care of in an impending v2 [1] (unless additional changes are required beyond the moved register offset). Ok, we will wait till you post v2 and see if that works for us without any of our local changes. Signed-off-by: Marijn Suijten --- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 78 +++ .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 6 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 12 +++ .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 3 + 5 files changed, 68 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 1cfe94494135..b9b9b5b0b615 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -86,6 +86,15 @@ #define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN) +#define IRQ_MSM8998_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ + BIT(MDP_SSPP_TOP0_INTR2) | \ + BIT(MDP_SSPP_TOP0_HIST_INTR) | \ + BIT(MDP_INTF0_INTR) | \ + BIT(MDP_INTF1_INTR) | \ + BIT(MDP_INTF2_INTR) | \ + BIT(MDP_INTF3_INTR) | \ + BIT(MDP_INTF4_INTR)) + #define IRQ_SDM845_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ BIT(MDP_SSPP_TOP0_INTR2) | \ BIT(MDP_SSPP_TOP0_HIST_INTR) | \ @@ -100,13 +109,15 @@ #define IRQ_QCM2290_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ BIT(MDP_SSPP_TOP0_INTR2) | \ BIT(MDP_SSPP_TOP0_HIST_INTR) | \ - BIT(MDP_INTF1_INTR)) + BIT(MDP_INTF1_INTR) | \ + BIT(MDP_INTF1_TEAR_INTR)) #define IRQ_SC7180_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ BIT(MDP_SSPP_TOP0_INTR2) | \ BIT(MDP_SSPP_TOP0_HIST_INTR) | \ BIT(MDP_INTF0_INTR) | \ - BIT(MDP_INTF1_INTR)) + BIT(MDP_INTF1_INTR) | \ + BIT(MDP_INTF1_TEAR_INTR)) #define IRQ_SC7280_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ BIT(MDP_SSPP_TOP0_INTR2) | \ @@ -120,7 +131,9 @@ BIT(MDP_SSPP_TOP0_HIST_INTR) | \ BIT(MDP_INTF0_INTR) | \ BIT(MDP_INTF1_INTR) | \ + BIT(MDP_INTF1_TEAR_INTR) | \ BIT(MDP_INTF2_INTR) | \ + BIT(MDP_INTF2_TEAR_INTR) | \ BIT(MDP_INTF3_INTR) | \ BIT(MDP_INTF4_INTR)) @@ -129,7 +142,9 @@ BIT(MDP_SSPP_TOP0_HIST_INTR) | \ BIT(MDP_INTF0_INTR) | \ BIT(MDP_INTF1_INTR) | \ + BIT(MDP_INTF1_TEAR_INTR) | \ BIT(MDP_INTF2_INTR) | \ + BIT(MDP_INTF2_TEAR_INTR) | \ BIT(MDP_INTF3_INTR) |
Re: [Freedreno] [PATCH 4/4] drm/msm/a5xx: fix context faults during ring switch
On Mon, Feb 13, 2023 at 6:10 PM Dmitry Baryshkov
wrote:
>
> The rptr_addr is set in the preempt_init_ring(), which is called from
> a5xx_gpu_init(). It uses shadowptr() to set the address, however the
> shadow_iova is not yet initialized at that time. Move the rptr_addr
> setting to the a5xx_preempt_hw_init() which is called after setting the
> shadow_iova, getting the correct value for the address.
>
> Fixes: 8907afb476ac ("drm/msm: Allow a5xx to mark the RPTR shadow as
> privileged")
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/7555
> Suggested-by: Rob Clark
> Signed-off-by: Dmitry Baryshkov
> ---
> drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> index 7e0affd60993..f58dd564d122 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_preempt.c
> @@ -207,6 +207,7 @@ void a5xx_preempt_hw_init(struct msm_gpu *gpu)
> a5xx_gpu->preempt[i]->wptr = 0;
> a5xx_gpu->preempt[i]->rptr = 0;
> a5xx_gpu->preempt[i]->rbase = gpu->rb[i]->iova;
> + a5xx_gpu->preempt[i]->rptr_addr = shadowptr(a5xx_gpu,
> gpu->rb[i]);
> }
>
> /* Write a 0 to signal that we aren't switching pagetables */
> @@ -257,7 +258,6 @@ static int preempt_init_ring(struct a5xx_gpu *a5xx_gpu,
> ptr->data = 0;
> ptr->cntl = MSM_GPU_RB_CNTL_DEFAULT | AXXX_CP_RB_CNTL_NO_UPDATE;
>
> - ptr->rptr_addr = shadowptr(a5xx_gpu, ring);
> ptr->counter = counters_iova;
>
> return 0;
> --
> 2.30.2
>
[Freedreno] [PATCH v2 00/14] GMU-less A6xx support (A610, A619_holi)
Subject: [PATCH v2 00/14] GMU-less A6xx support (A610, A619_holi) Date: Tue, 14 Feb 2023 18:31:31 +0100 From: Konrad Dybcio To: linux-arm-...@vger.kernel.org, anders...@kernel.org, agr...@kernel.org CC: marijn.suij...@somainline.org, Konrad Dybcio v1 -> v2: - Fix A630 values in [2/14] - Fix [6/14] for GMU-equipped GPUs Link to v1: https://lore.kernel.org/linux-arm-msm/20230126151618.225127-1-konrad.dyb...@linaro.org/ This series concludes my couple-weeks-long suffering of figuring out the ins and outs of the "non-standard" A6xx GPUs which feature no GMU. The GMU functionality is essentially emulated by parting out a "GMU wrapper" region, which is essentially just a register space within the GPU. It's modeled to be as similar to the actual GMU as possible while staying as unnecessary as we can make it - there's no IRQs, communicating with a microcontroller, no RPMh communication etc. etc. I tried to reuse as much code as possible without making a mess where every even line is used for GMU and every odd line is used for GMU wrapper.. This series contains: - plumbing for non-GMU operation, if-ing out GMU calls based on GMU presence - GMU wrapper support - A610 support (w/ speedbin) - A619 support (w/ speedbin) - couple of minor fixes and improvements - VDDCX/VDDGX scaling fix for non-GMU GPUs (concerns more than just A6xx) - Enablement of opp interconnect properties A619_holi works perfectly fine using the already-present A619 support in mesa. A610 needs more work on that front, but can already replay command traces captures on downstream. NOTE: the "drm/msm/a6xx: Add support for A619_holi" patch contains two occurences of 0x18 used in place of a register #define, as it's supposed to be RBBM_GPR0_CNTL, but that will only be present after mesa-side changes are merged and headers are synced from there. Speedbin patches depend on: https://lore.kernel.org/linux-arm-msm/20230120172233.1905761-1-konrad.dyb...@linaro.org/ Konrad Dybcio (14): drm/msm/a6xx: De-staticize sptprac en/disable functions drm/msm/a6xx: Extend UBWC config drm/msm/a6xx: Introduce GMU wrapper support drm/msm/a6xx: Remove both GBIF and RBBM GBIF halt on hw init drm/msm/adreno: Disable has_cached_coherent for A610/A619_holi drm/msm/gpu: Use dev_pm_opp_set_rate for non-GMU GPUs drm/msm/a6xx: Add support for A619_holi drm/msm/a6xx: Add A610 support drm/msm/a6xx: Fix some A619 tunables drm/msm/a6xx: Fix up A6XX protected registers drm/msm/a6xx: Enable optional icc voting from OPP tables drm/msm/a6xx: Use "else if" in GPU speedbin rev matching drm/msm/a6xx: Add A619_holi speedbin support drm/msm/a6xx: Add A610 speedbin support drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 55 ++- drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 2 + drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 427 +--- drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 + drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 14 +- drivers/gpu/drm/msm/adreno/adreno_device.c | 34 +- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 4 + drivers/gpu/drm/msm/adreno/adreno_gpu.h | 19 +- drivers/gpu/drm/msm/msm_gpu_devfreq.c | 2 +- 9 files changed, 492 insertions(+), 66 deletions(-) -- 2.39.1
[Freedreno] [PATCH v2 14/14] drm/msm/a6xx: Add A610 speedbin support
A610 is implemented on at least three SoCs: SM6115 (bengal), SM6125
(trinket) and SM6225 (khaje). Trinket does not support speed binning
(only a single SKU exists) and we don't yet support khaje upstream.
Hence, add a fuse mapping table for bengal to allow for per-chip
frequency limiting.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 94b4d93619ed..f2679f9cc137 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -2082,6 +2082,30 @@ static bool a6xx_progress(struct msm_gpu *gpu, struct
msm_ringbuffer *ring)
return progress;
}
+static u32 a610_get_speed_bin(u32 fuse)
+{
+ /*
+* There are (at least) three SoCs implementing A610: SM6125 (trinket),
+* SM6115 (bengal) and SM6225 (khaje). Trinket does not have
speedbinning,
+* as only a single SKU exists and we don't support khaje upstream yet.
+* Hence, this matching table is only valid for bengal and can be easily
+* expanded if need be.
+*/
+
+ if (fuse == 0)
+ return 0;
+ else if (fuse == 206)
+ return 1;
+ else if (fuse == 200)
+ return 2;
+ else if (fuse == 157)
+ return 3;
+ else if (fuse == 127)
+ return 4;
+
+ return UINT_MAX;
+}
+
static u32 a618_get_speed_bin(u32 fuse)
{
if (fuse == 0)
@@ -2178,6 +2202,9 @@ static u32 fuse_to_supp_hw(struct device *dev, struct
adreno_rev rev, u32 fuse)
{
u32 val = UINT_MAX;
+ if (adreno_cmp_rev(ADRENO_REV(6, 1, 0, ANY_ID), rev))
+ val = a610_get_speed_bin(fuse);
+
if (adreno_cmp_rev(ADRENO_REV(6, 1, 8, ANY_ID), rev))
val = a618_get_speed_bin(fuse);
--
2.39.1
[Freedreno] [PATCH v2 11/14] drm/msm/a6xx: Enable optional icc voting from OPP tables
On GMU-equipped GPUs, the GMU requests appropriate bandwidth votes for us. This is however not the case for the other GPUs. Add the dev_pm_opp_of_find_icc_paths() call to let the OPP framework handle bus voting as part of power level setting. Signed-off-by: Konrad Dybcio --- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c index d6b38bfdb3b4..b08ed127f8c4 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c @@ -2338,5 +2338,9 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev) msm_mmu_set_fault_handler(gpu->aspace->mmu, gpu, a6xx_fault_handler); + ret = dev_pm_opp_of_find_icc_paths(>dev, NULL); + if (ret) + return ERR_PTR(ret); + return gpu; } -- 2.39.1
[Freedreno] [PATCH v2 13/14] drm/msm/a6xx: Add A619_holi speedbin support
A619_holi is implemented on at least two SoCs: SM4350 (holi) and SM6375
(blair). This is what seems to be a first occurrence of this happening,
but it's easy to overcome by guarding the SoC-specific fuse values with
of_machine_is_compatible(). Do just that to enable frequency limiting
on these SoCs.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 31 +++
1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index ffe0fd431a76..94b4d93619ed 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -2094,6 +2094,34 @@ static u32 a618_get_speed_bin(u32 fuse)
return UINT_MAX;
}
+static u32 a619_holi_get_speed_bin(u32 fuse)
+{
+ /*
+* There are (at least) two SoCs implementing A619_holi: SM4350 (holi)
+* and SM6375 (blair). Limit the fuse matching to the corresponding
+* SoC to prevent bogus frequency setting (as improbable as it may be,
+* given unexpected fuse values are.. unexpected! But still possible.)
+*/
+
+ if (fuse == 0)
+ return 0;
+
+ if (of_machine_is_compatible("qcom,sm4350")) {
+ if (fuse == 138)
+ return 1;
+ else if (fuse == 92)
+ return 2;
+ } else if (of_machine_is_compatible("qcom,sm6375")) {
+ if (fuse == 190)
+ return 1;
+ else if (fuse == 177)
+ return 2;
+ } else
+ pr_warn("Unknown SoC implementing A619_holi!\n");
+
+ return UINT_MAX;
+}
+
static u32 a619_get_speed_bin(u32 fuse)
{
if (fuse == 0)
@@ -2153,6 +2181,9 @@ static u32 fuse_to_supp_hw(struct device *dev, struct
adreno_rev rev, u32 fuse)
if (adreno_cmp_rev(ADRENO_REV(6, 1, 8, ANY_ID), rev))
val = a618_get_speed_bin(fuse);
+ else if (adreno_cmp_rev(ADRENO_REV(6, 1, 9, 1), rev))
+ val = a619_holi_get_speed_bin(fuse);
+
else if (adreno_cmp_rev(ADRENO_REV(6, 1, 9, ANY_ID), rev))
val = a619_get_speed_bin(fuse);
--
2.39.1
[Freedreno] [PATCH v2 12/14] drm/msm/a6xx: Use "else if" in GPU speedbin rev matching
The GPU can only be one at a time. Turn a series of ifs into if +
elseifs to save some CPU cycles.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index b08ed127f8c4..ffe0fd431a76 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -2153,16 +2153,16 @@ static u32 fuse_to_supp_hw(struct device *dev, struct
adreno_rev rev, u32 fuse)
if (adreno_cmp_rev(ADRENO_REV(6, 1, 8, ANY_ID), rev))
val = a618_get_speed_bin(fuse);
- if (adreno_cmp_rev(ADRENO_REV(6, 1, 9, ANY_ID), rev))
+ else if (adreno_cmp_rev(ADRENO_REV(6, 1, 9, ANY_ID), rev))
val = a619_get_speed_bin(fuse);
- if (adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), rev))
+ else if (adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), rev))
val = adreno_7c3_get_speed_bin(fuse);
- if (adreno_cmp_rev(ADRENO_REV(6, 4, 0, ANY_ID), rev))
+ else if (adreno_cmp_rev(ADRENO_REV(6, 4, 0, ANY_ID), rev))
val = a640_get_speed_bin(fuse);
- if (adreno_cmp_rev(ADRENO_REV(6, 5, 0, ANY_ID), rev))
+ else if (adreno_cmp_rev(ADRENO_REV(6, 5, 0, ANY_ID), rev))
val = a650_get_speed_bin(fuse);
if (val == UINT_MAX) {
--
2.39.1
[Freedreno] [PATCH v2 09/14] drm/msm/a6xx: Fix some A619 tunables
Adreno 619 expects some tunables to be set differently. Make up for it.
Fixes: b7616b5c69e6 ("drm/msm/adreno: Add A619 support")
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 1e259e9901ca..503c750216e6 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1174,6 +1174,8 @@ static int hw_init(struct msm_gpu *gpu)
gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00200200);
else if (adreno_is_a650(adreno_gpu) || adreno_is_a660(adreno_gpu))
gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00300200);
+ else if (adreno_is_a619(adreno_gpu))
+ gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00018000);
else if (adreno_is_a610(adreno_gpu))
gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x0008);
else
@@ -1191,7 +1193,9 @@ static int hw_init(struct msm_gpu *gpu)
a6xx_set_ubwc_config(gpu);
/* Enable fault detection */
- if (adreno_is_a610(adreno_gpu))
+ if (adreno_is_a619(adreno_gpu))
+ gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 << 30)
| 0x3f);
+ else if (adreno_is_a610(adreno_gpu))
gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 << 30)
| 0x3);
else
gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 << 30)
| 0x1f);
--
2.39.1
[Freedreno] [PATCH v2 10/14] drm/msm/a6xx: Fix up A6XX protected registers
One of the protected ranges was too small (compared to the data we
have downstream). Fix it.
Fixes: 408434036958 ("drm/msm/a6xx: update/fix CP_PROTECT initialization")
Signed-off-by: Konrad Dybcio
---
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 503c750216e6..d6b38bfdb3b4 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -690,7 +690,7 @@ static const u32 a6xx_protect[] = {
A6XX_PROTECT_NORDWR(0x00800, 0x0082),
A6XX_PROTECT_NORDWR(0x008a0, 0x0008),
A6XX_PROTECT_NORDWR(0x008ab, 0x0024),
- A6XX_PROTECT_RDONLY(0x008de, 0x00ae),
+ A6XX_PROTECT_RDONLY(0x008d0, 0x00bc),
A6XX_PROTECT_NORDWR(0x00900, 0x004d),
A6XX_PROTECT_NORDWR(0x0098d, 0x0272),
A6XX_PROTECT_NORDWR(0x00e00, 0x0001),
--
2.39.1
[Freedreno] [PATCH v2 08/14] drm/msm/a6xx: Add A610 support
A610 is one of (if not the) lowest-tier SKUs in the A6XX family. It
features no GMU, as it's implemented solely on SoCs with SMD_RPM.
What's more interesting is that it does not feature a VDDGX line
either, being powered solely by VDDCX and has an unfortunate hardware
quirk that makes its reset line broken - after a couple of assert/
deassert cycles, it will hang for good and will not wake up again.
This GPU requires mesa changes for proper rendering, and lots of them
at that. The command streams are quite far away from any other A6XX
GPU and hence it needs special care. This patch was validated both
by running an (incomplete) downstream mesa with some hacks (frames
rendered correctly, though some instructions made the GPU hangcheck
which is expected - garbage in, garbage out) and by replaying RD
traces captured with the downstream KGSL driver - no crashes there,
ever.
Add support for this GPU on the kernel side, which comes down to
pretty simply adding A612 HWCG tables, altering a few values and
adding a special case for handling the reset line.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 95 --
drivers/gpu/drm/msm/adreno/adreno_device.c | 13 +++
drivers/gpu/drm/msm/adreno/adreno_gpu.h| 8 +-
3 files changed, 106 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index c168712a0dc4..1e259e9901ca 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -254,6 +254,56 @@ static void a6xx_submit(struct msm_gpu *gpu, struct
msm_gem_submit *submit)
a6xx_flush(gpu, ring);
}
+const struct adreno_reglist a612_hwcg[] = {
+ {REG_A6XX_RBBM_CLOCK_CNTL_SP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_CNTL2_SP0, 0x0220},
+ {REG_A6XX_RBBM_CLOCK_DELAY_SP0, 0x0081},
+ {REG_A6XX_RBBM_CLOCK_HYST_SP0, 0xf3cf},
+ {REG_A6XX_RBBM_CLOCK_CNTL_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_CNTL2_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_CNTL3_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_CNTL4_TP0, 0x0002},
+ {REG_A6XX_RBBM_CLOCK_DELAY_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_DELAY2_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_DELAY3_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_DELAY4_TP0, 0x0001},
+ {REG_A6XX_RBBM_CLOCK_HYST_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_HYST2_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_HYST3_TP0, 0x},
+ {REG_A6XX_RBBM_CLOCK_HYST4_TP0, 0x0007},
+ {REG_A6XX_RBBM_CLOCK_CNTL_RB0, 0x},
+ {REG_A6XX_RBBM_CLOCK_CNTL2_RB0, 0x0120},
+ {REG_A6XX_RBBM_CLOCK_CNTL_CCU0, 0x2220},
+ {REG_A6XX_RBBM_CLOCK_HYST_RB_CCU0, 0x00040f00},
+ {REG_A6XX_RBBM_CLOCK_CNTL_RAC, 0x05522022},
+ {REG_A6XX_RBBM_CLOCK_CNTL2_RAC, 0x},
+ {REG_A6XX_RBBM_CLOCK_DELAY_RAC, 0x0011},
+ {REG_A6XX_RBBM_CLOCK_HYST_RAC, 0x00445044},
+ {REG_A6XX_RBBM_CLOCK_CNTL_TSE_RAS_RBBM, 0x0422},
+ {REG_A6XX_RBBM_CLOCK_MODE_VFD, 0x},
+ {REG_A6XX_RBBM_CLOCK_MODE_GPC, 0x0222},
+ {REG_A6XX_RBBM_CLOCK_DELAY_HLSQ_2, 0x0002},
+ {REG_A6XX_RBBM_CLOCK_MODE_HLSQ, 0x},
+ {REG_A6XX_RBBM_CLOCK_DELAY_TSE_RAS_RBBM, 0x4000},
+ {REG_A6XX_RBBM_CLOCK_DELAY_VFD, 0x},
+ {REG_A6XX_RBBM_CLOCK_DELAY_GPC, 0x0200},
+ {REG_A6XX_RBBM_CLOCK_DELAY_HLSQ, 0x},
+ {REG_A6XX_RBBM_CLOCK_HYST_TSE_RAS_RBBM, 0x},
+ {REG_A6XX_RBBM_CLOCK_HYST_VFD, 0x},
+ {REG_A6XX_RBBM_CLOCK_HYST_GPC, 0x04104004},
+ {REG_A6XX_RBBM_CLOCK_HYST_HLSQ, 0x},
+ {REG_A6XX_RBBM_CLOCK_CNTL_UCHE, 0x},
+ {REG_A6XX_RBBM_CLOCK_HYST_UCHE, 0x0004},
+ {REG_A6XX_RBBM_CLOCK_DELAY_UCHE, 0x0002},
+ {REG_A6XX_RBBM_ISDB_CNT, 0x0182},
+ {REG_A6XX_RBBM_RAC_THRESHOLD_CNT, 0x},
+ {REG_A6XX_RBBM_SP_HYST_CNT, 0x},
+ {REG_A6XX_RBBM_CLOCK_CNTL_GMU_GX, 0x0222},
+ {REG_A6XX_RBBM_CLOCK_DELAY_GMU_GX, 0x0111},
+ {REG_A6XX_RBBM_CLOCK_HYST_GMU_GX, 0x0555},
+ {},
+};
+
/* For a615 family (a615, a616, a618 and a619) */
const struct adreno_reglist a615_hwcg[] = {
{REG_A6XX_RBBM_CLOCK_CNTL_SP0, 0x0222},
@@ -604,6 +654,8 @@ static void a6xx_set_hwcg(struct msm_gpu *gpu, bool state)
if (adreno_is_a630(adreno_gpu))
clock_cntl_on = 0x8aa8aa02;
+ else if (adreno_is_a610(adreno_gpu))
+ clock_cntl_on = 0xaaa8aa82;
else
clock_cntl_on = 0x8aa8aa82;
@@ -798,6 +850,11 @@ static void a6xx_set_ubwc_config(struct msm_gpu *gpu)
u32 min_acc_len = 0;
u32 ubwc_mode = 0;
+ if (adreno_is_a610(adreno_gpu)) {
+ min_acc_len = 1;
+ ubwc_mode = 1;
+ }
+
/* a618 is using the hw default values */
if
[Freedreno] [PATCH v2 07/14] drm/msm/a6xx: Add support for A619_holi
A619_holi is a GMU-less variant of the already-supported A619 GPU.
It's present on at least SM4350 (holi) and SM6375 (blair). No mesa
changes are required. Add the required kernel-side support for it.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 37 +-
drivers/gpu/drm/msm/adreno/adreno_device.c | 13
drivers/gpu/drm/msm/adreno/adreno_gpu.h| 5 +++
3 files changed, 47 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 75cf94b03c29..c168712a0dc4 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -614,14 +614,14 @@ static void a6xx_set_hwcg(struct msm_gpu *gpu, bool state)
return;
/* Disable SP clock before programming HWCG registers */
- if (!adreno_has_gmu_wrapper(adreno_gpu))
+ if ((!adreno_has_gmu_wrapper(adreno_gpu) ||
adreno_is_a619_holi(adreno_gpu)))
gmu_rmw(gmu, REG_A6XX_GPU_GMU_GX_SPTPRAC_CLOCK_CONTROL, 1, 0);
for (i = 0; (reg = _gpu->info->hwcg[i], reg->offset); i++)
gpu_write(gpu, reg->offset, state ? reg->value : 0);
/* Enable SP clock */
- if (!adreno_has_gmu_wrapper(adreno_gpu))
+ if ((!adreno_has_gmu_wrapper(adreno_gpu) ||
adreno_is_a619_holi(adreno_gpu)))
gmu_rmw(gmu, REG_A6XX_GPU_GMU_GX_SPTPRAC_CLOCK_CONTROL, 0, 1);
gpu_write(gpu, REG_A6XX_RBBM_CLOCK_CNTL, state ? clock_cntl_on : 0);
@@ -1007,7 +1007,12 @@ static int hw_init(struct msm_gpu *gpu)
}
/* Clear GBIF halt in case GX domain was not collapsed */
- if (a6xx_has_gbif(adreno_gpu)) {
+ if (adreno_is_a619_holi(adreno_gpu)) {
+ gpu_write(gpu, REG_A6XX_GBIF_HALT, 0);
+ gpu_write(gpu, 0x18, 0);
+ /* Let's make extra sure that the GPU can access the memory.. */
+ mb();
+ } else if (a6xx_has_gbif(adreno_gpu)) {
gpu_write(gpu, REG_A6XX_GBIF_HALT, 0);
gpu_write(gpu, REG_A6XX_RBBM_GBIF_HALT, 0);
/* Let's make extra sure that the GPU can access the memory.. */
@@ -1016,6 +1021,9 @@ static int hw_init(struct msm_gpu *gpu)
gpu_write(gpu, REG_A6XX_RBBM_SECVID_TSB_CNTL, 0);
+ if (adreno_is_a619_holi(adreno_gpu))
+ a6xx_sptprac_enable(gmu);
+
/*
* Disable the trusted memory range - we don't actually supported secure
* memory rendering at this point in time and we don't want to block off
@@ -1293,7 +1301,8 @@ static void a6xx_dump(struct msm_gpu *gpu)
#define GBIF_CLIENT_HALT_MASK BIT(0)
#define GBIF_ARB_HALT_MASK BIT(1)
#define VBIF_RESET_ACK_TIMEOUT 100
-#define VBIF_RESET_ACK_MASK0x00f0
+#define VBIF_RESET_ACK_MASK0xF0
+#define GPR0_GBIF_HALT_REQUEST 0x1E0
static void a6xx_recover(struct msm_gpu *gpu)
{
@@ -1350,10 +1359,16 @@ static void a6xx_recover(struct msm_gpu *gpu)
/* Software-reset the GPU */
if (adreno_has_gmu_wrapper(adreno_gpu)) {
- /* Halt the GX side of GBIF */
- gpu_write(gpu, REG_A6XX_RBBM_GBIF_HALT, GBIF_GX_HALT_MASK);
- spin_until(gpu_read(gpu, REG_A6XX_RBBM_GBIF_HALT_ACK) &
- GBIF_GX_HALT_MASK);
+ if (adreno_is_a619_holi(adreno_gpu)) {
+ gpu_write(gpu, 0x18, GPR0_GBIF_HALT_REQUEST);
+ spin_until((gpu_read(gpu,
REG_A6XX_RBBM_VBIF_GX_RESET_STATUS) &
+ (VBIF_RESET_ACK_MASK)) ==
VBIF_RESET_ACK_MASK);
+ } else {
+ /* Halt the GX side of GBIF */
+ gpu_write(gpu, REG_A6XX_RBBM_GBIF_HALT,
GBIF_GX_HALT_MASK);
+ spin_until(gpu_read(gpu, REG_A6XX_RBBM_GBIF_HALT_ACK) &
+ GBIF_GX_HALT_MASK);
+ }
/* Halt new client requests on GBIF */
gpu_write(gpu, REG_A6XX_GBIF_HALT, GBIF_CLIENT_HALT_MASK);
@@ -1763,6 +1778,9 @@ static int a6xx_pm_resume(struct msm_gpu *gpu)
if (ret)
return ret;
+ if (adreno_is_a619_holi(adreno_gpu))
+ a6xx_sptprac_enable(gmu);
+
mutex_unlock(_gpu->gmu.lock);
msm_devfreq_resume(gpu);
@@ -1795,6 +1813,9 @@ static int a6xx_pm_suspend(struct msm_gpu *gpu)
mutex_lock(_gpu->gmu.lock);
+ if (adreno_is_a619_holi(adreno_gpu))
+ a6xx_sptprac_disable(gmu);
+
ret = clk_prepare_enable(gpu->ebi1_clk);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 82757f005a1a..71faeb3fd466 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++
[Freedreno] [PATCH v2 06/14] drm/msm/gpu: Use dev_pm_opp_set_rate for non-GMU GPUs
Currently we only utilize the OPP table connected to the GPU for
getting (available) frequencies. We do however need to scale the
voltage rail(s) accordingly to ensure that we aren't trying to
run the GPU at 1GHz with a VDD_LOW vote, as that would result in
an otherwise inexplainable hang.
Tell the OPP framework that we want to scale the "core" clock
and swap out the clk_set_rate to a dev_pm_opp_set_rate in
msm_devfreq_target() to enable usage of required-opps and by
extension proper voltage level/corner scaling.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 4
drivers/gpu/drm/msm/msm_gpu_devfreq.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index ce6b76c45b6f..15e405e4f977 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -1047,6 +1047,10 @@ int adreno_gpu_init(struct drm_device *drm, struct
platform_device *pdev,
const char *gpu_name;
u32 speedbin;
+ /* This can only be done here, or devm_pm_opp_set_supported_hw will
WARN_ON() */
+ if (!IS_ERR(devm_clk_get(dev, "core")))
+ devm_pm_opp_set_clkname(dev, "core");
+
adreno_gpu->funcs = funcs;
adreno_gpu->info = adreno_info(config->rev);
adreno_gpu->gmem = adreno_gpu->info->gmem;
diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
index e27dbf12b5e8..ea70c1c32d94 100644
--- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
+++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
@@ -48,7 +48,7 @@ static int msm_devfreq_target(struct device *dev, unsigned
long *freq,
gpu->funcs->gpu_set_freq(gpu, opp, df->suspended);
mutex_unlock(>lock);
} else {
- clk_set_rate(gpu->core_clk, *freq);
+ dev_pm_opp_set_rate(dev, *freq);
}
dev_pm_opp_put(opp);
--
2.39.1
[Freedreno] [PATCH v2 05/14] drm/msm/adreno: Disable has_cached_coherent for A610/A619_holi
These SKUs don't support the feature. Disable it to make the GPU stop
crashing after almost each and every submission - the received data on
the GPU end was simply incomplete in garbled, resulting in almost nothing
being executed properly.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 36f062c7582f..82757f005a1a 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -540,7 +540,13 @@ static int adreno_bind(struct device *dev, struct device
*master, void *data)
config.rev.minor, config.rev.patchid);
priv->is_a2xx = config.rev.core == 2;
- priv->has_cached_coherent = config.rev.core >= 6;
+
+ if (config.rev.core >= 6) {
+ /* Exclude A610 and A619_holi */
+ if (!(adreno_cmp_rev(ADRENO_REV(6, 1, 0, ANY_ID), config.rev) ||
+ adreno_cmp_rev(ADRENO_REV(6, 1, 9, 1), config.rev)))
+ priv->has_cached_coherent = true;
+ }
gpu = info->init(drm);
if (IS_ERR(gpu)) {
--
2.39.1
[Freedreno] [PATCH v2 01/14] drm/msm/a6xx: De-staticize sptprac en/disable functions
These two will be reused by at least A619_holi in the non-gmu
paths. De-staticize them to make it possible.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 4 ++--
drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index f3c9600221d4..90e636dcdd5b 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -354,7 +354,7 @@ void a6xx_gmu_clear_oob(struct a6xx_gmu *gmu, enum
a6xx_gmu_oob_state state)
}
/* Enable CPU control of SPTP power power collapse */
-static int a6xx_sptprac_enable(struct a6xx_gmu *gmu)
+int a6xx_sptprac_enable(struct a6xx_gmu *gmu)
{
int ret;
u32 val;
@@ -376,7 +376,7 @@ static int a6xx_sptprac_enable(struct a6xx_gmu *gmu)
}
/* Disable CPU control of SPTP power power collapse */
-static void a6xx_sptprac_disable(struct a6xx_gmu *gmu)
+void a6xx_sptprac_disable(struct a6xx_gmu *gmu)
{
u32 val;
int ret;
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index e034935b3986..ec28abdd327b 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -186,5 +186,7 @@ int a6xx_hfi_set_freq(struct a6xx_gmu *gmu, int index);
bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu);
bool a6xx_gmu_sptprac_is_on(struct a6xx_gmu *gmu);
+void a6xx_sptprac_disable(struct a6xx_gmu *gmu);
+int a6xx_sptprac_enable(struct a6xx_gmu *gmu);
#endif
--
2.39.1
[Freedreno] [PATCH v2 04/14] drm/msm/a6xx: Remove both GBIF and RBBM GBIF halt on hw init
Currently we're only deasserting REG_A6XX_RBBM_GBIF_HALT, but we also
need REG_A6XX_GBIF_HALT to be set to 0. For GMU-equipped GPUs this is
done in a6xx_bus_clear_pending_transactions(), but for the GMU-less
ones we have to do it *somewhere*. Unhalting both side by side sounds
like a good plan and it won't cause any issues if it's unnecessary.
Also, add a memory barrier to ensure it's gone through.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 72bf5c9f7ff1..75cf94b03c29 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1007,8 +1007,12 @@ static int hw_init(struct msm_gpu *gpu)
}
/* Clear GBIF halt in case GX domain was not collapsed */
- if (a6xx_has_gbif(adreno_gpu))
+ if (a6xx_has_gbif(adreno_gpu)) {
+ gpu_write(gpu, REG_A6XX_GBIF_HALT, 0);
gpu_write(gpu, REG_A6XX_RBBM_GBIF_HALT, 0);
+ /* Let's make extra sure that the GPU can access the memory.. */
+ mb();
+ }
gpu_write(gpu, REG_A6XX_RBBM_SECVID_TSB_CNTL, 0);
--
2.39.1
[Freedreno] [PATCH v2 03/14] drm/msm/a6xx: Introduce GMU wrapper support
Some (particularly SMD_RPM, a.k.a non-RPMh) SoCs implement A6XX GPUs
but don't implement the associated GMUs. This is due to the fact that
the GMU directly pokes at RPMh. Sadly, this means we have to take care
of enabling & scaling power rails, clocks and bandwidth ourselves.
Reuse existing Adreno-common code and modify the deeply-GMU-infused
A6XX code to facilitate these GPUs. This involves if-ing out lots
of GMU callbacks and introducing a new type of GMU - GMU wrapper.
This is essentially a register region which is convenient to model
as a device. We'll use it for managing the GDSCs.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 51 -
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 198 +---
drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 +
drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 14 +-
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 6 +
5 files changed, 233 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 90e636dcdd5b..5aa9f3ef41c2 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -1474,6 +1474,7 @@ static int a6xx_gmu_get_irq(struct a6xx_gmu *gmu, struct
platform_device *pdev,
void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu)
{
+ struct adreno_gpu *adreno_gpu = _gpu->base;
struct a6xx_gmu *gmu = _gpu->gmu;
struct platform_device *pdev = to_platform_device(gmu->dev);
@@ -1493,10 +1494,12 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu)
gmu->mmio = NULL;
gmu->rscc = NULL;
- a6xx_gmu_memory_free(gmu);
+ if (!adreno_has_gmu_wrapper(adreno_gpu)) {
+ a6xx_gmu_memory_free(gmu);
- free_irq(gmu->gmu_irq, gmu);
- free_irq(gmu->hfi_irq, gmu);
+ free_irq(gmu->gmu_irq, gmu);
+ free_irq(gmu->hfi_irq, gmu);
+ }
/* Drop reference taken in of_find_device_by_node */
put_device(gmu->dev);
@@ -1504,6 +1507,48 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu)
gmu->initialized = false;
}
+int a6xx_gmu_wrapper_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
+{
+ struct platform_device *pdev = of_find_device_by_node(node);
+ struct a6xx_gmu *gmu = _gpu->gmu;
+ int ret;
+
+ if (!pdev)
+ return -ENODEV;
+
+ gmu->dev = >dev;
+
+ of_dma_configure(gmu->dev, node, true);
+
+ pm_runtime_enable(gmu->dev);
+
+ /* Mark legacy for manual SPTPRAC control */
+ gmu->legacy = true;
+
+ /* Map the GMU registers */
+ gmu->mmio = a6xx_gmu_get_mmio(pdev, "gmu");
+ if (IS_ERR(gmu->mmio)) {
+ ret = PTR_ERR(gmu->mmio);
+ goto err_mmio;
+ }
+
+ /* Get a link to the GX power domain to reset the GPU */
+ gmu->gxpd = dev_pm_domain_attach_by_name(gmu->dev, "gx");
+
+ gmu->initialized = true;
+
+ return 0;
+
+err_mmio:
+ iounmap(gmu->mmio);
+ ret = -ENODEV;
+
+ /* Drop reference taken in of_find_device_by_node */
+ put_device(gmu->dev);
+
+ return ret;
+}
+
int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
{
struct adreno_gpu *adreno_gpu = _gpu->base;
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 8855d798bbb3..72bf5c9f7ff1 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -20,9 +20,11 @@ static inline bool _a6xx_check_idle(struct msm_gpu *gpu)
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
- /* Check that the GMU is idle */
- if (!a6xx_gmu_isidle(_gpu->gmu))
- return false;
+ if (!adreno_has_gmu_wrapper(adreno_gpu)) {
+ /* Check that the GMU is idle */
+ if (!a6xx_gmu_isidle(_gpu->gmu))
+ return false;
+ }
/* Check tha the CX master is idle */
if (gpu_read(gpu, REG_A6XX_RBBM_STATUS) &
@@ -612,13 +614,15 @@ static void a6xx_set_hwcg(struct msm_gpu *gpu, bool state)
return;
/* Disable SP clock before programming HWCG registers */
- gmu_rmw(gmu, REG_A6XX_GPU_GMU_GX_SPTPRAC_CLOCK_CONTROL, 1, 0);
+ if (!adreno_has_gmu_wrapper(adreno_gpu))
+ gmu_rmw(gmu, REG_A6XX_GPU_GMU_GX_SPTPRAC_CLOCK_CONTROL, 1, 0);
for (i = 0; (reg = _gpu->info->hwcg[i], reg->offset); i++)
gpu_write(gpu, reg->offset, state ? reg->value : 0);
/* Enable SP clock */
- gmu_rmw(gmu, REG_A6XX_GPU_GMU_GX_SPTPRAC_CLOCK_CONTROL, 0, 1);
+ if (!adreno_has_gmu_wrapper(adreno_gpu))
+ gmu_rmw(gmu, REG_A6XX_GPU_GMU_GX_SPTPRAC_CLOCK_CONTROL, 0, 1);
gpu_write(gpu, REG_A6XX_RBBM_CLOCK_CNTL, state ? clock_cntl_on : 0);
}
@@ -994,10 +998,13 @@ static int
[Freedreno] [PATCH v2 02/14] drm/msm/a6xx: Extend UBWC config
Port setting min_access_length, ubwc_mode and upper_bit from downstream.
Values were validated using downstream device trees for SM8[123]50 and
left default (as per downstream) elsewhere.
Signed-off-by: Konrad Dybcio
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 29 +++
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index c5f5d0bb3fdc..8855d798bbb3 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -786,17 +786,25 @@ static void a6xx_set_cp_protect(struct msm_gpu *gpu)
static void a6xx_set_ubwc_config(struct msm_gpu *gpu)
{
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
- u32 lower_bit = 2;
+ u32 lower_bit = 1;
+ u32 upper_bit = 0;
u32 amsbc = 0;
u32 rgb565_predicator = 0;
u32 uavflagprd_inv = 0;
+ u32 min_acc_len = 0;
+ u32 ubwc_mode = 0;
/* a618 is using the hw default values */
if (adreno_is_a618(adreno_gpu))
return;
- if (adreno_is_a640_family(adreno_gpu))
+ if (adreno_is_a630(adreno_gpu))
+ lower_bit = 2;
+
+ if (adreno_is_a640_family(adreno_gpu)) {
amsbc = 1;
+ lower_bit = 2;
+ }
if (adreno_is_a650(adreno_gpu) || adreno_is_a660(adreno_gpu)) {
/* TODO: get ddr type from bootloader and use 2 for LPDDR4 */
@@ -807,18 +815,23 @@ static void a6xx_set_ubwc_config(struct msm_gpu *gpu)
}
if (adreno_is_7c3(adreno_gpu)) {
- lower_bit = 1;
amsbc = 1;
rgb565_predicator = 1;
uavflagprd_inv = 2;
}
gpu_write(gpu, REG_A6XX_RB_NC_MODE_CNTL,
- rgb565_predicator << 11 | amsbc << 4 | lower_bit << 1);
- gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, lower_bit << 1);
- gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL,
- uavflagprd_inv << 4 | lower_bit << 1);
- gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, lower_bit << 21);
+ rgb565_predicator << 11 | upper_bit << 10 | amsbc << 4 |
+ min_acc_len << 3 | lower_bit << 1 | ubwc_mode);
+
+ gpu_write(gpu, REG_A6XX_TPL1_NC_MODE_CNTL, upper_bit << 4 |
+ min_acc_len << 3 | lower_bit << 1 | ubwc_mode);
+
+ gpu_write(gpu, REG_A6XX_SP_NC_MODE_CNTL, upper_bit << 10 |
+ uavflagprd_inv << 4 | min_acc_len << 3 |
+ lower_bit << 1 | ubwc_mode);
+
+ gpu_write(gpu, REG_A6XX_UCHE_MODE_CNTL, min_acc_len << 23 | lower_bit
<< 21);
}
static int a6xx_cp_init(struct msm_gpu *gpu)
--
2.39.1
Re: [Freedreno] [RFT PATCH v2 3/3] drm/msm/dsi: More properly handle errors in regards to dsi_mgr_bridge_power_on()
Hi,
On Mon, Feb 13, 2023 at 6:02 PM Abhinav Kumar wrote:
>
> Hi Doug
>
> Sorry for the delayed response.
>
> On 2/2/2023 2:46 PM, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Feb 2, 2023 at 2:37 PM Abhinav Kumar
> > wrote:
> >>
> >> Hi Doug
> >>
> >> On 1/31/2023 2:18 PM, Douglas Anderson wrote:
> >>> In commit 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to modeset
> >>> time") the error handling with regards to dsi_mgr_bridge_power_on()
> >>> got a bit worse. Specifically if we failed to power the bridge on then
> >>> nothing would really notice. The modeset function couldn't return an
> >>> error and thus we'd blindly go forward and try to do the pre-enable.
> >>>
> >>> In commit ec7981e6c614 ("drm/msm/dsi: don't powerup at modeset time
> >>> for parade-ps8640") we added a special case to move the powerup back
> >>> to pre-enable time for ps8640. When we did that, we didn't try to
> >>> recover the old/better error handling just for ps8640.
> >>>
> >>> In the patch ("drm/msm/dsi: Stop unconditionally powering up DSI hosts
> >>> at modeset") we've now moved the powering up back to exclusively being
> >>> during pre-enable. That means we can add the better error handling
> >>> back in, so let's do it. To do so we'll add a new function
> >>> dsi_mgr_bridge_power_off() that's matches how errors were handled
> >>> prior to commit 7d8e9a90509f ("drm/msm/dsi: move DSI host powerup to
> >>> modeset time").
> >>>
> >>> NOTE: Now that we have dsi_mgr_bridge_power_off(), it feels as if we
> >>> should be calling it in dsi_mgr_bridge_post_disable(). That would make
> >>> some sense, but doing so would change the current behavior and thus
> >>> should be a separate patch. Specifically:
> >>> * dsi_mgr_bridge_post_disable() always calls dsi_mgr_phy_disable()
> >>> even in the slave-DSI case of bonded DSI. We'd need to add special
> >>> handling for this if it's truly needed.
> >>> * dsi_mgr_bridge_post_disable() calls msm_dsi_phy_pll_save_state()
> >>> midway through the poweroff.
> >>> * dsi_mgr_bridge_post_disable() has a different order of some of the
> >>> poweroffs / IRQ disables.
> >>> For now we'll leave dsi_mgr_bridge_post_disable() alone.
> >>>
> >>> Signed-off-by: Douglas Anderson
> >>> ---
> >>>
> >>> Changes in v2:
> >>> - ("More properly handle errors...") new for v2.
> >>>
> >>>drivers/gpu/drm/msm/dsi/dsi_manager.c | 32 ++-
> >>>1 file changed, 26 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> >>> b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> >>> index 2197a54b9b96..28b8012a21f2 100644
> >>> --- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
> >>> +++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
> >>> @@ -228,7 +228,7 @@ static void msm_dsi_manager_set_split_display(u8 id)
> >>>}
> >>>}
> >>>
> >>> -static void dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
> >>> +static int dsi_mgr_bridge_power_on(struct drm_bridge *bridge)
> >>>{
> >>>int id = dsi_mgr_bridge_get_id(bridge);
> >>>struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
> >>> @@ -268,14 +268,31 @@ static void dsi_mgr_bridge_power_on(struct
> >>> drm_bridge *bridge)
> >>>if (is_bonded_dsi && msm_dsi1)
> >>>msm_dsi_host_enable_irq(msm_dsi1->host);
> >>>
> >>> - return;
> >>> + return 0;
> >>>
> >>>host1_on_fail:
> >>>msm_dsi_host_power_off(host);
> >>>host_on_fail:
> >>>dsi_mgr_phy_disable(id);
> >>>phy_en_fail:
> >>> - return;
> >>> + return ret;
> >>> +}
> >>> +
> >>> +static void dsi_mgr_bridge_power_off(struct drm_bridge *bridge)
> >>> +{
> >>> + int id = dsi_mgr_bridge_get_id(bridge);
> >>> + struct msm_dsi *msm_dsi = dsi_mgr_get_dsi(id);
> >>> + struct msm_dsi *msm_dsi1 = dsi_mgr_get_dsi(DSI_1);
> >>> + struct mipi_dsi_host *host = msm_dsi->host;
> >>> + bool is_bonded_dsi = IS_BONDED_DSI();
> >>> +
> >>> + msm_dsi_host_disable_irq(host);
> >>> + if (is_bonded_dsi && msm_dsi1) {
> >>> + msm_dsi_host_disable_irq(msm_dsi1->host);
> >>> + msm_dsi_host_power_off(msm_dsi1->host);
> >>> + }
> >>
> >> The order of disabling the IRQs should be opposite of how they were
> >> enabled.
> >>
> >> So while enabling it was DSI0 and then DSI1.
> >>
> >> Hence while disabling it should be DSI1 and then DSI0.
> >>
> >> So the order here should be
> >>
> >> DSI1 irq disable
> >> DSI0 irq disable
> >> DSI1 host power off
> >> DSI0 host power off
> >
> > Right. Normally you want to go opposite. I guess a few points, though:
> >
> > 1. As talked about in the commit message, the order I have matches the
> > order we had prior to commit 7d8e9a90509f ("drm/msm/dsi: move DSI host
> > powerup to modeset time").
> >
> > 2. I'd be curious if it matters. The order you request means we need
> > to check for `(is_bonded_dsi && msm_dsi1)` twice. While that's not a
> > big deal if it's important, it's nice not to have to do
Re: [Freedreno] [RFC PATCH 5/7] drm/msm/dpu: Document and enable TEAR interrupts on DSI interfaces
On 2023-02-13 19:09:32, Abhinav Kumar wrote: > > > On 2/13/2023 1:46 PM, Dmitry Baryshkov wrote: > > On 13/02/2023 21:37, Jessica Zhang wrote: > >> > >> > >> On 12/31/2022 1:50 PM, Marijn Suijten wrote: > >>> All SoCs since DPU 5.0.0 (and seemingly up until and including 6.0.0, > >>> but excluding 7.x.x) have the tear interrupt and control registers moved > >>> out of the PINGPONG block and into the INTF block. Wire up the > >>> necessary interrupts and IRQ masks on all supported hardware. > >> > >> Hi Marijn, > >> > >> Thanks for the patch. > >> > >> I saw that in your commit msg, you mentioned that 7.x doesn't have > >> tearcheck in the INTF block -- can you double check that this is correct? It wasn't correct and has already been removed for v2 [1] after rebasing on top of SM8[345]50 support, where the registers reside at a different (named 7 downstream) offset. [1] https://github.com/SoMainline/linux/commit/886d3fb9eed925e7e9c8d6ca63d2439eaec1c702 > >> I'm working on SM8350 (DPU v7) and I'm seeing that it does have > >> tearcheck in INTF block. > > > > I confirm, according to the vendor drivers INTF TE should be used for > > all DPU >= 5.0, including 7.x and 8.x > > > > However I think I know what Marijn meant here. For 5.x and 6.x these > > IRQs are handled at the address MDSS + 0x6e800 / + 0x6e900 (which means > > offset here should 0x6d800 and 0x6d900) for INTF_1 and INTF_2. Since DPU > > 7.x these IRQ registers were moved close to the main INTF block (0x36800 > > and 0x37800 = INTF + 0x800). That might have been the case. > Got it, then the commit text should remove "control" and just say tear > interrupt registers. It got a bit confusing. The wording here points to both the interrupt (MDP_INTFx_TEAR_INTR) registers and control (INTF_TEAR_xxx) registers separately. Feel free to bikeshed the wording in preliminary v2 [1]; should I drop the mention of the control registers being "moved" from PP to INTF entirely, leaving just the wording about the interrupt registers moving from MDP_SSPP_TOP0_INTR to a dedicated MDP_INTFx_TEAR_INTR region? > We will add the 7xxx intf tear check support on top of this series. No need, that is already taken care of in an impending v2 [1] (unless additional changes are required beyond the moved register offset). > >>> Signed-off-by: Marijn Suijten > >>> --- > >>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 78 +++ > >>> .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 6 +- > >>> .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 12 +++ > >>> .../gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h | 2 + > >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 3 + > >>> 5 files changed, 68 insertions(+), 33 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >>> index 1cfe94494135..b9b9b5b0b615 100644 > >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > >>> @@ -86,6 +86,15 @@ > >>> #define INTF_SC7280_MASK INTF_SC7180_MASK | BIT(DPU_DATA_HCTL_EN) > >>> +#define IRQ_MSM8998_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > >>> + BIT(MDP_SSPP_TOP0_INTR2) | \ > >>> + BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > >>> + BIT(MDP_INTF0_INTR) | \ > >>> + BIT(MDP_INTF1_INTR) | \ > >>> + BIT(MDP_INTF2_INTR) | \ > >>> + BIT(MDP_INTF3_INTR) | \ > >>> + BIT(MDP_INTF4_INTR)) > >>> + > >>> #define IRQ_SDM845_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > >>> BIT(MDP_SSPP_TOP0_INTR2) | \ > >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > >>> @@ -100,13 +109,15 @@ > >>> #define IRQ_QCM2290_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > >>> BIT(MDP_SSPP_TOP0_INTR2) | \ > >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > >>> - BIT(MDP_INTF1_INTR)) > >>> + BIT(MDP_INTF1_INTR) | \ > >>> + BIT(MDP_INTF1_TEAR_INTR)) > >>> #define IRQ_SC7180_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > >>> BIT(MDP_SSPP_TOP0_INTR2) | \ > >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > >>> BIT(MDP_INTF0_INTR) | \ > >>> - BIT(MDP_INTF1_INTR)) > >>> + BIT(MDP_INTF1_INTR) | \ > >>> + BIT(MDP_INTF1_TEAR_INTR)) > >>> #define IRQ_SC7280_MASK (BIT(MDP_SSPP_TOP0_INTR) | \ > >>> BIT(MDP_SSPP_TOP0_INTR2) | \ > >>> @@ -120,7 +131,9 @@ > >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > >>> BIT(MDP_INTF0_INTR) | \ > >>> BIT(MDP_INTF1_INTR) | \ > >>> + BIT(MDP_INTF1_TEAR_INTR) | \ > >>> BIT(MDP_INTF2_INTR) | \ > >>> + BIT(MDP_INTF2_TEAR_INTR) | \ > >>> BIT(MDP_INTF3_INTR) | \ > >>> BIT(MDP_INTF4_INTR)) > >>> @@ -129,7 +142,9 @@ > >>> BIT(MDP_SSPP_TOP0_HIST_INTR) | \ > >>> BIT(MDP_INTF0_INTR) | \ >
[Freedreno] [PATCH 3/3] drm/msm/a5xx: add devcoredump support to the fault handler
Use adreno_fault_handler() to implement a5xx_fault_handler(). This
enables devcoredump support on a5xx platforms, allowing one to capture
the crashed GPU state at the time of context fault.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index b5270324f5f8..d38ebfb5965b 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1099,16 +1099,19 @@ bool a5xx_idle(struct msm_gpu *gpu, struct
msm_ringbuffer *ring)
static int a5xx_fault_handler(void *arg, unsigned long iova, int flags, void
*data)
{
struct msm_gpu *gpu = arg;
- pr_warn_ratelimited("*** gpu fault: iova=%08lx, flags=%d
(%u,%u,%u,%u)\n",
- iova, flags,
+ struct adreno_smmu_fault_info *info = data;
+ char block[12] = "unknown";
+ u32 scratch[] = {
gpu_read(gpu, REG_A5XX_CP_SCRATCH_REG(4)),
gpu_read(gpu, REG_A5XX_CP_SCRATCH_REG(5)),
gpu_read(gpu, REG_A5XX_CP_SCRATCH_REG(6)),
- gpu_read(gpu, REG_A5XX_CP_SCRATCH_REG(7)));
+ gpu_read(gpu, REG_A5XX_CP_SCRATCH_REG(7)),
+ };
- gpu->aspace->mmu->funcs->resume_translation(gpu->aspace->mmu);
+ if (info)
+ snprintf(block, sizeof(block), "%x", info->fsynr1);
- return 0;
+ return adreno_fault_handler(gpu, iova, flags, info, block, scratch);
}
static void a5xx_cp_err_irq(struct msm_gpu *gpu)
--
2.30.2
[Freedreno] [PATCH 2/3] drm/msm/adreno: split a6xx fault handler into generic and a6xx parts
Split the a6xx_fault_handler() into the generic adreno_fault_handler()
and platform-specific parts. The adreno_fault_handler() can further be
used by a5xx and hopefully by a4xx (at some point).
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 64 +++--
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 60 +++
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 4 ++
3 files changed, 71 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index aae60cbd9164..faee45135ca8 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1361,73 +1361,23 @@ static const char *a6xx_fault_block(struct msm_gpu
*gpu, u32 id)
return a6xx_uche_fault_block(gpu, id);
}
-#define ARM_SMMU_FSR_TF BIT(1)
-#define ARM_SMMU_FSR_PFBIT(3)
-#define ARM_SMMU_FSR_EFBIT(4)
-
static int a6xx_fault_handler(void *arg, unsigned long iova, int flags, void
*data)
{
struct msm_gpu *gpu = arg;
struct adreno_smmu_fault_info *info = data;
- const char *type = "UNKNOWN";
- const char *block;
- bool do_devcoredump = info && !READ_ONCE(gpu->crashstate);
-
- /*
-* If we aren't going to be resuming later from fault_worker, then do
-* it now.
-*/
- if (!do_devcoredump) {
- gpu->aspace->mmu->funcs->resume_translation(gpu->aspace->mmu);
- }
+ const char *block = "unknown";
- /*
-* Print a default message if we couldn't get the data from the
-* adreno-smmu-priv
-*/
- if (!info) {
- pr_warn_ratelimited("*** gpu fault: iova=%.16lx flags=%d
(%u,%u,%u,%u)\n",
- iova, flags,
+ u32 scratch[] = {
gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(4)),
gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(5)),
gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(6)),
- gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(7)));
-
- return 0;
- }
-
- if (info->fsr & ARM_SMMU_FSR_TF)
- type = "TRANSLATION";
- else if (info->fsr & ARM_SMMU_FSR_PF)
- type = "PERMISSION";
- else if (info->fsr & ARM_SMMU_FSR_EF)
- type = "EXTERNAL";
-
- block = a6xx_fault_block(gpu, info->fsynr1 & 0xff);
-
- pr_warn_ratelimited("*** gpu fault: ttbr0=%.16llx iova=%.16lx dir=%s
type=%s source=%s (%u,%u,%u,%u)\n",
- info->ttbr0, iova,
- flags & IOMMU_FAULT_WRITE ? "WRITE" : "READ",
- type, block,
- gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(4)),
- gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(5)),
- gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(6)),
- gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(7)));
-
- if (do_devcoredump) {
- /* Turn off the hangcheck timer to keep it from bothering us */
- del_timer(>hangcheck_timer);
+ gpu_read(gpu, REG_A6XX_CP_SCRATCH_REG(7)),
+ };
- gpu->fault_info.ttbr0 = info->ttbr0;
- gpu->fault_info.iova = iova;
- gpu->fault_info.flags = flags;
- gpu->fault_info.type = type;
- gpu->fault_info.block = block;
+ if (info)
+ block = a6xx_fault_block(gpu, info->fsynr1 & 0xff);
- kthread_queue_work(gpu->worker, >fault_work);
- }
-
- return 0;
+ return adreno_fault_handler(gpu, iova, flags, info, block, scratch);
}
static void a6xx_cp_hw_err_irq(struct msm_gpu *gpu)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index a05d48ecae15..9bbc298fe6d6 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -246,6 +246,66 @@ u64 adreno_private_address_space_size(struct msm_gpu *gpu)
return SZ_4G;
}
+#define ARM_SMMU_FSR_TF BIT(1)
+#define ARM_SMMU_FSR_PFBIT(3)
+#define ARM_SMMU_FSR_EFBIT(4)
+
+int adreno_fault_handler(struct msm_gpu *gpu, unsigned long iova, int flags,
+struct adreno_smmu_fault_info *info, const char *block,
+u32 scratch[4])
+{
+ const char *type = "UNKNOWN";
+ bool do_devcoredump = info && !READ_ONCE(gpu->crashstate);
+
+ /*
+* If we aren't going to be resuming later from fault_worker, then do
+* it now.
+*/
+ if (!do_devcoredump) {
+ gpu->aspace->mmu->funcs->resume_translation(gpu->aspace->mmu);
+ }
+
+ /*
+* Print a default message if we couldn't get the data from the
+* adreno-smmu-priv
+*/
[Freedreno] [PATCH 1/3] drm/msm/adreno: stall translation on fault for all GPU families
The commit e25e92e08e32 ("drm/msm: devcoredump iommu fault support")
enabled SMMU stalling to collect GPU state, but only for a6xx. It tied
enabling the stall with tha per-instance pagetables creation.
Since that commit SoCs with a5xx also gained support for
adreno-smmu-priv. Move stalling into generic code and add corresponding
resume_translation calls.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 2 ++
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
drivers/gpu/drm/msm/msm_iommu.c | 38 ++---
drivers/gpu/drm/msm/msm_mmu.h | 1 +
4 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 047c5e8c87ff..b5270324f5f8 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1106,6 +1106,8 @@ static int a5xx_fault_handler(void *arg, unsigned long
iova, int flags, void *da
gpu_read(gpu, REG_A5XX_CP_SCRATCH_REG(6)),
gpu_read(gpu, REG_A5XX_CP_SCRATCH_REG(7)));
+ gpu->aspace->mmu->funcs->resume_translation(gpu->aspace->mmu);
+
return 0;
}
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 3bc02dbed9a7..a05d48ecae15 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -208,7 +208,7 @@ adreno_iommu_create_address_space(struct msm_gpu *gpu,
struct msm_gem_address_space *aspace;
u64 start, size;
- mmu = msm_iommu_new(>dev, quirks);
+ mmu = msm_iommu_gpu_new(>dev, gpu, quirks);
if (IS_ERR_OR_NULL(mmu))
return ERR_CAST(mmu);
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index c2507582ecf3..418e1e06cdde 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -237,13 +237,6 @@ struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu
*parent)
if (!ttbr1_cfg)
return ERR_PTR(-ENODEV);
- /*
-* Defer setting the fault handler until we have a valid adreno_smmu
-* to avoid accidentially installing a GPU specific fault handler for
-* the display's iommu
-*/
- iommu_set_fault_handler(iommu->domain, msm_fault_handler, iommu);
-
pagetable = kzalloc(sizeof(*pagetable), GFP_KERNEL);
if (!pagetable)
return ERR_PTR(-ENOMEM);
@@ -271,9 +264,6 @@ struct msm_mmu *msm_iommu_pagetable_create(struct msm_mmu
*parent)
* the arm-smmu driver as a trigger to set up TTBR0
*/
if (atomic_inc_return(>pagetables) == 1) {
- /* Enable stall on iommu fault: */
- adreno_smmu->set_stall(adreno_smmu->cookie, true);
-
ret = adreno_smmu->set_ttbr0_cfg(adreno_smmu->cookie,
_cfg);
if (ret) {
free_io_pgtable_ops(pagetable->pgtbl_ops);
@@ -302,6 +292,7 @@ static int msm_fault_handler(struct iommu_domain *domain,
struct device *dev,
unsigned long iova, int flags, void *arg)
{
struct msm_iommu *iommu = arg;
+ struct msm_mmu *mmu = >base;
struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(iommu->base.dev);
struct adreno_smmu_fault_info info, *ptr = NULL;
@@ -314,6 +305,10 @@ static int msm_fault_handler(struct iommu_domain *domain,
struct device *dev,
return iommu->base.handler(iommu->base.arg, iova, flags, ptr);
pr_warn_ratelimited("*** fault: iova=%16lx, flags=%d\n", iova, flags);
+
+ if (mmu->funcs->resume_translation)
+ mmu->funcs->resume_translation(mmu);
+
return 0;
}
@@ -321,7 +316,8 @@ static void msm_iommu_resume_translation(struct msm_mmu
*mmu)
{
struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(mmu->dev);
- adreno_smmu->resume_translation(adreno_smmu->cookie, true);
+ if (adreno_smmu->resume_translation)
+ adreno_smmu->resume_translation(adreno_smmu->cookie, true);
}
static void msm_iommu_detach(struct msm_mmu *mmu)
@@ -406,3 +402,23 @@ struct msm_mmu *msm_iommu_new(struct device *dev, unsigned
long quirks)
return >base;
}
+
+struct msm_mmu *msm_iommu_gpu_new(struct device *dev, struct msm_gpu *gpu,
unsigned long quirks)
+{
+ struct adreno_smmu_priv *adreno_smmu = dev_get_drvdata(dev);
+ struct msm_iommu *iommu;
+ struct msm_mmu *mmu;
+
+ mmu = msm_iommu_new(dev, quirks);
+ if (IS_ERR(mmu))
+ return mmu;
+
+ iommu = to_msm_iommu(mmu);
+ iommu_set_fault_handler(iommu->domain, msm_fault_handler, iommu);
+
+ /* Enable stall on iommu fault: */
+ if (adreno_smmu->set_stall)
+ adreno_smmu->set_stall(adreno_smmu->cookie, true);
+
+ return mmu;
+}
diff --git a/drivers/gpu/drm/msm/msm_mmu.h
[Freedreno] [PATCH 0/3] drm/msm/adreno: implement devcoredump support for a5xx
During the debug of the a5xx preempt issue, having the devcoredump support was a great help. These patches port necessary code bits from being a6xx-specific to generic code path and then enables devcoredump for a5xx. Dmitry Baryshkov (3): drm/msm/adreno: stall translation on fault for all GPU families drm/msm/adreno: split a6xx fault handler into generic and a6xx parts drm/msm/a5xx: add devcoredump support to the fault handler drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 13 +++-- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 64 +++-- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 62 +++- drivers/gpu/drm/msm/adreno/adreno_gpu.h | 4 ++ drivers/gpu/drm/msm/msm_iommu.c | 38 ++- drivers/gpu/drm/msm/msm_mmu.h | 1 + 6 files changed, 109 insertions(+), 73 deletions(-) -- 2.30.2
Re: [Freedreno] [PATCH 09/14] drm/msm/a6xx: Fix some A619 tunables
On 8.02.2023 19:21, Jordan Crouse wrote:
> On Thu, Jan 26, 2023 at 04:16:13PM +0100, Konrad Dybcio wrote:
>> Adreno 619 expects some tunables to be set differently. Make up for it.
>>
>> Fixes: b7616b5c69e6 ("drm/msm/adreno: Add A619 support")
>> Signed-off-by: Konrad Dybcio
>> ---
>> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 6 +-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> index 7a480705f407..f34ab3f39f09 100644
>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> @@ -1171,6 +1171,8 @@ static int hw_init(struct msm_gpu *gpu)
>> gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00200200);
>> else if (adreno_is_a650(adreno_gpu) || adreno_is_a660(adreno_gpu))
>> gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00300200);
>> + else if (adreno_is_a619(adreno_gpu))
>> + gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x00018000);
>> else if (adreno_is_a610(adreno_gpu))
>> gpu_write(gpu, REG_A6XX_PC_DBG_ECO_CNTL, 0x0008);
>> else
>> @@ -1188,7 +1190,9 @@ static int hw_init(struct msm_gpu *gpu)
>> a6xx_set_ubwc_config(gpu);
>>
>> /* Enable fault detection */
>> - if (adreno_is_a610(adreno_gpu))
>> + if (adreno_is_a619(adreno_gpu))
>> + gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 <<
>> 30) | 0x3f);
>> + else if (adreno_is_a610(adreno_gpu))
>> gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 <<
>> 30) | 0x3);
>> else
>> gpu_write(gpu, REG_A6XX_RBBM_INTERFACE_HANG_INT_CNTL, (1 <<
>> 30) | 0x1f);
>
> The number appended to the register is the number of clock ticks to wait
> before declaring a hang. 0x3f happens to be the largest value that
> can be set for the a6xx family (excepting the 610 which, IIRC, used older
> hardware that had a smaller field for the counter).
Makes sense!
Downstream the
> number would creep up over time as unexplained hangs were discovered and
> diagnosed or covered up as "just wait longer".
lol..
>
> So in theory you could leave this with the "default value" or even bump
> up the default value to 0x3f for all targets if you wanted to. An
> alternate solution (that downstream does) is to put this as a
> pre-defined configuration in gpulist[].
I'm not sure it's a good idea to let things loose, as that may let some
bugs slip through.. Perhaps let's leave that as-is until we have a seriously
otherwise-unresolvable situation..
Konrad
>
> Jordan
