Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

media: i2c: imx219: Selection compliance fixes #3983

Merged
merged 1 commit into from
Dec 2, 2020

Conversation

jmondi
Copy link
Contributor

@jmondi jmondi commented Dec 2, 2020

To comply with the intended usage of the V4L2 selection target when
used to retrieve a sensor image properties, adjust the rectangles
returned by the imx219 driver.

The top/left crop coordinates of the TGT_CROP rectangle were set to
(0, 0) instead of (8, 8) which is the offset from the larger physical
pixel array rectangle. This was also a mismatch with the default values
crop rectangle value, so this is corrected. Found with v4l2-compliance.

While at it, add V4L2_SEL_TGT_CROP_BOUNDS support: CROP_DEFAULT and
CROP_BOUNDS have the same size as the non-active pixels are not readable
using the selection API. Found with v4l2-compliance.

Fixes: e6d4ef7 ("media: i2c: imx219: Implement get_selection")
Signed-off-by: Hans Verkuil hverkuil-cisco@xs4all.nl
[reword commit message, use macros for pixel offsets]
Signed-off-by: Jacopo Mondi jacopo@jmondi.org

To comply with the intended usage of the V4L2 selection target when
used to retrieve a sensor image properties, adjust the rectangles
returned by the imx219 driver.

The top/left crop coordinates of the TGT_CROP rectangle were set to
(0, 0) instead of (8, 8) which is the offset from the larger physical
pixel array rectangle. This was also a mismatch with the default values
crop rectangle value, so this is corrected. Found with v4l2-compliance.

While at it, add V4L2_SEL_TGT_CROP_BOUNDS support: CROP_DEFAULT and
CROP_BOUNDS have the same size as the non-active pixels are not readable
using the selection API. Found with v4l2-compliance.

Fixes: e6d4ef7 ("media: i2c: imx219: Implement get_selection")
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
[reword commit message, use macros for pixel offsets]
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
@6by9
Copy link
Contributor

6by9 commented Dec 2, 2020

Has this actually been merged upstream yet? I don't see it in https://git.linuxtv.org/media_tree.git/log/drivers/media/i2c/imx219.c

https://patchwork.linuxtv.org/project/linux-media/patch/b580ac9d-5ae4-29ce-c81a-a1f98b1d953b@xs4all.nl/ seems to have had a lot of discussion, but is there an actual conclusion?
And https://patchwork.linuxtv.org/project/linux-media/patch/20200805105721.15445-5-jacopo@jmondi.org/ seems to be in limbo too.

The question is more whether we expect to see further updates to this rather than objecting to the content.

@jmondi
Copy link
Contributor Author

jmondi commented Dec 2, 2020

Hi Dave,

Has this actually been merged upstream yet? I don't see it in https://git.linuxtv.org/media_tree.git/log/drivers/media/i2c/imx219.c

https://patchwork.linuxtv.org/project/linux-media/patch/b580ac9d-5ae4-29ce-c81a-a1f98b1d953b@xs4all.nl/ seems to have had a lot of discussion, but is there an actual conclusion?
And https://patchwork.linuxtv.org/project/linux-media/patch/20200805105721.15445-5-jacopo@jmondi.org/ seems to be in limbo too.

I got a confirmation from Sakari that he's going to collect:
https://patchwork.linuxtv.org/project/linux-media/patch/20200805105721.15445-5-jacopo@jmondi.org/

The patch itself has merits unrelated to the discussion on the V4L2 targets definition (which didn't end up with any actual change)

The question is more whether we expect to see further updates to this rather than objecting to the content.

I'm fine postponing merging this pull request until upstream does not collect the imx219 patch.

@6by9
Copy link
Contributor

6by9 commented Dec 2, 2020

If Sakari is going to pick this patch up as is, then I'm happy to merge it now.

If we ask @pelwell nicely he may cherry-pick it through to our 5.10 branch too.

@jmondi
Copy link
Contributor Author

jmondi commented Dec 2, 2020

I -literally- just received this automatic message:

This is an automatic generated email to let you know that the following patch were queued:
Subject: media: i2c: imx219: Selection compliance fixes

The patch should now be in media master.
Let's see if @pelwell is going to be nice with us!

@pelwell pelwell merged commit 543790f into raspberrypi:rpi-5.4.y Dec 2, 2020
@pelwell
Copy link
Contributor

pelwell commented Dec 2, 2020

Done.

popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Dec 7, 2020
kernel: overlays: Add PCF85063 and PCF85063A to i2c-rtc

kernel: configs: Add RTC_DRV_PCF85063=m

kernel: ARM: dts: CM4 audio pins are not connected

kernel: PCI: brcmstb: Advertise MSI-X support

kernel: media: bcm2835-unicam: Clear clock state when stopping streaming
See: raspberrypi/linux#3986

kernel: media: bcm2835-unicam: Correctly handle error states on stream on/off
See: raspberrypi/linux#3984

kernel: media: i2c: imx219: Selection compliance fixes
See: raspberrypi/linux#3983

kernel: drm/vc4: Correct DSI register definition
See: raspberrypi/linux#3980

firmware: arm_dt: Handle parent interrupt controllers when masking

firmware: config: Add cm4 and pi400 config section filters

firmware: MMAL/IL/ISP component: Set the ISP boost frequency once on open

firmware: sdcard: Remove legacy NOOBS support to support booting from primary partition 4

firmware: arm_loader: Move 2711 RAM to PCIe address 16GB

firmware: video_decode: Add parameter to disable timestamp validation

firmware: Imx477 camera tuning fixes
See: https://www.raspberrypi.org/forums/viewtopic.php?f=43&t=291032#p1770287
See: https://www.raspberrypi.org/forums/viewtopic.php?f=43&t=291032&start=25#p1771066
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Dec 7, 2020
kernel: overlays: Add PCF85063 and PCF85063A to i2c-rtc

kernel: configs: Add RTC_DRV_PCF85063=m

kernel: ARM: dts: CM4 audio pins are not connected

kernel: PCI: brcmstb: Advertise MSI-X support

kernel: media: bcm2835-unicam: Clear clock state when stopping streaming
See: raspberrypi/linux#3986

kernel: media: bcm2835-unicam: Correctly handle error states on stream on/off
See: raspberrypi/linux#3984

kernel: media: i2c: imx219: Selection compliance fixes
See: raspberrypi/linux#3983

kernel: drm/vc4: Correct DSI register definition
See: raspberrypi/linux#3980

firmware: arm_dt: Handle parent interrupt controllers when masking

firmware: config: Add cm4 and pi400 config section filters

firmware: MMAL/IL/ISP component: Set the ISP boost frequency once on open

firmware: sdcard: Remove legacy NOOBS support to support booting from primary partition 4

firmware: arm_loader: Move 2711 RAM to PCIe address 16GB

firmware: video_decode: Add parameter to disable timestamp validation

firmware: Imx477 camera tuning fixes
See: https://www.raspberrypi.org/forums/viewtopic.php?f=43&t=291032#p1770287
See: https://www.raspberrypi.org/forums/viewtopic.php?f=43&t=291032&start=25#p1771066
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Dec 8, 2020
kernel: overlays: Add PCF85063 and PCF85063A to i2c-rtc

kernel: configs: Add RTC_DRV_PCF85063=m

kernel: ARM: dts: CM4 audio pins are not connected

kernel: media: bcm2835-unicam: Clear clock state when stopping streaming
See: raspberrypi/linux#3986

kernel: media: bcm2835-unicam: Correctly handle error states on stream on/off
See: raspberrypi/linux#3984

kernel: media: i2c: imx219: Selection compliance fixes
See: raspberrypi/linux#3983

firmware: arm_dt: Handle parent interrupt controllers when masking

firmware: config: Add cm4 and pi400 config section filters

firmware: MMAL/IL/ISP component: Set the ISP boost frequency once on open

firmware: sdcard: Remove legacy NOOBS support to support booting from primary partition 4

firmware: arm_loader: Move 2711 RAM to PCIe address 16GB

firmware: video_decode: Add parameter to disable timestamp validation

firmware: Imx477 camera tuning fixes
See: https://www.raspberrypi.org/forums/viewtopic.php?f=43&t=291032#p1770287
See: https://www.raspberrypi.org/forums/viewtopic.php?f=43&t=291032&start=25#p1771066
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Dec 9, 2020
kernel: overlays: Add PCF85063 and PCF85063A to i2c-rtc

kernel: configs: Add RTC_DRV_PCF85063=m

kernel: ARM: dts: CM4 audio pins are not connected

kernel: media: bcm2835-unicam: Clear clock state when stopping streaming
See: raspberrypi/linux#3986

kernel: media: bcm2835-unicam: Correctly handle error states on stream on/off
See: raspberrypi/linux#3984

kernel: media: i2c: imx219: Selection compliance fixes
See: raspberrypi/linux#3983

firmware: arm_dt: Handle parent interrupt controllers when masking

firmware: config: Add cm4 and pi400 config section filters

firmware: MMAL/IL/ISP component: Set the ISP boost frequency once on open

firmware: sdcard: Remove legacy NOOBS support to support booting from primary partition 4

firmware: arm_loader: Move 2711 RAM to PCIe address 16GB

firmware: video_decode: Add parameter to disable timestamp validation

firmware: Imx477 camera tuning fixes
See: https://www.raspberrypi.org/forums/viewtopic.php?f=43&t=291032#p1770287
See: https://www.raspberrypi.org/forums/viewtopic.php?f=43&t=291032&start=25#p1771066
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants