Re: [PATCH v4 10/18] soc: imx: add i.MX8M blk-ctrl driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Le 05/10/2021 à 12:03, Lucas Stach a écrit :
Hi Benjamin,

Am Montag, dem 04.10.2021 um 16:27 +0200 schrieb Benjamin Gaignard:
Le 02/10/2021 à 03:07, Lucas Stach a écrit :
Hi Benjamin,

Am Dienstag, dem 14.09.2021 um 17:46 +0200 schrieb Benjamin Gaignard:
Le 10/09/2021 à 22:26, Lucas Stach a écrit :
This adds a driver for the blk-ctrl blocks found in the i.MX8M* line of
SoCs. The blk-ctrl is a top-level peripheral located in the various *MIX
power domains and interacts with the GPC power controller to provide the
peripherals in the power domain access to the NoC and ensures that those
peripherals are properly reset when their respective power domain is
brought back to life.

Software needs to do different things to make the bus handshake happen
after the GPC *MIX domain is powered up and before it is powered down.
As the requirements are quite different between the various blk-ctrls
there is a callback function provided to hook in the proper sequence.

The peripheral domains are quite uniform, they handle the soft clock
enables and resets in the blk-ctrl address space and sequencing with the
upstream GPC power domains.
Hi Lucas,

I have tried to use your patches for IMX8MQ but it seems that the hardware
have different architecture.
On IMX8MQ there is only one VPU domain for G1 and G2 and that doesn't match
with your implementation where it is needed to have "bus" and devices power domain.
   From what I experiment in current IMX8MQ implementation of blk-ctrl (inside VPU driver)
enabling the 3 clocks (bus, G1, G2) is needed to reset the VPUs.

Do you think you can update your design to take care of these hardware variations ?
The clocking/reset of the blk-ctrl and ADB in the i.MX8MQ VPU power
domain is really a bit strange, as the ADB reset is tied to the VPU
resets and the clk-ctrl seem to require all 3 VPU clocks, instead of
only the bus clock as in newer designs. However I was able to make it
work with the existing blk-ctrl driver design.

My current WIP patches (only tested with the G1 core so far) on top of
the v5 of the series I just sent out can be found here:
https://git.pengutronix.de/cgit/lst/linux/log/?h=imx8mq-vpu-blk-ctrl

Hope this helps.
Hi Lucas,

I have been able to test your branch on my iMX8MQ.
I confirm that G1 is working fine, I able to decode H264 files.

I wasn't able to make G2 works, I think it is coming from the reset sequence
done before each frame decoding in G2 driver.
I have change imx8mq_runtime_resume() and  imx8m_vpu_reset()
to call pm_runtime_put() and pm_runtime_get() to perform a reset like.
I think you need to use the _sync variants of those functions to make
sure the domain is going through the reset state. However that seems
like a pretty heavy-weight thing to do if the decoder really requires a
reset before each frame. Excuse my ignorance about the G2 block, but
this sounds like a quite odd requirement. Is this to work around some
hardware erratum?

When using _sync variants kernel stop when probing G2.
I don't have any documentation about the link between the control block and
G2. I have done lot of tests and resetting the hardware block between each frame
seems to be mandatory. That also the case in Hantro proprietary stack.


If we really need to reset the G2 before each frame, I think it would
be best to also expose a reset controller from the blk-ctrl driver, to
allow the G2 driver to do a light-weight reset, instead of doing this
runtime PM transition.

I do agree and I have already done a attempt in this way:
https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=440031&state=%2A&archive=both
but Philipp have fairly argue that clocks enabling is not the job of a reset driver.

Thanks for the time you spend on this topic.

Regards,
Benjamin


Regards,
Lucas

Without that G2 hangs when decoding the first frame.

One G1 it seems that doing a reset before each frame decoding is not needed.

On DT I had to assignee G1 and G2 on the both nodes to avoid a warning at probe time.
assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>,
					  <&clk IMX8MQ_CLK_VPU_G2>,
					  <&clk IMX8MQ_VPU_PLL_BYPASS>;
			assigned-clock-parents = <&clk IMX8MQ_VPU_PLL_OUT>,
						 <&clk IMX8MQ_VPU_PLL_OUT>,
						 <&clk IMX8MQ_VPU_PLL>;
			assigned-clock-rates = <600000000>, <300000000>, <0>;

I also set G2 clock at 300Mhz as specify in the TRM.
Even with all this G2 doesn't fire interrupts.

Benjamin

Regards,
Lucas





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux