2022-12-16 13:46:22

by Chen-Yu Tsai

[permalink] [raw]
Subject: [PATCH] media: hantro: Use core-generated bus_info value

The Hantro driver uses a hardcoded value for the bus_info field in the
media device and |struct v4l2_capability|. This worked well when there
was just one device. However with the iMX.8 series we are now seeing
two Hantro blocks on the same chip. The static bus_info is no longer
sufficient for differentiating devices.

Since commit f2d8b6917f3b ("media: v4l: ioctl: Set bus_info in
v4l_querycap()"), the V4L2 core provides a default value for the
bus_info field for platform and PCI devices. This value will match
the default value for media devices added by commit cef699749f37
("media: mc: Set bus_info in media_device_init()"). These defaults
are stable and device-specific.

Drop the static bus_info values from the hantro driver and use the
defaults.

Signed-off-by: Chen-Yu Tsai <[email protected]>
---
arch/arm64/boot/dts/freescale/Makefile | 1 +
drivers/media/platform/verisilicon/hantro_drv.c | 2 --
drivers/media/platform/verisilicon/hantro_v4l2.c | 2 --
3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index ef6f364eaa18..ef815442c78b 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -55,6 +55,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mm-data-modul-edm-sbc.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mm-emcon-avari.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
+dtb-$(CONFIG_ARCH_MXC) += imx8mm-hummingboard-pulse.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-ctouch2.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-edimm2.2.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8mm-innocomm-wb15-evk.dtb
diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
index 8cb4a68c9119..b0aeedae7b65 100644
--- a/drivers/media/platform/verisilicon/hantro_drv.c
+++ b/drivers/media/platform/verisilicon/hantro_drv.c
@@ -1050,8 +1050,6 @@ static int hantro_probe(struct platform_device *pdev)

vpu->mdev.dev = vpu->dev;
strscpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model));
- strscpy(vpu->mdev.bus_info, "platform: " DRIVER_NAME,
- sizeof(vpu->mdev.bus_info));
media_device_init(&vpu->mdev);
vpu->mdev.ops = &hantro_m2m_media_ops;
vpu->v4l2_dev.mdev = &vpu->mdev;
diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index 2c7a805289e7..c1a116031013 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -142,8 +142,6 @@ static int vidioc_querycap(struct file *file, void *priv,

strscpy(cap->driver, vpu->dev->driver->name, sizeof(cap->driver));
strscpy(cap->card, vdev->name, sizeof(cap->card));
- snprintf(cap->bus_info, sizeof(cap->bus_info), "platform: %s",
- vpu->dev->driver->name);
return 0;
}

--
2.39.0.314.g84b9a713c41-goog


2022-12-16 21:31:47

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] media: hantro: Use core-generated bus_info value

Hi Chen-Yu,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20221216]
[also build test ERROR on v6.1]
[cannot apply to media-tree/master robh/for-next pza/reset/next pza/imx-drm/next v6.1 v6.1-rc8 v6.1-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Chen-Yu-Tsai/media-hantro-Use-core-generated-bus_info-value/20221216-212545
patch link: https://lore.kernel.org/r/20221216132404.1173254-1-wenst%40chromium.org
patch subject: [PATCH] media: hantro: Use core-generated bus_info value
config: arm64-allyesconfig
compiler: aarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/241a1224cb2b93fedfe13d603007cab8e83b4ed7
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Chen-Yu-Tsai/media-hantro-Use-core-generated-bus_info-value/20221216-212545
git checkout 241a1224cb2b93fedfe13d603007cab8e83b4ed7
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> make[3]: *** No rule to make target 'arch/arm64/boot/dts/freescale/imx8mm-hummingboard-pulse.dtb', needed by 'arch/arm64/boot/dts/freescale/'.
make[3]: Target 'arch/arm64/boot/dts/freescale/' not remade because of errors.

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (2.07 kB)
config (372.26 kB)
Download all attachments

2022-12-20 19:51:32

by Adam Ford

[permalink] [raw]
Subject: Re: [PATCH] media: hantro: Use core-generated bus_info value

On Fri, Dec 16, 2022 at 7:46 AM Chen-Yu Tsai <[email protected]> wrote:
>
> The Hantro driver uses a hardcoded value for the bus_info field in the
> media device and |struct v4l2_capability|. This worked well when there
> was just one device. However with the iMX.8 series we are now seeing
> two Hantro blocks on the same chip. The static bus_info is no longer
> sufficient for differentiating devices.
>
> Since commit f2d8b6917f3b ("media: v4l: ioctl: Set bus_info in
> v4l_querycap()"), the V4L2 core provides a default value for the
> bus_info field for platform and PCI devices. This value will match
> the default value for media devices added by commit cef699749f37
> ("media: mc: Set bus_info in media_device_init()"). These defaults
> are stable and device-specific.
>
> Drop the static bus_info values from the hantro driver and use the
> defaults.
>
> Signed-off-by: Chen-Yu Tsai <[email protected]>
> ---
> arch/arm64/boot/dts/freescale/Makefile | 1 +
> drivers/media/platform/verisilicon/hantro_drv.c | 2 --
> drivers/media/platform/verisilicon/hantro_v4l2.c | 2 --
> 3 files changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index ef6f364eaa18..ef815442c78b 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -55,6 +55,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mm-data-modul-edm-sbc.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-emcon-avari.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mm-hummingboard-pulse.dtb

I think the makefile was edited by mistake.

> dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-ctouch2.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-icore-mx8mm-edimm2.2.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mm-innocomm-wb15-evk.dtb
> diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c
> index 8cb4a68c9119..b0aeedae7b65 100644
> --- a/drivers/media/platform/verisilicon/hantro_drv.c
> +++ b/drivers/media/platform/verisilicon/hantro_drv.c
> @@ -1050,8 +1050,6 @@ static int hantro_probe(struct platform_device *pdev)
>
> vpu->mdev.dev = vpu->dev;
> strscpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model));
> - strscpy(vpu->mdev.bus_info, "platform: " DRIVER_NAME,
> - sizeof(vpu->mdev.bus_info));
> media_device_init(&vpu->mdev);
> vpu->mdev.ops = &hantro_m2m_media_ops;
> vpu->v4l2_dev.mdev = &vpu->mdev;
> diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
> index 2c7a805289e7..c1a116031013 100644
> --- a/drivers/media/platform/verisilicon/hantro_v4l2.c
> +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
> @@ -142,8 +142,6 @@ static int vidioc_querycap(struct file *file, void *priv,
>
> strscpy(cap->driver, vpu->dev->driver->name, sizeof(cap->driver));
> strscpy(cap->card, vdev->name, sizeof(cap->card));
> - snprintf(cap->bus_info, sizeof(cap->bus_info), "platform: %s",
> - vpu->dev->driver->name);
> return 0;
> }
>
> --
> 2.39.0.314.g84b9a713c41-goog
>

2022-12-26 05:49:05

by Chen-Yu Tsai

[permalink] [raw]
Subject: Re: [PATCH] media: hantro: Use core-generated bus_info value

On Wed, Dec 21, 2022 at 3:39 AM Adam Ford <[email protected]> wrote:
>
> On Fri, Dec 16, 2022 at 7:46 AM Chen-Yu Tsai <[email protected]> wrote:
> >
> > The Hantro driver uses a hardcoded value for the bus_info field in the
> > media device and |struct v4l2_capability|. This worked well when there
> > was just one device. However with the iMX.8 series we are now seeing
> > two Hantro blocks on the same chip. The static bus_info is no longer
> > sufficient for differentiating devices.
> >
> > Since commit f2d8b6917f3b ("media: v4l: ioctl: Set bus_info in
> > v4l_querycap()"), the V4L2 core provides a default value for the
> > bus_info field for platform and PCI devices. This value will match
> > the default value for media devices added by commit cef699749f37
> > ("media: mc: Set bus_info in media_device_init()"). These defaults
> > are stable and device-specific.
> >
> > Drop the static bus_info values from the hantro driver and use the
> > defaults.
> >
> > Signed-off-by: Chen-Yu Tsai <[email protected]>
> > ---
> > arch/arm64/boot/dts/freescale/Makefile | 1 +
> > drivers/media/platform/verisilicon/hantro_drv.c | 2 --
> > drivers/media/platform/verisilicon/hantro_v4l2.c | 2 --
> > 3 files changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> > index ef6f364eaa18..ef815442c78b 100644
> > --- a/arch/arm64/boot/dts/freescale/Makefile
> > +++ b/arch/arm64/boot/dts/freescale/Makefile
> > @@ -55,6 +55,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mm-data-modul-edm-sbc.dtb
> > dtb-$(CONFIG_ARCH_MXC) += imx8mm-ddr4-evk.dtb
> > dtb-$(CONFIG_ARCH_MXC) += imx8mm-emcon-avari.dtb
> > dtb-$(CONFIG_ARCH_MXC) += imx8mm-evk.dtb
> > +dtb-$(CONFIG_ARCH_MXC) += imx8mm-hummingboard-pulse.dtb
>
> I think the makefile was edited by mistake.

It was. I'll send a v2.

On this topic though, has anyone else tried the i.MX8 Mini variant of
the Hummingboard Pulse?

I forward-ported the DT from SolidRun's 5.4 kernel and got it running
and correct to a certain degree, but if the PMIC regulator driver is
enabled, the board cuts out as soon as it is probed.


ChenYu