linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lucas Stach <l.stach@pengutronix.de>
To: Benjamin Gaignard <benjamin.gaignard@collabora.com>,
	ezequiel@collabora.com, p.zabel@pengutronix.de,
	mchehab@kernel.org, robh+dt@kernel.org, shawnguo@kernel.org,
	s.hauer@pengutronix.de, festevam@gmail.com, lee.jones@linaro.org,
	gregkh@linuxfoundation.org, mripard@kernel.org,
	paul.kocialkowski@bootlin.com, wens@csie.org,
	jernej.skrabec@siol.net, hverkuil-cisco@xs4all.nl,
	emil.l.velikov@gmail.com, "Peng Fan (OSS)" <peng.fan@oss.nxp.com>,
	Jacky Bai <ping.bai@nxp.com>
Cc: devel@driverdev.osuosl.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org,
	linux-imx@nxp.com, kernel@pengutronix.de, kernel@collabora.com,
	cphealy@gmail.com, linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org
Subject: Re: [PATCH v9 03/13] media: hantro: Use syscon instead of 'ctrl' register
Date: Fri, 16 Apr 2021 12:54:16 +0200	[thread overview]
Message-ID: <7bcbb787d82f21d42563d8fb7e3c2e7d40123932.camel@pengutronix.de> (raw)
In-Reply-To: <20210407073534.376722-4-benjamin.gaignard@collabora.com>

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 <benjamin.gaignard@collabora.com>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> 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 <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/videodev2.h>
>  #include <linux/wait.h>
>  #include <linux/clk.h>
> @@ -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 <linux/clk.h>
>  #include <linux/delay.h>
> +#include <linux/mfd/syscon.h>
>  
> 
> 
> 
>  #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(0xffffffff, vpu->ctrl_base + CTRL_G1_DEC_FUSE);
> -	writel(0xffffffff, vpu->ctrl_base + CTRL_G1_PP_FUSE);
> -	writel(0xffffffff, vpu->ctrl_base + CTRL_G2_DEC_FUSE);
> +	regmap_write(vpu->ctrl_base, CTRL_G1_DEC_FUSE, 0xffffffff);
> +	regmap_write(vpu->ctrl_base, CTRL_G1_PP_FUSE, 0xffffffff);
> +	regmap_write(vpu->ctrl_base, CTRL_G2_DEC_FUSE, 0xffffffff);
>  
> 
> 
> 
>  	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))
> +			return PTR_ERR(ctrl);
> +
> +		vpu->ctrl_base = devm_regmap_init_mmio(vpu->dev, ctrl, &ctrl_regmap_ctrl);
> +		if (IS_ERR(vpu->ctrl_base))
> +			return PTR_ERR(vpu->ctrl_base);
> +	}
>  
> 
> 
> 
>  	return 0;
>  }
> @@ -198,7 +211,6 @@ static const struct hantro_irq imx8mq_irqs[] = {
>  };
>  
> 
> 
> 
>  static const char * const imx8mq_clk_names[] = { "g1", "g2", "bus" };
> -static const char * const imx8mq_reg_names[] = { "g1", "g2", "ctrl" };
>  
> 
> 
> 
>  const struct hantro_variant imx8mq_vpu_variant = {
>  	.dec_fmts = imx8m_vpu_dec_fmts,
> @@ -215,6 +227,4 @@ const struct hantro_variant imx8mq_vpu_variant = {
>  	.num_irqs = ARRAY_SIZE(imx8mq_irqs),
>  	.clk_names = imx8mq_clk_names,
>  	.num_clocks = ARRAY_SIZE(imx8mq_clk_names),
> -	.reg_names = imx8mq_reg_names,
> -	.num_regs = ARRAY_SIZE(imx8mq_reg_names)
>  };



  reply	other threads:[~2021-04-16 10:54 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-07  7:35 [PATCH v9 00/13] Add HANTRO G2/HEVC decoder support for IMX8MQ Benjamin Gaignard
2021-04-07  7:35 ` [PATCH v9 01/13] dt-bindings: mfd: Add 'nxp,imx8mq-vpu-ctrl' to syscon list Benjamin Gaignard
2021-04-07  7:35 ` [PATCH v9 02/13] dt-bindings: media: nxp,imx8mq-vpu: Update the bindings for G2 support Benjamin Gaignard
2021-11-29 20:13   ` [PATCH v9 02/13] dt-bindings: media: nxp, imx8mq-vpu: " Adam Ford
2021-11-30 12:34     ` Benjamin Gaignard
2021-04-07  7:35 ` [PATCH v9 03/13] media: hantro: Use syscon instead of 'ctrl' register Benjamin Gaignard
2021-04-16 10:54   ` Lucas Stach [this message]
2021-04-16 13:08     ` Benjamin Gaignard
2021-04-16 15:14       ` Lucas Stach
2021-04-20  9:10         ` Benjamin Gaignard
2021-04-20  9:16           ` Hans Verkuil
2021-04-20  9:31             ` Benjamin Gaignard
2021-04-20 11:35               ` Hans Verkuil
2021-05-16 22:40     ` Ezequiel Garcia
2021-05-17 10:52       ` Lucas Stach
2021-05-17 13:23         ` Ezequiel Garcia
2021-05-17 13:46           ` Lucas Stach
2021-06-04 17:37         ` Ezequiel Garcia
2021-06-28 13:35     ` Benjamin Gaignard
2021-06-28 15:18       ` Ezequiel Garcia
2021-04-07  7:35 ` [PATCH v9 04/13] media: hevc: Add fields and flags for hevc PPS Benjamin Gaignard
2021-04-07  7:35 ` [PATCH v9 05/13] media: hevc: Add decode params control Benjamin Gaignard
2021-04-07  7:35 ` [PATCH v9 06/13] media: hantro: change hantro_codec_ops run prototype to return errors Benjamin Gaignard
2021-04-07  7:35 ` [PATCH v9 07/13] media: hantro: Define HEVC codec profiles and supported features Benjamin Gaignard
2021-04-07  7:35 ` [PATCH v9 08/13] media: hantro: Only use postproc when post processed formats are defined Benjamin Gaignard
2021-04-07  7:35 ` [PATCH v9 09/13] media: uapi: Add a control for HANTRO driver Benjamin Gaignard
2021-04-07  7:35 ` [PATCH v9 10/13] media: hantro: handle V4L2_PIX_FMT_HEVC_SLICE control Benjamin Gaignard
2021-04-07  7:35 ` [PATCH v9 11/13] media: hantro: Introduce G2/HEVC decoder Benjamin Gaignard
2021-04-07  7:35 ` [PATCH v9 12/13] media: hantro: IMX8M: add variant for G2/HEVC codec Benjamin Gaignard
2021-04-07  7:35 ` [PATCH v9 13/13] arm64: dts: imx8mq: Add node to G2 hardware Benjamin Gaignard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7bcbb787d82f21d42563d8fb7e3c2e7d40123932.camel@pengutronix.de \
    --to=l.stach@pengutronix.de \
    --cc=benjamin.gaignard@collabora.com \
    --cc=cphealy@gmail.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=devicetree@vger.kernel.org \
    --cc=emil.l.velikov@gmail.com \
    --cc=ezequiel@collabora.com \
    --cc=festevam@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=jernej.skrabec@siol.net \
    --cc=kernel@collabora.com \
    --cc=kernel@pengutronix.de \
    --cc=lee.jones@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-imx@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mchehab@kernel.org \
    --cc=mripard@kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=paul.kocialkowski@bootlin.com \
    --cc=peng.fan@oss.nxp.com \
    --cc=ping.bai@nxp.com \
    --cc=robh+dt@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).