Re: [PATCH v9 03/13] media: hantro: Use syscon instead of 'ctrl' register

2021-04-20 Thread Hans Verkuil
On 20/04/2021 11:31, Benjamin Gaignard wrote:
> 
> Le 20/04/2021 à 11:16, Hans Verkuil a écrit :
>> On 20/04/2021 11:10, Benjamin Gaignard wrote:
>>> Le 16/04/2021 à 17:14, Lucas Stach a écrit :
 Am Freitag, dem 16.04.2021 um 15:08 +0200 schrieb Benjamin Gaignard:
> Le 16/04/2021 à 12:54, Lucas Stach a écrit :
>> Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard:
>>> In order to be able to share the control hardware block between
>>> VPUs use a syscon instead a ioremap it in the driver.
>>> To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl'
>>> phandle is not found look at 'ctrl' reg-name.
>>> With the method it becomes useless to provide a list of register
>>> names so remove it.
>> Sorry for putting a spoke in the wheel after many iterations of the
>> series.
>>
>> We just discussed a way forward on how to handle the clocks and resets
>> provided by the blkctl block on i.MX8MM and later and it seems there is
>> a consensus on trying to provide virtual power domains from a blkctl
>> driver, controlling clocks and resets for the devices in the power
>> domain. I would like to avoid introducing yet another way of handling
>> the blkctl and thus would like to align the i.MX8MQ VPU blkctl with
>> what we are planning to do on the later chip generations.
>>
>> CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this
>> virtual power domain thing a shot.
> That could replace the 3 first patches and Dt patche of this series
> but that will not impact the hevc part, so I wonder if pure hevc patches
> could be merged anyway ?
> They are reviewed and don't depend of how the ctrl block is managed.
 I'm not really in a position to give any informed opinion about that
 hvec patches, as I only skimmed them, but I don't see any reason to
 delay patches 04-11 from this series until the i.MX8M platform issues
 are sorted. AFAICS those things are totally orthogonal.
>>> Hi Hans,
>>> What do you think about this proposal to split this series ?
>>> Get hevc part merged could allow me to continue to add features
>>> like scaling lists, compressed reference buffers and 10-bit supports.
>> Makes sense to me!
> 
> Great !
> If the latest version match your expectations how would you like to processed 
> ?
> Can you merged patches 4 to 12 ? or should I resend them in a new shorted 
> series ?

A separate patch series would be easier for me.

Regards,

Hans

> 
> Regards,
> Benjamin
> 
>>
>> Regards,
>>
>>  Hans
>>



Re: [PATCH v9 03/13] media: hantro: Use syscon instead of 'ctrl' register

2021-04-20 Thread Benjamin Gaignard



Le 20/04/2021 à 11:16, Hans Verkuil a écrit :

On 20/04/2021 11:10, Benjamin Gaignard wrote:

Le 16/04/2021 à 17:14, Lucas Stach a écrit :

Am Freitag, dem 16.04.2021 um 15:08 +0200 schrieb Benjamin Gaignard:

Le 16/04/2021 à 12:54, Lucas Stach a écrit :

Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard:

In order to be able to share the control hardware block between
VPUs use a syscon instead a ioremap it in the driver.
To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl'
phandle is not found look at 'ctrl' reg-name.
With the method it becomes useless to provide a list of register
names so remove it.

Sorry for putting a spoke in the wheel after many iterations of the
series.

We just discussed a way forward on how to handle the clocks and resets
provided by the blkctl block on i.MX8MM and later and it seems there is
a consensus on trying to provide virtual power domains from a blkctl
driver, controlling clocks and resets for the devices in the power
domain. I would like to avoid introducing yet another way of handling
the blkctl and thus would like to align the i.MX8MQ VPU blkctl with
what we are planning to do on the later chip generations.

CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this
virtual power domain thing a shot.

That could replace the 3 first patches and Dt patche of this series
but that will not impact the hevc part, so I wonder if pure hevc patches
could be merged anyway ?
They are reviewed and don't depend of how the ctrl block is managed.

I'm not really in a position to give any informed opinion about that
hvec patches, as I only skimmed them, but I don't see any reason to
delay patches 04-11 from this series until the i.MX8M platform issues
are sorted. AFAICS those things are totally orthogonal.

Hi Hans,
What do you think about this proposal to split this series ?
Get hevc part merged could allow me to continue to add features
like scaling lists, compressed reference buffers and 10-bit supports.

Makes sense to me!


Great !
If the latest version match your expectations how would you like to processed ?
Can you merged patches 4 to 12 ? or should I resend them in a new shorted 
series ?

Regards,
Benjamin



Regards,

Hans



Re: [PATCH v9 03/13] media: hantro: Use syscon instead of 'ctrl' register

2021-04-20 Thread Hans Verkuil
On 20/04/2021 11:10, Benjamin Gaignard wrote:
> 
> Le 16/04/2021 à 17:14, Lucas Stach a écrit :
>> Am Freitag, dem 16.04.2021 um 15:08 +0200 schrieb Benjamin Gaignard:
>>> Le 16/04/2021 à 12:54, Lucas Stach a écrit :
 Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard:
> In order to be able to share the control hardware block between
> VPUs use a syscon instead a ioremap it in the driver.
> To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl'
> phandle is not found look at 'ctrl' reg-name.
> With the method it becomes useless to provide a list of register
> names so remove it.
 Sorry for putting a spoke in the wheel after many iterations of the
 series.

 We just discussed a way forward on how to handle the clocks and resets
 provided by the blkctl block on i.MX8MM and later and it seems there is
 a consensus on trying to provide virtual power domains from a blkctl
 driver, controlling clocks and resets for the devices in the power
 domain. I would like to avoid introducing yet another way of handling
 the blkctl and thus would like to align the i.MX8MQ VPU blkctl with
 what we are planning to do on the later chip generations.

 CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this
 virtual power domain thing a shot.
>>> That could replace the 3 first patches and Dt patche of this series
>>> but that will not impact the hevc part, so I wonder if pure hevc patches
>>> could be merged anyway ?
>>> They are reviewed and don't depend of how the ctrl block is managed.
>> I'm not really in a position to give any informed opinion about that
>> hvec patches, as I only skimmed them, but I don't see any reason to
>> delay patches 04-11 from this series until the i.MX8M platform issues
>> are sorted. AFAICS those things are totally orthogonal.
> 
> Hi Hans,
> What do you think about this proposal to split this series ?
> Get hevc part merged could allow me to continue to add features
> like scaling lists, compressed reference buffers and 10-bit supports.

Makes sense to me!

Regards,

Hans


Re: [PATCH v9 03/13] media: hantro: Use syscon instead of 'ctrl' register

2021-04-20 Thread Benjamin Gaignard



Le 16/04/2021 à 17:14, Lucas Stach a écrit :

Am Freitag, dem 16.04.2021 um 15:08 +0200 schrieb Benjamin Gaignard:

Le 16/04/2021 à 12:54, Lucas Stach a écrit :

Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard:

In order to be able to share the control hardware block between
VPUs use a syscon instead a ioremap it in the driver.
To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl'
phandle is not found look at 'ctrl' reg-name.
With the method it becomes useless to provide a list of register
names so remove it.

Sorry for putting a spoke in the wheel after many iterations of the
series.

We just discussed a way forward on how to handle the clocks and resets
provided by the blkctl block on i.MX8MM and later and it seems there is
a consensus on trying to provide virtual power domains from a blkctl
driver, controlling clocks and resets for the devices in the power
domain. I would like to avoid introducing yet another way of handling
the blkctl and thus would like to align the i.MX8MQ VPU blkctl with
what we are planning to do on the later chip generations.

CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this
virtual power domain thing a shot.

That could replace the 3 first patches and Dt patche of this series
but that will not impact the hevc part, so I wonder if pure hevc patches
could be merged anyway ?
They are reviewed and don't depend of how the ctrl block is managed.

I'm not really in a position to give any informed opinion about that
hvec patches, as I only skimmed them, but I don't see any reason to
delay patches 04-11 from this series until the i.MX8M platform issues
are sorted. AFAICS those things are totally orthogonal.


Hi Hans,
What do you think about this proposal to split this series ?
Get hevc part merged could allow me to continue to add features
like scaling lists, compressed reference buffers and 10-bit supports.

Regards,
Benjamin



Regards,
Lucas


Regards,
Lucas


Signed-off-by: Benjamin Gaignard 
Reviewed-by: Philipp Zabel 
---
version 9:
   - Corrections in commit message

version 7:
   - Add Philipp reviewed-by tag.
   - Change syscon phandle name.
   







version 5:
   - use syscon instead of VPU reset driver.
   - if DT doesn't provide syscon keep backward compatibilty by using
 'ctrl' reg-name.

   drivers/staging/media/hantro/hantro.h   |  5 +-
   drivers/staging/media/hantro/imx8m_vpu_hw.c | 52 -
   2 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro.h 
b/drivers/staging/media/hantro/hantro.h
index 6c1b888abe75..37b9ce04bd4e 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -13,6 +13,7 @@
   #define HANTRO_H_
   







   #include 
+#include 
   #include 
   #include 
   #include 
@@ -167,7 +168,7 @@ hantro_vdev_to_func(struct video_device *vdev)
    * @reg_bases:   Mapped addresses of VPU registers.
    * @enc_base:Mapped address of VPU encoder register for 
convenience.
    * @dec_base:Mapped address of VPU decoder register for 
convenience.
- * @ctrl_base: Mapped address of VPU control block.
+ * @ctrl_base: Regmap of VPU control block.
    * @vpu_mutex:   Mutex to synchronize V4L2 calls.
    * @irqlock: Spinlock to synchronize access to data structures
    *   shared with interrupt handlers.
@@ -186,7 +187,7 @@ struct hantro_dev {
    void __iomem **reg_bases;
    void __iomem *enc_base;
    void __iomem *dec_base;
-   void __iomem *ctrl_base;
+   struct regmap *ctrl_base;
   







    struct mutex vpu_mutex; /* video_device lock */
    spinlock_t irqlock;
diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c 
b/drivers/staging/media/hantro/imx8m_vpu_hw.c
index c222de075ef4..8d0c3425234b 100644
--- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
+++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
@@ -7,6 +7,7 @@
   







   #include 
   #include 
+#include 
   







   #include "hantro.h"
   #include "hantro_jpeg.h"
@@ -24,30 +25,28 @@
   #define CTRL_G1_PP_FUSE  0x0c
   #define CTRL_G2_DEC_FUSE 0x10
   







+static const struct regmap_config ctrl_regmap_ctrl = {
+   .reg_bits = 32,
+   .val_bits = 32,
+   .reg_stride = 0x14,
+};
+
   static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits)
   {
-   u32 val;
-
    /* Assert */
-   val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
-   val &= ~reset_bits;
-   writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
+   regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, reset_bits, 0);
   







    udelay(2);
   







    /* Release */
-   val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
-   val |= reset_bits;
-   writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
+   regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET,
+

Re: [PATCH v9 03/13] media: hantro: Use syscon instead of 'ctrl' register

2021-04-16 Thread Lucas Stach
Am Freitag, dem 16.04.2021 um 15:08 +0200 schrieb Benjamin Gaignard:
> Le 16/04/2021 à 12:54, Lucas Stach a écrit :
> > Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard:
> > > In order to be able to share the control hardware block between
> > > VPUs use a syscon instead a ioremap it in the driver.
> > > To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl'
> > > phandle is not found look at 'ctrl' reg-name.
> > > With the method it becomes useless to provide a list of register
> > > names so remove it.
> > Sorry for putting a spoke in the wheel after many iterations of the
> > series.
> > 
> > We just discussed a way forward on how to handle the clocks and resets
> > provided by the blkctl block on i.MX8MM and later and it seems there is
> > a consensus on trying to provide virtual power domains from a blkctl
> > driver, controlling clocks and resets for the devices in the power
> > domain. I would like to avoid introducing yet another way of handling
> > the blkctl and thus would like to align the i.MX8MQ VPU blkctl with
> > what we are planning to do on the later chip generations.
> > 
> > CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this
> > virtual power domain thing a shot.
> 
> That could replace the 3 first patches and Dt patche of this series
> but that will not impact the hevc part, so I wonder if pure hevc patches
> could be merged anyway ?
> They are reviewed and don't depend of how the ctrl block is managed.

I'm not really in a position to give any informed opinion about that
hvec patches, as I only skimmed them, but I don't see any reason to
delay patches 04-11 from this series until the i.MX8M platform issues
are sorted. AFAICS those things are totally orthogonal.

Regards,
Lucas

> > 
> > Regards,
> > Lucas
> > 
> > > Signed-off-by: Benjamin Gaignard 
> > > Reviewed-by: Philipp Zabel 
> > > ---
> > > version 9:
> > >   - Corrections in commit message
> > > 
> > > version 7:
> > >   - Add Philipp reviewed-by tag.
> > >   - Change syscon phandle name.
> > >   
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > version 5:
> > >   - use syscon instead of VPU reset driver.
> > >   - if DT doesn't provide syscon keep backward compatibilty by using
> > > 'ctrl' reg-name.
> > > 
> > >   drivers/staging/media/hantro/hantro.h   |  5 +-
> > >   drivers/staging/media/hantro/imx8m_vpu_hw.c | 52 -
> > >   2 files changed, 34 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/drivers/staging/media/hantro/hantro.h 
> > > b/drivers/staging/media/hantro/hantro.h
> > > index 6c1b888abe75..37b9ce04bd4e 100644
> > > --- a/drivers/staging/media/hantro/hantro.h
> > > +++ b/drivers/staging/media/hantro/hantro.h
> > > @@ -13,6 +13,7 @@
> > >   #define HANTRO_H_
> > >   
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > >   #include 
> > > +#include 
> > >   #include 
> > >   #include 
> > >   #include 
> > > @@ -167,7 +168,7 @@ hantro_vdev_to_func(struct video_device *vdev)
> > >    * @reg_bases:  Mapped addresses of VPU registers.
> > >    * @enc_base:   Mapped address of VPU encoder register for 
> > > convenience.
> > >    * @dec_base:   Mapped address of VPU decoder register for 
> > > convenience.
> > > - * @ctrl_base:   Mapped address of VPU control block.
> > > + * @ctrl_base:   Regmap of VPU control block.
> > >    * @vpu_mutex:  Mutex to synchronize V4L2 calls.
> > >    * @irqlock:Spinlock to synchronize access to data 
> > > structures
> > >    *  shared with interrupt handlers.
> > > @@ -186,7 +187,7 @@ struct hantro_dev {
> > >   void __iomem **reg_bases;
> > >   void __iomem *enc_base;
> > >   void __iomem *dec_base;
> > > - void __iomem *ctrl_base;
> > > + struct regmap *ctrl_base;
> > >   
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > >   struct mutex vpu_mutex; /* video_device lock */
> > >   spinlock_t irqlock;
> > > diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c 
> > > b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > > index c222de075ef4..8d0c3425234b 100644
> > > --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > > +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > > @@ -7,6 +7,7 @@
> > >   
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > >   #include 
> > >   #include 
> > > +#include 
> > >   
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > >   #include "hantro.h"
> > >   #include "hantro_jpeg.h"
> > > @@ -24,30 +25,28 @@
> > >   #define CTRL_G1_PP_FUSE 0x0c
> > >   #define CTRL_G2_DEC_FUSE0x10
> > >   
> > > 
> > > 
> > > 
> > > 
> > > 
> > > 
> > > +static const struct regmap_config ctrl_regmap_ctrl = {
> > > + .reg_bits = 32,
> > > + .val_bits = 32,
> > > + .reg_stride = 0x14,
> > > +};
> > > +
> > >   static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits)
> > >   {
> > > - u32 val;
> > > -
> > >   /* Assert */
> > > - val = 

Re: [PATCH v9 03/13] media: hantro: Use syscon instead of 'ctrl' register

2021-04-16 Thread Benjamin Gaignard



Le 16/04/2021 à 12:54, Lucas Stach a écrit :

Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard:

In order to be able to share the control hardware block between
VPUs use a syscon instead a ioremap it in the driver.
To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl'
phandle is not found look at 'ctrl' reg-name.
With the method it becomes useless to provide a list of register
names so remove it.

Sorry for putting a spoke in the wheel after many iterations of the
series.

We just discussed a way forward on how to handle the clocks and resets
provided by the blkctl block on i.MX8MM and later and it seems there is
a consensus on trying to provide virtual power domains from a blkctl
driver, controlling clocks and resets for the devices in the power
domain. I would like to avoid introducing yet another way of handling
the blkctl and thus would like to align the i.MX8MQ VPU blkctl with
what we are planning to do on the later chip generations.

CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this
virtual power domain thing a shot.


That could replace the 3 first patches and Dt patche of this series
but that will not impact the hevc part, so I wonder if pure hevc patches
could be merged anyway ?
They are reviewed and don't depend of how the ctrl block is managed.

Regards,
Benjamin



Regards,
Lucas


Signed-off-by: Benjamin Gaignard 
Reviewed-by: Philipp Zabel 
---
version 9:
  - Corrections in commit message

version 7:
  - Add Philipp reviewed-by tag.
  - Change syscon phandle name.
  




version 5:
  - use syscon instead of VPU reset driver.
  - if DT doesn't provide syscon keep backward compatibilty by using
    'ctrl' reg-name.

  drivers/staging/media/hantro/hantro.h   |  5 +-
  drivers/staging/media/hantro/imx8m_vpu_hw.c | 52 -
  2 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro.h 
b/drivers/staging/media/hantro/hantro.h
index 6c1b888abe75..37b9ce04bd4e 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -13,6 +13,7 @@
  #define HANTRO_H_
  




  #include 
+#include 
  #include 
  #include 
  #include 
@@ -167,7 +168,7 @@ hantro_vdev_to_func(struct video_device *vdev)
   * @reg_bases:Mapped addresses of VPU registers.
   * @enc_base: Mapped address of VPU encoder register for convenience.
   * @dec_base: Mapped address of VPU decoder register for convenience.
- * @ctrl_base: Mapped address of VPU control block.
+ * @ctrl_base: Regmap of VPU control block.
   * @vpu_mutex:Mutex to synchronize V4L2 calls.
   * @irqlock:  Spinlock to synchronize access to data structures
   *shared with interrupt handlers.
@@ -186,7 +187,7 @@ struct hantro_dev {
    void __iomem **reg_bases;
    void __iomem *enc_base;
    void __iomem *dec_base;
-   void __iomem *ctrl_base;
+   struct regmap *ctrl_base;
  




    struct mutex vpu_mutex; /* video_device lock */
    spinlock_t irqlock;
diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c 
b/drivers/staging/media/hantro/imx8m_vpu_hw.c
index c222de075ef4..8d0c3425234b 100644
--- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
+++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
@@ -7,6 +7,7 @@
  




  #include 
  #include 
+#include 
  




  #include "hantro.h"
  #include "hantro_jpeg.h"
@@ -24,30 +25,28 @@
  #define CTRL_G1_PP_FUSE   0x0c
  #define CTRL_G2_DEC_FUSE  0x10
  




+static const struct regmap_config ctrl_regmap_ctrl = {
+   .reg_bits = 32,
+   .val_bits = 32,
+   .reg_stride = 0x14,
+};
+
  static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits)
  {
-   u32 val;
-
    /* Assert */
-   val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
-   val &= ~reset_bits;
-   writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
+   regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, reset_bits, 0);
  




    udelay(2);
  




    /* Release */
-   val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
-   val |= reset_bits;
-   writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
+   regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET,
+  reset_bits, reset_bits);
  }
  




  static void imx8m_clk_enable(struct hantro_dev *vpu, u32 clock_bits)
  {
-   u32 val;
-
-   val = readl(vpu->ctrl_base + CTRL_CLOCK_ENABLE);
-   val |= clock_bits;
-   writel(val, vpu->ctrl_base + CTRL_CLOCK_ENABLE);
+   regmap_update_bits(vpu->ctrl_base, CTRL_CLOCK_ENABLE,
+  clock_bits, clock_bits);
  }
  




  static int imx8mq_runtime_resume(struct hantro_dev *vpu)
@@ -64,9 +63,9 @@ static int imx8mq_runtime_resume(struct hantro_dev *vpu)
    imx8m_clk_enable(vpu, CLOCK_G1 | CLOCK_G2);
  




    /* Set values of the fuse registers */
-   writel(0x, 

Re: [PATCH v9 03/13] media: hantro: Use syscon instead of 'ctrl' register

2021-04-16 Thread Lucas Stach
Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard:
> In order to be able to share the control hardware block between
> VPUs use a syscon instead a ioremap it in the driver.
> To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl'
> phandle is not found look at 'ctrl' reg-name.
> With the method it becomes useless to provide a list of register
> names so remove it.

Sorry for putting a spoke in the wheel after many iterations of the
series.

We just discussed a way forward on how to handle the clocks and resets
provided by the blkctl block on i.MX8MM and later and it seems there is
a consensus on trying to provide virtual power domains from a blkctl
driver, controlling clocks and resets for the devices in the power
domain. I would like to avoid introducing yet another way of handling
the blkctl and thus would like to align the i.MX8MQ VPU blkctl with
what we are planning to do on the later chip generations.

CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this
virtual power domain thing a shot.

Regards,
Lucas

> Signed-off-by: Benjamin Gaignard 
> Reviewed-by: Philipp Zabel 
> ---
> version 9:
>  - Corrections in commit message
> 
> version 7:
>  - Add Philipp reviewed-by tag.
>  - Change syscon phandle name.
>  
> 
> 
> 
> version 5:
>  - use syscon instead of VPU reset driver.
>  - if DT doesn't provide syscon keep backward compatibilty by using
>    'ctrl' reg-name.
> 
>  drivers/staging/media/hantro/hantro.h   |  5 +-
>  drivers/staging/media/hantro/imx8m_vpu_hw.c | 52 -
>  2 files changed, 34 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/staging/media/hantro/hantro.h 
> b/drivers/staging/media/hantro/hantro.h
> index 6c1b888abe75..37b9ce04bd4e 100644
> --- a/drivers/staging/media/hantro/hantro.h
> +++ b/drivers/staging/media/hantro/hantro.h
> @@ -13,6 +13,7 @@
>  #define HANTRO_H_
>  
> 
> 
> 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -167,7 +168,7 @@ hantro_vdev_to_func(struct video_device *vdev)
>   * @reg_bases:   Mapped addresses of VPU registers.
>   * @enc_base:Mapped address of VPU encoder register for 
> convenience.
>   * @dec_base:Mapped address of VPU decoder register for 
> convenience.
> - * @ctrl_base:   Mapped address of VPU control block.
> + * @ctrl_base:   Regmap of VPU control block.
>   * @vpu_mutex:   Mutex to synchronize V4L2 calls.
>   * @irqlock: Spinlock to synchronize access to data structures
>   *   shared with interrupt handlers.
> @@ -186,7 +187,7 @@ struct hantro_dev {
>   void __iomem **reg_bases;
>   void __iomem *enc_base;
>   void __iomem *dec_base;
> - void __iomem *ctrl_base;
> + struct regmap *ctrl_base;
>  
> 
> 
> 
>   struct mutex vpu_mutex; /* video_device lock */
>   spinlock_t irqlock;
> diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c 
> b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> index c222de075ef4..8d0c3425234b 100644
> --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
> +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> @@ -7,6 +7,7 @@
>  
> 
> 
> 
>  #include 
>  #include 
> +#include 
>  
> 
> 
> 
>  #include "hantro.h"
>  #include "hantro_jpeg.h"
> @@ -24,30 +25,28 @@
>  #define CTRL_G1_PP_FUSE  0x0c
>  #define CTRL_G2_DEC_FUSE 0x10
>  
> 
> 
> 
> +static const struct regmap_config ctrl_regmap_ctrl = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 0x14,
> +};
> +
>  static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits)
>  {
> - u32 val;
> -
>   /* Assert */
> - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
> - val &= ~reset_bits;
> - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
> + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, reset_bits, 0);
>  
> 
> 
> 
>   udelay(2);
>  
> 
> 
> 
>   /* Release */
> - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
> - val |= reset_bits;
> - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
> + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET,
> +reset_bits, reset_bits);
>  }
>  
> 
> 
> 
>  static void imx8m_clk_enable(struct hantro_dev *vpu, u32 clock_bits)
>  {
> - u32 val;
> -
> - val = readl(vpu->ctrl_base + CTRL_CLOCK_ENABLE);
> - val |= clock_bits;
> - writel(val, vpu->ctrl_base + CTRL_CLOCK_ENABLE);
> + regmap_update_bits(vpu->ctrl_base, CTRL_CLOCK_ENABLE,
> +clock_bits, clock_bits);
>  }
>  
> 
> 
> 
>  static int imx8mq_runtime_resume(struct hantro_dev *vpu)
> @@ -64,9 +63,9 @@ static int imx8mq_runtime_resume(struct hantro_dev *vpu)
>   imx8m_clk_enable(vpu, CLOCK_G1 | CLOCK_G2);
>  
> 
> 
> 
>   /* Set values of the fuse registers */
> - writel(0x, vpu->ctrl_base + CTRL_G1_DEC_FUSE);
> - writel(0x, vpu->ctrl_base + CTRL_G1_PP_FUSE);
> - 

[PATCH v9 03/13] media: hantro: Use syscon instead of 'ctrl' register

2021-04-07 Thread Benjamin Gaignard
In order to be able to share the control hardware block between
VPUs use a syscon instead a ioremap it in the driver.
To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl'
phandle is not found look at 'ctrl' reg-name.
With the method it becomes useless to provide a list of register
names so remove it.

Signed-off-by: Benjamin Gaignard 
Reviewed-by: Philipp Zabel 
---
version 9:
 - Corrections in commit message

version 7:
 - Add Philipp reviewed-by tag.
 - Change syscon phandle name.
 
version 5:
 - use syscon instead of VPU reset driver.
 - if DT doesn't provide syscon keep backward compatibilty by using
   'ctrl' reg-name.

 drivers/staging/media/hantro/hantro.h   |  5 +-
 drivers/staging/media/hantro/imx8m_vpu_hw.c | 52 -
 2 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro.h 
b/drivers/staging/media/hantro/hantro.h
index 6c1b888abe75..37b9ce04bd4e 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -13,6 +13,7 @@
 #define HANTRO_H_
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -167,7 +168,7 @@ hantro_vdev_to_func(struct video_device *vdev)
  * @reg_bases: Mapped addresses of VPU registers.
  * @enc_base:  Mapped address of VPU encoder register for convenience.
  * @dec_base:  Mapped address of VPU decoder register for convenience.
- * @ctrl_base: Mapped address of VPU control block.
+ * @ctrl_base: Regmap of VPU control block.
  * @vpu_mutex: Mutex to synchronize V4L2 calls.
  * @irqlock:   Spinlock to synchronize access to data structures
  * shared with interrupt handlers.
@@ -186,7 +187,7 @@ struct hantro_dev {
void __iomem **reg_bases;
void __iomem *enc_base;
void __iomem *dec_base;
-   void __iomem *ctrl_base;
+   struct regmap *ctrl_base;
 
struct mutex vpu_mutex; /* video_device lock */
spinlock_t irqlock;
diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c 
b/drivers/staging/media/hantro/imx8m_vpu_hw.c
index c222de075ef4..8d0c3425234b 100644
--- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
+++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
@@ -7,6 +7,7 @@
 
 #include 
 #include 
+#include 
 
 #include "hantro.h"
 #include "hantro_jpeg.h"
@@ -24,30 +25,28 @@
 #define CTRL_G1_PP_FUSE0x0c
 #define CTRL_G2_DEC_FUSE   0x10
 
+static const struct regmap_config ctrl_regmap_ctrl = {
+   .reg_bits = 32,
+   .val_bits = 32,
+   .reg_stride = 0x14,
+};
+
 static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits)
 {
-   u32 val;
-
/* Assert */
-   val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
-   val &= ~reset_bits;
-   writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
+   regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, reset_bits, 0);
 
udelay(2);
 
/* Release */
-   val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
-   val |= reset_bits;
-   writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
+   regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET,
+  reset_bits, reset_bits);
 }
 
 static void imx8m_clk_enable(struct hantro_dev *vpu, u32 clock_bits)
 {
-   u32 val;
-
-   val = readl(vpu->ctrl_base + CTRL_CLOCK_ENABLE);
-   val |= clock_bits;
-   writel(val, vpu->ctrl_base + CTRL_CLOCK_ENABLE);
+   regmap_update_bits(vpu->ctrl_base, CTRL_CLOCK_ENABLE,
+  clock_bits, clock_bits);
 }
 
 static int imx8mq_runtime_resume(struct hantro_dev *vpu)
@@ -64,9 +63,9 @@ static int imx8mq_runtime_resume(struct hantro_dev *vpu)
imx8m_clk_enable(vpu, CLOCK_G1 | CLOCK_G2);
 
/* Set values of the fuse registers */
-   writel(0x, vpu->ctrl_base + CTRL_G1_DEC_FUSE);
-   writel(0x, vpu->ctrl_base + CTRL_G1_PP_FUSE);
-   writel(0x, vpu->ctrl_base + CTRL_G2_DEC_FUSE);
+   regmap_write(vpu->ctrl_base, CTRL_G1_DEC_FUSE, 0x);
+   regmap_write(vpu->ctrl_base, CTRL_G1_PP_FUSE, 0x);
+   regmap_write(vpu->ctrl_base, CTRL_G2_DEC_FUSE, 0x);
 
clk_bulk_disable_unprepare(vpu->variant->num_clocks, vpu->clocks);
 
@@ -150,8 +149,22 @@ static irqreturn_t imx8m_vpu_g1_irq(int irq, void *dev_id)
 
 static int imx8mq_vpu_hw_init(struct hantro_dev *vpu)
 {
-   vpu->dec_base = vpu->reg_bases[0];
-   vpu->ctrl_base = vpu->reg_bases[vpu->variant->num_regs - 1];
+   struct device_node *np = vpu->dev->of_node;
+
+   vpu->ctrl_base = syscon_regmap_lookup_by_phandle(np, 
"nxp,imx8m-vpu-ctrl");
+   if (IS_ERR(vpu->ctrl_base)) {
+   struct resource *res;
+   void __iomem *ctrl;
+
+   res = platform_get_resource_byname(vpu->pdev, IORESOURCE_MEM, 
"ctrl");
+   ctrl = devm_ioremap_resource(vpu->dev, res);
+   if (IS_ERR(ctrl))
+