On Thu, Jul 6, 2023 at 8:16 PM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> wrote:
>
> On 07/07/2023 02:35, Konrad Dybcio wrote:
> > On 6.07.2023 23:10, Rob Clark wrote:
> >> From: Rob Clark <robdclark@xxxxxxxxxxxx>
> >>
> >> Sometimes it is useful to know the sub-generation (or "family"). And in
> >> any case, this helps us get away from infering the generation from the
> >> numerical chip-id.
> >>
> >> Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx>
> >> ---
> > [...]
> >
> >> .rev = ADRENO_REV(5, 0, 8, ANY_ID),
> >> + .family = ADRENO_5XX,
> >> .revn = 508,
> > 508 is from 530 fam
> >
> >> .fw = {
> >> [ADRENO_FW_PM4] = "a530_pm4.fw",
> >> @@ -156,6 +168,7 @@ static const struct adreno_info gpulist[] = {
> >> .zapfw = "a508_zap.mdt",
> >> }, {
> >> .rev = ADRENO_REV(5, 0, 9, ANY_ID),
> >> + .family = ADRENO_5XX,
> >> .revn = 509,
> > 509 and 512 are from 540 fam
> >
> >> .fw = {
> >> [ADRENO_FW_PM4] = "a530_pm4.fw",
> >> @@ -173,6 +186,7 @@ static const struct adreno_info gpulist[] = {
> >> .zapfw = "a512_zap.mdt",
> >> }, {
> >> .rev = ADRENO_REV(5, 1, 0, ANY_ID),
> >> + .family = ADRENO_5XX,
> >> .revn = 510,
> > 510 is 530ish but I think it's closer to 505 or whatever the
> > 8953 gpu was called
>
> I'd say, there were following generations here:
>
> - a505 / a506 / a508
> - a509 / a512
> - a510
> - a530
> - a540
>
> Indeed a50x were close to a530 in some aspects and a509/512 being closer
> to a540, but I don't think they were the same family.
As a practical matter, I chose to defer splitting a3xx/a4xx/a5xx into
sub-generations, simply because we didn't have any use for that yet.
For a2xx and a6xx there was a clear immediate use for (most of) it,
and what isn't used falls out of usage of sub-generation
classification we have in mesa, so I have a lot of confidence in the
split.
We can try and map it all out for the other gens now.. or simply just
wait until there is a use for it. I'm not super against mapping it
out better now, but didn't feel that there was any down side to just
punting on that. It would be easy enough to do follow-up patches that
refactor the code and split out the subgen enums at the same time.
BR,
-R
> >
> > [...]
> >
> >> - priv->is_a2xx = config.rev.core == 2;
> >> + priv->is_a2xx = info->family < ADRENO_3XX;
> > is this variable even needed now that there are explicit family values?
> >
> > Konrad
> >> priv->has_cached_coherent =
> >> !!(info->quirks & ADRENO_QUIRK_HAS_CACHED_COHERENT);
> >>
> >> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> >> index 2e62a7ce9f13..75ff7fb46099 100644
> >> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> >> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> >> @@ -1079,8 +1079,13 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
> >> u32 speedbin;
> >> int ret;
> >>
> >> + adreno_gpu->funcs = funcs;
> >> + adreno_gpu->info = adreno_info(config->rev);
> >> + adreno_gpu->rev = *rev;
> >> +
> >> /* Only handle the core clock when GMU is not in use (or is absent). */
> >> - if (adreno_has_gmu_wrapper(adreno_gpu) || config->rev.core < 6) {
> >> + if (adreno_has_gmu_wrapper(adreno_gpu) ||
> >> + adreno_gpu->info->family < ADRENO_6XX_GEN1) {
> >> /*
> >> * This can only be done before devm_pm_opp_of_add_table(), or
> >> * dev_pm_opp_set_config() will WARN_ON()
> >> @@ -1096,10 +1101,6 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
> >> devm_pm_opp_set_clkname(dev, "core");
> >> }
> >>
> >> - adreno_gpu->funcs = funcs;
> >> - adreno_gpu->info = adreno_info(config->rev);
> >> - adreno_gpu->rev = *rev;
> >> -
> >> if (adreno_read_speedbin(dev, &speedbin) || !speedbin)
> >> speedbin = 0xffff;
> >> adreno_gpu->speedbin = (uint16_t) (0xffff & speedbin);
> >> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> >> index 6066cfaaea52..2fa14dcd4e40 100644
> >> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> >> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> >> @@ -29,6 +29,25 @@ enum {
> >> ADRENO_FW_MAX,
> >> };
> >>
> >> +/**
> >> + * @enum adreno_family: identify generation and possibly sub-generation
> >> + *
> >> + * In some cases there are distinct sub-generations within a major revision
> >> + * so it helps to be able to group the GPU devices by generation and if
> >> + * necessary sub-generation.
> >> + */
> >> +enum adreno_family {
> >> + ADRENO_2XX_GEN1, /* a20x */
> >> + ADRENO_2XX_GEN2, /* a22x */
> >> + ADRENO_3XX,
> >> + ADRENO_4XX,
> >> + ADRENO_5XX,
> >> + ADRENO_6XX_GEN1, /* a630 family */
> >> + ADRENO_6XX_GEN2, /* a640 family */
> >> + ADRENO_6XX_GEN3, /* a650 family */
> >> + ADRENO_6XX_GEN4, /* a660 family */
> >> +};
> >> +
> >> #define ADRENO_QUIRK_TWO_PASS_USE_WFI BIT(0)
> >> #define ADRENO_QUIRK_FAULT_DETECT_MASK BIT(1)
> >> #define ADRENO_QUIRK_LMLOADKILL_DISABLE BIT(2)
> >> @@ -63,6 +82,7 @@ extern const struct adreno_reglist a660_hwcg[], a690_hwcg[];
> >> struct adreno_info {
> >> const char *machine;
> >> struct adreno_rev rev;
> >> + enum adreno_family family;
> >> uint32_t revn;
> >> const char *fw[ADRENO_FW_MAX];
> >> uint32_t gmem;
> >> @@ -188,14 +208,14 @@ static inline bool adreno_is_a2xx(const struct adreno_gpu *gpu)
> >> {
> >> if (WARN_ON_ONCE(!gpu->info))
> >> return false;
> >> - return (gpu->info->revn < 300);
> >> + return gpu->info->family < ADRENO_2XX_GEN2;
> >> }
> >>
> >> static inline bool adreno_is_a20x(const struct adreno_gpu *gpu)
> >> {
> >> if (WARN_ON_ONCE(!gpu->info))
> >> return false;
> >> - return (gpu->info->revn < 210);
> >> + return gpu->info->family == ADRENO_2XX_GEN1;
> >> }
> >>
> >> static inline bool adreno_is_a225(const struct adreno_gpu *gpu)
> >> @@ -338,29 +358,31 @@ static inline int adreno_is_a690(const struct adreno_gpu *gpu)
> >> /* check for a615, a616, a618, a619 or any a630 derivatives */
> >> static inline int adreno_is_a630_family(const struct adreno_gpu *gpu)
> >> {
> >> - return adreno_is_revn(gpu, 630) ||
> >> - adreno_is_revn(gpu, 615) ||
> >> - adreno_is_revn(gpu, 616) ||
> >> - adreno_is_revn(gpu, 618) ||
> >> - adreno_is_revn(gpu, 619);
> >> + if (WARN_ON_ONCE(!gpu->info))
> >> + return false;
> >> + return gpu->info->family == ADRENO_6XX_GEN1;
> >> }
> >>
> >> static inline int adreno_is_a660_family(const struct adreno_gpu *gpu)
> >> {
> >> - return adreno_is_a660(gpu) || adreno_is_a690(gpu) || adreno_is_7c3(gpu);
> >> + if (WARN_ON_ONCE(!gpu->info))
> >> + return false;
> >> + return gpu->info->family == ADRENO_6XX_GEN4;
> >> }
> >>
> >> /* check for a650, a660, or any derivatives */
> >> static inline int adreno_is_a650_family(const struct adreno_gpu *gpu)
> >> {
> >> - return adreno_is_revn(gpu, 650) ||
> >> - adreno_is_revn(gpu, 620) ||
> >> - adreno_is_a660_family(gpu);
> >> + if (WARN_ON_ONCE(!gpu->info))
> >> + return false;
> >> + return gpu->info->family >= ADRENO_6XX_GEN3;
> >> }
> >>
> >> static inline int adreno_is_a640_family(const struct adreno_gpu *gpu)
> >> {
> >> - return adreno_is_a640(gpu) || adreno_is_a680(gpu);
> >> + if (WARN_ON_ONCE(!gpu->info))
> >> + return false;
> >> + return gpu->info->family == ADRENO_6XX_GEN2;
> >> }
> >>
> >> u64 adreno_private_address_space_size(struct msm_gpu *gpu);
>
> --
> With best wishes
> Dmitry
>
