Hi,
On 11/8/23 12:59, Hans de Goede wrote:
> Hi Bingbu,
>
> On 10/24/23 13:29, bingbu.cao@xxxxxxxxx wrote:
>> From: Bingbu Cao <bingbu.cao@xxxxxxxxx>
>>
>> This patch series adds a driver for Intel IPU6 input system.
>> IPU6 is the sixth generation of Imaging Processing Unit, it is a PCI
>> device which can be found in some Intel Client Platforms. User can use
>> IPU6 to capture images from MIPI camera sensors.
>>
>> IPU6 has its own firmware which exposes ABIs to driver, and communicates
>> with CSE to do firmware authentication. IPU6 has its MMU hardware, so
>> the driver sets up a page table to allow IPU6 DMA to access the system
>> memory.
>>
>> IPU6 input system driver uses MC and V4L2 sub-device APIs besides V4L2.
>
> I have been testing this on a TigerLake system, a Dell Latitude 9420
> with ov01a1s and the packed 10 bit bayer pixel fmt: V4L2_PIX_FMT_SGRBG10P,
> which libcamera together with (WIP) software debayer support picks
> by default does not work. There are many many CSI errors in dmesg
> and only the first 10 or so lines of the image show.
>
> Disabling the packed format by removing it from ipu6_isys_pfmts[],
> making libcamera pick the unpacked (every 10 bits per pixel data
> stored in a 16 bit word in output buffer) fixes this.
>
> Are the packed bayer formats supposed to work on Tiger Lake, or
> is this a known issue which Intel's own userspace stack avoids
> by always picking the unpacked format ?
I have just rebased my personal tree with these patches in it
on top of media_staging. I had to make some changes to make
this series work on top of media_staging.
I'm attaching these changes here as a patch in case they
are useful for someone else. Feel free to squash these
into the next version.
Talking about a next version I would think it is about
time to post a v3 of this ?
From 93d23e346aaa2e2ba59b4bc95ca029b674e01195 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@xxxxxxxxxx>
Date: Mon, 4 Dec 2023 12:13:02 +0100
Subject: [PATCH] media: ipu6: Add support for kernel 6.8
Add support for the upcoming v4l2-subdev API changes in kernel 6.8.
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 8 +++-----
.../media/pci/intel/ipu6/ipu6-isys-subdev.c | 19 +++++++++++--------
.../media/pci/intel/ipu6/ipu6-isys-subdev.h | 2 --
.../media/pci/intel/ipu6/ipu6-isys-video.c | 3 +--
4 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
index 2da82a74fd18..1d40bfa992a6 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
@@ -386,12 +386,11 @@ static int ipu6_isys_csi2_set_sel(struct v4l2_subdev *sd,
if (!sink_ffmt)
return -EINVAL;
- src_ffmt = v4l2_subdev_state_get_stream_format(state, sel->pad,
- sel->stream);
+ src_ffmt = v4l2_subdev_state_get_format(state, sel->pad, sel->stream);
if (!src_ffmt)
return -EINVAL;
- crop = v4l2_subdev_state_get_stream_crop(state, sel->pad, sel->stream);
+ crop = v4l2_subdev_state_get_crop(state, sel->pad, sel->stream);
if (!crop)
return -EINVAL;
@@ -436,7 +435,7 @@ static int ipu6_isys_csi2_get_sel(struct v4l2_subdev *sd,
if (!sink_ffmt)
return -EINVAL;
- crop = v4l2_subdev_state_get_stream_crop(state, sel->pad, sel->stream);
+ crop = v4l2_subdev_state_get_crop(state, sel->pad, sel->stream);
if (!crop)
return -EINVAL;
@@ -463,7 +462,6 @@ static const struct v4l2_subdev_video_ops csi2_sd_video_ops = {
};
static const struct v4l2_subdev_pad_ops csi2_sd_pad_ops = {
- .init_cfg = ipu6_isys_subdev_init_cfg,
.get_fmt = v4l2_subdev_get_fmt,
.set_fmt = ipu6_isys_subdev_set_fmt,
.get_selection = ipu6_isys_csi2_get_sel,
diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c
index 7f104205d153..598bdf69b364 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c
@@ -143,8 +143,7 @@ int ipu6_isys_subdev_set_fmt(struct v4l2_subdev *sd,
format->format.field = V4L2_FIELD_NONE;
/* Store the format and propagate it to the source pad. */
- fmt = v4l2_subdev_state_get_stream_format(state, format->pad,
- format->stream);
+ fmt = v4l2_subdev_state_get_format(state, format->pad, format->stream);
if (!fmt)
return -EINVAL;
@@ -169,8 +168,7 @@ int ipu6_isys_subdev_set_fmt(struct v4l2_subdev *sd,
if (ret)
return -EINVAL;
- crop = v4l2_subdev_state_get_stream_crop(state, other_pad,
- other_stream);
+ crop = v4l2_subdev_state_get_crop(state, other_pad, other_stream);
/* reset crop */
crop->left = 0;
crop->top = 0;
@@ -229,7 +227,7 @@ int ipu6_isys_get_stream_pad_fmt(struct v4l2_subdev *sd, u32 pad, u32 stream,
return -EINVAL;
state = v4l2_subdev_lock_and_get_active_state(sd);
- fmt = v4l2_subdev_state_get_stream_format(state, pad, stream);
+ fmt = v4l2_subdev_state_get_format(state, pad, stream);
if (fmt) {
*format = *fmt;
ret = 0;
@@ -250,7 +248,7 @@ int ipu6_isys_get_stream_pad_crop(struct v4l2_subdev *sd, u32 pad, u32 stream,
return -EINVAL;
state = v4l2_subdev_lock_and_get_active_state(sd);
- rect = v4l2_subdev_state_get_stream_crop(state, pad, stream);
+ rect = v4l2_subdev_state_get_crop(state, pad, stream);
if (rect) {
*crop = *rect;
ret = 0;
@@ -284,8 +282,8 @@ u32 ipu6_isys_get_src_stream_by_src_pad(struct v4l2_subdev *sd, u32 pad)
return source_stream;
}
-int ipu6_isys_subdev_init_cfg(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *state)
+static int ipu6_isys_subdev_init_state(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state)
{
struct v4l2_subdev_route route = {
.sink_pad = 0,
@@ -310,6 +308,10 @@ int ipu6_isys_subdev_set_routing(struct v4l2_subdev *sd,
return subdev_set_routing(sd, state, routing);
}
+static const struct v4l2_subdev_internal_ops ipu6_isys_subdev_internal_ops = {
+ .init_state = ipu6_isys_subdev_init_state,
+};
+
int ipu6_isys_subdev_init(struct ipu6_isys_subdev *asd,
const struct v4l2_subdev_ops *ops,
unsigned int nr_ctrls,
@@ -327,6 +329,7 @@ int ipu6_isys_subdev_init(struct ipu6_isys_subdev *asd,
V4L2_SUBDEV_FL_STREAMS;
asd->sd.owner = THIS_MODULE;
asd->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
+ asd->sd.internal_ops = &ipu6_isys_subdev_internal_ops;
asd->pad = devm_kcalloc(&asd->isys->adev->auxdev.dev, num_pads,
sizeof(*asd->pad), GFP_KERNEL);
diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h
index 13e2c57efaea..ef787102ed98 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h
@@ -43,8 +43,6 @@ int ipu6_isys_get_stream_pad_fmt(struct v4l2_subdev *sd, u32 pad, u32 stream,
struct v4l2_mbus_framefmt *format);
int ipu6_isys_get_stream_pad_crop(struct v4l2_subdev *sd, u32 pad, u32 stream,
struct v4l2_rect *crop);
-int ipu6_isys_subdev_init_cfg(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *state);
int ipu6_isys_subdev_set_routing(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
enum v4l2_subdev_format_whence which,
diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
index e73a46eb7fd3..dc1dab713bb6 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-isys-video.c
@@ -281,8 +281,7 @@ static int link_validate(struct media_link *link)
v4l2_subdev_lock_state(s_state);
- s_fmt = v4l2_subdev_state_get_stream_format(s_state, s_pad->index,
- s_stream);
+ s_fmt = v4l2_subdev_state_get_format(s_state, s_pad->index, s_stream);
if (!s_fmt) {
dev_err(dev, "failed to get source pad format\n");
goto unlock;
--
2.41.0
