[PATCH] media: renesas: vsp1: blacklist r8a7795 ES1.*

[PATCH] media: renesas: vsp1: blacklist r8a7795 ES1.*

[Wolfram Sang]

The earliest revision of these SoC may hang when underrunning. Later
revisions have that fixed. Bail out when we detect a problematic
version.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

The BSP tries to work around the issue, yet this is neither upstreamable
nor are we sure the solution is complete. Because the early SoC revision
is hardly in use, we simply "document" the problem upstream.

 drivers/media/platform/renesas/vsp1/vsp1_drv.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
index c260d318d298..b395e8eb0af9 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
@@ -17,6 +17,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/reset.h>
+#include <linux/sys_soc.h>
 #include <linux/videodev2.h>
 
 #include <media/rcar-fcp.h>
@@ -875,13 +876,24 @@ static const struct vsp1_device_info *vsp1_lookup_info(struct vsp1_device *vsp1)
 	return NULL;
 }
 
+static const struct soc_device_attribute vsp1_blacklist[]  = {
+	/* H3 ES1.* has underrun hang issues */
+	{ .soc_id = "r8a7795", .revision = "ES1.*" },
+	{ /* Sentinel */ }
+};
+
 static int vsp1_probe(struct platform_device *pdev)
 {
+	const struct soc_device_attribute *attr;
 	struct vsp1_device *vsp1;
 	struct device_node *fcp_node;
 	int ret;
 	int irq;
 
+	attr = soc_device_match(vsp1_blacklist);
+	if (attr)
+		return -ENOTSUPP;
+
 	vsp1 = devm_kzalloc(&pdev->dev, sizeof(*vsp1), GFP_KERNEL);
 	if (vsp1 == NULL)
 		return -ENOMEM;
-- 
2.30.2

Re: [PATCH] media: renesas: vsp1: blacklist r8a7795 ES1.*

[Laurent Pinchart]

Hi Wolfram,

Thank you for the patch.

On Wed, Jan 18, 2023 at 01:20:02PM +0100, Wolfram Sang wrote:
> The earliest revision of these SoC may hang when underrunning. Later
> revisions have that fixed. Bail out when we detect a problematic
> version.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> The BSP tries to work around the issue, yet this is neither upstreamable
> nor are we sure the solution is complete. Because the early SoC revision
> is hardly in use, we simply "document" the problem upstream.

The workaround isn't upstreamable as-is, but I think it could be
upstreamed after being cleaned up.

Overall, how much support do we still have upstream for H3 ES1.x, and do
we need to keep it ? H3 ES.1x is relatively old, does someone still rely
on it ?

>  drivers/media/platform/renesas/vsp1/vsp1_drv.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drv.c b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> index c260d318d298..b395e8eb0af9 100644
> --- a/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_drv.c
> @@ -17,6 +17,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/reset.h>
> +#include <linux/sys_soc.h>
>  #include <linux/videodev2.h>
>  
>  #include <media/rcar-fcp.h>
> @@ -875,13 +876,24 @@ static const struct vsp1_device_info *vsp1_lookup_info(struct vsp1_device *vsp1)
>  	return NULL;
>  }
>  
> +static const struct soc_device_attribute vsp1_blacklist[]  = {
> +	/* H3 ES1.* has underrun hang issues */
> +	{ .soc_id = "r8a7795", .revision = "ES1.*" },
> +	{ /* Sentinel */ }
> +};
> +
>  static int vsp1_probe(struct platform_device *pdev)
>  {
> +	const struct soc_device_attribute *attr;
>  	struct vsp1_device *vsp1;
>  	struct device_node *fcp_node;
>  	int ret;
>  	int irq;
>  
> +	attr = soc_device_match(vsp1_blacklist);
> +	if (attr)
> +		return -ENOTSUPP;
> +
>  	vsp1 = devm_kzalloc(&pdev->dev, sizeof(*vsp1), GFP_KERNEL);
>  	if (vsp1 == NULL)
>  		return -ENOMEM;

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: renesas: vsp1: blacklist r8a7795 ES1.*

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 574 bytes --]


> Overall, how much support do we still have upstream for H3 ES1.x, and do
> we need to keep it ? H3 ES.1x is relatively old, does someone still rely
> on it ?

SDHI also had numerous problems with H3 ES1. The agreement was: If it is
"easily" upstreamable, then we want to support ES1 as much as possible.
If it is complex and a maintenance burden, then we don't upstream it.
This is why ES1 has no HS400 support for eMMC. It would have involved
activating the SDHI sequencer which we don't use otherwise.

I copied this behaviour and think it makes sense (for upstream).


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] media: renesas: vsp1: blacklist r8a7795 ES1.*

[Geert Uytterhoeven]

Hi Laurent,

On Wed, Jan 18, 2023 at 2:21 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Wed, Jan 18, 2023 at 01:20:02PM +0100, Wolfram Sang wrote:
> > The earliest revision of these SoC may hang when underrunning. Later
> > revisions have that fixed. Bail out when we detect a problematic
> > version.
> >
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> >
> > The BSP tries to work around the issue, yet this is neither upstreamable
> > nor are we sure the solution is complete. Because the early SoC revision
> > is hardly in use, we simply "document" the problem upstream.
>
> The workaround isn't upstreamable as-is, but I think it could be
> upstreamed after being cleaned up.
>
> Overall, how much support do we still have upstream for H3 ES1.x, and do
> we need to keep it ? H3 ES.1x is relatively old, does someone still rely
> on it ?

I think the upstream support level for R-Car H3 ES1.x is about the same
as for H3 ES2.0.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] media: renesas: vsp1: blacklist r8a7795 ES1.*

[Laurent Pinchart]

Hi Geert, Wolfram,

On Wed, Jan 18, 2023 at 02:30:51PM +0100, Geert Uytterhoeven wrote:
> On Wed, Jan 18, 2023 at 2:21 PM Laurent Pinchart wrote:
> > On Wed, Jan 18, 2023 at 01:20:02PM +0100, Wolfram Sang wrote:
> > > The earliest revision of these SoC may hang when underrunning. Later
> > > revisions have that fixed. Bail out when we detect a problematic
> > > version.
> > >
> > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > ---
> > >
> > > The BSP tries to work around the issue, yet this is neither upstreamable
> > > nor are we sure the solution is complete. Because the early SoC revision
> > > is hardly in use, we simply "document" the problem upstream.
> >
> > The workaround isn't upstreamable as-is, but I think it could be
> > upstreamed after being cleaned up.
> >
> > Overall, how much support do we still have upstream for H3 ES1.x, and do
> > we need to keep it ? H3 ES.1x is relatively old, does someone still rely
> > on it ?
> 
> I think the upstream support level for R-Car H3 ES1.x is about the same
> as for H3 ES2.0.

Question is, do we need to keep it ? :-) And if we do, instead of
black-listing devices in the VSP driver, how about dropping them from
r8a77950.dtsi ? We already delete quite a lot of devices there.

Note that without VSP support, you will get no display either, so the
DU device (and the LVDS encoder) so also be deleted.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: renesas: vsp1: blacklist r8a7795 ES1.*

[Geert Uytterhoeven]

Hi Laurent,

On Wed, Jan 18, 2023 at 2:39 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Wed, Jan 18, 2023 at 02:30:51PM +0100, Geert Uytterhoeven wrote:
> > On Wed, Jan 18, 2023 at 2:21 PM Laurent Pinchart wrote:
> > > On Wed, Jan 18, 2023 at 01:20:02PM +0100, Wolfram Sang wrote:
> > > > The earliest revision of these SoC may hang when underrunning. Later
> > > > revisions have that fixed. Bail out when we detect a problematic
> > > > version.
> > > >
> > > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > > ---
> > > >
> > > > The BSP tries to work around the issue, yet this is neither upstreamable
> > > > nor are we sure the solution is complete. Because the early SoC revision
> > > > is hardly in use, we simply "document" the problem upstream.
> > >
> > > The workaround isn't upstreamable as-is, but I think it could be
> > > upstreamed after being cleaned up.
> > >
> > > Overall, how much support do we still have upstream for H3 ES1.x, and do
> > > we need to keep it ? H3 ES.1x is relatively old, does someone still rely
> > > on it ?
> >
> > I think the upstream support level for R-Car H3 ES1.x is about the same
> > as for H3 ES2.0.
>
> Question is, do we need to keep it ? :-) And if we do, instead of
> black-listing devices in the VSP driver, how about dropping them from
> r8a77950.dtsi ?

I prefer blacklisting in the driver, as dropping them from r8a77950.dtsi
wouldn't disable them when used with an older or out-of-tree DTB.

>  We already delete quite a lot of devices there.

... because they don't exist on H3 ES1.x.

> Note that without VSP support, you will get no display either, so the
> DU device (and the LVDS encoder) so also be deleted.

True...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] media: renesas: vsp1: blacklist r8a7795 ES1.*

[Laurent Pinchart]

Hi Geert,

On Wed, Jan 18, 2023 at 03:02:53PM +0100, Geert Uytterhoeven wrote:
> On Wed, Jan 18, 2023 at 2:39 PM Laurent Pinchart wrote:
> > On Wed, Jan 18, 2023 at 02:30:51PM +0100, Geert Uytterhoeven wrote:
> > > On Wed, Jan 18, 2023 at 2:21 PM Laurent Pinchart wrote:
> > > > On Wed, Jan 18, 2023 at 01:20:02PM +0100, Wolfram Sang wrote:
> > > > > The earliest revision of these SoC may hang when underrunning. Later
> > > > > revisions have that fixed. Bail out when we detect a problematic
> > > > > version.
> > > > >
> > > > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > > > ---
> > > > >
> > > > > The BSP tries to work around the issue, yet this is neither upstreamable
> > > > > nor are we sure the solution is complete. Because the early SoC revision
> > > > > is hardly in use, we simply "document" the problem upstream.
> > > >
> > > > The workaround isn't upstreamable as-is, but I think it could be
> > > > upstreamed after being cleaned up.
> > > >
> > > > Overall, how much support do we still have upstream for H3 ES1.x, and do
> > > > we need to keep it ? H3 ES.1x is relatively old, does someone still rely
> > > > on it ?
> > >
> > > I think the upstream support level for R-Car H3 ES1.x is about the same
> > > as for H3 ES2.0.
> >
> > Question is, do we need to keep it ? :-) And if we do, instead of
> > black-listing devices in the VSP driver, how about dropping them from
> > r8a77950.dtsi ?
> 
> I prefer blacklisting in the driver, as dropping them from r8a77950.dtsi
> wouldn't disable them when used with an older or out-of-tree DTB.

Is that really a use case we need to care about ? Who will run a recent
kernel with an old DTB on a H3 ES1.x, without an easy way to update to a
mainline device tree ? It's not like those devices went into production.

> >  We already delete quite a lot of devices there.
> 
> ... because they don't exist on H3 ES1.x.
> 
> > Note that without VSP support, you will get no display either, so the
> > DU device (and the LVDS encoder) so also be deleted.
> 
> True...

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: renesas: vsp1: blacklist r8a7795 ES1.*

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 647 bytes --]

Hi!

> > I prefer blacklisting in the driver, as dropping them from r8a77950.dtsi
> > wouldn't disable them when used with an older or out-of-tree DTB.
> 
> Is that really a use case we need to care about ? Who will run a recent
> kernel with an old DTB on a H3 ES1.x, without an easy way to update to a
> mainline device tree ? It's not like those devices went into production.

There's some agreement that DTBs are an ABI, and that they should work
with old and new kernels. Disabling it in the driver seems like right
solution.

Best regards,
								Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] media: renesas: vsp1: blacklist r8a7795 ES1.*

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

Hi Pavel,

> There's some agreement that DTBs are an ABI, and that they should work
> with old and new kernels. Disabling it in the driver seems like right
> solution.

We agreed to remove support for this specific SoC entirely from the
kernel. With this merge window, it won't boot anymore. It was for
internal development only anyhow. So, instead of adding new quirks for
it, I will remove all existing ones once rc1 is released. So, this patch
can be dropped.

Thank you for your review,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]