| Message ID | 20230620173305.896438-1-festevam@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/msm/a3xx: Pass the revision information | expand |
On 20/06/2023 20:33, Fabio Estevam wrote: > From: Fabio Estevam <festevam@denx.de> > > Commit cc943f43ece7 ("drm/msm/adreno: warn if chip revn is verified > before being set") exposes the need of setting the GPU revision fields > prior to using the adreno_is_xxx() functions. > > Pass the GPU revision information to avoid run-time warning. > > Signed-off-by: Fabio Estevam <festevam@denx.de> I'll take a glance later. Generic comment, you missed freedreno@, so these patches will not pop up in our patch tracker. Also could you please use git send-email passing all patches to the command, so that they are threaded? > --- > Build-tested only. > > drivers/gpu/drm/msm/adreno/a3xx_gpu.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c > index c86b377f6f0d..fc23810d7684 100644 > --- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c > @@ -530,6 +530,8 @@ struct msm_gpu *a3xx_gpu_init(struct drm_device *dev) > struct msm_gpu *gpu; > 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 icc_path *ocmem_icc_path; > struct icc_path *icc_path; > int ret; > @@ -558,6 +560,25 @@ struct msm_gpu *a3xx_gpu_init(struct drm_device *dev) > if (ret) > goto fail; > > + /* > + * 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) { > + ret = -EINVAL; > + goto fail; > + } > + > + /* Assign these early so that we can use the is_aXYZ helpers */ > + /* Numeric revision IDs (e.g. 630) */ > + adreno_gpu->revn = info->revn; > + /* New-style ADRENO_REV()-only */ > + adreno_gpu->rev = info->rev; > + /* Quirk data */ > + adreno_gpu->info = info; This looks like a boilerplate being added to all aYxx drivers (and then these fields are also set in adreno_gpu_init()). Can we remove duplication somehow? > + > /* if needed, allocate gmem: */ > if (adreno_is_a330(adreno_gpu)) { > ret = adreno_gpu_ocmem_init(&adreno_gpu->base.pdev->dev,
On 20/06/2023 14:40, Dmitry Baryshkov wrote: > This looks like a boilerplate being added to all aYxx drivers (and > then these fields are also set in adreno_gpu_init()). Can we remove > duplication somehow? Sorry, I missed this comment prior to sending v2. Maybe a simpler fix for a2xx_gpu would be: --- a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c @@ -540,6 +540,10 @@ struct msm_gpu *a2xx_gpu_init(struct drm_device *dev) gpu->perfcntrs = perfcntrs; gpu->num_perfcntrs = ARRAY_SIZE(perfcntrs); + ret = adreno_gpu_init(dev, pdev, adreno_gpu, &funcs, 1); + if (ret) + goto fail; + if (adreno_is_a20x(adreno_gpu)) adreno_gpu->registers = a200_registers; else if (adreno_is_a225(adreno_gpu)) @@ -547,10 +551,6 @@ struct msm_gpu *a2xx_gpu_init(struct drm_device *dev) else adreno_gpu->registers = a220_registers; - ret = adreno_gpu_init(dev, pdev, adreno_gpu, &funcs, 1); - if (ret) - goto fail; - if (!gpu->aspace) { dev_err(dev->dev, "No memory protection without MMU\n"); if (!allow_vram_carveout) { What do you think? a3xx and a4xx call adreno_gpu_init() prior to adreno_is_xxx() so they don't have issues.
On Tue, 20 Jun 2023 at 21:14, Fabio Estevam <festevam@denx.de> wrote: > > On 20/06/2023 14:40, Dmitry Baryshkov wrote: > > > This looks like a boilerplate being added to all aYxx drivers (and > > then these fields are also set in adreno_gpu_init()). Can we remove > > duplication somehow? > > Sorry, I missed this comment prior to sending v2. > > Maybe a simpler fix for a2xx_gpu would be: > > --- a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c > @@ -540,6 +540,10 @@ struct msm_gpu *a2xx_gpu_init(struct drm_device > *dev) > gpu->perfcntrs = perfcntrs; > gpu->num_perfcntrs = ARRAY_SIZE(perfcntrs); > > + ret = adreno_gpu_init(dev, pdev, adreno_gpu, &funcs, 1); > + if (ret) > + goto fail; > + > if (adreno_is_a20x(adreno_gpu)) > adreno_gpu->registers = a200_registers; > else if (adreno_is_a225(adreno_gpu)) > @@ -547,10 +551,6 @@ struct msm_gpu *a2xx_gpu_init(struct drm_device > *dev) > else > adreno_gpu->registers = a220_registers; > > - ret = adreno_gpu_init(dev, pdev, adreno_gpu, &funcs, 1); > - if (ret) > - goto fail; > - > if (!gpu->aspace) { > dev_err(dev->dev, "No memory protection without MMU\n"); > if (!allow_vram_carveout) { > > What do you think? > > a3xx and a4xx call adreno_gpu_init() prior to adreno_is_xxx() so they > don't have issues. Yes, this seems like a perfect solution. Please send it with the proper commit message. Also please add: Fixes: 21af872cd8c6 ("drm/msm/adreno: add a2xx")
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c index c86b377f6f0d..fc23810d7684 100644 --- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c +++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c @@ -530,6 +530,8 @@ struct msm_gpu *a3xx_gpu_init(struct drm_device *dev) struct msm_gpu *gpu; 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 icc_path *ocmem_icc_path; struct icc_path *icc_path; int ret; @@ -558,6 +560,25 @@ struct msm_gpu *a3xx_gpu_init(struct drm_device *dev) if (ret) goto fail; + /* + * 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) { + ret = -EINVAL; + goto fail; + } + + /* Assign these early so that we can use the is_aXYZ helpers */ + /* Numeric revision IDs (e.g. 630) */ + adreno_gpu->revn = info->revn; + /* New-style ADRENO_REV()-only */ + adreno_gpu->rev = info->rev; + /* Quirk data */ + adreno_gpu->info = info; + /* if needed, allocate gmem: */ if (adreno_is_a330(adreno_gpu)) { ret = adreno_gpu_ocmem_init(&adreno_gpu->base.pdev->dev,