Re: [RFC,v3 9/9] media: platform: Add Mediatek ISP P1 shared memory device
Hi Jungo,
On Tue, Jun 11, 2019 at 11:53:44AM +0800, Jungo Lin wrote:
> The purpose of this child device is to provide shared
> memory management for exchanging tuning data between co-processor
> and the Pass 1 unit of the camera ISP system, including cache
> buffer handling.
>
Looks like we haven't really progressed on getting this replaced with
something that doesn't require so much custom code. Let me propose something
better then.
We already have a reserved memory mode in DT. If it has a compatible string
of "shared-dma-pool", it would be registered in the coherent DMA framework
[1]. That would make it available for consumer devices to look-up.
Now if we add a "memory-region" property to the SCP device node and point it
to our reserved memory node, the SCP driver could look it up and hook to the
DMA mapping API using of_reserved_mem_device_init_by_idx[2].
That basically makes any dma_alloc_*(), dma_map_*(), etc. calls on the SCP
struct device use the coherent DMA ops, which operate on the assigned memory
pool. With that, the P1 driver could just directly use those calls to
manage the memory, without any custom code.
There is an example how this setup works in the s5p-mfc driver[3], but it
needs to be noted that it creates child nodes, because it can have more than
1 DMA port, which may need its own memory pool. In our case, we wouldn't
need child nodes and could just use the SCP device directly.
[1] https://elixir.bootlin.com/linux/v5.2-rc7/source/kernel/dma/coherent.c#L345
[2]
https://elixir.bootlin.com/linux/v5.2-rc7/source/drivers/of/of_reserved_mem.c#L312
[3]
https://elixir.bootlin.com/linux/v5.2-rc7/source/drivers/media/platform/s5p-mfc/s5p_mfc.c#L1075
Let me also post some specific comments below, in case we end up still
needing any of the code.
> Signed-off-by: Jungo Lin
> ---
> This patch depends on "Add support for mt8183 SCP"[1].
>
> [1] https://patchwork.kernel.org/cover/10972143/
> ---
> .../platform/mtk-isp/isp_50/cam/Makefile | 1 +
> .../mtk-isp/isp_50/cam/mtk_cam-smem.c | 304 ++
> .../mtk-isp/isp_50/cam/mtk_cam-smem.h | 18 ++
> 3 files changed, 323 insertions(+)
> create mode 100644 drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-smem.c
> create mode 100644 drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-smem.h
>
> diff --git a/drivers/media/platform/mtk-isp/isp_50/cam/Makefile
> b/drivers/media/platform/mtk-isp/isp_50/cam/Makefile
> index 95f0b1c8fa1c..d545ca6f09c5 100644
> --- a/drivers/media/platform/mtk-isp/isp_50/cam/Makefile
> +++ b/drivers/media/platform/mtk-isp/isp_50/cam/Makefile
> @@ -4,5 +4,6 @@ mtk-cam-isp-objs += mtk_cam-ctrl.o
> mtk-cam-isp-objs += mtk_cam-v4l2-util.o
> mtk-cam-isp-objs += mtk_cam.o
> mtk-cam-isp-objs += mtk_cam-scp.o
> +mtk-cam-isp-objs += mtk_cam-smem.o
>
> obj-$(CONFIG_VIDEO_MEDIATEK_ISP_PASS1) += mtk-cam-isp.o
> \ No newline at end of file
> diff --git a/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-smem.c
> b/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-smem.c
> new file mode 100644
> index ..a9845668ce10
> --- /dev/null
> +++ b/drivers/media/platform/mtk-isp/isp_50/cam/mtk_cam-smem.c
> @@ -0,0 +1,304 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (c) 2018 MediaTek Inc.
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "mtk_cam-smem.h"
> +
> +static struct dma_map_ops smem_dma_ops;
> +
> +struct mtk_cam_smem_dev {
> + struct device *dev;
> + struct sg_table sgt;
> + struct page **smem_pages;
> + dma_addr_t smem_base;
> + dma_addr_t smem_dma_base;
> + int smem_size;
> +};
> +
> +struct dma_coherent_mem {
> + void*virt_base;
> + dma_addr_t device_base;
> + unsigned long pfn_base;
> + int size;
> + int flags;
> + unsigned long *bitmap;
> + spinlock_t spinlock; /* dma_coherent_mem attributes protection */
> + booluse_dev_dma_pfn_offset;
> +};
> +
> +dma_addr_t mtk_cam_smem_iova_to_scp_addr(struct device *dev,
> + dma_addr_t iova)
> +{
> + struct iommu_domain *domain;
> + dma_addr_t addr, limit;
> + struct mtk_cam_smem_dev *smem_dev = dev_get_drvdata(dev);
> +
> + domain = iommu_get_domain_for_dev(dev);
> + if (!domain) {
> + dev_warn(dev, "No iommu group domain\n");
> + return 0;
> + }
> +
> + addr = iommu_iova_to_phys(domain, iova);
> + limit = smem_dev->smem_base + smem_dev->smem_size;
> + if (addr < smem_dev->smem_base || addr >= limit) {
> + dev_err(dev,
> + "Unexpected scp_addr:%pad must >= %pad and < %pad)\n",
> + &addr, &smem_dev->smem_base, &limit);
> + return 0;
> + }
> + return addr;
> +}
This isn't correct. One could pass an IOVA that wasn't allocated for t
Re: [PATCH 01/16] am437x/davinci: set device_caps in struct video_device
Hi Hans,
Thank you for the patch.
On Wed, Jun 26, 2019 at 8:44 AM Hans Verkuil wrote:
>
> Instead of filling in the struct v4l2_capability device_caps
> field, fill in the struct video_device device_caps field.
>
> That way the V4L2 core knows what the capabilities of the
> video device are.
>
> Signed-off-by: Hans Verkuil
> Cc: prabhakar.cse...@gmail.com
> ---
> drivers/media/platform/am437x/am437x-vpfe.c | 6 ++
> drivers/media/platform/davinci/vpbe_display.c | 3 +--
> drivers/media/platform/davinci/vpfe_capture.c | 3 +--
> drivers/media/platform/davinci/vpif_capture.c | 3 +--
> drivers/media/platform/davinci/vpif_display.c | 3 +--
> 5 files changed, 6 insertions(+), 12 deletions(-)
>
Acked-by: Lad, Prabhakar
Cheers,
Prabhakar Lad
> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c
> b/drivers/media/platform/am437x/am437x-vpfe.c
> index fe7b937eb5f2..ef635f80d645 100644
> --- a/drivers/media/platform/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
> @@ -1412,10 +1412,6 @@ static int vpfe_querycap(struct file *file, void
> *priv,
> strscpy(cap->card, "TI AM437x VPFE", sizeof(cap->card));
> snprintf(cap->bus_info, sizeof(cap->bus_info),
> "platform:%s", vpfe->v4l2_dev.name);
> - cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> - V4L2_CAP_READWRITE;
> - cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
> -
> return 0;
> }
>
> @@ -2393,6 +2389,8 @@ static int vpfe_probe_complete(struct vpfe_device *vpfe)
> vdev->vfl_dir = VFL_DIR_RX;
> vdev->queue = q;
> vdev->lock = &vpfe->lock;
> + vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> + V4L2_CAP_READWRITE;
> video_set_drvdata(vdev, vpfe);
> err = video_register_device(&vpfe->video_dev, VFL_TYPE_GRABBER, -1);
> if (err) {
> diff --git a/drivers/media/platform/davinci/vpbe_display.c
> b/drivers/media/platform/davinci/vpbe_display.c
> index 000b191c42d8..8d864b4da65e 100644
> --- a/drivers/media/platform/davinci/vpbe_display.c
> +++ b/drivers/media/platform/davinci/vpbe_display.c
> @@ -633,8 +633,6 @@ static int vpbe_display_querycap(struct file *file, void
> *priv,
> struct vpbe_layer *layer = video_drvdata(file);
> struct vpbe_device *vpbe_dev = layer->disp_dev->vpbe_dev;
>
> - cap->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
> - cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
> snprintf(cap->driver, sizeof(cap->driver), "%s",
> dev_name(vpbe_dev->pdev));
> snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> @@ -1319,6 +1317,7 @@ static int init_vpbe_layer(int i, struct vpbe_display
> *disp_dev,
> vbd->v4l2_dev = &disp_dev->vpbe_dev->v4l2_dev;
> vbd->lock = &vpbe_display_layer->opslock;
> vbd->vfl_dir= VFL_DIR_TX;
> + vbd->device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
>
> if (disp_dev->vpbe_dev->current_timings.timings_type &
> VPBE_ENC_STD)
> diff --git a/drivers/media/platform/davinci/vpfe_capture.c
> b/drivers/media/platform/davinci/vpfe_capture.c
> index 295fbf1a49cf..852fc357e19d 100644
> --- a/drivers/media/platform/davinci/vpfe_capture.c
> +++ b/drivers/media/platform/davinci/vpfe_capture.c
> @@ -877,8 +877,6 @@ static int vpfe_querycap(struct file *file, void *priv,
>
> v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_querycap\n");
>
> - cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> - cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
> strscpy(cap->driver, CAPTURE_DRV_NAME, sizeof(cap->driver));
> strscpy(cap->bus_info, "VPFE", sizeof(cap->bus_info));
> strscpy(cap->card, vpfe_dev->cfg->card_name, sizeof(cap->card));
> @@ -1785,6 +1783,7 @@ static int vpfe_probe(struct platform_device *pdev)
> vfd->ioctl_ops = &vpfe_ioctl_ops;
> vfd->tvnorms= 0;
> vfd->v4l2_dev = &vpfe_dev->v4l2_dev;
> + vfd->device_caps= V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> snprintf(vfd->name, sizeof(vfd->name),
> "%s_V%d.%d.%d",
> CAPTURE_DRV_NAME,
> diff --git a/drivers/media/platform/davinci/vpif_capture.c
> b/drivers/media/platform/davinci/vpif_capture.c
> index f0f7ef638c56..af22fc5050c3 100644
> --- a/drivers/media/platform/davinci/vpif_capture.c
> +++ b/drivers/media/platform/davinci/vpif_capture.c
> @@ -1085,8 +1085,6 @@ static int vpif_querycap(struct file *file, void *priv,
> {
> struct vpif_capture_config *config = vpif_dev->platform_data;
>
> - cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> - cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
> strscpy(cap->driver, VPIF_DRIVE
Re: [PATCH 5/5] media/platform: don't set description in ENUM_FMT
Hi Hans,
Thank you for the patch.
On Wed, Jun 26, 2019 at 12:27 PM Hans Verkuil wrote:
>
> The V4L2 core sets the format description and flags for the driver in order
> to ensure consistent naming.
>
> So drop the strscpy of the description in drivers. Also remove any
> description strings in driver-internal structures since those are
> no longer needed.
>
> Signed-off-by: Hans Verkuil
> Cc: Laurent Pinchart
> Cc: Prabhakar Lad
> Cc: Sylwester Nawrocki
> Cc: Jonathan Corbet
> Cc: Jacopo Mondi
> Cc: Benoit Parrot
> ---
> drivers/media/platform/am437x/am437x-vpfe.c | 18 ++---
> drivers/media/platform/davinci/vpbe_display.c | 14 ++
> drivers/media/platform/davinci/vpif_capture.c | 11 ++--
> drivers/media/platform/davinci/vpif_display.c | 4 ---
For the above,
Acked-by: Lad, Prabhakar
Cheers,
Prabhakar Lad
> drivers/media/platform/exynos-gsc/gsc-core.c | 22 ---
> drivers/media/platform/exynos-gsc/gsc-core.h | 2 --
> .../media/platform/exynos4-is/fimc-capture.c | 3 ---
> drivers/media/platform/exynos4-is/fimc-core.c | 20 --
> .../platform/exynos4-is/fimc-isp-video.c | 1 -
> drivers/media/platform/exynos4-is/fimc-isp.c | 3 ---
> drivers/media/platform/exynos4-is/fimc-lite.c | 8 --
> drivers/media/platform/exynos4-is/fimc-m2m.c | 1 -
> drivers/media/platform/m2m-deinterlace.c | 4 ---
> .../media/platform/marvell-ccic/mcam-core.c | 10 ---
> drivers/media/platform/mx2_emmaprp.c | 4 ---
> drivers/media/platform/omap/omap_vout.c | 7 -
> .../media/platform/s3c-camif/camif-capture.c | 11 +++-
> drivers/media/platform/s3c-camif/camif-core.c | 6 -
> drivers/media/platform/s3c-camif/camif-core.h | 1 -
> drivers/media/platform/s5p-g2d/g2d.c | 6 -
> drivers/media/platform/s5p-g2d/g2d.h | 1 -
> drivers/media/platform/s5p-jpeg/jpeg-core.c | 27 ---
> drivers/media/platform/s5p-jpeg/jpeg-core.h | 2 --
> .../media/platform/s5p-mfc/s5p_mfc_common.h | 1 -
> drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 15 ---
> drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 10 ---
> drivers/media/platform/sh_veu.c | 19 ++---
> drivers/media/platform/sh_vou.c | 12 ++---
> drivers/media/platform/ti-vpe/vpe.c | 12 -
> drivers/media/platform/via-camera.c | 4 ---
> drivers/media/platform/xilinx/xilinx-dma.c| 2 --
> drivers/media/platform/xilinx/xilinx-vip.c| 16 +--
> drivers/media/platform/xilinx/xilinx-vip.h| 2 --
> include/media/drv-intf/exynos-fimc.h | 2 --
> 34 files changed, 28 insertions(+), 253 deletions(-)
>
> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c
> b/drivers/media/platform/am437x/am437x-vpfe.c
> index fe7b937eb5f2..7582c26f8459 100644
> --- a/drivers/media/platform/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
> @@ -76,7 +76,6 @@ struct bus_format {
>
> /*
> * struct vpfe_fmt - VPFE media bus format information
> - * @name: V4L2 format description
> * @code: V4L2 media bus format code
> * @shifted: V4L2 media bus format code for the same pixel layout but
> * shifted to be 8 bits per pixel. =0 if format is not shiftable.
> @@ -86,7 +85,6 @@ struct bus_format {
> * @supported: Indicates format supported by subdev
> */
> struct vpfe_fmt {
> - const char *name;
> u32 fourcc;
> u32 code;
> struct bus_format l;
> @@ -97,7 +95,6 @@ struct vpfe_fmt {
>
> static struct vpfe_fmt formats[] = {
> {
> - .name = "YUV 4:2:2 packed, YCbYCr",
> .fourcc = V4L2_PIX_FMT_YUYV,
> .code = MEDIA_BUS_FMT_YUYV8_2X8,
> .l.width= 10,
> @@ -106,7 +103,6 @@ static struct vpfe_fmt formats[] = {
> .s.bpp = 2,
> .supported = false,
> }, {
> - .name = "YUV 4:2:2 packed, CbYCrY",
> .fourcc = V4L2_PIX_FMT_UYVY,
> .code = MEDIA_BUS_FMT_UYVY8_2X8,
> .l.width= 10,
> @@ -115,7 +111,6 @@ static struct vpfe_fmt formats[] = {
> .s.bpp = 2,
> .supported = false,
> }, {
> - .name = "YUV 4:2:2 packed, YCrYCb",
> .fourcc = V4L2_PIX_FMT_YVYU,
> .code = MEDIA_BUS_FMT_YVYU8_2X8,
> .l.width= 10,
> @@ -124,7 +119,6 @@ static struct vpfe_fmt formats[] = {
> .s.bpp = 2,
> .supported = false,
> }, {
> - .name = "YUV 4:2:2 packed, CrYCbY",
> .fourcc = V4L2_PIX_FMT_VYUY,
> .code = MEDIA_BUS_FMT_VYUY8_2X8,
> .l.width= 10,
> @@ -133,7 +1
Re: [PATCH for v5.3] v4l2-subdev: fix regression in check_pad()
Hi Hans,
On Sat, Jun 29, 2019 at 03:08:23PM +0200, Hans Verkuil wrote:
> On 6/29/19 2:57 PM, Hans Verkuil wrote:
> > On 6/29/19 2:06 PM, Janusz Krzysztofik wrote:
> >> Hi Hans,
> >>
> >> On Saturday, June 29, 2019 12:00:24 P.M. CEST Hans Verkuil wrote:
> >>> sd->entity.graph_obj.mdev can be NULL when this function is called, and
> >>> that breaks existing drivers (rcar-vin, but probably others as well).
> >>>
> >>> Check if sd->entity.num_pads is non-zero instead since that doesn't depend
> >>> on mdev.
> >>>
> >>> Signed-off-by: Hans Verkuil
> >>> Reported-by: Niklas Söderlund
> >>> Fixes: a8fa55078a77 ("media: v4l2-subdev: Verify arguments in
> >>> v4l2_subdev_call()") ---
> >>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
> >>> b/drivers/media/v4l2-core/v4l2-subdev.c index 21fb90d66bfc..bc32fc1e0c0b
> >>> 100644
> >>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> >>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> >>> @@ -124,16 +124,11 @@ static inline int check_which(__u32 which)
> >>> static inline int check_pad(struct v4l2_subdev *sd, __u32 pad)
> >>> {
> >>> #if defined(CONFIG_MEDIA_CONTROLLER)
> >>> - if (sd->entity.graph_obj.mdev) {
> >>> - if (pad >= sd->entity.num_pads)
> >>> - return -EINVAL;
> >>> - return 0;
> >>> - }
> >>> + if (sd->entity.num_pads)
> >>
> >> This reverts a change introduced on Sakari's request in v7 of the series
> >> which
> >> is the source of the regression. The intention was to fail if num_pads ==
> >> 0
> >> on a valid media entity. Maybe we should still keep that restriction and
> >> fail
> >> in case mdev is not NULL? In other words:
> >>
> >> - if (sd->entity.num_pads)
> >> + if (sd->entity.num_pads || sd->entity.graph_obj.mdev)
> >
> > If an entity has no pads, then it doesn't have pad ops either and this
> > function
> > would never be called.
>
> Sakari, do you think it is ever possible that an entity may have pad ops,
> but num_pads goes at some point to 0?
>
> If so, then we can check for sd->entity.function != 0. All MC subdevs must
> set that to a non-0 value, otherwise the core will issue a WARN_ON. I think
> it is the only reliable indicator that can be used.
I don't think checking the num_pads field is is a great way to test whether
an entity supports the pad ops; I don't have a better proposal either as it
seems that some drivers call these ops before registering the subdev.
There are sub-device drivers that expose their own device node and thus
work with MC-enabled master drivers but have no pad ops: this is what makes
the fundamental difference in API support, it's not the number of pads. We
just happen to see this in pad ops only (I guess).
Currently the drivers may set the HAS_DEVNODE flag (that really tells about
MC compatibility) just before registering the sub-device. This might be a
better test for allowing pad ops, but the name of the flag is somewhat
misleading. I wouldn't want to start testing this now however, it'd risk
exposing these issues to a much wider audience.
I guess in practice testing for num_pads still works; it however does leave
some gray area between MC-enabled sub-device drivers and the rest. Perhaps
something to improve in the future, but for a later kernel release.
So,
Acked-by: Sakari Ailus
--
Kind regards,
Sakari Ailus
sakari.ai...@linux.intel.com
Re: [PATCH 2/2] media: i2c: isl7998x: Add driver for Intersil ISL7998x
Hello Marek, long time due review, sorry about this. On Mon, May 20, 2019 at 10:18:12PM +0200, Marek Vasut wrote: > Add driver for the Intersil ISL7998x BT656-to-MIPI-CSI2 video decoder. analog to CSI-2 or analog to BT.656 if you want to support both chip versions. > This chip supports 1/2/4 analog video inputs and converts them into > 1/2/4 VCs in MIPI CSI2 stream. > > This driver currently supports ISL79987 and both 720x480 and 720x576 > resolutions, however as per specification, all inputs must use the > same resolution and standard. The only supported pixel format is now > YUYV/YUV422. The chip should support RGB565 on the CSI2 as well, but it's a media bus format, just mention YUYV > this is currently unsupported. > > Signed-off-by: Marek Vasut > Cc: Sakari Ailus > Cc: Mauro Carvalho Chehab > Cc: Rob Herring > To: linux-media@vger.kernel.org > --- > drivers/media/i2c/Kconfig|6 + > drivers/media/i2c/Makefile |1 + > drivers/media/i2c/isl7998x.c | ++ > 3 files changed, 1118 insertions(+) > create mode 100644 drivers/media/i2c/isl7998x.c > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index 7793358ab8b3..af2ea08d6c59 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -929,6 +929,12 @@ config VIDEO_NOON010PC30 > help > This driver supports NOON010PC30 CIF camera from Siliconfile > > +config VIDEO_ISL7998X > + tristate "Intersil ISL7998x support" > + depends on I2C && VIDEO_V4L2 Don't you depend on OF too ? > + help > + This driver supports Intersil ISL7998x BT656-to-MIPI-CSI2 bridge. Please update this description as well as this chip is either CSI-2 or BT.565, not both. > + > source "drivers/media/i2c/m5mols/Kconfig" > > config VIDEO_RJ54N1 > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > index d8ad9dad495d..a66f105a7bcc 100644 > --- a/drivers/media/i2c/Makefile > +++ b/drivers/media/i2c/Makefile > @@ -113,6 +113,7 @@ obj-$(CONFIG_VIDEO_IMX258)+= imx258.o > obj-$(CONFIG_VIDEO_IMX274) += imx274.o > obj-$(CONFIG_VIDEO_IMX319) += imx319.o > obj-$(CONFIG_VIDEO_IMX355) += imx355.o > +obj-$(CONFIG_VIDEO_ISL7998X) += isl7998x.o > obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o > > obj-$(CONFIG_SDR_MAX2175) += max2175.o > diff --git a/drivers/media/i2c/isl7998x.c b/drivers/media/i2c/isl7998x.c > new file mode 100644 > index ..858e98d8b494 > --- /dev/null > +++ b/drivers/media/i2c/isl7998x.c > @@ -0,0 +1, @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Intersil ISL7998x BT656-to-MIPI-CSI2 driver Here as well. > + * > + * Copyright (C) 2018-2019 Marek Vasut > + */ > +#include Sorting inclusion alphabetically is always nice and help finding duplicates. > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define ISL7998x_WIDTH 720 > +#define ISL7998x_HEIGHT 480 > +#define ISL7998x_INPUTS 4 > + > +#define V4L2_CID_TEST_PATTERN_COLOR (V4L2_CID_USER_BASE | 0x1001) > +#define V4L2_CID_TEST_PATTERN_CHANNELS (V4L2_CID_USER_BASE | 0x1002) > +#define V4L2_CID_TEST_PATTERN_BARS (V4L2_CID_USER_BASE | 0x1003) > + > +#define ISL7998x_REG(page, reg) (((page) << 8) | (reg)) > + > +#define ISL7998x_REG_Pn_SIZE 256 > +#define ISL7998x_REG_Pn_BASE(n) ((n) * > ISL7998x_REG_Pn_SIZE) > + > +#define ISL7998x_REG_Px_DEC_PAGE(page) ISL7998x_REG((page), > 0xff) > +#define ISL7998x_REG_Px_DEC_PAGE_MASK0xf > +#define ISL7998x_REG_P0_PRODUCT_ID_CODE ISL7998x_REG(0, 0x00) > +#define ISL7998x_REG_P0_PRODUCT_REV_CODE ISL7998x_REG(0, 0x01) > +#define ISL7998x_REG_P0_SW_RESET_CTL ISL7998x_REG(0, 0x02) > +#define ISL7998x_REG_P0_IO_BUFFER_CTLISL7998x_REG(0, 0x03) > +#define ISL7998x_REG_P0_IO_BUFFER_CTL_1_1ISL7998x_REG(0, 0x04) > +#define ISL7998x_REG_P0_IO_PAD_PULL_EN_CTL ISL7998x_REG(0, 0x05) > +#define ISL7998x_REG_P0_IO_BUFFER_CTL_1_2ISL7998x_REG(0, 0x06) > +#define ISL7998x_REG_P0_VIDEO_IN_CHAN_CTLISL7998x_REG(0, 0x07) > +#define ISL7998x_REG_P0_CLK_CTL_1ISL7998x_REG(0, 0x08) > +#define ISL7998x_REG_P0_CLK_CTL_2ISL7998x_REG(0, 0x09) > +#define ISL7998x_REG_P0_CLK_CTL_3ISL7998x_REG(0, 0x0a) > +#define ISL7998x_REG_P0_CLK_CTL_4ISL7998x_REG(0, 0x0b) > +#define ISL7998x_REG_P0_MPP1_SYNC_CTLISL7998x_REG(0, 0x0c) > +#define ISL7998x_REG_P0_MPP2_SYNC_CTLISL7998x_REG(0, 0x0d) > +#define ISL7998x_REG_P0_IRQ_SYNC_CTL ISL7998x_REG(0, 0x0e) > +#define ISL7998x_REG_P0_INTERRUPT_STATUS ISL7998x_REG(0, 0x10) > +#define ISL7998x_REG_P0_CHAN_1_IRQ ISL7998x_REG(0, 0x11) > +#define IS
Re: [PATCH 1/2] media: dt-bindings: Add Intersil ISL7998x DT bindings
Hi Marek,
On Mon, May 20, 2019 at 10:18:11PM +0200, Marek Vasut wrote:
> Add bindings for the Intersil ISL7998x BT656-to-MIPI-CSI2 decoder.
>
> Signed-off-by: Marek Vasut
> Cc: Sakari Ailus
> Cc: Mauro Carvalho Chehab
> Cc: Rob Herring
> Cc: devicet...@vger.kernel.org
> To: linux-media@vger.kernel.org
> ---
> .../bindings/media/i2c/isl7998x.txt | 37 +++
> 1 file changed, 37 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/isl7998x.txt
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/isl7998x.txt
> b/Documentation/devicetree/bindings/media/i2c/isl7998x.txt
> new file mode 100644
> index ..c21703983360
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/isl7998x.txt
> @@ -0,0 +1,37 @@
> +Intersil ISL7998x BT656-to-MIPI-CSI2 decoder
> +
> +The Intersil ISL7998x is a BT656-to-MIPI-CSI decoder which, capable of
> +receiving up to four analog stream and multiplexing them into up to four
> +MIPI CSI2 virtual channels, using one MIPI clock lane and 1/2 data lanes.
> +
> +Required Properties:
> +- compatible: value should be "isil,isl79987"
> +- pd-gpios: a GPIO spec for the Power Down pin (active high)
> +
> +Option Properties:
> +- isil,num-inputs: Number of connected inputs (1, 2 or 4)
The presence of ports describing connected Bt.656 inputs tells this.
> +
> +For further reading on port node refer to
> +Documentation/devicetree/bindings/media/video-interfaces.txt.
Which endpoint properties are relevant for the endpoint(s) in the CSI-2 port?
How about the ports describing the Bt.656 interfaces? You should have
those, too...
> +
> +Example:
> +
> + i2c_master {
> + isl7998x_mipi@44 {
> + compatible = "isil,isl79987";
> + reg = <0x44>;
> + isil,num-inputs = <4>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_videoadc>;
> + pd-gpios = <&gpio3 27 GPIO_ACTIVE_HIGH>;
> + status = "okay";
> +
> + port {
> + isl79987_to_mipi_csi2: endpoint {
> + remote-endpoint = <&mipi_csi2_in>;
> + clock-lanes = <0>;
> + data-lanes = <1 2>;
> + };
> + };
> + };
> + };
>
--
Regards,
Sakari Ailus
sakari.ai...@linux.intel.com
[PATCH 0/5] cec: convert remaining drivers to the new notifier API
This series updates remaining drivers in DRM to use new CEC notifier API. Those complement the "cec: improve notifier support, add connector info" patch series and also replace 2 following patches from there: - [PATCHv8 09/13] dw-hdmi: use cec_notifier_conn_(un)register - [PATCHv9 12/13] tda998x: use cec_notifier_conn_(un)register None of those changes were not tested on a real hardware. Dariusz Marcinkiewicz (5): drm: tda998x: use cec_notifier_conn_(un)register drm: sti: use cec_notifier_conn_(un)register drm: tegra: use cec_notifier_conn_(un)register drm: dw-hdmi: use cec_notifier_conn_(un)register drm: exynos: exynos_hdmi: use cec_notifier_conn_(un)register drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 32 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 33 ++- drivers/gpu/drm/i2c/tda998x_drv.c | 27 --- drivers/gpu/drm/sti/sti_hdmi.c| 20 +- drivers/gpu/drm/tegra/output.c| 18 - 5 files changed, 81 insertions(+), 49 deletions(-) -- 2.22.0.410.gd8fdbe21b5-goog
[PATCH 3/5] drm: tegra: use cec_notifier_conn_(un)register
Use the new cec_notifier_conn_(un)register() functions to
(un)register the notifier for the HDMI connector, and fill in
the cec_connector_info.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/tegra/output.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index 9c2b9dad55c30..ea92e72280868 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -80,6 +80,9 @@ tegra_output_connector_detect(struct drm_connector
*connector, bool force)
void tegra_output_connector_destroy(struct drm_connector *connector)
{
+ struct tegra_output *output = connector_to_output(connector);
+
+ cec_notifier_conn_unregister(output->cec);
drm_connector_unregister(connector);
drm_connector_cleanup(connector);
}
@@ -174,18 +177,11 @@ int tegra_output_probe(struct tegra_output *output)
disable_irq(output->hpd_irq);
}
- output->cec = cec_notifier_get(output->dev);
- if (!output->cec)
- return -ENOMEM;
-
return 0;
}
void tegra_output_remove(struct tegra_output *output)
{
- if (output->cec)
- cec_notifier_put(output->cec);
-
if (gpio_is_valid(output->hpd_gpio)) {
free_irq(output->hpd_irq, output);
gpio_free(output->hpd_gpio);
@@ -197,6 +193,7 @@ void tegra_output_remove(struct tegra_output *output)
int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
{
+ struct cec_connector_info conn_info;
int err;
if (output->panel) {
@@ -212,6 +209,13 @@ int tegra_output_init(struct drm_device *drm, struct
tegra_output *output)
if (gpio_is_valid(output->hpd_gpio))
enable_irq(output->hpd_irq);
+ cec_fill_conn_info_from_drm(&conn_info, &output->connector);
+
+ output->cec = cec_notifier_conn_register(output->dev, NULL, &conn_info);
+ if (!output->cec)
+ return -ENOMEM;
+
+
return 0;
}
--
2.22.0.410.gd8fdbe21b5-goog
[PATCH 1/5] drm: tda998x: use cec_notifier_conn_(un)register
Use the new cec_notifier_conn_(un)register() functions to
(un)register the notifier for the HDMI connector, and fill
in the cec_connector_info.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/i2c/tda998x_drv.c | 27 ++-
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
b/drivers/gpu/drm/i2c/tda998x_drv.c
index 7f34601bb5155..d8d51c6a2d390 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -794,9 +794,14 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data)
if (lvl & CEC_RXSHPDLEV_HPD) {
tda998x_edid_delay_start(priv);
} else {
+ struct cec_notifier *notify;
+
schedule_work(&priv->detect_work);
- cec_notifier_set_phys_addr(priv->cec_notify,
- CEC_PHYS_ADDR_INVALID);
+
+ notify = READ_ONCE(priv->cec_notify);
+ if (notify)
+ cec_notifier_set_phys_addr(notify,
+ CEC_PHYS_ADDR_INVALID);
}
handled = true;
@@ -1253,6 +1258,8 @@ static int tda998x_connector_init(struct tda998x_priv
*priv,
struct drm_device *drm)
{
struct drm_connector *connector = &priv->connector;
+ struct cec_connector_info conn_info;
+ struct cec_notifier *notifier;
int ret;
connector->interlace_allowed = 1;
@@ -1269,6 +1276,14 @@ static int tda998x_connector_init(struct tda998x_priv
*priv,
if (ret)
return ret;
+ cec_fill_conn_info_from_drm(&conn_info, connector);
+
+ notifier = cec_notifier_conn_register(priv->cec_glue.parent,
+ NULL, &conn_info);
+ if (!notifier)
+ return -ENOMEM;
+ WRITE_ONCE(priv->cec_notify, notifier);
+
drm_connector_attach_encoder(&priv->connector,
priv->bridge.encoder);
@@ -1651,7 +1666,7 @@ static void tda998x_destroy(struct device *dev)
i2c_unregister_device(priv->cec);
if (priv->cec_notify)
- cec_notifier_put(priv->cec_notify);
+ cec_notifier_conn_unregister(priv->cec_notify);
}
static int tda998x_create(struct device *dev)
@@ -1776,12 +1791,6 @@ static int tda998x_create(struct device *dev)
cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD);
}
- priv->cec_notify = cec_notifier_get(dev);
- if (!priv->cec_notify) {
- ret = -ENOMEM;
- goto fail;
- }
-
priv->cec_glue.parent = dev;
priv->cec_glue.data = priv;
priv->cec_glue.init = tda998x_cec_hook_init;
--
2.22.0.410.gd8fdbe21b5-goog
[PATCH 4/5] drm: dw-hdmi: use cec_notifier_conn_(un)register
Use the new cec_notifier_conn_(un)register() functions to
(un)register the notifier for the HDMI connector, and fill in
the cec_connector_info.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 32 ++-
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ab7968c8f6a29..b0308ee08f2a1 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2118,6 +2118,8 @@ static int dw_hdmi_bridge_attach(struct drm_bridge
*bridge)
struct dw_hdmi *hdmi = bridge->driver_private;
struct drm_encoder *encoder = bridge->encoder;
struct drm_connector *connector = &hdmi->connector;
+ struct cec_connector_info conn_info;
+ struct cec_notifier *notifier;
connector->interlace_allowed = 1;
connector->polled = DRM_CONNECTOR_POLL_HPD;
@@ -2129,6 +2131,13 @@ static int dw_hdmi_bridge_attach(struct drm_bridge
*bridge)
drm_connector_attach_encoder(connector, encoder);
+ cec_fill_conn_info_from_drm(&conn_info, connector);
+
+ notifier = cec_notifier_conn_register(hdmi->dev, NULL, &conn_info);
+ if (!notifier)
+ return -ENOMEM;
+ WRITE_ONCE(hdmi->cec_notifier, notifier);
+
return 0;
}
@@ -2295,9 +2304,15 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
phy_stat & HDMI_PHY_HPD,
phy_stat & HDMI_PHY_RX_SENSE);
- if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
- cec_notifier_set_phys_addr(hdmi->cec_notifier,
- CEC_PHYS_ADDR_INVALID);
+ if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0) {
+ struct cec_notifier *notifer;
+
+ notifer = READ_ONCE(hdmi->cec_notifier);
+ if (notifer)
+ cec_notifier_set_phys_addr(
+ notifer,
+ CEC_PHYS_ADDR_INVALID);
+ }
}
if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
@@ -2600,12 +2615,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
if (ret)
goto err_iahb;
- hdmi->cec_notifier = cec_notifier_get(dev);
- if (!hdmi->cec_notifier) {
- ret = -ENOMEM;
- goto err_iahb;
- }
-
/*
* To prevent overflows in HDMI_IH_FC_STAT2, set the clk regenerator
* N and cts values before enabling phy
@@ -2693,9 +2702,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
hdmi->ddc = NULL;
}
- if (hdmi->cec_notifier)
- cec_notifier_put(hdmi->cec_notifier);
-
clk_disable_unprepare(hdmi->iahb_clk);
if (hdmi->cec_clk)
clk_disable_unprepare(hdmi->cec_clk);
@@ -2718,7 +2724,7 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0);
if (hdmi->cec_notifier)
- cec_notifier_put(hdmi->cec_notifier);
+ cec_notifier_conn_unregister(hdmi->cec_notifier);
clk_disable_unprepare(hdmi->iahb_clk);
clk_disable_unprepare(hdmi->isfr_clk);
--
2.22.0.410.gd8fdbe21b5-goog
[PATCH 2/5] drm: sti: use cec_notifier_conn_(un)register
Use the new cec_notifier_conn_(un)register() functions to
(un)register the notifier for the HDMI connector, and fill
in the cec_connector_info.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/sti/sti_hdmi.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index 6000df6249807..5519b0c397c72 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -1250,6 +1250,7 @@ static int sti_hdmi_bind(struct device *dev, struct
device *master, void *data)
struct drm_device *drm_dev = data;
struct drm_encoder *encoder;
struct sti_hdmi_connector *connector;
+ struct cec_connector_info conn_info;
struct drm_connector *drm_connector;
struct drm_bridge *bridge;
int err;
@@ -1310,6 +1311,14 @@ static int sti_hdmi_bind(struct device *dev, struct
device *master, void *data)
goto err_sysfs;
}
+ cec_fill_conn_info_from_drm(&conn_info, drm_connector);
+ hdmi->notifier = cec_notifier_conn_register(&hdmi->dev, NULL,
+ &conn_info);
+ if (!hdmi->notifier) {
+ hdmi->drm_connector = NULL;
+ return -ENOMEM;
+ }
+
/* Enable default interrupts */
hdmi_write(hdmi, HDMI_DEFAULT_INT, HDMI_INT_EN);
@@ -1323,6 +1332,10 @@ static int sti_hdmi_bind(struct device *dev, struct
device *master, void *data)
static void sti_hdmi_unbind(struct device *dev,
struct device *master, void *data)
{
+ struct sti_hdmi *hdmi = dev_get_drvdata(dev);
+
+ cec_notifier_set_phys_addr(hdmi->notifier, CEC_PHYS_ADDR_INVALID);
+ cec_notifier_conn_unregister(hdmi->notifier);
}
static const struct component_ops sti_hdmi_ops = {
@@ -1428,10 +1441,6 @@ static int sti_hdmi_probe(struct platform_device *pdev)
goto release_adapter;
}
- hdmi->notifier = cec_notifier_get(&pdev->dev);
- if (!hdmi->notifier)
- goto release_adapter;
-
hdmi->reset = devm_reset_control_get(dev, "hdmi");
/* Take hdmi out of reset */
if (!IS_ERR(hdmi->reset))
@@ -1451,14 +1460,11 @@ static int sti_hdmi_remove(struct platform_device *pdev)
{
struct sti_hdmi *hdmi = dev_get_drvdata(&pdev->dev);
- cec_notifier_set_phys_addr(hdmi->notifier, CEC_PHYS_ADDR_INVALID);
-
i2c_put_adapter(hdmi->ddc_adapt);
if (hdmi->audio_pdev)
platform_device_unregister(hdmi->audio_pdev);
component_del(&pdev->dev, &sti_hdmi_ops);
- cec_notifier_put(hdmi->notifier);
return 0;
}
--
2.22.0.410.gd8fdbe21b5-goog
[PATCH 5/5] drm: exynos: exynos_hdmi: use cec_notifier_conn_(un)register
Use the new cec_notifier_conn_(un)register() functions to
(un)register the notifier for the HDMI connector, and fill in
the cec_connector_info.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/exynos/exynos_hdmi.c | 33 +---
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 19c252f659dd0..dec4149435de1 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -858,6 +858,11 @@ static enum drm_connector_status hdmi_detect(struct
drm_connector *connector,
static void hdmi_connector_destroy(struct drm_connector *connector)
{
+ struct hdmi_context *hdata = connector_to_hdmi(connector);
+
+ cec_notifier_set_phys_addr(hdata->notifier, CEC_PHYS_ADDR_INVALID);
+ cec_notifier_conn_unregister(hdata->notifier);
+
drm_connector_unregister(connector);
drm_connector_cleanup(connector);
}
@@ -941,6 +946,7 @@ static int hdmi_create_connector(struct drm_encoder
*encoder)
{
struct hdmi_context *hdata = encoder_to_hdmi(encoder);
struct drm_connector *connector = &hdata->connector;
+ struct cec_connector_info conn_info;
int ret;
connector->interlace_allowed = true;
@@ -963,6 +969,15 @@ static int hdmi_create_connector(struct drm_encoder
*encoder)
DRM_DEV_ERROR(hdata->dev, "Failed to attach bridge\n");
}
+ cec_fill_conn_info_from_drm(&conn_info, connector);
+
+ hdata->notifier = cec_notifier_conn_register(hdata->dev, NULL,
+&conn_info);
+ if (hdata->notifier == NULL) {
+ ret = -ENOMEM;
+ DRM_DEV_ERROR(hdata->dev, "Failed to allocate CEC notifier\n");
+ }
+
return ret;
}
@@ -1534,8 +1549,9 @@ static void hdmi_disable(struct drm_encoder *encoder)
*/
mutex_unlock(&hdata->mutex);
cancel_delayed_work(&hdata->hotplug_work);
- cec_notifier_set_phys_addr(hdata->notifier,
- CEC_PHYS_ADDR_INVALID);
+ if (hdata->notifier)
+ cec_notifier_set_phys_addr(hdata->notifier,
+ CEC_PHYS_ADDR_INVALID);
return;
}
@@ -2012,12 +2028,6 @@ static int hdmi_probe(struct platform_device *pdev)
}
}
- hdata->notifier = cec_notifier_get(&pdev->dev);
- if (hdata->notifier == NULL) {
- ret = -ENOMEM;
- goto err_hdmiphy;
- }
-
pm_runtime_enable(dev);
audio_infoframe = &hdata->audio.infoframe;
@@ -2029,7 +2039,7 @@ static int hdmi_probe(struct platform_device *pdev)
ret = hdmi_register_audio_device(hdata);
if (ret)
- goto err_notifier_put;
+ goto err_runtime_disable;
ret = component_add(&pdev->dev, &hdmi_component_ops);
if (ret)
@@ -2040,8 +2050,7 @@ static int hdmi_probe(struct platform_device *pdev)
err_unregister_audio:
platform_device_unregister(hdata->audio.pdev);
-err_notifier_put:
- cec_notifier_put(hdata->notifier);
+err_runtime_disable:
pm_runtime_disable(dev);
err_hdmiphy:
@@ -2060,12 +2069,10 @@ static int hdmi_remove(struct platform_device *pdev)
struct hdmi_context *hdata = platform_get_drvdata(pdev);
cancel_delayed_work_sync(&hdata->hotplug_work);
- cec_notifier_set_phys_addr(hdata->notifier, CEC_PHYS_ADDR_INVALID);
component_del(&pdev->dev, &hdmi_component_ops);
platform_device_unregister(hdata->audio.pdev);
- cec_notifier_put(hdata->notifier);
pm_runtime_disable(&pdev->dev);
if (!IS_ERR(hdata->reg_hdmi_en))
--
2.22.0.410.gd8fdbe21b5-goog
Re: [PATCH 2/2] media: i2c: isl7998x: Add driver for Intersil ISL7998x
Hi Marek, Thank you for the patchset --- and my apologies for reviewing it so late. On Mon, May 20, 2019 at 10:18:12PM +0200, Marek Vasut wrote: > Add driver for the Intersil ISL7998x BT656-to-MIPI-CSI2 video decoder. > This chip supports 1/2/4 analog video inputs and converts them into > 1/2/4 VCs in MIPI CSI2 stream. > > This driver currently supports ISL79987 and both 720x480 and 720x576 > resolutions, however as per specification, all inputs must use the > same resolution and standard. The only supported pixel format is now > YUYV/YUV422. The chip should support RGB565 on the CSI2 as well, but > this is currently unsupported. > > Signed-off-by: Marek Vasut > Cc: Sakari Ailus > Cc: Mauro Carvalho Chehab > Cc: Rob Herring > To: linux-media@vger.kernel.org > --- > drivers/media/i2c/Kconfig|6 + > drivers/media/i2c/Makefile |1 + > drivers/media/i2c/isl7998x.c | ++ > 3 files changed, 1118 insertions(+) > create mode 100644 drivers/media/i2c/isl7998x.c > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index 7793358ab8b3..af2ea08d6c59 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -929,6 +929,12 @@ config VIDEO_NOON010PC30 > help > This driver supports NOON010PC30 CIF camera from Siliconfile > > +config VIDEO_ISL7998X > + tristate "Intersil ISL7998x support" > + depends on I2C && VIDEO_V4L2 > + help > + This driver supports Intersil ISL7998x BT656-to-MIPI-CSI2 bridge. > + > source "drivers/media/i2c/m5mols/Kconfig" > > config VIDEO_RJ54N1 > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > index d8ad9dad495d..a66f105a7bcc 100644 > --- a/drivers/media/i2c/Makefile > +++ b/drivers/media/i2c/Makefile > @@ -113,6 +113,7 @@ obj-$(CONFIG_VIDEO_IMX258)+= imx258.o > obj-$(CONFIG_VIDEO_IMX274) += imx274.o > obj-$(CONFIG_VIDEO_IMX319) += imx319.o > obj-$(CONFIG_VIDEO_IMX355) += imx355.o > +obj-$(CONFIG_VIDEO_ISL7998X) += isl7998x.o > obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o > > obj-$(CONFIG_SDR_MAX2175) += max2175.o > diff --git a/drivers/media/i2c/isl7998x.c b/drivers/media/i2c/isl7998x.c > new file mode 100644 > index ..858e98d8b494 > --- /dev/null > +++ b/drivers/media/i2c/isl7998x.c > @@ -0,0 +1, @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Intersil ISL7998x BT656-to-MIPI-CSI2 driver > + * > + * Copyright (C) 2018-2019 Marek Vasut > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include Alphabetical order, please. > + > +#define ISL7998x_WIDTH 720 > +#define ISL7998x_HEIGHT 480 > +#define ISL7998x_INPUTS 4 > + > +#define V4L2_CID_TEST_PATTERN_COLOR (V4L2_CID_USER_BASE | 0x1001) > +#define V4L2_CID_TEST_PATTERN_CHANNELS (V4L2_CID_USER_BASE | 0x1002) > +#define V4L2_CID_TEST_PATTERN_BARS (V4L2_CID_USER_BASE | 0x1003) > + > +#define ISL7998x_REG(page, reg) (((page) << 8) | (reg)) > + > +#define ISL7998x_REG_Pn_SIZE 256 > +#define ISL7998x_REG_Pn_BASE(n) ((n) * > ISL7998x_REG_Pn_SIZE) > + > +#define ISL7998x_REG_Px_DEC_PAGE(page) ISL7998x_REG((page), > 0xff) > +#define ISL7998x_REG_Px_DEC_PAGE_MASK0xf > +#define ISL7998x_REG_P0_PRODUCT_ID_CODE ISL7998x_REG(0, 0x00) > +#define ISL7998x_REG_P0_PRODUCT_REV_CODE ISL7998x_REG(0, 0x01) > +#define ISL7998x_REG_P0_SW_RESET_CTL ISL7998x_REG(0, 0x02) > +#define ISL7998x_REG_P0_IO_BUFFER_CTLISL7998x_REG(0, 0x03) > +#define ISL7998x_REG_P0_IO_BUFFER_CTL_1_1ISL7998x_REG(0, 0x04) > +#define ISL7998x_REG_P0_IO_PAD_PULL_EN_CTL ISL7998x_REG(0, 0x05) > +#define ISL7998x_REG_P0_IO_BUFFER_CTL_1_2ISL7998x_REG(0, 0x06) > +#define ISL7998x_REG_P0_VIDEO_IN_CHAN_CTLISL7998x_REG(0, 0x07) > +#define ISL7998x_REG_P0_CLK_CTL_1ISL7998x_REG(0, 0x08) > +#define ISL7998x_REG_P0_CLK_CTL_2ISL7998x_REG(0, 0x09) > +#define ISL7998x_REG_P0_CLK_CTL_3ISL7998x_REG(0, 0x0a) > +#define ISL7998x_REG_P0_CLK_CTL_4ISL7998x_REG(0, 0x0b) > +#define ISL7998x_REG_P0_MPP1_SYNC_CTLISL7998x_REG(0, 0x0c) > +#define ISL7998x_REG_P0_MPP2_SYNC_CTLISL7998x_REG(0, 0x0d) > +#define ISL7998x_REG_P0_IRQ_SYNC_CTL ISL7998x_REG(0, 0x0e) > +#define ISL7998x_REG_P0_INTERRUPT_STATUS ISL7998x_REG(0, 0x10) > +#define ISL7998x_REG_P0_CHAN_1_IRQ ISL7998x_REG(0, 0x11) > +#define ISL7998x_REG_P0_CHAN_2_IRQ ISL7998x_REG(0, 0x12) > +#define ISL7998x_REG_P0_CHAN_3_IRQ ISL7998x_REG(0, 0x13) > +#define ISL7998x_REG_P0_CHAN_4_IRQ ISL7998x_REG(0, 0x14) > +#define ISL7998x_REG_P0_SHORT_DIAG_IRQ ISL7998x_REG(0, 0x15)
Re: [ANN] Media summit in Lisbon at September
Hi Mauro, On Sun, Jun 30, 2019 at 01:44:04PM -0300, Mauro Carvalho Chehab wrote: > Hi all, > > We are organizing a media mini-summit in Lisbon to happen in September, > at the same week as the Linux Plumber Conference and the Kernel Summit. > > We're still discussing the details about that. > > In principle, it will be a free event for the ones registered > to Linux Plumbers Conference, happening between Sept 9-11. > They have a room available that we could use for the meeting on that > period of time, but we need to adjust to avoid conflicts with other > interesting micro-conferences that will happen in parallel (or > eventually do it outside that period, but that would be harder to > organize). > > I'll let you know more details once we got it. > > If you plan to attend, please let me know. It is open for all, but > we'll have a limited number of seats. So, the earliest we get the > number of interested people, the best. I'd like to attend. > > - > > At the last summit, we were supposed to do a Key Signing Party, but > we end not doing it, due to lack of time. I suggest we add this again, > but doing it earlier, in order to avoid getting out of time for doing > it. > > From my side, I'd like to discuss what criteria we should adopt for > the code that comes via /drivers/staging/media, in particular: > how much time should we keep a code there that doesn't receive any > patch addressing the drivers real issues (excluding codepatch, > cleanups and kAPI changes)? > > What other themes should be discussed? Nothing from my side. I wouldn't mind a brief chat with you about possible dvb improvements. I have started a few things already but it would be great to hear your ideas. Not sure this is interesting for the wider audience. Thanks Sean
Re: Setting up the links for imx7-mipi-csis
Hi Guys, On Sat 29 Jun 2019 at 12:42, Fabio Estevam wrote: > Hi Sakari, > > On Sat, Jun 29, 2019 at 6:13 AM Sakari Ailus wrote: > >> What does media-ctp -p say? > > Thanks for this hint! > > I managed to fix the media-ctl failure by looking at media-ctl -p output. > > Documentation/media/v4l-drivers/imx7.rst mentions "csi_mux", but > media-ctl says "csi-mux", so I needed to change it to: > > media-ctl -l "'imx7-mipi-csis.0':1 -> 'csi-mux':1[1]" > > and then I see no errors after the media-ctl command. > > I will send a patch fixing Documentation/media/v4l-drivers/imx7.rst . > arriving late to the party. Thanks for the fix. yup, there is a typo in there. Thanks Sakari. --- Cheers, Rui
Re: [PATCH] media: imx7.rst: Fix the references to the CSI multiplexer
Oi Fabio,
On Sat 29 Jun 2019 at 13:16, Fabio Estevam wrote:
> In imx7s.dtsi the node name for the CSI multiplexer is "csi-mux", not
> "csi_mux", so fix all the references in the document.
>
> This fixes the following error when the instructions are followed:
>
> # media-ctl -l "'imx7-mipi-csis.0':1 -> 'csi_mux':1[1]"
> Unable to parse link: Invalid argument (22)
Yeah, it was a last minute rename that did not reflect in the
documentation.
>
> While at it, provide the "media-ctl -p" output from 5.2 kernel
> version, so that users can see a more updated output.
Also thanks for this.
>
> Fixes: fa88fbdafb4a ("media: imx7.rst: add documentation for i.MX7 media
> driver")
> Signed-off-by: Fabio Estevam
>
Reviewed-by: Rui Miguel Silva
---
Cheers,
Rui
>
> ---
> Documentation/media/v4l-drivers/imx7.rst | 127 +++
> 1 file changed, 63 insertions(+), 64 deletions(-)
>
> diff --git a/Documentation/media/v4l-drivers/imx7.rst
> b/Documentation/media/v4l-drivers/imx7.rst
> index fe411f65c01c..ab9e17d111bf 100644
> --- a/Documentation/media/v4l-drivers/imx7.rst
> +++ b/Documentation/media/v4l-drivers/imx7.rst
> @@ -41,7 +41,7 @@ data from MIPI CSI-2 camera sensor. It has one source pad,
> corresponding to the
> virtual channel 0. This module is compliant to previous version of Samsung
> D-phy, and supports two D-PHY Rx Data lanes.
>
> -csi_mux
> +csi-mux
> ---
>
> This is the video multiplexer. It has two sink pads to select from either
> camera
> @@ -56,7 +56,7 @@ can interface directly with Parallel and MIPI CSI-2 buses.
> It has 256 x 64 FIFO
> to store received image pixel data and embedded DMA controllers to transfer
> data
> from the FIFO through AHB bus.
>
> -This entity has one sink pad that receives from the csi_mux entity and a
> single
> +This entity has one sink pad that receives from the csi-mux entity and a
> single
> source pad that routes video frames directly to memory buffers. This pad is
> routed to a capture device node.
>
> @@ -81,14 +81,14 @@ an output of 800x600, and BGGR 10 bit bayer format:
>
> # Setup links
> media-ctl -l "'ov2680 1-0036':0 -> 'imx7-mipi-csis.0':0[1]"
> - media-ctl -l "'imx7-mipi-csis.0':1 -> 'csi_mux':1[1]"
> - media-ctl -l "'csi_mux':2 -> 'csi':0[1]"
> + media-ctl -l "'imx7-mipi-csis.0':1 -> 'csi-mux':1[1]"
> + media-ctl -l "'csi-mux':2 -> 'csi':0[1]"
> media-ctl -l "'csi':1 -> 'csi capture':0[1]"
>
> # Configure pads for pipeline
> media-ctl -V "'ov2680 1-0036':0 [fmt:SBGGR10_1X10/800x600 field:none]"
> - media-ctl -V "'csi_mux':1 [fmt:SBGGR10_1X10/800x600 field:none]"
> - media-ctl -V "'csi_mux':2 [fmt:SBGGR10_1X10/800x600 field:none]"
> + media-ctl -V "'csi-mux':1 [fmt:SBGGR10_1X10/800x600 field:none]"
> + media-ctl -V "'csi-mux':2 [fmt:SBGGR10_1X10/800x600 field:none]"
> media-ctl -V "'imx7-mipi-csis.0':0 [fmt:SBGGR10_1X10/800x600 field:none]"
> media-ctl -V "'csi':0 [fmt:SBGGR10_1X10/800x600 field:none]"
>
> @@ -97,64 +97,63 @@ the resolutions supported by the sensor.
>
> .. code-block:: none
>
> -root@imx7s-warp:~# media-ctl -p
> -Media controller API version 4.17.0
> -
> -Media device information
> -
> -driver imx-media
> -model imx-media
> -serial
> -bus info
> -hw revision 0x0
> -driver version 4.17.0
> -
> -Device topology
> -- entity 1: csi (2 pads, 2 links)
> - type V4L2 subdev subtype Unknown flags 0
> - device node name /dev/v4l-subdev0
> - pad0: Sink
> - [fmt:SBGGR10_1X10/800x600 field:none]
> - <- "csi_mux":2 [ENABLED]
> - pad1: Source
> - [fmt:SBGGR10_1X10/800x600 field:none]
> - -> "csi capture":0 [ENABLED]
> -
> -- entity 4: csi capture (1 pad, 1 link)
> - type Node subtype V4L flags 0
> - device node name /dev/video0
> - pad0: Sink
> - <- "csi":1 [ENABLED]
> -
> -- entity 10: csi_mux (3 pads, 2 links)
> - type V4L2 subdev subtype Unknown flags 0
> - device node name /dev/v4l-subdev1
> - pad0: Sink
> - [fmt:unknown/0x0]
> - pad1: Sink
> - [fmt:unknown/800x600 field:none]
> - <- "imx7-mipi-csis.0":1 [ENABLED]
> - pad2: Source
> - [fmt:unknown/800x600 field:none]
> - -> "csi":0 [ENABLED]
> -
> -- entity 14: imx7-mipi-csis.0 (2 pads, 2 links)
> - type V4L2 subdev subtype Unknown flags 0
> - device node name /dev/v4l-subdev2
> - pad0: Sink
> - [fmt:SBGGR10_1X10/800x600 field:none]
> - <- "ov2680 1-0036":0 [ENABLED]
> - pad1: Source
> - [fmt:SBGGR10_1X10/800x600 field:none]
> - -> "csi_mux":1 [ENABLED]
> -
> -- entity 17: ov2680 1-0036 (1 pad, 1 link)
> - type V4L2
Re: [PATCH 2/2] media: i2c: isl7998x: Add driver for Intersil ISL7998x
One more comment...
On Mon, May 20, 2019 at 10:18:12PM +0200, Marek Vasut wrote:
...
> +static int isl7998x_set_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_format *format)
> +{
> + struct isl7998x *isl7998x = sd_to_isl7998x(sd);
> + struct v4l2_mbus_framefmt *mf = &format->format;
> + int ret;
> +
> + if (format->pad != 0)
> + return -EINVAL;
> +
> + if (format->format.width != 720 ||
> + (format->format.height != 480 && format->format.height != 576))
> + return -EINVAL;
> +
> + if (format->format.code != MEDIA_BUS_FMT_YUYV8_2X8)
> + return -EINVAL;
> +
> + mf->width = format->format.width;
> + mf->height = format->format.height;
> + mf->code= format->format.code;
> + mf->field = V4L2_FIELD_INTERLACED;
> + mf->colorspace = 0;
> +
> + if (format->which != V4L2_SUBDEV_FORMAT_TRY) {
> + ret = isl7998x_update_std(isl7998x);
> + if (ret)
> + return ret;
> + mf->width = isl7998x->width;
> + mf->height = isl7998x->height;
> + isl7998x->fmt = &isl7998x_colour_fmts[0];
> + }
Note that as the driver exposes a sub-device node to the user space, it's
responsible for serialising the access to its own data structures that are
accessed by other drivers or through its uAPI nodes. Most drivers use the
same mutex as the control handler, as controls usually deal with at least
some of the same data.
--
Regards,
Sakari Ailus
sakari.ai...@linux.intel.com
regarding commit "v4l2-ctl: get fmt after source change" in v4l-utils repository
commit 84219e2b5d013709ee5259621715966c46eec746
Author: Hans Verkuil
Date: Sat Mar 30 14:16:25 2019 +0100
v4l2-ctl: get fmt after source change
When a source change event arrives during decoding get the new
format at that point instead of after restarting streaming.
If there is another source change queued up, then when you call
streamon for CAPTURE again it might send the new source change
event and update the format for that one, so reading the format
after streamon might give the wrong format.
Signed-off-by: Hans Verkuil
diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
index bb656584..3695a027 100644
--- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
@@ -2363,12 +2363,11 @@ static void stateful_m2m(cv4l_fd &fd,
cv4l_queue &in, cv4l_queue &out,
if (in_source_change_event) {
in_source_change_event = false;
last_buffer = false;
+ fd.g_fmt(fmt_in, in.g_type());
if (capture_setup(fd, in, exp_fd_p))
return;
fps_ts[CAP].reset();
fps_ts[OUT].reset();
- fd.g_fmt(fmt_out, out.g_type());
- fd.g_fmt(fmt_in, in.g_type());
Removing those lines cause inconsistency when the user send the wanted
capture pixel format when decoding with the `v4l2-ctl` command. In
this case the value of `-v pixelformat=...` is updated in the kernel
with the capture_setup function but it is not updated in the fmt_in
variable and so the command will try to save the decoded video in a
different format from what is configured in the kernel.
cap_streaming = true;
} else {
break;
Dafna
Re: [PATCH v3] keytable: Add keymap test
On Fri, Jun 28, 2019 at 11:45:29AM +0200, Bastien Nocera wrote:
> This new test will try to parse all the ".toml" files in the directory
> path passed to it, error'ing out on the first parsing problem.
>
> Run as "make check" in the keytable directory.
>
> Signed-off-by: Bastien Nocera
> ---
> Changes since v2:
> - Added SoB
What about the other comments?
>
> Changes since v1:
> - Fix patch formatting
>
> At least 4 keymaps look broken in the current git:
> it913x_v2.toml
> pinnacle310e.toml
> hisi_poplar.toml
> imon_mce.toml
>
> Let me know if you want patches to remove the duplicate entries from
> those.
Actually we need those patches before we can merge this.
>
> utils/keytable/Makefile.am| 6
> utils/keytable/test_keymaps.c | 68 +++
> 2 files changed, 74 insertions(+)
> create mode 100644 utils/keytable/test_keymaps.c
>
> diff --git a/utils/keytable/Makefile.am b/utils/keytable/Makefile.am
> index 148b9446..086d53c2 100644
> --- a/utils/keytable/Makefile.am
> +++ b/utils/keytable/Makefile.am
> @@ -1,9 +1,12 @@
> bin_PROGRAMS = ir-keytable
> +noinst_PROGRAMS = test-keymaps
> man_MANS = ir-keytable.1 rc_keymap.5
> sysconf_DATA = rc_maps.cfg
> keytablesystem_DATA = $(srcdir)/rc_keymaps/*
> udevrules_DATA = 70-infrared.rules
>
> +test_keymaps_SOURCES = toml.c toml.h test_keymaps.c
> +
> ir_keytable_SOURCES = keytable.c parse.h ir-encode.c ir-encode.h toml.c
> toml.h
>
> if WITH_BPF
> @@ -21,6 +24,9 @@ endif
> EXTRA_DIST = 70-infrared.rules rc_keymaps rc_keymaps_userspace
> gen_keytables.pl ir-keytable.1 rc_maps.cfg rc_keymap.5
>
> # custom target
> +check: test-keymaps
> + $(builddir)/test-keymaps $(srcdir)/rc_keymaps/
> +
> install-data-local:
> $(install_sh) -d "$(DESTDIR)$(keytableuserdir)"
>
> diff --git a/utils/keytable/test_keymaps.c b/utils/keytable/test_keymaps.c
> new file mode 100644
> index ..23084331
> --- /dev/null
> +++ b/utils/keytable/test_keymaps.c
> @@ -0,0 +1,68 @@
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "toml.h"
> +
> +static int
> +has_suffix(const char *str, const char *suffix)
> +{
> + if (strlen(str) < strlen(suffix))
> + return 0;
> + if (strncmp(str + strlen(str) - strlen(suffix), suffix, strlen(suffix))
> == 0)
> + return 1;
> + return 0;
> +}
> +
> +int main (int argc, char **argv)
> +{
> + DIR *dir;
> + struct dirent *entry;
> +
> + if (argc != 2) {
> + fprintf(stderr, "Usage: %s KEYMAPS-DIRECTORY\n", argv[0]);
> + return 1;
> + }
> +
> + dir = opendir(argv[1]);
> + if (!dir) {
> + perror("Could not open directory");
> + return 1;
> + }
> +
> + while ((entry = readdir(dir)) != NULL) {
> + struct toml_table_t *root;
> + FILE *fin;
> + char buf[200];
> + char path[2048];
> +
> + if (!has_suffix(entry->d_name, ".toml")) {
> + /* Skipping file */
> + continue;
> + }
> +
> + memset(path, 0, sizeof(path));
> + strcpy(path, argv[1]);
> + strcpy(path + strlen(argv[1]), "/");
> + strcpy(path + strlen(argv[1]) + 1, entry->d_name);
> + strcpy(path + strlen(argv[1]) + 1 + strlen(entry->d_name),
> "\0");
> +
> + fin = fopen(path, "r");
> + if (!fin) {
> + fprintf(stderr, "Could not open file %s: %s", path,
> strerror(errno));
> + return 1;
> + }
> +
> + root = toml_parse_file(fin, buf, sizeof(buf));
> + fclose(fin);
> + if (!root) {
> + fprintf(stderr, "Failed to parse %s: %s\n", path, buf);
> + return 1;
> + }
> + toml_free(root);
> + }
> +
> + return 0;
> +}
Re: [PATCH v3] keytable: Add keymap test
On Mon, 2019-07-01 at 12:28 +0100, Sean Young wrote: > On Fri, Jun 28, 2019 at 11:45:29AM +0200, Bastien Nocera wrote: > > This new test will try to parse all the ".toml" files in the > > directory > > path passed to it, error'ing out on the first parsing problem. > > > > Run as "make check" in the keytable directory. > > > > Signed-off-by: Bastien Nocera > > --- > > Changes since v2: > > - Added SoB > > What about the other comments? Please mention as early as possible in your review mails that there's more comments below. I can't guess that from the mail you sent, see the screenshot attached.
Re: [PATCH v2] keytable: Add keymap test
On Thu, 2019-06-27 at 20:33 +0100, Sean Young wrote:
> On Thu, Jun 27, 2019 at 10:13:56AM +0200, Bastien Nocera wrote:
> > This new test will try to parse all the ".toml" files in the
> > directory
> > path passed to it, error'ing out on the first parsing problem.
> >
> > Run as "make check" in the keytable directory.
>
> Good catch, and I like your solution.
>
> It needs a Signed-off-by.
>
> > ---
> > Changes since v1:
> > - Fix patch formatting
> >
> > At least 4 keymaps look broken in the current git:
> > it913x_v2.toml
> > pinnacle310e.toml
> > hisi_poplar.toml
> > imon_mce.toml
> >
> > Let me know if you want patches to remove the duplicate entries
> > from
> > those.
>
> That would be great. They have to be patched in the kernel tree, they
> are generated from there.
It's customary to put a comment at the top of generated files
indicating that they shouldn't be modified and list the name and
version of the tools used to generate that file.
So, what's the name of the tool used, and where does it live? :)
> > utils/keytable/Makefile.am| 6
> > utils/keytable/test_keymaps.c | 68
> > +++
> > 2 files changed, 74 insertions(+)
> > create mode 100644 utils/keytable/test_keymaps.c
> >
> > diff --git a/utils/keytable/Makefile.am
> > b/utils/keytable/Makefile.am
> > index 148b9446..086d53c2 100644
> > --- a/utils/keytable/Makefile.am
> > +++ b/utils/keytable/Makefile.am
> > @@ -1,9 +1,12 @@
> > bin_PROGRAMS = ir-keytable
> > +noinst_PROGRAMS = test-keymaps
> > man_MANS = ir-keytable.1 rc_keymap.5
> > sysconf_DATA = rc_maps.cfg
> > keytablesystem_DATA = $(srcdir)/rc_keymaps/*
> > udevrules_DATA = 70-infrared.rules
> >
> > +test_keymaps_SOURCES = toml.c toml.h test_keymaps.c
> > +
>
> It could be called check keymaps in line with the make target.
I usually call tests "test" as they aren't beholden to the "check"
target to be run, but up to you.
> > ir_keytable_SOURCES = keytable.c parse.h ir-encode.c ir-encode.h
> > toml.c toml.h
> >
> > if WITH_BPF
> > @@ -21,6 +24,9 @@ endif
> > EXTRA_DIST = 70-infrared.rules rc_keymaps rc_keymaps_userspace
> > gen_keytables.pl ir-keytable.1 rc_maps.cfg rc_keymap.5
> >
> > # custom target
> > +check: test-keymaps
> > + $(builddir)/test-keymaps $(srcdir)/rc_keymaps/
> > +
> > install-data-local:
> > $(install_sh) -d "$(DESTDIR)$(keytableuserdir)"
> >
> > diff --git a/utils/keytable/test_keymaps.c
> > b/utils/keytable/test_keymaps.c
> > new file mode 100644
> > index ..23084331
> > --- /dev/null
> > +++ b/utils/keytable/test_keymaps.c
> > @@ -0,0 +1,68 @@
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "toml.h"
> > +
> > +static int
> > +has_suffix(const char *str, const char *suffix)
> > +{
> > + if (strlen(str) < strlen(suffix))
> > + return 0;
> > + if (strncmp(str + strlen(str) - strlen(suffix), suffix,
> > strlen(suffix)) == 0)
> > + return 1;
> > + return 0;
> > +}
> > +
> > +int main (int argc, char **argv)
> > +{
> > + DIR *dir;
> > + struct dirent *entry;
> > +
> > + if (argc != 2) {
> > + fprintf(stderr, "Usage: %s KEYMAPS-DIRECTORY\n",
> > argv[0]);
> > + return 1;
> > + }
> > +
> > + dir = opendir(argv[1]);
> > + if (!dir) {
> > + perror("Could not open directory");
> > + return 1;
> > + }
> > +
> > + while ((entry = readdir(dir)) != NULL) {
> > + struct toml_table_t *root;
> > + FILE *fin;
> > + char buf[200];
> > + char path[2048];
> > +
> > + if (!has_suffix(entry->d_name, ".toml")) {
> > + /* Skipping file */
> > + continue;
> > + }
> > +
> > + memset(path, 0, sizeof(path));
> > + strcpy(path, argv[1]);
> > + strcpy(path + strlen(argv[1]), "/");
> > + strcpy(path + strlen(argv[1]) + 1, entry->d_name);
> > + strcpy(path + strlen(argv[1]) + 1 + strlen(entry-
> > >d_name), "\0");
>
> These five lines could be replaced with a single snprintf().
You're right, it's marginally better.
> > +
> > + fin = fopen(path, "r");
> > + if (!fin) {
> > + fprintf(stderr, "Could not open file %s: %s",
> > path, strerror(errno));
> > + return 1;
> > + }
> > +
> > + root = toml_parse_file(fin, buf, sizeof(buf));
> > + fclose(fin);
> > + if (!root) {
> > + fprintf(stderr, "Failed to parse %s: %s\n",
> > path, buf);
> > + return 1;
> > + }
> > + toml_free(root);
> > + }
> > +
> > + return 0;
> > +}
>
> Great idea!
>
> Thanks
> Sean
[PATCH v2 2/5] drm: sti: use cec_notifier_conn_(un)register
Use the new cec_notifier_conn_(un)register() functions to
(un)register the notifier for the HDMI connector, and fill
in the cec_connector_info.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/sti/sti_hdmi.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index 6000df6249807..5519b0c397c72 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -1250,6 +1250,7 @@ static int sti_hdmi_bind(struct device *dev, struct
device *master, void *data)
struct drm_device *drm_dev = data;
struct drm_encoder *encoder;
struct sti_hdmi_connector *connector;
+ struct cec_connector_info conn_info;
struct drm_connector *drm_connector;
struct drm_bridge *bridge;
int err;
@@ -1310,6 +1311,14 @@ static int sti_hdmi_bind(struct device *dev, struct
device *master, void *data)
goto err_sysfs;
}
+ cec_fill_conn_info_from_drm(&conn_info, drm_connector);
+ hdmi->notifier = cec_notifier_conn_register(&hdmi->dev, NULL,
+ &conn_info);
+ if (!hdmi->notifier) {
+ hdmi->drm_connector = NULL;
+ return -ENOMEM;
+ }
+
/* Enable default interrupts */
hdmi_write(hdmi, HDMI_DEFAULT_INT, HDMI_INT_EN);
@@ -1323,6 +1332,10 @@ static int sti_hdmi_bind(struct device *dev, struct
device *master, void *data)
static void sti_hdmi_unbind(struct device *dev,
struct device *master, void *data)
{
+ struct sti_hdmi *hdmi = dev_get_drvdata(dev);
+
+ cec_notifier_set_phys_addr(hdmi->notifier, CEC_PHYS_ADDR_INVALID);
+ cec_notifier_conn_unregister(hdmi->notifier);
}
static const struct component_ops sti_hdmi_ops = {
@@ -1428,10 +1441,6 @@ static int sti_hdmi_probe(struct platform_device *pdev)
goto release_adapter;
}
- hdmi->notifier = cec_notifier_get(&pdev->dev);
- if (!hdmi->notifier)
- goto release_adapter;
-
hdmi->reset = devm_reset_control_get(dev, "hdmi");
/* Take hdmi out of reset */
if (!IS_ERR(hdmi->reset))
@@ -1451,14 +1460,11 @@ static int sti_hdmi_remove(struct platform_device *pdev)
{
struct sti_hdmi *hdmi = dev_get_drvdata(&pdev->dev);
- cec_notifier_set_phys_addr(hdmi->notifier, CEC_PHYS_ADDR_INVALID);
-
i2c_put_adapter(hdmi->ddc_adapt);
if (hdmi->audio_pdev)
platform_device_unregister(hdmi->audio_pdev);
component_del(&pdev->dev, &sti_hdmi_ops);
- cec_notifier_put(hdmi->notifier);
return 0;
}
--
2.22.0.410.gd8fdbe21b5-goog
[PATCH v2 1/5] drm: tda998x: use cec_notifier_conn_(un)register
Use the new cec_notifier_conn_(un)register() functions to
(un)register the notifier for the HDMI connector, and fill
in the cec_connector_info.
Changes since v1:
Add memory barrier to make sure that the notifier
becomes visible to the irq thread once it is
fully constructed.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/i2c/tda998x_drv.c | 32 ++-
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
b/drivers/gpu/drm/i2c/tda998x_drv.c
index 7f34601bb5155..7844f4113a839 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -794,9 +794,14 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data)
if (lvl & CEC_RXSHPDLEV_HPD) {
tda998x_edid_delay_start(priv);
} else {
+ struct cec_notifier *notify;
+
schedule_work(&priv->detect_work);
- cec_notifier_set_phys_addr(priv->cec_notify,
- CEC_PHYS_ADDR_INVALID);
+
+ notify = READ_ONCE(priv->cec_notify);
+ if (notify)
+ cec_notifier_set_phys_addr(notify,
+ CEC_PHYS_ADDR_INVALID);
}
handled = true;
@@ -1253,6 +1258,8 @@ static int tda998x_connector_init(struct tda998x_priv
*priv,
struct drm_device *drm)
{
struct drm_connector *connector = &priv->connector;
+ struct cec_connector_info conn_info;
+ struct cec_notifier *notifier;
int ret;
connector->interlace_allowed = 1;
@@ -1269,6 +1276,19 @@ static int tda998x_connector_init(struct tda998x_priv
*priv,
if (ret)
return ret;
+ cec_fill_conn_info_from_drm(&conn_info, connector);
+
+ notifier = cec_notifier_conn_register(priv->cec_glue.parent,
+ NULL, &conn_info);
+ if (!notifier)
+ return -ENOMEM;
+ /*
+* Make sure that tda998x_irq_thread does see the notifier
+* when it fully constructed.
+*/
+ smp_wmb();
+ priv->cec_notify = notifier;
+
drm_connector_attach_encoder(&priv->connector,
priv->bridge.encoder);
@@ -1651,7 +1671,7 @@ static void tda998x_destroy(struct device *dev)
i2c_unregister_device(priv->cec);
if (priv->cec_notify)
- cec_notifier_put(priv->cec_notify);
+ cec_notifier_conn_unregister(priv->cec_notify);
}
static int tda998x_create(struct device *dev)
@@ -1776,12 +1796,6 @@ static int tda998x_create(struct device *dev)
cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD);
}
- priv->cec_notify = cec_notifier_get(dev);
- if (!priv->cec_notify) {
- ret = -ENOMEM;
- goto fail;
- }
-
priv->cec_glue.parent = dev;
priv->cec_glue.data = priv;
priv->cec_glue.init = tda998x_cec_hook_init;
--
2.22.0.410.gd8fdbe21b5-goog
[PATCH v2 0/5] cec: convert remaining drivers to the new notifier API
This series updates remaining drivers in DRM to use new CEC notifier API. Those complement the "cec: improve notifier support, add connector info" patch series and also replace 2 following patches from there: - [PATCHv8 09/13] dw-hdmi: use cec_notifier_conn_(un)register - [PATCHv9 12/13] tda998x: use cec_notifier_conn_(un)register None of those changes were not tested on a real hardware. Changes since v1: Those patches delay creation of notifiers until respective connectors are constructed. It seems that those patches, for a couple of drivers, by adding the delay, introduce a race between notifiers' creation and the IRQs handling threads - at least I don't see anything obvious in there that would explicitly forbid such races to occur. v2 adds a write barrier to make sure IRQ threads see the notifier once it is created (replacing the WRITE_ONCE I put in v1). The best thing to do here, I believe, would be not to have any synchronization and make sure that an IRQ only gets enabled after the notifier is created. Dariusz Marcinkiewicz (5): drm: tda998x: use cec_notifier_conn_(un)register drm: sti: use cec_notifier_conn_(un)register drm: tegra: use cec_notifier_conn_(un)register drm: dw-hdmi: use cec_notifier_conn_(un)register drm: exynos: exynos_hdmi: use cec_notifier_conn_(un)register drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 37 +++ drivers/gpu/drm/exynos/exynos_hdmi.c | 33 drivers/gpu/drm/i2c/tda998x_drv.c | 32 ++-- drivers/gpu/drm/sti/sti_hdmi.c| 20 +++- drivers/gpu/drm/tegra/output.c| 18 ++- 5 files changed, 91 insertions(+), 49 deletions(-) -- 2.22.0.410.gd8fdbe21b5-goog
[PATCH v2 4/5] drm: dw-hdmi: use cec_notifier_conn_(un)register
Use the new cec_notifier_conn_(un)register() functions to
(un)register the notifier for the HDMI connector, and fill in
the cec_connector_info.
Changes since v1:
Add memory barrier to make sure that the notifier
becomes visible to the irq thread once it is fully
constructed.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 37 +++
1 file changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ab7968c8f6a29..c0f4eb3c12b18 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2118,6 +2118,8 @@ static int dw_hdmi_bridge_attach(struct drm_bridge
*bridge)
struct dw_hdmi *hdmi = bridge->driver_private;
struct drm_encoder *encoder = bridge->encoder;
struct drm_connector *connector = &hdmi->connector;
+ struct cec_connector_info conn_info;
+ struct cec_notifier *notifier;
connector->interlace_allowed = 1;
connector->polled = DRM_CONNECTOR_POLL_HPD;
@@ -2129,6 +2131,18 @@ static int dw_hdmi_bridge_attach(struct drm_bridge
*bridge)
drm_connector_attach_encoder(connector, encoder);
+ cec_fill_conn_info_from_drm(&conn_info, connector);
+
+ notifier = cec_notifier_conn_register(hdmi->dev, NULL, &conn_info);
+ if (!notifier)
+ return -ENOMEM;
+ /*
+* Make sure that dw_hdmi_irq thread does see the notifier
+* when it fully constructed.
+*/
+ smp_wmb();
+ hdmi->cec_notifier = notifier;
+
return 0;
}
@@ -2295,9 +2309,15 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
phy_stat & HDMI_PHY_HPD,
phy_stat & HDMI_PHY_RX_SENSE);
- if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
- cec_notifier_set_phys_addr(hdmi->cec_notifier,
- CEC_PHYS_ADDR_INVALID);
+ if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0) {
+ struct cec_notifier *notifer;
+
+ notifer = READ_ONCE(hdmi->cec_notifier);
+ if (notifer)
+ cec_notifier_set_phys_addr(
+ notifer,
+ CEC_PHYS_ADDR_INVALID);
+ }
}
if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
@@ -2600,12 +2620,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
if (ret)
goto err_iahb;
- hdmi->cec_notifier = cec_notifier_get(dev);
- if (!hdmi->cec_notifier) {
- ret = -ENOMEM;
- goto err_iahb;
- }
-
/*
* To prevent overflows in HDMI_IH_FC_STAT2, set the clk regenerator
* N and cts values before enabling phy
@@ -2693,9 +2707,6 @@ __dw_hdmi_probe(struct platform_device *pdev,
hdmi->ddc = NULL;
}
- if (hdmi->cec_notifier)
- cec_notifier_put(hdmi->cec_notifier);
-
clk_disable_unprepare(hdmi->iahb_clk);
if (hdmi->cec_clk)
clk_disable_unprepare(hdmi->cec_clk);
@@ -2718,7 +2729,7 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi)
hdmi_writeb(hdmi, ~0, HDMI_IH_MUTE_PHY_STAT0);
if (hdmi->cec_notifier)
- cec_notifier_put(hdmi->cec_notifier);
+ cec_notifier_conn_unregister(hdmi->cec_notifier);
clk_disable_unprepare(hdmi->iahb_clk);
clk_disable_unprepare(hdmi->isfr_clk);
--
2.22.0.410.gd8fdbe21b5-goog
[PATCH v2 3/5] drm: tegra: use cec_notifier_conn_(un)register
Use the new cec_notifier_conn_(un)register() functions to
(un)register the notifier for the HDMI connector, and fill in
the cec_connector_info.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/tegra/output.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index 9c2b9dad55c30..ea92e72280868 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -80,6 +80,9 @@ tegra_output_connector_detect(struct drm_connector
*connector, bool force)
void tegra_output_connector_destroy(struct drm_connector *connector)
{
+ struct tegra_output *output = connector_to_output(connector);
+
+ cec_notifier_conn_unregister(output->cec);
drm_connector_unregister(connector);
drm_connector_cleanup(connector);
}
@@ -174,18 +177,11 @@ int tegra_output_probe(struct tegra_output *output)
disable_irq(output->hpd_irq);
}
- output->cec = cec_notifier_get(output->dev);
- if (!output->cec)
- return -ENOMEM;
-
return 0;
}
void tegra_output_remove(struct tegra_output *output)
{
- if (output->cec)
- cec_notifier_put(output->cec);
-
if (gpio_is_valid(output->hpd_gpio)) {
free_irq(output->hpd_irq, output);
gpio_free(output->hpd_gpio);
@@ -197,6 +193,7 @@ void tegra_output_remove(struct tegra_output *output)
int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
{
+ struct cec_connector_info conn_info;
int err;
if (output->panel) {
@@ -212,6 +209,13 @@ int tegra_output_init(struct drm_device *drm, struct
tegra_output *output)
if (gpio_is_valid(output->hpd_gpio))
enable_irq(output->hpd_irq);
+ cec_fill_conn_info_from_drm(&conn_info, &output->connector);
+
+ output->cec = cec_notifier_conn_register(output->dev, NULL, &conn_info);
+ if (!output->cec)
+ return -ENOMEM;
+
+
return 0;
}
--
2.22.0.410.gd8fdbe21b5-goog
[PATCH v2 5/5] drm: exynos: exynos_hdmi: use cec_notifier_conn_(un)register
Use the new cec_notifier_conn_(un)register() functions to
(un)register the notifier for the HDMI connector, and fill in
the cec_connector_info.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/exynos/exynos_hdmi.c | 33 +---
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 19c252f659dd0..dec4149435de1 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -858,6 +858,11 @@ static enum drm_connector_status hdmi_detect(struct
drm_connector *connector,
static void hdmi_connector_destroy(struct drm_connector *connector)
{
+ struct hdmi_context *hdata = connector_to_hdmi(connector);
+
+ cec_notifier_set_phys_addr(hdata->notifier, CEC_PHYS_ADDR_INVALID);
+ cec_notifier_conn_unregister(hdata->notifier);
+
drm_connector_unregister(connector);
drm_connector_cleanup(connector);
}
@@ -941,6 +946,7 @@ static int hdmi_create_connector(struct drm_encoder
*encoder)
{
struct hdmi_context *hdata = encoder_to_hdmi(encoder);
struct drm_connector *connector = &hdata->connector;
+ struct cec_connector_info conn_info;
int ret;
connector->interlace_allowed = true;
@@ -963,6 +969,15 @@ static int hdmi_create_connector(struct drm_encoder
*encoder)
DRM_DEV_ERROR(hdata->dev, "Failed to attach bridge\n");
}
+ cec_fill_conn_info_from_drm(&conn_info, connector);
+
+ hdata->notifier = cec_notifier_conn_register(hdata->dev, NULL,
+&conn_info);
+ if (hdata->notifier == NULL) {
+ ret = -ENOMEM;
+ DRM_DEV_ERROR(hdata->dev, "Failed to allocate CEC notifier\n");
+ }
+
return ret;
}
@@ -1534,8 +1549,9 @@ static void hdmi_disable(struct drm_encoder *encoder)
*/
mutex_unlock(&hdata->mutex);
cancel_delayed_work(&hdata->hotplug_work);
- cec_notifier_set_phys_addr(hdata->notifier,
- CEC_PHYS_ADDR_INVALID);
+ if (hdata->notifier)
+ cec_notifier_set_phys_addr(hdata->notifier,
+ CEC_PHYS_ADDR_INVALID);
return;
}
@@ -2012,12 +2028,6 @@ static int hdmi_probe(struct platform_device *pdev)
}
}
- hdata->notifier = cec_notifier_get(&pdev->dev);
- if (hdata->notifier == NULL) {
- ret = -ENOMEM;
- goto err_hdmiphy;
- }
-
pm_runtime_enable(dev);
audio_infoframe = &hdata->audio.infoframe;
@@ -2029,7 +2039,7 @@ static int hdmi_probe(struct platform_device *pdev)
ret = hdmi_register_audio_device(hdata);
if (ret)
- goto err_notifier_put;
+ goto err_runtime_disable;
ret = component_add(&pdev->dev, &hdmi_component_ops);
if (ret)
@@ -2040,8 +2050,7 @@ static int hdmi_probe(struct platform_device *pdev)
err_unregister_audio:
platform_device_unregister(hdata->audio.pdev);
-err_notifier_put:
- cec_notifier_put(hdata->notifier);
+err_runtime_disable:
pm_runtime_disable(dev);
err_hdmiphy:
@@ -2060,12 +2069,10 @@ static int hdmi_remove(struct platform_device *pdev)
struct hdmi_context *hdata = platform_get_drvdata(pdev);
cancel_delayed_work_sync(&hdata->hotplug_work);
- cec_notifier_set_phys_addr(hdata->notifier, CEC_PHYS_ADDR_INVALID);
component_del(&pdev->dev, &hdmi_component_ops);
platform_device_unregister(hdata->audio.pdev);
- cec_notifier_put(hdata->notifier);
pm_runtime_disable(&pdev->dev);
if (!IS_ERR(hdata->reg_hdmi_en))
--
2.22.0.410.gd8fdbe21b5-goog
[PATCH 1/2] keytable: Add source information in generated keymaps
Add comments to mention that keymap files are generated, and that they shouldn't be modified by hand. Also list which tool was used to generate them and the kernel source filename. Signed-off-by: Bastien Nocera --- utils/keytable/gen_keytables.pl | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/utils/keytable/gen_keytables.pl b/utils/keytable/gen_keytables.pl index 4124e366..3dc74ba6 100755 --- a/utils/keytable/gen_keytables.pl +++ b/utils/keytable/gen_keytables.pl @@ -36,10 +36,17 @@ sub flush($$) my $filename = shift; my $legacy = shift; my $defined; + my $relative_filename = $filename; return if (!$keyname || !$out); - print "Creating $dir/$keyname.toml\n"; + $relative_filename =~ s/^$kernel_dir//; + $relative_filename =~ s/^\///; + print "Creating $dir/$keyname.toml from $relative_filename\n"; open OUT, ">$dir/$keyname.toml"; + print OUT "# This file is a generated data file, do not modify manually\n"; + print OUT "#\n"; + print OUT "# Generated with gen_keytables.pl in v4l-utils\n"; + print OUT "# using $relative_filename as a source file\n"; print OUT "[[protocols]]\n"; print OUT "name = \"$keyname\"\n"; print OUT "protocol = \"$type\"\n"; -- 2.21.0
[PATCH 2/2] keytable: Remove comments before processing keymaps
Do our best to remove comments from each line we process from the keymap
sources, so as to avoid commented duplicates and false positives
sneaking in to the keymap definitions.
Signed-off-by: Bastien Nocera
---
utils/keytable/gen_keytables.pl | 3 +++
1 file changed, 3 insertions(+)
diff --git a/utils/keytable/gen_keytables.pl b/utils/keytable/gen_keytables.pl
index 3dc74ba6..d73daf58 100755
--- a/utils/keytable/gen_keytables.pl
+++ b/utils/keytable/gen_keytables.pl
@@ -138,6 +138,9 @@ sub parse_file($$)
}
if ($read) {
+ # Remove comments
+ ~ s#/\*.*?\*/##sg;
+ ~ s#.*\*/##sg;
if (m/(0x[\dA-Fa-f]+)[\s\,]+(KEY|BTN)(\_[^\s\,\}]+)/) {
$out .= "$1 = \"$2$3\"\n";
next;
--
2.21.0
Keymap with duplicate entries
Hey Aapo,
There seems to be a problem in the keymap you provided for inclusion
in the kernel.
In drivers/media/usb/dvb-usb/m920x.c, 2 keys are triggered by the same
keycode:
790 ↦ { 0x08, KEY_SUBTITLE },•
794 ↦ { 0x08, KEY_VIDEO },↦ ↦ /* A/V */•
Could you please check what the correct values are for both of those
keys?
Thanks!
Keymap with duplicate entries
Hey Malcolm,
There seems to be a problem in the keymap you provided for inclusion
in the kernel.
In media/rc/keymaps/rc-it913x-v2.c, 2 keys are triggered by the same
keycode:
53 ↦ { 0x866b18, KEY_VOLUMEDOWN },•
55 ↦ { 0x866b18, KEY_CHANNELDOWN },•
Could you please check what the correct values are for both of those
keys?
Thanks!
Keymap with duplicate entries
Hey Younian,
There seems to be a problem in the keymap you provided for inclusion
in the kernel.
In drivers/media/rc/keymaps/rc-hisi-poplar.c, 2 keys are triggered by
the same keycode:
27 ↦ { 0xb2c5, KEY_DELETE},•
34 ↦ { 0xb2c5, KEY_BACK},•
Could you please check what the correct values are for both of those
keys?
Thanks!
Re: [PATCH v2] keytable: Add keymap test
On Mon, 2019-07-01 at 16:27 +0200, Bastien Nocera wrote: > > That would be great. They have to be patched in the kernel tree, > > they > > are generated from there. > > It's customary to put a comment at the top of generated files > indicating that they shouldn't be modified and list the name and > version of the tools used to generate that file. > > So, what's the name of the tool used, and where does it live? :) Apparently it's called gen_keytables.pl, and I've sent a couple of patches to it because it didn't ignore comments, and then sent a couple of mails to broken definitions in the kernel. Once the 3 broken definitions in the kernel are fixed, fixing all the keymaps in v4l-utils will be as easy as regenerating the keymaps, and I'll send the updated patch for the "check" target. Don't forget to remove the files before generating them again, a number of keymaps were removed from the upstream kernel. Cheers
Re: Keymap with duplicate entries
On Mon, 2019-07-01 at 18:38 +0200, Bastien Nocera wrote:
> Hey Aapo,
>
> There seems to be a problem in the keymap you provided for inclusion
> in the kernel.
>
> In drivers/media/usb/dvb-usb/m920x.c, 2 keys are triggered by the
> same
> keycode:
> 790 ↦ { 0x08, KEY_SUBTITLE },•
> 794 ↦ { 0x08, KEY_VIDEO },↦ ↦ /* A/V */•
>
> Could you please check what the correct values are for both of those
> keys?
Apparently Aapo's address bounced hard. If someone knows how to contact
them.
Cheers
Re: [PATCH for v5.3] v4l2-subdev: fix regression in check_pad()
Hi Sakari,
On Monday, July 1, 2019 9:50:55 A.M. CEST Sakari Ailus wrote:
> Hi Hans,
>
> On Sat, Jun 29, 2019 at 03:08:23PM +0200, Hans Verkuil wrote:
> > On 6/29/19 2:57 PM, Hans Verkuil wrote:
> > > On 6/29/19 2:06 PM, Janusz Krzysztofik wrote:
> > >> Hi Hans,
> > >>
> > >> On Saturday, June 29, 2019 12:00:24 P.M. CEST Hans Verkuil wrote:
> > >>> sd->entity.graph_obj.mdev can be NULL when this function is called,
> > >>> and
> > >>> that breaks existing drivers (rcar-vin, but probably others as well).
> > >>>
> > >>> Check if sd->entity.num_pads is non-zero instead since that doesn't
> > >>> depend
> > >>> on mdev.
> > >>>
> > >>> Signed-off-by: Hans Verkuil
> > >>> Reported-by: Niklas Söderlund
> > >>> Fixes: a8fa55078a77 ("media: v4l2-subdev: Verify arguments in
> > >>> v4l2_subdev_call()") ---
> > >>> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
> > >>> b/drivers/media/v4l2-core/v4l2-subdev.c index
> > >>> 21fb90d66bfc..bc32fc1e0c0b
> > >>> 100644
> > >>> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > >>> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > >>> @@ -124,16 +124,11 @@ static inline int check_which(__u32 which)
> > >>>
> > >>> static inline int check_pad(struct v4l2_subdev *sd, __u32 pad)
> > >>> {
> > >>> #if defined(CONFIG_MEDIA_CONTROLLER)
> > >>>
> > >>> - if (sd->entity.graph_obj.mdev) {
> > >>> - if (pad >= sd->entity.num_pads)
> > >>> - return -EINVAL;
> > >>> - return 0;
> > >>> - }
> > >>> + if (sd->entity.num_pads)
> > >>
> > >> This reverts a change introduced on Sakari's request in v7 of the
> > >> series which is the source of the regression. The intention was to
> > >> fail if num_pads == 0 on a valid media entity. Maybe we should still
> > >> keep that restriction and fail in case mdev is not NULL? In other
> > >> words:
> > >>
> > >> -if (sd->entity.num_pads)
> > >> +if (sd->entity.num_pads || sd->entity.graph_obj.mdev)
> > >
> > > If an entity has no pads, then it doesn't have pad ops either and this
> > > function would never be called.
> >
> > Sakari, do you think it is ever possible that an entity may have pad ops,
> > but num_pads goes at some point to 0?
> >
> > If so, then we can check for sd->entity.function != 0. All MC subdevs must
> > set that to a non-0 value, otherwise the core will issue a WARN_ON. I
> > think
> > it is the only reliable indicator that can be used.
>
> I don't think checking the num_pads field is is a great way to test whether
> an entity supports the pad ops; I don't have a better proposal either as it
> seems that some drivers call these ops before registering the subdev.
>
> There are sub-device drivers that expose their own device node and thus
> work with MC-enabled master drivers but have no pad ops: this is what makes
> the fundamental difference in API support, it's not the number of pads. We
> just happen to see this in pad ops only (I guess).
>
> Currently the drivers may set the HAS_DEVNODE flag (that really tells about
> MC compatibility) just before registering the sub-device. This might be a
> better test for allowing pad ops, but the name of the flag is somewhat
> misleading. I wouldn't want to start testing this now however, it'd risk
> exposing these issues to a much wider audience.
>
> I guess in practice testing for num_pads still works; it however does leave
> some gray area between MC-enabled sub-device drivers and the rest.
That's why I proposed to cover that area by still checking for a non-NULL mdev
member alternatively.
Are you sure HAS_DEVNODE flag is applicable only for media entities? AFAICS
subdev drivers may expose some IOCTLs regardless of CONFIG_MEDIA_CONTROLLER
and CONFIG_V4L2_SUBDEV_API settings.
Thanks,
Janusz
> Perhaps
> something to improve in the future, but for a later kernel release.
>
> So,
>
> Acked-by: Sakari Ailus
Re: [PATCH 2/2] media: ov5645: Use regulator_bulk() functions
Hello Fabio, On Thu, Jun 27, 2019 at 9:24 PM Fabio Estevam wrote: > > Hi Sakari, > > On Thu, Jun 27, 2019 at 1:27 PM Sakari Ailus wrote: > > > This appears to change the order in which the regulators are enabled. Is > > that intentional? Does the sensor support this order as well? > > Good catch! I have sent a v2 that preserves the regulator enable order. Thank you for the patch. The question about using the regulator_bulk API seems to come regularly from time to time. This has been discussed on [1] and I believe the conclusion has been that the regulator_bulk API doesn't guarantee the order of enabling of the regulators. So in theory this is possible to cause problems in some corner cases and we have agreed to leave the order explicit. Best regards, Todor [1] https://patchwork.linuxtv.org/patch/36918/ P.S. Sorry for previous emails, hopefully my client is set up correctly now.
Analyze syzbot report technisat_usb2_rc_query KASAN
Hello,
I did a checking for this report of syzbot [1]
From the call stack of dump log:
There shows that a problem within technisat_usb2_get_ir()
BUG: KASAN: slab-out-of-bounds in technisat_usb2_get_ir
drivers/media/usb/dvb-usb/technisat-usb2.c:664 [inline]
BUG: KASAN: slab-out-of-bounds in technisat_usb2_rc_query+0x5fa/0x660
drivers/media/usb/dvb-usb/technisat-usb2.c:679
Read of size 1 at addr 8880a8791ea8 by task kworker/0:1/12
Take a look into while loop in technisat_usb2_get_ir().
I recognized that a problem. The loop will not break out with the
condition doesn't reach. Then "b++" will go wrong and buffer will be
overflow.
while (1) {
[...]
b++;
if (*b == 0xff) {
ev.pulse = 0;
ev.duration = 88*2;
ir_raw_event_store(d->rc_dev, &ev);
break;
}
}
I would propose changing the loop condition by checking the address of
the buffer. If acceptable, I will send this patch to the mailing-list.
eg:
- while (1) {
+ while (b != (buf + 63)) {
[...]
}
Tested with syzbot, result is good [2].
[1] https://syzkaller.appspot.com/bug?extid=eaaaf38a95427be88f4b
[2] https://groups.google.com/d/msg/syzkaller-bugs/CySBCKuUOOs/0hKq1CdjCwAJ
Phong
[PATCH] rcar-vin: Clean up correct notifier in error path
When adding the v4l2_async_notifier_cleanup() callas the wrong notifier
was cleaned up if the parallel notifier registration failed. Fix this by
cleaning up the correct one.
Fixes: 9863bc8695bc36e3 ("media: rcar-vin: Cleanup notifier in error path")
Signed-off-by: Niklas Söderlund
---
drivers/media/platform/rcar-vin/rcar-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
b/drivers/media/platform/rcar-vin/rcar-core.c
index 64f9cf790445d14e..a6efe1a8099a6ae6 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -633,7 +633,7 @@ static int rvin_parallel_init(struct rvin_dev *vin)
ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
if (ret < 0) {
vin_err(vin, "Notifier registration failed\n");
- v4l2_async_notifier_cleanup(&vin->group->notifier);
+ v4l2_async_notifier_cleanup(&vin->notifier);
return ret;
}
--
2.21.0
Re: [Linux-kernel-mentees] Analyze syzbot report technisat_usb2_rc_query KASAN
On Tue, Jul 02, 2019 at 07:49:26AM +0700, Phong Tran wrote:
> Hello,
>
> I did a checking for this report of syzbot [1]
> From the call stack of dump log:
>
> There shows that a problem within technisat_usb2_get_ir()
>
> BUG: KASAN: slab-out-of-bounds in technisat_usb2_get_ir
> drivers/media/usb/dvb-usb/technisat-usb2.c:664 [inline]
> BUG: KASAN: slab-out-of-bounds in technisat_usb2_rc_query+0x5fa/0x660
> drivers/media/usb/dvb-usb/technisat-usb2.c:679
> Read of size 1 at addr 8880a8791ea8 by task kworker/0:1/12
>
> Take a look into while loop in technisat_usb2_get_ir().
> I recognized that a problem. The loop will not break out with the condition
> doesn't reach. Then "b++" will go wrong and buffer will be overflow.
>
> while (1) {
> [...]
> b++;
> if (*b == 0xff) {
> ev.pulse = 0;
> ev.duration = 88*2;
> ir_raw_event_store(d->rc_dev, &ev);
> break;
> }
> }
>
> I would propose changing the loop condition by checking the address of the
> buffer. If acceptable, I will send this patch to the mailing-list.
> eg:
>
> - while (1) {
> + while (b != (buf + 63)) {
> [...]
> }
>
> Tested with syzbot, result is good [2].
>
> [1] https://syzkaller.appspot.com/bug?extid=eaaaf38a95427be88f4b
> [2] https://groups.google.com/d/msg/syzkaller-bugs/CySBCKuUOOs/0hKq1CdjCwAJ
Great, can you submit a patch for this?
thanks,
greg k-h
