Re: [PATCH v2 07/14] drm/msm/a6xx: Add support for A619_holi

[Dmitry Baryshkov]

On 17/02/2023 23:21, Konrad Dybcio wrote:
> On 17.02.2023 22:19, Dmitry Baryshkov wrote:
> > On 14/02/2023 19:31, Konrad Dybcio wrote:
> > > 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 <konrad.dybcio@xxxxxxxxxx>
> > > ---
> > >    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/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
> > > +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> > > @@ -264,6 +264,19 @@ static const struct adreno_info gpulist[] = {
> > >            .gmem = SZ_512K,
> > >            .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > >            .init = a6xx_gpu_init,
> > > +    }, {
> > > +        .rev = ADRENO_REV(6, 1, 9, 1),
> >
> > I think this deserves a comment that GMU-enabled sm6350 has patch_id 0 (if I interpreted the vendor dtsi correctly).
> >
> > Another option might be to actually check for the qcom,gmu presense and add that to the selection conditional.
> We pass the GMU wrapper in qcom,gmu = <>, though perhaps setting
> the holi-ness based on whether it's "qcom,gmu-x.y.z.a" or
> "qcom,gmu-wrapper" would be wiser.. The patch ID is indeterminate
> and I *think* one GMU-wrapper A619 has patch id 0..

I was not aware that GMU-wrapper also adds a GMU device. In this case, 
checking the GMU's compatible strings sounds like a logical approach to me.

> Konrad
> > > +        .revn = 619,
> > > +        .name = "A619_holi",
> > > +        .fw = {
> > > +            [ADRENO_FW_SQE] = "a630_sqe.fw",
> > > +        },
> > > +        .gmem = SZ_512K,
> > > +        .inactive_period = DRM_MSM_INACTIVE_PERIOD,
> > > +        .quirks = ADRENO_QUIRK_GMU_WRAPPER,
> > > +        .init = a6xx_gpu_init,
> > > +        .zapfw = "a615_zap.mdt",
> > > +        .hwcg = a615_hwcg,
> > >        }, {
> > >            .rev = ADRENO_REV(6, 1, 9, ANY_ID),
> > >            .revn = 619,
> > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > > index 7c5e0a90b5fb..16241368c2e4 100644
> > > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > > @@ -252,6 +252,11 @@ static inline int adreno_is_a619(struct adreno_gpu *gpu)
> > >        return gpu->revn == 619;
> > >    }
> > >    +static inline int adreno_is_a619_holi(struct adreno_gpu *gpu)
> > > +{
> > > +    return adreno_is_a619(gpu) && adreno_has_gmu_wrapper(gpu);
> > > +}
> > > +
> > >    static inline int adreno_is_a630(struct adreno_gpu *gpu)
> > >    {
> > >        return gpu->revn == 630;

--
With best wishes
Dmitry