| Message ID | 20230118122003.132905-1-wsa+renesas@sang-engineering.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | media: renesas: vsp1: blacklist r8a7795 ES1.* | expand |
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;
> 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).
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
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.
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
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...
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
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
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;
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(+)