* [RFC PATCH] drm/msm: Introduce Adreno Features
@ 2021-07-29 15:30 Akhil P Oommen
2021-07-29 15:56 ` Rob Clark
0 siblings, 1 reply; 3+ messages in thread
From: Akhil P Oommen @ 2021-07-29 15:30 UTC (permalink / raw)
To: freedreno, dri-devel, linux-arm-msm, Rob Clark
Cc: Jordan Crouse, Douglas Anderson, Jonathan Marek,
Matthias Kaehlcke, AngeloGioacchino Del Regno, Daniel Vetter,
David Airlie, Iskren Chernev, Konrad Dybcio, Sai Prakash Ranjan,
Sean Paul, Sharat Masetty, linux-kernel
Introduce a feature flag in gpulist to easily identify the capabilities
of each gpu revision. This will help to avoid a lot of adreno_is_axxx()
check when we add new features. In the current patch, HW APRIV feature
is converted to a feature flag.
Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
---
This patch is rebased on top of the below series:
https://patchwork.freedesktop.org/series/93192/
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 12 ------------
drivers/gpu/drm/msm/adreno/adreno_device.c | 3 +++
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 9 +++++++++
4 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 1881e09..b28305b 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1765,7 +1765,6 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
struct msm_drm_private *priv = dev->dev_private;
struct platform_device *pdev = priv->gpu_pdev;
struct adreno_platform_config *config = pdev->dev.platform_data;
- const struct adreno_info *info;
struct device_node *node;
struct a6xx_gpu *a6xx_gpu;
struct adreno_gpu *adreno_gpu;
@@ -1781,17 +1780,6 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
adreno_gpu->registers = NULL;
- /*
- * We need to know the platform type before calling into adreno_gpu_init
- * so that the hw_apriv flag can be correctly set. Snoop into the info
- * and grab the revision number
- */
- info = adreno_info(config->rev);
-
- if (info && (info->revn == 650 || info->revn == 660 ||
- adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), info->rev)))
- adreno_gpu->base.hw_apriv = true;
-
a6xx_llc_slices_init(pdev, a6xx_gpu);
ret = a6xx_set_supported_hw(&pdev->dev, config->rev);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 7b9d605..44321ec 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -276,6 +276,7 @@ static const struct adreno_info gpulist[] = {
.rev = ADRENO_REV(6, 5, 0, ANY_ID),
.revn = 650,
.name = "A650",
+ .features = ADRENO_APRIV,
.fw = {
[ADRENO_FW_SQE] = "a650_sqe.fw",
[ADRENO_FW_GMU] = "a650_gmu.bin",
@@ -289,6 +290,7 @@ static const struct adreno_info gpulist[] = {
.rev = ADRENO_REV(6, 6, 0, ANY_ID),
.revn = 660,
.name = "A660",
+ .features = ADRENO_APRIV,
.fw = {
[ADRENO_FW_SQE] = "a660_sqe.fw",
[ADRENO_FW_GMU] = "a660_gmu.bin",
@@ -301,6 +303,7 @@ static const struct adreno_info gpulist[] = {
}, {
.rev = ADRENO_REV(6, 3, 5, ANY_ID),
.name = "Adreno 7c Gen 3",
+ .features = ADRENO_APRIV,
.fw = {
[ADRENO_FW_SQE] = "a660_sqe.fw",
[ADRENO_FW_GMU] = "a660_gmu.bin",
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 9f5a302..e8acadf5 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -945,6 +945,9 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
pm_runtime_use_autosuspend(dev);
pm_runtime_enable(dev);
+ if (ADRENO_FEAT(adreno_gpu, ADRENO_APRIV))
+ adreno_gpu->base.hw_apriv = true;
+
return msm_gpu_init(drm, pdev, &adreno_gpu->base, &funcs->base,
adreno_gpu->info->name, &adreno_gpu_config);
}
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 50b4d53..61797c3 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -35,6 +35,11 @@ enum adreno_quirks {
ADRENO_QUIRK_LMLOADKILL_DISABLE = 3,
};
+enum adreno_features {
+ /* ADRENO has HW APRIV feature */
+ ADRENO_APRIV,
+};
+
struct adreno_rev {
uint8_t core;
uint8_t major;
@@ -63,6 +68,7 @@ struct adreno_info {
struct adreno_rev rev;
uint32_t revn;
const char *name;
+ u32 features;
const char *fw[ADRENO_FW_MAX];
uint32_t gmem;
enum adreno_quirks quirks;
@@ -388,6 +394,9 @@ static inline uint32_t get_wptr(struct msm_ringbuffer *ring)
return (ring->cur - ring->start) % (MSM_GPU_RINGBUFFER_SZ >> 2);
}
+#define ADRENO_FEAT(adreno_gpu, feature) \
+ (adreno_gpu->info->features & (1 << feature))
+
/*
* Given a register and a count, return a value to program into
* REG_CP_PROTECT_REG(n) - this will block both reads and writes for _len
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] drm/msm: Introduce Adreno Features
2021-07-29 15:30 [RFC PATCH] drm/msm: Introduce Adreno Features Akhil P Oommen
@ 2021-07-29 15:56 ` Rob Clark
2021-07-29 18:45 ` Akhil P Oommen
0 siblings, 1 reply; 3+ messages in thread
From: Rob Clark @ 2021-07-29 15:56 UTC (permalink / raw)
To: Akhil P Oommen
Cc: freedreno, dri-devel, linux-arm-msm, Jordan Crouse,
Douglas Anderson, Jonathan Marek, Matthias Kaehlcke,
AngeloGioacchino Del Regno, Daniel Vetter, David Airlie,
Iskren Chernev, Konrad Dybcio, Sai Prakash Ranjan, Sean Paul,
Sharat Masetty, Linux Kernel Mailing List
On Thu, Jul 29, 2021 at 8:31 AM Akhil P Oommen <akhilpo@codeaurora.org> wrote:
>
> Introduce a feature flag in gpulist to easily identify the capabilities
> of each gpu revision. This will help to avoid a lot of adreno_is_axxx()
> check when we add new features. In the current patch, HW APRIV feature
> is converted to a feature flag.
>
> Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
> ---
> This patch is rebased on top of the below series:
> https://patchwork.freedesktop.org/series/93192/
>
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 12 ------------
> drivers/gpu/drm/msm/adreno/adreno_device.c | 3 +++
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 9 +++++++++
> 4 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 1881e09..b28305b 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -1765,7 +1765,6 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
> struct msm_drm_private *priv = dev->dev_private;
> struct platform_device *pdev = priv->gpu_pdev;
> struct adreno_platform_config *config = pdev->dev.platform_data;
> - const struct adreno_info *info;
> struct device_node *node;
> struct a6xx_gpu *a6xx_gpu;
> struct adreno_gpu *adreno_gpu;
> @@ -1781,17 +1780,6 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
>
> adreno_gpu->registers = NULL;
>
> - /*
> - * We need to know the platform type before calling into adreno_gpu_init
> - * so that the hw_apriv flag can be correctly set. Snoop into the info
> - * and grab the revision number
> - */
> - info = adreno_info(config->rev);
> -
> - if (info && (info->revn == 650 || info->revn == 660 ||
> - adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), info->rev)))
> - adreno_gpu->base.hw_apriv = true;
> -
> a6xx_llc_slices_init(pdev, a6xx_gpu);
>
> ret = a6xx_set_supported_hw(&pdev->dev, config->rev);
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 7b9d605..44321ec 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -276,6 +276,7 @@ static const struct adreno_info gpulist[] = {
> .rev = ADRENO_REV(6, 5, 0, ANY_ID),
> .revn = 650,
> .name = "A650",
> + .features = ADRENO_APRIV,
I guess this should be:
.features = BIT(ADRENO_APRIV),
> .fw = {
> [ADRENO_FW_SQE] = "a650_sqe.fw",
> [ADRENO_FW_GMU] = "a650_gmu.bin",
> @@ -289,6 +290,7 @@ static const struct adreno_info gpulist[] = {
> .rev = ADRENO_REV(6, 6, 0, ANY_ID),
> .revn = 660,
> .name = "A660",
> + .features = ADRENO_APRIV,
> .fw = {
> [ADRENO_FW_SQE] = "a660_sqe.fw",
> [ADRENO_FW_GMU] = "a660_gmu.bin",
> @@ -301,6 +303,7 @@ static const struct adreno_info gpulist[] = {
> }, {
> .rev = ADRENO_REV(6, 3, 5, ANY_ID),
> .name = "Adreno 7c Gen 3",
> + .features = ADRENO_APRIV,
> .fw = {
> [ADRENO_FW_SQE] = "a660_sqe.fw",
> [ADRENO_FW_GMU] = "a660_gmu.bin",
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 9f5a302..e8acadf5 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -945,6 +945,9 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
> pm_runtime_use_autosuspend(dev);
> pm_runtime_enable(dev);
>
> + if (ADRENO_FEAT(adreno_gpu, ADRENO_APRIV))
> + adreno_gpu->base.hw_apriv = true;
> +
> return msm_gpu_init(drm, pdev, &adreno_gpu->base, &funcs->base,
> adreno_gpu->info->name, &adreno_gpu_config);
> }
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index 50b4d53..61797c3 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -35,6 +35,11 @@ enum adreno_quirks {
> ADRENO_QUIRK_LMLOADKILL_DISABLE = 3,
> };
>
> +enum adreno_features {
> + /* ADRENO has HW APRIV feature */
> + ADRENO_APRIV,
> +};
> +
> struct adreno_rev {
> uint8_t core;
> uint8_t major;
> @@ -63,6 +68,7 @@ struct adreno_info {
> struct adreno_rev rev;
> uint32_t revn;
> const char *name;
> + u32 features;
> const char *fw[ADRENO_FW_MAX];
> uint32_t gmem;
> enum adreno_quirks quirks;
> @@ -388,6 +394,9 @@ static inline uint32_t get_wptr(struct msm_ringbuffer *ring)
> return (ring->cur - ring->start) % (MSM_GPU_RINGBUFFER_SZ >> 2);
> }
>
> +#define ADRENO_FEAT(adreno_gpu, feature) \
> + (adreno_gpu->info->features & (1 << feature))
And also use BIT() here
But I suppose we could also do something like:
#define ADRENO_FEAT(feature) BIT(ADRENO_ ## feature)
#define ADRENO_HAS_FEAT(adreno_gpu, feature) \
((adreno_gpu)->info->features & ADRENO_FEAT(feature))
and then in the gpulist table:
.features = ADRENO_FEAT(APRIV) | ADRENO_FEAT(FOO) | ADRENO_FEAT(BAR)
that way there is no confusion about whether or not to use BIT()
Otherwise, I like the idea.
BR,
-R
> +
> /*
> * Given a register and a count, return a value to program into
> * REG_CP_PROTECT_REG(n) - this will block both reads and writes for _len
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation.
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] drm/msm: Introduce Adreno Features
2021-07-29 15:56 ` Rob Clark
@ 2021-07-29 18:45 ` Akhil P Oommen
0 siblings, 0 replies; 3+ messages in thread
From: Akhil P Oommen @ 2021-07-29 18:45 UTC (permalink / raw)
To: Rob Clark
Cc: Sean Paul, Sai Prakash Ranjan, Jonathan Marek, David Airlie,
linux-arm-msm, Sharat Masetty, Konrad Dybcio, Douglas Anderson,
dri-devel, Jordan Crouse, Matthias Kaehlcke, Iskren Chernev,
AngeloGioacchino Del Regno, freedreno, Linux Kernel Mailing List
On 7/29/2021 9:26 PM, Rob Clark wrote:
> On Thu, Jul 29, 2021 at 8:31 AM Akhil P Oommen <akhilpo@codeaurora.org> wrote:
>>
>> Introduce a feature flag in gpulist to easily identify the capabilities
>> of each gpu revision. This will help to avoid a lot of adreno_is_axxx()
>> check when we add new features. In the current patch, HW APRIV feature
>> is converted to a feature flag.
>>
>> Signed-off-by: Akhil P Oommen <akhilpo@codeaurora.org>
>> ---
>> This patch is rebased on top of the below series:
>> https://patchwork.freedesktop.org/series/93192/
>>
>> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 12 ------------
>> drivers/gpu/drm/msm/adreno/adreno_device.c | 3 +++
>> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 3 +++
>> drivers/gpu/drm/msm/adreno/adreno_gpu.h | 9 +++++++++
>> 4 files changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> index 1881e09..b28305b 100644
>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
>> @@ -1765,7 +1765,6 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
>> struct msm_drm_private *priv = dev->dev_private;
>> struct platform_device *pdev = priv->gpu_pdev;
>> struct adreno_platform_config *config = pdev->dev.platform_data;
>> - const struct adreno_info *info;
>> struct device_node *node;
>> struct a6xx_gpu *a6xx_gpu;
>> struct adreno_gpu *adreno_gpu;
>> @@ -1781,17 +1780,6 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
>>
>> adreno_gpu->registers = NULL;
>>
>> - /*
>> - * We need to know the platform type before calling into adreno_gpu_init
>> - * so that the hw_apriv flag can be correctly set. Snoop into the info
>> - * and grab the revision number
>> - */
>> - info = adreno_info(config->rev);
>> -
>> - if (info && (info->revn == 650 || info->revn == 660 ||
>> - adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), info->rev)))
>> - adreno_gpu->base.hw_apriv = true;
>> -
>> a6xx_llc_slices_init(pdev, a6xx_gpu);
>>
>> ret = a6xx_set_supported_hw(&pdev->dev, config->rev);
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> index 7b9d605..44321ec 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
>> @@ -276,6 +276,7 @@ static const struct adreno_info gpulist[] = {
>> .rev = ADRENO_REV(6, 5, 0, ANY_ID),
>> .revn = 650,
>> .name = "A650",
>> + .features = ADRENO_APRIV,
>
> I guess this should be:
>
> .features = BIT(ADRENO_APRIV),
D'oh!
>
>> .fw = {
>> [ADRENO_FW_SQE] = "a650_sqe.fw",
>> [ADRENO_FW_GMU] = "a650_gmu.bin",
>> @@ -289,6 +290,7 @@ static const struct adreno_info gpulist[] = {
>> .rev = ADRENO_REV(6, 6, 0, ANY_ID),
>> .revn = 660,
>> .name = "A660",
>> + .features = ADRENO_APRIV,
>> .fw = {
>> [ADRENO_FW_SQE] = "a660_sqe.fw",
>> [ADRENO_FW_GMU] = "a660_gmu.bin",
>> @@ -301,6 +303,7 @@ static const struct adreno_info gpulist[] = {
>> }, {
>> .rev = ADRENO_REV(6, 3, 5, ANY_ID),
>> .name = "Adreno 7c Gen 3",
>> + .features = ADRENO_APRIV,
>> .fw = {
>> [ADRENO_FW_SQE] = "a660_sqe.fw",
>> [ADRENO_FW_GMU] = "a660_gmu.bin",
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
>> index 9f5a302..e8acadf5 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
>> @@ -945,6 +945,9 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>> pm_runtime_use_autosuspend(dev);
>> pm_runtime_enable(dev);
>>
>> + if (ADRENO_FEAT(adreno_gpu, ADRENO_APRIV))
>> + adreno_gpu->base.hw_apriv = true;
>> +
>> return msm_gpu_init(drm, pdev, &adreno_gpu->base, &funcs->base,
>> adreno_gpu->info->name, &adreno_gpu_config);
>> }
>> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
>> index 50b4d53..61797c3 100644
>> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
>> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
>> @@ -35,6 +35,11 @@ enum adreno_quirks {
>> ADRENO_QUIRK_LMLOADKILL_DISABLE = 3,
>> };
>>
>> +enum adreno_features {
>> + /* ADRENO has HW APRIV feature */
>> + ADRENO_APRIV,
>> +};
>> +
>> struct adreno_rev {
>> uint8_t core;
>> uint8_t major;
>> @@ -63,6 +68,7 @@ struct adreno_info {
>> struct adreno_rev rev;
>> uint32_t revn;
>> const char *name;
>> + u32 features;
>> const char *fw[ADRENO_FW_MAX];
>> uint32_t gmem;
>> enum adreno_quirks quirks;
>> @@ -388,6 +394,9 @@ static inline uint32_t get_wptr(struct msm_ringbuffer *ring)
>> return (ring->cur - ring->start) % (MSM_GPU_RINGBUFFER_SZ >> 2);
>> }
>>
>> +#define ADRENO_FEAT(adreno_gpu, feature) \
>> + (adreno_gpu->info->features & (1 << feature))
>
> And also use BIT() here
>
> But I suppose we could also do something like:
>
> #define ADRENO_FEAT(feature) BIT(ADRENO_ ## feature)
> #define ADRENO_HAS_FEAT(adreno_gpu, feature) \
> ((adreno_gpu)->info->features & ADRENO_FEAT(feature))
>
> and then in the gpulist table:
>
> .features = ADRENO_FEAT(APRIV) | ADRENO_FEAT(FOO) | ADRENO_FEAT(BAR)
>
> that way there is no confusion about whether or not to use BIT()
>
> Otherwise, I like the idea.
Sounds good to me.
Thanks for the feedback.
-Akhil.
>
> BR,
> -R
>
>> +
>> /*
>> * Given a register and a count, return a value to program into
>> * REG_CP_PROTECT_REG(n) - this will block both reads and writes for _len
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
>> of Code Aurora Forum, hosted by The Linux Foundation.
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-07-29 18:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-29 15:30 [RFC PATCH] drm/msm: Introduce Adreno Features Akhil P Oommen
2021-07-29 15:56 ` Rob Clark
2021-07-29 18:45 ` Akhil P Oommen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).