The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is part
of the Series6XT, another variation of the Rogue family of GPUs.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/gpu/drm/imagination/pvr_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
index 5c3b2d58d766..3d1a933c8303 100644
--- a/drivers/gpu/drm/imagination/pvr_drv.c
+++ b/drivers/gpu/drm/imagination/pvr_drv.c
@@ -1475,6 +1475,7 @@ pvr_remove(struct platform_device *plat_dev)
 
 static const struct of_device_id dt_match[] = {
 	{ .compatible = "img,img-axe", .data = NULL },
+	{ .compatible = "img,powervr-6xt", .data = NULL },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dt_match);
-- 
2.45.1.288.g0e0cd299f1-goog

On Thu, 2024-05-30 at 16:35 +0800, Chen-Yu Tsai wrote:
> The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is part
> of the Series6XT, another variation of the Rogue family of GPUs.
> 
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
>  drivers/gpu/drm/imagination/pvr_drv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
> index 5c3b2d58d766..3d1a933c8303 100644
> --- a/drivers/gpu/drm/imagination/pvr_drv.c
> +++ b/drivers/gpu/drm/imagination/pvr_drv.c
> @@ -1475,6 +1475,7 @@ pvr_remove(struct platform_device *plat_dev)
>  
>  static const struct of_device_id dt_match[] = {
>  	{ .compatible = "img,img-axe", .data = NULL },
> +	{ .compatible = "img,powervr-6xt", .data = NULL },

I assume that by adding this to the list of supported devices we're essentially
freezing the existing uapi. This concerns me, as we've not yet started running
Vulkan conformance on any Series6XT GPUs and there's a chance we may need to
make some tweaks.

I'm not really sure what the accepted approach is to hardware enablement /
experimental support. I'm not sure if it's sufficient to hide support behind a
Kconfig option and/or module parameter or whether we just have to hold this
patch back for the time being.

Thanks
Frank

>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, dt_match);

在 2024-05-31星期五的 11:18 +0000，Frank Binns写道：
> On Thu, 2024-05-30 at 16:35 +0800, Chen-Yu Tsai wrote:
> > The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is
> > part
> > of the Series6XT, another variation of the Rogue family of GPUs.
> > 
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> >  drivers/gpu/drm/imagination/pvr_drv.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/imagination/pvr_drv.c
> > b/drivers/gpu/drm/imagination/pvr_drv.c
> > index 5c3b2d58d766..3d1a933c8303 100644
> > --- a/drivers/gpu/drm/imagination/pvr_drv.c
> > +++ b/drivers/gpu/drm/imagination/pvr_drv.c
> > @@ -1475,6 +1475,7 @@ pvr_remove(struct platform_device *plat_dev)
> >  
> >  static const struct of_device_id dt_match[] = {
> >         { .compatible = "img,img-axe", .data = NULL },
> > +       { .compatible = "img,powervr-6xt", .data = NULL },
> 
> I assume that by adding this to the list of supported devices we're
> essentially
> freezing the existing uapi. This concerns me, as we've not yet
> started running
> Vulkan conformance on any Series6XT GPUs and there's a chance we may
> need to
> make some tweaks.

Is there anything in the Series 6 XT GPUs that will affect conformance
test and need new ABI to drive? Well I think the GX6250 GPU has TLA
despite AXE (and BXE) has none, but what TLA does seems to be for
transfer jobs, which we already support by using fragment pipeline?

In addition, if we add bits to the ABI, we can recognize the new ABI by
raising the version number returned by the DRM driver.

And, if my understand is right, I think we're keeping the command
stream the same among different GPUs, so if the FWIF is changed, it's
quite possible that every GPU, not only S6XT but also AXE will be
affected too.

> 
> I'm not really sure what the accepted approach is to hardware
> enablement /
> experimental support. I'm not sure if it's sufficient to hide support
> behind a
> Kconfig option and/or module parameter or whether we just have to
> hold this
> patch back for the time being.
> 
> Thanks
> Frank
> 
> >         {}
> >  };
> >  MODULE_DEVICE_TABLE(of, dt_match);

On Fri, May 31, 2024 at 7:18 PM Frank Binns <Frank.Binns@imgtec.com> wrote:
>
> On Thu, 2024-05-30 at 16:35 +0800, Chen-Yu Tsai wrote:
> > The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is part
> > of the Series6XT, another variation of the Rogue family of GPUs.
> >
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> >  drivers/gpu/drm/imagination/pvr_drv.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
> > index 5c3b2d58d766..3d1a933c8303 100644
> > --- a/drivers/gpu/drm/imagination/pvr_drv.c
> > +++ b/drivers/gpu/drm/imagination/pvr_drv.c
> > @@ -1475,6 +1475,7 @@ pvr_remove(struct platform_device *plat_dev)
> >
> >  static const struct of_device_id dt_match[] = {
> >       { .compatible = "img,img-axe", .data = NULL },
> > +     { .compatible = "img,powervr-6xt", .data = NULL },
>
> I assume that by adding this to the list of supported devices we're essentially
> freezing the existing uapi. This concerns me, as we've not yet started running
> Vulkan conformance on any Series6XT GPUs and there's a chance we may need to
> make some tweaks.
>
> I'm not really sure what the accepted approach is to hardware enablement /
> experimental support. I'm not sure if it's sufficient to hide support behind a
> Kconfig option and/or module parameter or whether we just have to hold this
> patch back for the time being.

I guess this is more of a question for the DRM maintainers.
Added a couple Panfrost/Panthor folks for ideas.


ChenYu

> Thanks
> Frank
>
> >       {}
> >  };
> >  MODULE_DEVICE_TABLE(of, dt_match);

On 03/06/2024 04:29, Chen-Yu Tsai wrote:
> On Fri, May 31, 2024 at 7:18 PM Frank Binns <Frank.Binns@imgtec.com> wrote:
>>
>> On Thu, 2024-05-30 at 16:35 +0800, Chen-Yu Tsai wrote:
>>> The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is part
>>> of the Series6XT, another variation of the Rogue family of GPUs.
>>>
>>> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>>> ---
>>>  drivers/gpu/drm/imagination/pvr_drv.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
>>> index 5c3b2d58d766..3d1a933c8303 100644
>>> --- a/drivers/gpu/drm/imagination/pvr_drv.c
>>> +++ b/drivers/gpu/drm/imagination/pvr_drv.c
>>> @@ -1475,6 +1475,7 @@ pvr_remove(struct platform_device *plat_dev)
>>>
>>>  static const struct of_device_id dt_match[] = {
>>>       { .compatible = "img,img-axe", .data = NULL },
>>> +     { .compatible = "img,powervr-6xt", .data = NULL },
>>
>> I assume that by adding this to the list of supported devices we're essentially
>> freezing the existing uapi. This concerns me, as we've not yet started running
>> Vulkan conformance on any Series6XT GPUs and there's a chance we may need to
>> make some tweaks.
>>
>> I'm not really sure what the accepted approach is to hardware enablement /
>> experimental support. I'm not sure if it's sufficient to hide support behind a
>> Kconfig option and/or module parameter or whether we just have to hold this
>> patch back for the time being.
> 
> I guess this is more of a question for the DRM maintainers.
> Added a couple Panfrost/Panthor folks for ideas.

I'm not sure quite what scale of "tweaks" you are expecting. Obviously
adding new uAPI is possible at any time - the only requirement is "don't
break user space" - i.e. don't remove old uAPI. Although obviously you
want to be careful about adding it because that means supporting it
forever more.

Panfrost has had an "unstable_ioctls" module parameter that we've hidden
performance counters behind. (Performance counters are hard from a uAPI
perspective - Panthor has similar issues).

We've also added support for GPUs in a deliberately "crippled" manner
(e.g. only one core group - see panfrost_get_core_mask()). I think we're
mostly just hoping those 'awkward' GPUs are not interesting enough and
we'll never implement full support for them - but if we did I expect
we'd implement support by providing a new uAPI for enabling the second
core group so old user space can continue working with just the single
core group.

Of course if the support for this platform is actually 'broken' (the
talk of GPU resets makes me think so - on Mali requiring a reset is a
"should never happen" situation, but we do have errata...) then it's
probably best holding off merging this until you've got something which
is minimally functional and then add support as necessary. For Vulkan
you can always have user space require a particular DRM kernel version
if you discover extra uAPI is needed.

Steve

Il 30/05/24 10:35, Chen-Yu Tsai ha scritto:
> The MediaTek MT8173 comes with a PowerVR Rogue GX6250, which is part
> of the Series6XT, another variation of the Rogue family of GPUs.
> 
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>