From: "Heiko Stübner" <heiko@sntech.de>
To: Sebastian Reichel <sebastian.reichel@collabora.com>
Cc: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
Philipp Zabel <p.zabel@pengutronix.de>,
Nicolas Frattaroli <frattaroli.nicolas@gmail.com>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Jianfeng Liu <liujianfeng1994@gmail.com>,
Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>,
Nicolas Dufresne <nicolas.dufresne@collabora.com>,
linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel@collabora.com
Subject: Re: [PATCH v5 3/5] media: hantro: Add RK3588 VEPU121 support
Date: Thu, 13 Jun 2024 10:01:09 +0200 [thread overview]
Message-ID: <14943967.O6BkTfRZtg@diego> (raw)
In-Reply-To: <g3zzuwv2pti6oudc5xlab3cdjpqbhjsdxdsphzch4mtcaisqxl@iwkxgillwi2f>
Am Donnerstag, 13. Juni 2024, 00:44:38 CEST schrieb Sebastian Reichel:
> Hi,
>
> On Wed, Jun 12, 2024 at 08:08:51PM GMT, Heiko Stübner wrote:
> > Am Mittwoch, 12. Juni 2024, 19:15:43 CEST schrieb Sebastian Reichel:
> > > Avoid exposing each of the 4 Hantro H1 cores separately to userspace.
> > > For now just expose the first one.
> > >
> > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > > ---
> > > .../media/platform/verisilicon/hantro_drv.c | 38 +++++++++++++++++++
> > > 1 file changed, 38 insertions(+)
> > >
> > > diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> > > index 34b123dafd89..b722a20c5fe3 100644
> > > --- a/drivers/media/platform/verisilicon/hantro_drv.c
> > > +++ b/drivers/media/platform/verisilicon/hantro_drv.c
> > > @@ -722,6 +722,7 @@ static const struct of_device_id of_hantro_match[] = {
> > > { .compatible = "rockchip,rk3399-vpu", .data = &rk3399_vpu_variant, },
> > > { .compatible = "rockchip,rk3568-vepu", .data = &rk3568_vepu_variant, },
> > > { .compatible = "rockchip,rk3568-vpu", .data = &rk3568_vpu_variant, },
> > > + { .compatible = "rockchip,rk3588-vepu121", .data = &rk3568_vpu_variant, },
> > > { .compatible = "rockchip,rk3588-av1-vpu", .data = &rk3588_vpu981_variant, },
> > > #endif
> > > #ifdef CONFIG_VIDEO_HANTRO_IMX8M
> > > @@ -992,6 +993,39 @@ static const struct media_device_ops hantro_m2m_media_ops = {
> > > .req_queue = v4l2_m2m_request_queue,
> > > };
> > >
> > > +/*
> > > + * Some SoCs, like RK3588 have multiple identical Hantro cores, but the
> > > + * kernel is currently missing support for multi-core handling. Exposing
> > > + * separate devices for each core to userspace is bad, since that does
> > > + * not allow scheduling tasks properly (and creates ABI). With this workaround
> > > + * the driver will only probe for the first core and early exit for the other
> > > + * cores. Once the driver gains multi-core support, the same technique
> > > + * for detecting the main core can be used to cluster all cores together.
> > > + */
> > > +static int hantro_disable_multicore(struct hantro_dev *vpu)
> > > +{
> > > + const char *compatible;
> > > + struct device_node *node;
> > > + int ret;
> > > +
> > > + /* Intentionally ignores the fallback strings */
> > > + ret = of_property_read_string(vpu->dev->of_node, "compatible", &compatible);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + /* first compatible node found from the root node is considered the main core */
> > > + node = of_find_compatible_node(NULL, NULL, compatible);
> > > + if (!node)
> > > + return -EINVAL; /* broken DT? */
> > > +
> > > + if (vpu->dev->of_node != node) {
> > > + dev_info(vpu->dev, "missing multi-core support, ignoring this instance\n");
> > > + return -ENODEV;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static int hantro_probe(struct platform_device *pdev)
> > > {
> > > const struct of_device_id *match;
> > > @@ -1011,6 +1045,10 @@ static int hantro_probe(struct platform_device *pdev)
> > > match = of_match_node(of_hantro_match, pdev->dev.of_node);
> > > vpu->variant = match->data;
> > >
> > > + ret = hantro_disable_multicore(vpu);
> > > + if (ret)
> > > + return ret;
> > > +
> >
> > I think this might be better as two patches?
> >
> > As this patch stands, the disable-multicore handling is done for _all_
> > hantro variants, so part of me wants this to be labeled as such.
> >
> > The whole reasoning is completely ok, but somehow having this under
> > the "add rk3588" umbrella feels strange ;-)
>
> I can do that, but the 'rockchip,rk3588-vepu121' part is only needed
> because of the multicore handling. If the kernel already had this bit
> in the past, the RK3568 compatible could be used for RK3588 (as a
> fallback compatible), just like for VPU121.
I meant, you're doing hantro_disable_multicore() here also for everyone
else (i.MX etc), hence I'd like that to be a separate commit in this
series like:
----- 8< ------
media: hantro: Disable multi-core handling for the time being
The VSI doc for the Hantro codec describes the grouping of up to 4 instances.
The kernel currently doesn't handle multi-core processing .... foo bar ....
----- 8< ------
And then add rk3588 support on top of that.
Heiko
next prev parent reply other threads:[~2024-06-13 8:01 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-12 17:15 [PATCH v5 0/5] RK3588 VEPU121/VPU121 support Sebastian Reichel
2024-06-12 17:15 ` [PATCH v5 1/5] media: dt-bindings: rk3568-vepu: Add RK3588 VEPU121 Sebastian Reichel
2024-06-12 18:26 ` Rob Herring (Arm)
2024-06-12 22:20 ` Sebastian Reichel
2024-06-13 6:23 ` Krzysztof Kozlowski
2024-06-12 17:15 ` [PATCH v5 2/5] media: dt-bindings: rockchip-vpu: Add RK3588 VPU121 Sebastian Reichel
2024-06-12 17:15 ` [PATCH v5 3/5] media: hantro: Add RK3588 VEPU121 support Sebastian Reichel
2024-06-12 18:08 ` Heiko Stübner
2024-06-12 22:44 ` Sebastian Reichel
2024-06-13 8:01 ` Heiko Stübner [this message]
2024-06-12 17:15 ` [PATCH v5 4/5] arm64: dts: rockchip: Add VEPU121 to RK3588 Sebastian Reichel
2024-06-12 20:20 ` Nicolas Dufresne
2024-06-12 22:48 ` Sebastian Reichel
2024-06-12 17:15 ` [PATCH v5 5/5] arm64: dts: rockchip: Add VPU121 support for RK3588 Sebastian Reichel
2024-06-12 18:10 ` Diederik de Haas
2024-06-12 22:50 ` Sebastian Reichel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=14943967.O6BkTfRZtg@diego \
--to=heiko@sntech.de \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=ezequiel@vanguardiasur.com.ar \
--cc=frattaroli.nicolas@gmail.com \
--cc=kernel@collabora.com \
--cc=krzk+dt@kernel.org \
--cc=linkmauve@linkmauve.fr \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=liujianfeng1994@gmail.com \
--cc=nicolas.dufresne@collabora.com \
--cc=p.zabel@pengutronix.de \
--cc=robh@kernel.org \
--cc=sebastian.reichel@collabora.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).