Re: [PATCH v2 2/4] media: rockchip: Introduce the rkvdec2 driver

From: Krzysztof Kozlowski <krzk@kernel.org>
To: Sebastian Reichel <sebastian.reichel@collabora.com>
Cc: Detlev Casanova <detlev.casanova@collabora.com>,
	linux-kernel@vger.kernel.org,
	Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Heiko Stuebner <heiko@sntech.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Dragan Simic <dsimic@manjaro.org>,
	Diederik de Haas <didi.debian@cknow.org>,
	Andy Yan <andy.yan@rock-chips.com>,
	Boris Brezillon <boris.brezillon@collabora.com>,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	Daniel Almeida <daniel.almeida@collabora.com>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	Nicolas Dufresne <nicolas.dufresne@collabora.com>,
	Benjamin Gaignard <benjamin.gaignard@collabora.com>,
	Jonas Karlman <jonas@kwiboo.se>, Alex Bee <knaerzche@gmail.com>,
	linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-staging@lists.linux.dev
Subject: Re: [PATCH v2 2/4] media: rockchip: Introduce the rkvdec2 driver
Date: Fri, 21 Jun 2024 08:10:26 +0200	[thread overview]
Message-ID: <3acc5a0a-260a-4958-aab2-1d473c835a1b@kernel.org> (raw)
In-Reply-To: <klzru2ak3yxccywqr57bslyctqtmrl2d3eghgv5letp7jkel6s@rlupljmxfijg>

On 20/06/2024 15:41, Sebastian Reichel wrote:
> Hi,
> 
> On Thu, Jun 20, 2024 at 12:30:00PM GMT, Krzysztof Kozlowski wrote:
>> On 19/06/2024 16:57, Detlev Casanova wrote:
>>> +static const char * const rkvdec2_clk_names[] = {
>>> +	"axi",
>>> +	"ahb",
>>> +	"core",
>>> +	"cabac",
>>> +	"hevc_cabac",
>>> +};
>>> +
>>> +/*
>>> + * Some SoCs, like RK3588 have multiple identical vdpu34x 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 rkvdec2_disable_multicore(struct rkvdec2_dev *rkvdec)
>>> +{
>>> +	const char *compatible;
>>> +	struct device_node *node;
>>> +	int ret;
>>> +
>>> +	/* Intentionally ignores the fallback strings */
>>> +	ret = of_property_read_string(rkvdec->dev->of_node, "compatible", &compatible);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	/* first compatible node found from the root node is considered the main core */
>>
>> So you rely on order of nodes? Before you claim "identical cores", but
>> now "main core" suggests one is different than others.
> 
> Heh, I wrote that comment for Hantro. By main core I was referencing
> the software side of things. With a number of equal cores and no DT
> node describing a cluster (from HW point of view it's just equal
> cores), they somehow need to be combined into a single entity to allow
> scheduling work between them. This solves the issue by making one of
> the devices the "main" device. From the HW point it's exactly the same
> as the others. The function could also use the last core or the second
> one. It does not matter as long as there is only one "main" core.

Sounds good. Maybe comment could be a bit extended with this
explanation, so the term "main" is clarified.

Best regards,
Krzysztof

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip