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

[Krzysztof Kozlowski]

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