| Message ID | 20210506010440.7016-1-peng.fan@oss.nxp.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | soc: imx: gpcv2: support i.MX8MM | expand |
On 06.05.21 03:04, Peng Fan (OSS) wrote: > From: Peng Fan <peng.fan@nxp.com> > > > V2: > - Add R-b/A-b tag > - Merge V1 patch 13 to V2 patch 6 > - Drop V1 patch 15 > - Merge V1 patch 16 to V2 patch 5 and add comments in patch 5 to explain > details > - Add explaination in patch 8 for "why the resets are not defined" > > This patchset is a pick up Lucas's gpcv2 work for i.MX8MM and several > minor changes from me to make it could work with i.MX BLK-CTL driver. > > Thanks for Lucas's work and suggestion, Frieder Schrempf for collecting > all the patches, Jacky Bai on help debug issues. I tested this series together with the BLK CTL patches by using the GPU and the display stack. Everything looks good to me. Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> > > Lucas Stach (12): > soc: imx: gpcv2: move to more ideomatic error handling in probe > soc: imx: gpcv2: move domain mapping to domain driver probe > soc: imx: gpcv2: switch to clk_bulk_* API > soc: imx: gpcv2: split power up and power down sequence control > soc: imx: gpcv2: wait for ADB400 handshake > soc: imx: gpcv2: add runtime PM support for power-domains > soc: imx: gpcv2: allow domains without power-sequence control > dt-bindings: imx: gpcv2: add support for optional resets > soc: imx: gpcv2: add support for optional resets > dt-bindings: power: add defines for i.MX8MM power domains > soc: imx: gpcv2: add support for i.MX8MM power domains > soc: imx: gpcv2: Add support for missing i.MX8MM VPU/DISPMIX power > domains > > Peng Fan (1): > soc: imx: gpcv2: move reset assert after requesting domain power up > > .../bindings/power/fsl,imx-gpcv2.yaml | 9 + > drivers/soc/imx/gpcv2.c | 542 ++++++++++++++---- > include/dt-bindings/power/imx8mm-power.h | 22 + > 3 files changed, 458 insertions(+), 115 deletions(-) > create mode 100644 include/dt-bindings/power/imx8mm-power.h >
On 06.05.21 10:32, Frieder Schrempf wrote: > On 06.05.21 03:04, Peng Fan (OSS) wrote: >> From: Peng Fan <peng.fan@nxp.com> >> >> >> V2: >> - Add R-b/A-b tag >> - Merge V1 patch 13 to V2 patch 6 >> - Drop V1 patch 15 >> - Merge V1 patch 16 to V2 patch 5 and add comments in patch 5 to explain >> details >> - Add explaination in patch 8 for "why the resets are not defined" >> >> This patchset is a pick up Lucas's gpcv2 work for i.MX8MM and several >> minor changes from me to make it could work with i.MX BLK-CTL driver. >> >> Thanks for Lucas's work and suggestion, Frieder Schrempf for collecting >> all the patches, Jacky Bai on help debug issues. > > I tested this series together with the BLK CTL patches by using the GPU and the display stack. Everything looks good to me. > > Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> So after some more testing on different hardware I stumbled upon the problem that USB autosuspend doesn't work properly anymore. I have an onboard LTE module that is connected to OTG2 on the i.MX8MM. When using the mainline TF-A (that enables USB power-domains by default) and removing the power-domain control from the kernel, the device comes up after a few seconds and is enumerated on the bus. Now, when I let the kernel control the power-domains, the device comes up at boot, but isn't enumerated on the USB bus. As soon as I disable autosuspend for the port, it comes up. Is this something that needs to be fixed on the USB driver side or is something to be considered for the GPCv2 driver?
On 19.05.21 18:09, Frieder Schrempf wrote: > On 06.05.21 10:32, Frieder Schrempf wrote: >> On 06.05.21 03:04, Peng Fan (OSS) wrote: >>> From: Peng Fan <peng.fan@nxp.com> >>> >>> >>> V2: >>> - Add R-b/A-b tag >>> - Merge V1 patch 13 to V2 patch 6 >>> - Drop V1 patch 15 >>> - Merge V1 patch 16 to V2 patch 5 and add comments in patch 5 to explain >>> details >>> - Add explaination in patch 8 for "why the resets are not defined" >>> >>> This patchset is a pick up Lucas's gpcv2 work for i.MX8MM and several >>> minor changes from me to make it could work with i.MX BLK-CTL driver. >>> >>> Thanks for Lucas's work and suggestion, Frieder Schrempf for collecting >>> all the patches, Jacky Bai on help debug issues. >> >> I tested this series together with the BLK CTL patches by using the GPU and the display stack. Everything looks good to me. >> >> Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> > > So after some more testing on different hardware I stumbled upon the problem that USB autosuspend doesn't work properly anymore. > > I have an onboard LTE module that is connected to OTG2 on the i.MX8MM. When using the mainline TF-A (that enables USB power-domains by default) and removing the power-domain control from the kernel, the device comes up after a few seconds and is enumerated on the bus. > > Now, when I let the kernel control the power-domains, the device comes up at boot, but isn't enumerated on the USB bus. As soon as I disable autosuspend for the port, it comes up. > > Is this something that needs to be fixed on the USB driver side or is something to be considered for the GPCv2 driver? So I think this is something that needs to be covered on the USB driver side. I would expect that a device appearing on the bus should resume the autosuspended bus, but I don't really know much about USB so there might be other things I miss. For now I will disable autosuspend in this case. A different, probably more severe problem is that I was still able to reliably run into lockups with suspend/resume and the GPU. I thought I had tested this before as it was one of the things that already failed with the previous implementation, but I must have missed something as it still fails with kernel v5.12.1 + v2 of the GPC patches. This is how I run into the lockup: echo mem > /sys/power/state # Sleep # Wake up again glmark2-es2-drm # Use the GPU # Device locks up Peng, is this something you can reproduce?
Hi Frieder, Am Donnerstag, dem 20.05.2021 um 17:16 +0200 schrieb Frieder Schrempf: > On 19.05.21 18:09, Frieder Schrempf wrote: > > On 06.05.21 10:32, Frieder Schrempf wrote: > > > On 06.05.21 03:04, Peng Fan (OSS) wrote: > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > > > > > > > V2: > > > > - Add R-b/A-b tag > > > > - Merge V1 patch 13 to V2 patch 6 > > > > - Drop V1 patch 15 > > > > - Merge V1 patch 16 to V2 patch 5 and add comments in patch 5 > > > > to explain > > > > details > > > > - Add explaination in patch 8 for "why the resets are not > > > > defined" > > > > > > > > This patchset is a pick up Lucas's gpcv2 work for i.MX8MM and > > > > several > > > > minor changes from me to make it could work with i.MX BLK-CTL > > > > driver. > > > > > > > > Thanks for Lucas's work and suggestion, Frieder Schrempf for > > > > collecting > > > > all the patches, Jacky Bai on help debug issues. > > > > > > I tested this series together with the BLK CTL patches by using > > > the GPU and the display stack. Everything looks good to me. > > > > > > Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> > > > > So after some more testing on different hardware I stumbled upon > > the problem that USB autosuspend doesn't work properly anymore. > > > > I have an onboard LTE module that is connected to OTG2 on the > > i.MX8MM. When using the mainline TF-A (that enables USB power- > > domains by default) and removing the power-domain control from the > > kernel, the device comes up after a few seconds and is enumerated > > on the bus. > > > > Now, when I let the kernel control the power-domains, the device > > comes up at boot, but isn't enumerated on the USB bus. As soon as I > > disable autosuspend for the port, it comes up. > > > > Is this something that needs to be fixed on the USB driver side or > > is something to be considered for the GPCv2 driver? > > So I think this is something that needs to be covered on the USB > driver side. I would expect that a device appearing on the bus should > resume the autosuspended bus, but I don't really know much about USB > so there might be other things I miss. For now I will disable > autosuspend in this case. > > A different, probably more severe problem is that I was still able to > reliably run into lockups with suspend/resume and the GPU. I thought > I had tested this before as it was one of the things that already > failed with the previous implementation, but I must have missed > something as it still fails with kernel v5.12.1 + v2 of the GPC > patches. > > This is how I run into the lockup: > > echo mem > /sys/power/state # Sleep > # Wake up again > glmark2-es2-drm # Use the GPU > # Device locks up > > Peng, is this something you can reproduce? I could reproduce this issue on my last GPC+BLK_CTRL series. This was caused by a bad interaction between our slightly unusual way to control the nested power domains via runtime PM and the system suspend/resume code, which lead to some of the power domains not properly coming up again in the resume path. v2 of my series fixes this issue and the above sequence works as expected. Regards, Lucas
Hi Lucas, On 21.07.21 22:51, Lucas Stach wrote: > Hi Frieder, > > Am Donnerstag, dem 20.05.2021 um 17:16 +0200 schrieb Frieder Schrempf: >> On 19.05.21 18:09, Frieder Schrempf wrote: >>> On 06.05.21 10:32, Frieder Schrempf wrote: >>>> On 06.05.21 03:04, Peng Fan (OSS) wrote: >>>>> From: Peng Fan <peng.fan@nxp.com> >>>>> >>>>> >>>>> V2: >>>>> - Add R-b/A-b tag >>>>> - Merge V1 patch 13 to V2 patch 6 >>>>> - Drop V1 patch 15 >>>>> - Merge V1 patch 16 to V2 patch 5 and add comments in patch 5 >>>>> to explain >>>>> details >>>>> - Add explaination in patch 8 for "why the resets are not >>>>> defined" >>>>> >>>>> This patchset is a pick up Lucas's gpcv2 work for i.MX8MM and >>>>> several >>>>> minor changes from me to make it could work with i.MX BLK-CTL >>>>> driver. >>>>> >>>>> Thanks for Lucas's work and suggestion, Frieder Schrempf for >>>>> collecting >>>>> all the patches, Jacky Bai on help debug issues. >>>> >>>> I tested this series together with the BLK CTL patches by using >>>> the GPU and the display stack. Everything looks good to me. >>>> >>>> Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> >>> >>> So after some more testing on different hardware I stumbled upon >>> the problem that USB autosuspend doesn't work properly anymore. >>> >>> I have an onboard LTE module that is connected to OTG2 on the >>> i.MX8MM. When using the mainline TF-A (that enables USB power- >>> domains by default) and removing the power-domain control from the >>> kernel, the device comes up after a few seconds and is enumerated >>> on the bus. >>> >>> Now, when I let the kernel control the power-domains, the device >>> comes up at boot, but isn't enumerated on the USB bus. As soon as I >>> disable autosuspend for the port, it comes up. >>> >>> Is this something that needs to be fixed on the USB driver side or >>> is something to be considered for the GPCv2 driver? >> >> So I think this is something that needs to be covered on the USB >> driver side. I would expect that a device appearing on the bus should >> resume the autosuspended bus, but I don't really know much about USB >> so there might be other things I miss. For now I will disable >> autosuspend in this case. >> >> A different, probably more severe problem is that I was still able to >> reliably run into lockups with suspend/resume and the GPU. I thought >> I had tested this before as it was one of the things that already >> failed with the previous implementation, but I must have missed >> something as it still fails with kernel v5.12.1 + v2 of the GPC >> patches. >> >> This is how I run into the lockup: >> >> echo mem > /sys/power/state # Sleep >> # Wake up again >> glmark2-es2-drm # Use the GPU >> # Device locks up >> >> Peng, is this something you can reproduce? > > I could reproduce this issue on my last GPC+BLK_CTRL series. This was > caused by a bad interaction between our slightly unusual way to control > the nested power domains via runtime PM and the system suspend/resume > code, which lead to some of the power domains not properly coming up > again in the resume path. v2 of my series fixes this issue and the > above sequence works as expected. Glad you could reproduce and fix this issue. Thanks for the effort. I will try to do some tests myself soon. Thanks Frieder
Hi Peng, Lucas, On Wed, 5 May 2021 at 21:32, Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote: > > From: Peng Fan <peng.fan@nxp.com> > > > V2: > - Add R-b/A-b tag > - Merge V1 patch 13 to V2 patch 6 > - Drop V1 patch 15 > - Merge V1 patch 16 to V2 patch 5 and add comments in patch 5 to explain > details > - Add explaination in patch 8 for "why the resets are not defined" > > This patchset is a pick up Lucas's gpcv2 work for i.MX8MM and several > minor changes from me to make it could work with i.MX BLK-CTL driver. > > Thanks for Lucas's work and suggestion, Frieder Schrempf for collecting > all the patches, Jacky Bai on help debug issues. > > Lucas Stach (12): > soc: imx: gpcv2: move to more ideomatic error handling in probe > soc: imx: gpcv2: move domain mapping to domain driver probe > soc: imx: gpcv2: switch to clk_bulk_* API > soc: imx: gpcv2: split power up and power down sequence control > soc: imx: gpcv2: wait for ADB400 handshake > soc: imx: gpcv2: add runtime PM support for power-domains > soc: imx: gpcv2: allow domains without power-sequence control > dt-bindings: imx: gpcv2: add support for optional resets > soc: imx: gpcv2: add support for optional resets > dt-bindings: power: add defines for i.MX8MM power domains > soc: imx: gpcv2: add support for i.MX8MM power domains > soc: imx: gpcv2: Add support for missing i.MX8MM VPU/DISPMIX power > domains > It's nice to see this finally moving forward! As you know, Hantro G2 support for i.MX8MQ (and i.MX8MP, i.MX8MM) is currently blocked, as you have requested: https://lore.kernel.org/driverdev-devel/5aa5700b862234895a7a6eb251ca3c80fdc1a6d3.camel@collabora.com/ So, I think we really need to include i.MX8MP and i.MX8MQ on this series. It's been quite a while and we really need to have that. To be honest, I fear that if we merge this series as-is, MX8MP and MX8MP support will fall in the oblivion, and Hantro G2 VPU will remain unusable. We are planning to submit Hantro G2 VP9 support soon, and we've been testing on various platforms, but it will also be blocked by lack of power-domains. In the future, please Cc the linux-media mailing list, as well as Benjamin, Andrzej and myself, so we can follow this. Thanks a lot for the hard work! Ezequiel
Hi Ezequiel, Am Mittwoch, dem 04.08.2021 um 11:30 -0300 schrieb Ezequiel Garcia: > Hi Peng, Lucas, > > On Wed, 5 May 2021 at 21:32, Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote: > > > > From: Peng Fan <peng.fan@nxp.com> > > > > > > V2: > > - Add R-b/A-b tag > > - Merge V1 patch 13 to V2 patch 6 > > - Drop V1 patch 15 > > - Merge V1 patch 16 to V2 patch 5 and add comments in patch 5 to explain > > details > > - Add explaination in patch 8 for "why the resets are not defined" > > > > This patchset is a pick up Lucas's gpcv2 work for i.MX8MM and several > > minor changes from me to make it could work with i.MX BLK-CTL driver. > > > > Thanks for Lucas's work and suggestion, Frieder Schrempf for collecting > > all the patches, Jacky Bai on help debug issues. > > > > Lucas Stach (12): > > soc: imx: gpcv2: move to more ideomatic error handling in probe > > soc: imx: gpcv2: move domain mapping to domain driver probe > > soc: imx: gpcv2: switch to clk_bulk_* API > > soc: imx: gpcv2: split power up and power down sequence control > > soc: imx: gpcv2: wait for ADB400 handshake > > soc: imx: gpcv2: add runtime PM support for power-domains > > soc: imx: gpcv2: allow domains without power-sequence control > > dt-bindings: imx: gpcv2: add support for optional resets > > soc: imx: gpcv2: add support for optional resets > > dt-bindings: power: add defines for i.MX8MM power domains > > soc: imx: gpcv2: add support for i.MX8MM power domains > > soc: imx: gpcv2: Add support for missing i.MX8MM VPU/DISPMIX power > > domains > > > > It's nice to see this finally moving forward! > > As you know, Hantro G2 support for i.MX8MQ (and i.MX8MP, i.MX8MM) is currently > blocked, as you have requested: > > https://lore.kernel.org/driverdev-devel/5aa5700b862234895a7a6eb251ca3c80fdc1a6d3.camel@collabora.com/ > > So, I think we really need to include i.MX8MP and i.MX8MQ on this series. > It's been quite a while and we really need to have that. To be honest, > I fear that > if we merge this series as-is, MX8MP and MX8MP support will fall in > the oblivion, > and Hantro G2 VPU will remain unusable. > > We are planning to submit Hantro G2 VP9 support soon, and we've been testing > on various platforms, but it will also be blocked by lack of power-domains. > > In the future, please Cc the linux-media mailing list, as well as > Benjamin, Andrzej and myself, so we can follow this. Please take a look at [1], which is the current state of this work. I intend to add both i.MX8MQ and i.MX8MP support to the series now, as it seems that there have been no big objections to my approach over the last 2 weeks, where I was on vacation. ;) Regards, Lucas [1] https://lore.kernel.org/linux-arm-kernel/20210716232916.3572966-14-l.stach@pengutronix.de/T/#m43cbf6b8615b2a37ff2abb0346e7e7f6594976d1
Le 09/08/2021 à 10:15, Lucas Stach a écrit : > Hi Ezequiel, > > Am Mittwoch, dem 04.08.2021 um 11:30 -0300 schrieb Ezequiel Garcia: >> Hi Peng, Lucas, >> >> On Wed, 5 May 2021 at 21:32, Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote: >>> From: Peng Fan <peng.fan@nxp.com> >>> >>> >>> V2: >>> - Add R-b/A-b tag >>> - Merge V1 patch 13 to V2 patch 6 >>> - Drop V1 patch 15 >>> - Merge V1 patch 16 to V2 patch 5 and add comments in patch 5 to explain >>> details >>> - Add explaination in patch 8 for "why the resets are not defined" >>> >>> This patchset is a pick up Lucas's gpcv2 work for i.MX8MM and several >>> minor changes from me to make it could work with i.MX BLK-CTL driver. >>> >>> Thanks for Lucas's work and suggestion, Frieder Schrempf for collecting >>> all the patches, Jacky Bai on help debug issues. >>> >>> Lucas Stach (12): >>> soc: imx: gpcv2: move to more ideomatic error handling in probe >>> soc: imx: gpcv2: move domain mapping to domain driver probe >>> soc: imx: gpcv2: switch to clk_bulk_* API >>> soc: imx: gpcv2: split power up and power down sequence control >>> soc: imx: gpcv2: wait for ADB400 handshake >>> soc: imx: gpcv2: add runtime PM support for power-domains >>> soc: imx: gpcv2: allow domains without power-sequence control >>> dt-bindings: imx: gpcv2: add support for optional resets >>> soc: imx: gpcv2: add support for optional resets >>> dt-bindings: power: add defines for i.MX8MM power domains >>> soc: imx: gpcv2: add support for i.MX8MM power domains >>> soc: imx: gpcv2: Add support for missing i.MX8MM VPU/DISPMIX power >>> domains >>> >> It's nice to see this finally moving forward! >> >> As you know, Hantro G2 support for i.MX8MQ (and i.MX8MP, i.MX8MM) is currently >> blocked, as you have requested: >> >> https://lore.kernel.org/driverdev-devel/5aa5700b862234895a7a6eb251ca3c80fdc1a6d3.camel@collabora.com/ >> >> So, I think we really need to include i.MX8MP and i.MX8MQ on this series. >> It's been quite a while and we really need to have that. To be honest, >> I fear that >> if we merge this series as-is, MX8MP and MX8MP support will fall in >> the oblivion, >> and Hantro G2 VPU will remain unusable. >> >> We are planning to submit Hantro G2 VP9 support soon, and we've been testing >> on various platforms, but it will also be blocked by lack of power-domains. >> >> In the future, please Cc the linux-media mailing list, as well as >> Benjamin, Andrzej and myself, so we can follow this. > Please take a look at [1], which is the current state of this work. I > intend to add both i.MX8MQ and i.MX8MP support to the series now, as it > seems that there have been no big objections to my approach over the > last 2 weeks, where I was on vacation. ;) Hi Lucas, I have tried to implement the block control driver for imx8mq. I didn't manage to get it working. My implementation is here: https://gitlab.collabora.com/benjamin.gaignard/for-upstream/-/tree/IMX8MQ_BLK_CTRL While you have the same in IMX8MM do you have also made changes in Hantro driver ? If it is that can you share these changes ? I have include mine in the above branch. Regards, Benjamin > > Regards, > Lucas > > [1] > https://lore.kernel.org/linux-arm-kernel/20210716232916.3572966-14-l.stach@pengutronix.de/T/#m43cbf6b8615b2a37ff2abb0346e7e7f6594976d1 > >
From: Peng Fan <peng.fan@nxp.com> V2: - Add R-b/A-b tag - Merge V1 patch 13 to V2 patch 6 - Drop V1 patch 15 - Merge V1 patch 16 to V2 patch 5 and add comments in patch 5 to explain details - Add explaination in patch 8 for "why the resets are not defined" This patchset is a pick up Lucas's gpcv2 work for i.MX8MM and several minor changes from me to make it could work with i.MX BLK-CTL driver. Thanks for Lucas's work and suggestion, Frieder Schrempf for collecting all the patches, Jacky Bai on help debug issues. Lucas Stach (12): soc: imx: gpcv2: move to more ideomatic error handling in probe soc: imx: gpcv2: move domain mapping to domain driver probe soc: imx: gpcv2: switch to clk_bulk_* API soc: imx: gpcv2: split power up and power down sequence control soc: imx: gpcv2: wait for ADB400 handshake soc: imx: gpcv2: add runtime PM support for power-domains soc: imx: gpcv2: allow domains without power-sequence control dt-bindings: imx: gpcv2: add support for optional resets soc: imx: gpcv2: add support for optional resets dt-bindings: power: add defines for i.MX8MM power domains soc: imx: gpcv2: add support for i.MX8MM power domains soc: imx: gpcv2: Add support for missing i.MX8MM VPU/DISPMIX power domains Peng Fan (1): soc: imx: gpcv2: move reset assert after requesting domain power up .../bindings/power/fsl,imx-gpcv2.yaml | 9 + drivers/soc/imx/gpcv2.c | 542 ++++++++++++++---- include/dt-bindings/power/imx8mm-power.h | 22 + 3 files changed, 458 insertions(+), 115 deletions(-) create mode 100644 include/dt-bindings/power/imx8mm-power.h