Re: [PATCH v2 0/2] rcar-du, vsp1: rcar-gen3: Add support for colorkey alpha blending
Alexandru Gheorghe writes: > Currently, rcar-du supports colorkeying only for rcar-gen2 and it uses > some hw capability of the display unit(DU) which is not available on gen3. > In order to implement colorkeying for gen3 we need to use the colorkey > capability of the VSPD, hence the need to change both drivers rcar-du and > vsp1. > > This patchset had been developed and tested on top of v4.9/rcar-3.5.1 from > git://git.kernel.org/pub/scm/linux/kernel/git/horms/renesas-bsp.git A few questions: Are other drivers interested in supporting this property? VC4 has the 24-bit RGB colorkey, but I don't see YCBCR support. Should it be documented in a generic location? Does your colorkey end up forcing alpha to 1 for the plane when it's not matched? signature.asc Description: PGP signature
Re: [PATCH 3/4] drm/vc4: Add register defines for CEC.
Hans Verkuil writes: > From: Eric Anholt > > Basic usage: > > poweron: HSM clock should be running. Set the bit clock divider, > set all the other _US timeouts based on bit clock rate. Bring RX/TX > reset up and then down. > > powerdown: Set RX/TX reset. > > interrupt: read CPU_STATUS, write bits that have been handled to > CPU_CLEAR. > > Bits are added to /debug/dri/0/hdmi_regs so you can check out the > power-on values. Let's drop my hints to you from the commit message here :) signature.asc Description: PGP signature
Re: [PATCH 2/4] drm/vc4: prepare for CEC support
Hans Verkuil writes:
> From: Hans Verkuil
>
> In order to support CEC the hsm clock needs to be enabled in
> vc4_hdmi_bind(), not in vc4_hdmi_encoder_enable(). Otherwise you wouldn't
> be able to support CEC when there is no hotplug detect signal, which is
> required by some monitors that turn off the HPD when in standby, but keep
> the CEC bus alive so they can be woken up.
>
> The HDMI core also has to be enabled in vc4_hdmi_bind() for the same
> reason.
>
> Signed-off-by: Hans Verkuil
Ccing Boris, I'd love to see what he thinks of this and if we can do any
better.
Hans, is it true that CEC needs to be on always, or could it only be
enabled when somebody is listening to messages?
> drivers/gpu/drm/vc4/vc4_hdmi.c | 59
> +-
> 1 file changed, 29 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index ed63d4e85762..e0104f96011e 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -463,11 +463,6 @@ static void vc4_hdmi_encoder_disable(struct drm_encoder
> *encoder)
> HD_WRITE(VC4_HD_VID_CTL,
>HD_READ(VC4_HD_VID_CTL) & ~VC4_HD_VID_CTL_ENABLE);
>
> - HD_WRITE(VC4_HD_M_CTL, VC4_HD_M_SW_RST);
> - udelay(1);
> - HD_WRITE(VC4_HD_M_CTL, 0);
> -
> - clk_disable_unprepare(hdmi->hsm_clock);
> clk_disable_unprepare(hdmi->pixel_clock);
>
> ret = pm_runtime_put(&hdmi->pdev->dev);
> @@ -509,16 +504,6 @@ static void vc4_hdmi_encoder_enable(struct drm_encoder
> *encoder)
> return;
> }
>
> - /* This is the rate that is set by the firmware. The number
> - * needs to be a bit higher than the pixel clock rate
> - * (generally 148.5Mhz).
> - */
> - ret = clk_set_rate(hdmi->hsm_clock, 163682864);
> - if (ret) {
> - DRM_ERROR("Failed to set HSM clock rate: %d\n", ret);
> - return;
> - }
> -
> ret = clk_set_rate(hdmi->pixel_clock,
> mode->clock * 1000 *
> ((mode->flags & DRM_MODE_FLAG_DBLCLK) ? 2 : 1));
> @@ -533,20 +518,6 @@ static void vc4_hdmi_encoder_enable(struct drm_encoder
> *encoder)
> return;
> }
>
> - ret = clk_prepare_enable(hdmi->hsm_clock);
> - if (ret) {
> - DRM_ERROR("Failed to turn on HDMI state machine clock: %d\n",
> - ret);
> - clk_disable_unprepare(hdmi->pixel_clock);
> - return;
> - }
> -
> - HD_WRITE(VC4_HD_M_CTL, VC4_HD_M_SW_RST);
> - udelay(1);
> - HD_WRITE(VC4_HD_M_CTL, 0);
> -
> - HD_WRITE(VC4_HD_M_CTL, VC4_HD_M_ENABLE);
> -
> HDMI_WRITE(VC4_HDMI_SW_RESET_CONTROL,
> VC4_HDMI_SW_RESET_HDMI |
> VC4_HDMI_SW_RESET_FORMAT_DETECT);
> @@ -1205,6 +1176,23 @@ static int vc4_hdmi_bind(struct device *dev, struct
> device *master, void *data)
> return -EPROBE_DEFER;
> }
>
> + /* This is the rate that is set by the firmware. The number
> + * needs to be a bit higher than the pixel clock rate
> + * (generally 148.5Mhz).
> + */
> + ret = clk_set_rate(hdmi->hsm_clock, 163682864);
> + if (ret) {
> + DRM_ERROR("Failed to set HSM clock rate: %d\n", ret);
> + goto err_put_i2c;
> + }
> +
> + ret = clk_prepare_enable(hdmi->hsm_clock);
> + if (ret) {
> + DRM_ERROR("Failed to turn on HDMI state machine clock: %d\n",
> + ret);
> + goto err_put_i2c;
> + }
> +
> /* Only use the GPIO HPD pin if present in the DT, otherwise
>* we'll use the HDMI core's register.
>*/
> @@ -1216,7 +1204,7 @@ static int vc4_hdmi_bind(struct device *dev, struct
> device *master, void *data)
>&hpd_gpio_flags);
> if (hdmi->hpd_gpio < 0) {
> ret = hdmi->hpd_gpio;
> - goto err_put_i2c;
> + goto err_unprepare_hsm;
> }
>
> hdmi->hpd_active_low = hpd_gpio_flags & OF_GPIO_ACTIVE_LOW;
> @@ -1224,6 +1212,14 @@ static int vc4_hdmi_bind(struct device *dev, struct
> device *master, void *data)
>
> vc4->hdmi = hdmi;
>
> + /* HDMI core must be enabled. */
> + if (!(HD_READ(VC4_HD_M_CTL) & VC4_HD_M_ENABLE)) {
> + HD_WRITE(VC4_HD_M_CTL, VC4_HD_M_SW_RST);
> + udelay(1);
> + HD_WRITE(VC4_HD_M_CTL, 0);
> +
> + HD_WRITE(VC4_HD_M_CTL, VC4_HD_M_ENABLE);
> + }
I'm wondering if there's any impact from leaving VC4_HD_M_ENABLE on
while the HDMI power domain is off. I don't quite understand the role
of the power domain, and I've fired off an email internally to check if
there are any experts on this hardware still around.
signature.asc
Description: PGP signature
Re: [PATCH 4/4] drm/vc4: add HDMI CEC support
Hans Verkuil writes:
> From: Hans Verkuil
>
> This patch adds support to VC4 for CEC.
>
> To prevent the firmware from eating the CEC interrupts you need to add this to
> your config.txt:
>
> mask_gpu_interrupt1=0x100
>
> Signed-off-by: Hans Verkuil
This looks pretty great. Just a couple of little comments.
> ---
> drivers/gpu/drm/vc4/Kconfig| 8 ++
> drivers/gpu/drm/vc4/vc4_hdmi.c | 203
> -
> drivers/gpu/drm/vc4/vc4_regs.h | 5 +
> 3 files changed, 211 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
> index 4361bdcfd28a..fdae18aeab4f 100644
> --- a/drivers/gpu/drm/vc4/Kconfig
> +++ b/drivers/gpu/drm/vc4/Kconfig
> @@ -19,3 +19,11 @@ config DRM_VC4
> This driver requires that "avoid_warnings=2" be present in
> the config.txt for the firmware, to keep it from smashing
> our display setup.
> +
> +config DRM_VC4_HDMI_CEC
> + bool "Broadcom VC4 HDMI CEC Support"
> + depends on DRM_VC4
> + select CEC_CORE
> + help
> + Choose this option if you have a Broadcom VC4 GPU
> + and want to use CEC.
Do we need a Kconfig for this? Couldn't we just #ifdef on CEC_CORE
instead?
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index b0521e6cc281..14e2ece5db94 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> +static int vc4_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
> +{
> + struct vc4_dev *vc4 = cec_get_drvdata(adap);
> + u32 hsm_clock = clk_get_rate(vc4->hdmi->hsm_clock);
> + u32 cntrl1 = HDMI_READ(VC4_HDMI_CEC_CNTRL_1);
> + u32 divclk = cntrl1 & VC4_HDMI_CEC_DIV_CLK_CNT_MASK;
We should probably be setting the divider to a value of our choice,
rather than relying on whatever default value is there.
(Bonus points if we were to do this using a common clk divider, so the
rate shows up in /debug/clk/clk_summary, but I won't require that)
> + /* clock period in microseconds */
> + u32 usecs = 100 / (hsm_clock / divclk);
> + u32 val = HDMI_READ(VC4_HDMI_CEC_CNTRL_5);
> +
> + val &= ~(VC4_HDMI_CEC_TX_SW_RESET | VC4_HDMI_CEC_RX_SW_RESET |
> + VC4_HDMI_CEC_CNT_TO_4700_US_MASK |
> + VC4_HDMI_CEC_CNT_TO_4500_US_MASK);
> + val |= ((4700 / usecs) << VC4_HDMI_CEC_CNT_TO_4700_US_SHIFT) |
> +((4500 / usecs) << VC4_HDMI_CEC_CNT_TO_4500_US_SHIFT);
> +
> + if (enable) {
> + cntrl1 &= VC4_HDMI_CEC_DIV_CLK_CNT_MASK |
> + VC4_HDMI_CEC_ADDR_MASK;
> +
> + HDMI_WRITE(VC4_HDMI_CEC_CNTRL_5, val |
> +VC4_HDMI_CEC_TX_SW_RESET | VC4_HDMI_CEC_RX_SW_RESET);
> + HDMI_WRITE(VC4_HDMI_CEC_CNTRL_5, val);
> + HDMI_WRITE(VC4_HDMI_CEC_CNTRL_2,
> + ((1500 / usecs) << VC4_HDMI_CEC_CNT_TO_1500_US_SHIFT) |
> + ((1300 / usecs) << VC4_HDMI_CEC_CNT_TO_1300_US_SHIFT) |
> + ((800 / usecs) << VC4_HDMI_CEC_CNT_TO_800_US_SHIFT) |
> + ((600 / usecs) << VC4_HDMI_CEC_CNT_TO_600_US_SHIFT) |
> + ((400 / usecs) << VC4_HDMI_CEC_CNT_TO_400_US_SHIFT));
> + HDMI_WRITE(VC4_HDMI_CEC_CNTRL_3,
> + ((2750 / usecs) << VC4_HDMI_CEC_CNT_TO_2750_US_SHIFT) |
> + ((2400 / usecs) << VC4_HDMI_CEC_CNT_TO_2400_US_SHIFT) |
> + ((2050 / usecs) << VC4_HDMI_CEC_CNT_TO_2050_US_SHIFT) |
> + ((1700 / usecs) << VC4_HDMI_CEC_CNT_TO_1700_US_SHIFT));
> + HDMI_WRITE(VC4_HDMI_CEC_CNTRL_4,
> + ((4300 / usecs) << VC4_HDMI_CEC_CNT_TO_4300_US_SHIFT) |
> + ((3900 / usecs) << VC4_HDMI_CEC_CNT_TO_3900_US_SHIFT) |
> + ((3600 / usecs) << VC4_HDMI_CEC_CNT_TO_3600_US_SHIFT) |
> + ((3500 / usecs) << VC4_HDMI_CEC_CNT_TO_3500_US_SHIFT));
> +
> + HDMI_WRITE(VC4_HDMI_CPU_MASK_CLEAR, VC4_HDMI_CPU_CEC);
> + } else {
> + HDMI_WRITE(VC4_HDMI_CPU_MASK_SET, VC4_HDMI_CPU_CEC);
> + HDMI_WRITE(VC4_HDMI_CEC_CNTRL_5, val |
> +VC4_HDMI_CEC_TX_SW_RESET | VC4_HDMI_CEC_RX_SW_RESET);
> + }
> + return 0;
> +}
> +static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
> + u32 signal_free_time, struct cec_msg *msg)
> +{
> + struct vc4_dev *vc4 = cec_get_drvdata(adap);
> + u32 val;
> + unsigned int i;
> +
> + for (i = 0; i < msg->len; i += 4)
> + HDMI_WRITE(VC4_HDMI_CEC_TX_DATA_1 + i,
> +(msg->msg[i]) |
> +(msg->msg[i + 1] << 8) |
> +(msg->msg[i + 2] << 16) |
> +(msg->msg[i + 3] << 24));
> +
> + val = HDMI_READ(VC4_HDMI_CEC_CNTRL_1);
> + val &= ~VC4_HDMI_CEC_START_
Re: [PATCH 4/4] drm/vc4: add HDMI CEC support
Hans Verkuil writes:
> On 12/07/17 21:43, Hans Verkuil wrote:
>> On 12/07/17 21:02, Eric Anholt wrote:
>>>> +static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8
>>>> attempts,
>>>> +u32 signal_free_time, struct cec_msg *msg)
>>>> +{
>>>> + struct vc4_dev *vc4 = cec_get_drvdata(adap);
>>>> + u32 val;
>>>> + unsigned int i;
>>>> +
>>>> + for (i = 0; i < msg->len; i += 4)
>>>> + HDMI_WRITE(VC4_HDMI_CEC_TX_DATA_1 + i,
>>>> + (msg->msg[i]) |
>>>> + (msg->msg[i + 1] << 8) |
>>>> + (msg->msg[i + 2] << 16) |
>>>> + (msg->msg[i + 3] << 24));
>>>> +
>>>> + val = HDMI_READ(VC4_HDMI_CEC_CNTRL_1);
>>>> + val &= ~VC4_HDMI_CEC_START_XMIT_BEGIN;
>>>> + HDMI_WRITE(VC4_HDMI_CEC_CNTRL_1, val);
>>>> + val &= ~VC4_HDMI_CEC_MESSAGE_LENGTH_MASK;
>>>> + val |= (msg->len - 1) << VC4_HDMI_CEC_MESSAGE_LENGTH_SHIFT;
>>>> + val |= VC4_HDMI_CEC_START_XMIT_BEGIN;
>>>
>>> It doesn't look to me like len should have 1 subtracted from it. The
>>> field has 4 bits for our up-to-16-byte length, and the firmware seems to
>>> be setting it to the same value as a memcpy for the message data uses.
>>
>> You need to subtract by one. The CEC protocol supports messages of 1-16
>> bytes in length. Since the message length mask is only 4 bits you need to
>> encode this in the value 0-15. Hence the '-1', otherwise you would never
>> be able to send 16 byte messages.
>>
>> I actually found this when debugging the messages it was transmitting: they
>> were one too long.
>>
>> This suggests that the firmware does this wrong. I don't have time tomorrow,
>> but I'll see if I can do a quick test on Friday to verify that.
>
> I double-checked this and both the driver and the firmware do the right thing.
> Just to be certain I also tried sending a message that uses the full 16 byte
> payload and that too went well. So the code is definitely correct.
Great, I'll assume that I just missed the subtraction somewhere in the
layers of firmware code. Thanks for checking!
signature.asc
Description: PGP signature
Re: [PATCHv2 3/3] drm/vc4: add HDMI CEC support
Hans Verkuil writes: > Hi Eric, > > On 16/07/17 12:48, Hans Verkuil wrote: >> From: Hans Verkuil >> >> This patch adds support to VC4 for CEC. >> >> Thanks to Eric Anholt for providing me with the CEC register information. >> >> To prevent the firmware from eating the CEC interrupts you need to add this >> to >> your config.txt: >> >> mask_gpu_interrupt1=0x100 > > I put this text in the commit log, but I think it should also go into the > source. > Do you agree? > > Should I also mention this in the kernel log via a 'dev_info'? It's not an > obvious > config option, after all. > > Or do you have other ideas regarding this? The firmware should have been masking this interrupt for ages (~2 years?), as long as it sees the kernel's DT instead of the downstream one. I've dropped the commit message comment about that, added a bit of explanation of having a separate Kconfig, and pushed. Huge thanks for your work on this! signature.asc Description: PGP signature
[PATCH 2/6] staging: bcm2835-v4l2: Update the driver to the current VCHI API.
49bec49fd7f2 ("staging: vc04_services: remove vchiq_copy_from_user")
removed the flags/msg_handle arguments, which were unused, and pushed
the implementation of copying using memcpy vs copy_from_user to the
caller.
Signed-off-by: Eric Anholt
---
drivers/staging/media/platform/bcm2835/mmal-vchiq.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
index 781322542d5a..24bd2948136c 100644
--- a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
+++ b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
@@ -378,6 +378,14 @@ static int inline_receive(struct vchiq_mmal_instance
*instance,
return 0;
}
+static ssize_t mmal_memcpy_wrapper(void *src, void *dst,
+ size_t offset, size_t size)
+{
+ memcpy(dst + offset, src + offset, size);
+
+ return size;
+}
+
/* queue the buffer availability with MMAL_MSG_TYPE_BUFFER_FROM_HOST */
static int
buffer_from_host(struct vchiq_mmal_instance *instance,
@@ -442,10 +450,9 @@ buffer_from_host(struct vchiq_mmal_instance *instance,
vchi_service_use(instance->handle);
- ret = vchi_msg_queue(instance->handle, &m,
+ ret = vchi_msg_queue(instance->handle, mmal_memcpy_wrapper, &m,
sizeof(struct mmal_msg_header) +
-sizeof(m.u.buffer_from_host),
-VCHI_FLAGS_BLOCK_UNTIL_QUEUED, NULL);
+sizeof(m.u.buffer_from_host));
if (ret != 0) {
release_msg_context(msg_context);
@@ -731,9 +738,9 @@ static int send_synchronous_mmal_msg(struct
vchiq_mmal_instance *instance,
vchi_service_use(instance->handle);
ret = vchi_msg_queue(instance->handle,
+mmal_memcpy_wrapper,
msg,
-sizeof(struct mmal_msg_header) + payload_len,
-VCHI_FLAGS_BLOCK_UNTIL_QUEUED, NULL);
+sizeof(struct mmal_msg_header) + payload_len);
vchi_service_release(instance->handle);
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver
Here's my first pass at importing the camera driver. There's a bunch of TODO left to it, most of which is documented, and the rest being standard checkpatch fare. Unfortunately, when I try modprobing it on my pi3, the USB network device dies, consistently. I'm not sure what's going on here yet, but I'm going to keep working on some debug of it. I've unfortunately changed a lot of variables (pi3 vs pi2, upstream vs downstream, vchi's updates while in staging, 4.9 vs 4.4), so I probably won't figure it out today. Note that the "Update the driver to the current VCHI API" patch will conflict with the outstanding "Add vchi_queue_kernel_message and vchi_queue_user_message" series, but the fix should be pretty obvious when that lands. I built this against 4.10-rc1, but a merge with staging-next was clean and still built fine. Eric Anholt (6): staging: Import the BCM2835 MMAL-based V4L2 camera driver. staging: bcm2835-v4l2: Update the driver to the current VCHI API. staging: bcm2835-v4l2: Add a build system for the module. staging: bcm2835-v4l2: Add a TODO file for improvements we need. staging: bcm2835-v4l2: Apply many whitespace fixes from checkpatch. staging: bcm2835-v4l2: Apply spelling fixes from checkpatch. drivers/staging/media/Kconfig |2 + drivers/staging/media/Makefile |1 + drivers/staging/media/platform/bcm2835/Kconfig | 10 + drivers/staging/media/platform/bcm2835/Makefile| 11 + drivers/staging/media/platform/bcm2835/TODO| 39 + .../media/platform/bcm2835/bcm2835-camera.c| 2024 .../media/platform/bcm2835/bcm2835-camera.h| 145 ++ drivers/staging/media/platform/bcm2835/controls.c | 1335 + .../staging/media/platform/bcm2835/mmal-common.h | 53 + .../media/platform/bcm2835/mmal-encodings.h| 127 ++ .../media/platform/bcm2835/mmal-msg-common.h | 50 + .../media/platform/bcm2835/mmal-msg-format.h | 81 + .../staging/media/platform/bcm2835/mmal-msg-port.h | 107 ++ drivers/staging/media/platform/bcm2835/mmal-msg.h | 404 .../media/platform/bcm2835/mmal-parameters.h | 689 +++ .../staging/media/platform/bcm2835/mmal-vchiq.c| 1920 +++ .../staging/media/platform/bcm2835/mmal-vchiq.h| 178 ++ 17 files changed, 7176 insertions(+) create mode 100644 drivers/staging/media/platform/bcm2835/Kconfig create mode 100644 drivers/staging/media/platform/bcm2835/Makefile create mode 100644 drivers/staging/media/platform/bcm2835/TODO create mode 100644 drivers/staging/media/platform/bcm2835/bcm2835-camera.c create mode 100644 drivers/staging/media/platform/bcm2835/bcm2835-camera.h create mode 100644 drivers/staging/media/platform/bcm2835/controls.c create mode 100644 drivers/staging/media/platform/bcm2835/mmal-common.h create mode 100644 drivers/staging/media/platform/bcm2835/mmal-encodings.h create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-common.h create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-format.h create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg-port.h create mode 100644 drivers/staging/media/platform/bcm2835/mmal-msg.h create mode 100644 drivers/staging/media/platform/bcm2835/mmal-parameters.h create mode 100644 drivers/staging/media/platform/bcm2835/mmal-vchiq.c create mode 100644 drivers/staging/media/platform/bcm2835/mmal-vchiq.h -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] staging: bcm2835-v4l2: Add a build system for the module.
This is derived from the downstream tree's build system, but with just a single Kconfig option. For now the driver only builds on 32-bit arm -- the aarch64 build breaks due to the driver using arm-specific cache flushing functions. Signed-off-by: Eric Anholt --- drivers/staging/media/Kconfig | 2 ++ drivers/staging/media/Makefile | 1 + drivers/staging/media/platform/bcm2835/Kconfig | 10 ++ drivers/staging/media/platform/bcm2835/Makefile | 11 +++ 4 files changed, 24 insertions(+) create mode 100644 drivers/staging/media/platform/bcm2835/Kconfig create mode 100644 drivers/staging/media/platform/bcm2835/Makefile diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig index ffb8fa72c3da..abd0e2d57c20 100644 --- a/drivers/staging/media/Kconfig +++ b/drivers/staging/media/Kconfig @@ -27,6 +27,8 @@ source "drivers/staging/media/davinci_vpfe/Kconfig" source "drivers/staging/media/omap4iss/Kconfig" +source "drivers/staging/media/platform/bcm2835/Kconfig" + source "drivers/staging/media/s5p-cec/Kconfig" # Keep LIRC at the end, as it has sub-menus diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile index a28e82cf6447..dc89325c463d 100644 --- a/drivers/staging/media/Makefile +++ b/drivers/staging/media/Makefile @@ -2,6 +2,7 @@ obj-$(CONFIG_I2C_BCM2048) += bcm2048/ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC) += s5p-cec/ obj-$(CONFIG_DVB_CXD2099) += cxd2099/ obj-$(CONFIG_LIRC_STAGING) += lirc/ +obj-$(CONFIG_VIDEO_BCM2835)+= platform/bcm2835/ obj-$(CONFIG_VIDEO_DM365_VPFE) += davinci_vpfe/ obj-$(CONFIG_VIDEO_OMAP4) += omap4iss/ obj-$(CONFIG_VIDEO_STI_HDMI_CEC) += st-cec/ diff --git a/drivers/staging/media/platform/bcm2835/Kconfig b/drivers/staging/media/platform/bcm2835/Kconfig new file mode 100644 index ..7c5245dc3225 --- /dev/null +++ b/drivers/staging/media/platform/bcm2835/Kconfig @@ -0,0 +1,10 @@ +config VIDEO_BCM2835 + tristate "Broadcom BCM2835 camera driver" + depends on VIDEO_V4L2 && (ARCH_BCM2835 || COMPILE_TEST) + depends on BCM2835_VCHIQ + depends on ARM + select VIDEOBUF2_VMALLOC + help + Say Y here to enable camera host interface devices for + Broadcom BCM2835 SoC. This operates over the VCHIQ interface + to a service running on VideoCore. diff --git a/drivers/staging/media/platform/bcm2835/Makefile b/drivers/staging/media/platform/bcm2835/Makefile new file mode 100644 index ..d7900a5951a8 --- /dev/null +++ b/drivers/staging/media/platform/bcm2835/Makefile @@ -0,0 +1,11 @@ +bcm2835-v4l2-$(CONFIG_VIDEO_BCM2835) := \ + bcm2835-camera.o \ + controls.o \ + mmal-vchiq.o + +obj-$(CONFIG_VIDEO_BCM2835) += bcm2835-v4l2.o + +ccflags-y += \ + -Idrivers/staging/vc04_services \ + -Idrivers/staging/vc04_services/interface/vcos/linuxkernel \ + -D__VCCOREVER__=0x0400 -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] staging: bcm2835-v4l2: Add a TODO file for improvements we need.
Signed-off-by: Eric Anholt --- drivers/staging/media/platform/bcm2835/TODO | 39 + 1 file changed, 39 insertions(+) create mode 100644 drivers/staging/media/platform/bcm2835/TODO diff --git a/drivers/staging/media/platform/bcm2835/TODO b/drivers/staging/media/platform/bcm2835/TODO new file mode 100644 index ..61a509992b9a --- /dev/null +++ b/drivers/staging/media/platform/bcm2835/TODO @@ -0,0 +1,39 @@ +1) Support dma-buf memory management. + +In order to zero-copy import camera images into the 3D or display +pipelines, we need to export our buffers through dma-buf so that the +vc4 driver can import them. This may involve bringing in the VCSM +driver (which allows long-term management of regions of memory in the +space that the VPU reserved and Linux otherwise doesn't have access +to), or building some new protocol that allows VCSM-style management +of Linux's CMA memory. + +2) Avoid extra copies for padding of images. + +We expose V4L2_PIX_FMT_* formats that have a specified stride/height +padding in the V4L2 spec, but that padding doesn't match what the +hardware can do. If we exposed the native padding requirements +through the V4L2 "multiplanar" formats, the firmware would have one +less copy it needed to do. + +3) Port to ARM64 + +The bulk_receive() does some manual cache flushing that are 32-bit ARM +only, which we should convert to proper cross-platform APIs. + +4) Convert to be a platform driver. + +Right now when the module probes, it tries to initialize VCHI and +errors out if it wasn't ready yet. If bcm2835-v4l2 was built in, then +VCHI generally isn't ready because it depends on both the firmware and +mailbox drivers having already loaded. + +We should have VCHI create a platform device once it's initialized, +and have this driver bind to it, so that we automatically load the +v4l2 module after VCHI loads. + +5) Drop the gstreamer workaround. + +This was a temporary workaround for a bug that was fixed mid-2014, and +we should remove it before stabilizing the driver. + -- 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/6] staging: bcm2835-v4l2: Apply spelling fixes from checkpatch.
Generated with checkpatch.pl --fix-inplace and git add -p out of the
results.
Signed-off-by: Eric Anholt
---
drivers/staging/media/platform/bcm2835/bcm2835-camera.c | 6 +++---
drivers/staging/media/platform/bcm2835/mmal-vchiq.c | 12 ++--
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
index 4541a363905c..105d88102cd9 100644
--- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
+++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
@@ -1027,7 +1027,7 @@ static int mmal_setup_components(struct bm2835_mmal_dev
*dev,
dev->capture.encode_component = NULL;
}
- /* format dependant port setup */
+ /* format dependent port setup */
switch (mfmt->mmal_component) {
case MMAL_COMPONENT_CAMERA:
/* Make a further decision on port based on resolution */
@@ -1336,7 +1336,7 @@ int vidioc_enum_framesizes(struct file *file, void *fh,
return 0;
}
-/* timeperframe is arbitrary and continous */
+/* timeperframe is arbitrary and continuous */
static int vidioc_enum_frameintervals(struct file *file, void *priv,
struct v4l2_frmivalenum *fival)
{
@@ -1359,7 +1359,7 @@ static int vidioc_enum_frameintervals(struct file *file,
void *priv,
fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
- /* fill in stepwise (step=1.0 is requred by V4L2 spec) */
+ /* fill in stepwise (step=1.0 is required by V4L2 spec) */
fival->stepwise.min = tpf_min;
fival->stepwise.max = tpf_max;
fival->stepwise.step = (struct v4l2_fract) {1, 1};
diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
index cc968442adc4..f71dc3e9c3ae 100644
--- a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
+++ b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
@@ -239,7 +239,7 @@ static int bulk_receive(struct vchiq_mmal_instance
*instance,
pr_err("buffer list empty trying to submit bulk receive\n");
/* todo: this is a serious error, we should never have
-* commited a buffer_to_host operation to the mmal
+* committed a buffer_to_host operation to the mmal
* port without the buffer to back it up (underflow
* handling) and there is no obvious way to deal with
* this - how is the mmal servie going to react when
@@ -352,7 +352,7 @@ static int inline_receive(struct vchiq_mmal_instance
*instance,
pr_err("buffer list empty trying to receive inline\n");
/* todo: this is a serious error, we should never have
-* commited a buffer_to_host operation to the mmal
+* committed a buffer_to_host operation to the mmal
* port without the buffer to back it up (with
* underflow handling) and there is no obvious way to
* deal with this. Less bad than the bulk case as we
@@ -653,7 +653,7 @@ static void service_callback(void *param,
break;
default:
- /* messages dependant on header context to complete */
+ /* messages dependent on header context to complete */
/* todo: the msg.context really ought to be sanity
* checked before we just use it, afaict it comes back
@@ -780,7 +780,7 @@ static void dump_port_info(struct vchiq_mmal_port *port)
port->current_buffer.num,
port->current_buffer.size, port->current_buffer.alignment);
- pr_debug("elementry stream: type:%d encoding:0x%x varient:0x%x\n",
+ pr_debug("elementry stream: type:%d encoding:0x%x variant:0x%x\n",
port->format.type,
port->format.encoding, port->format.encoding_variant);
@@ -883,7 +883,7 @@ static int port_info_set(struct vchiq_mmal_instance
*instance,
return ret;
}
-/* use port info get message to retrive port information */
+/* use port info get message to retrieve port information */
static int port_info_get(struct vchiq_mmal_instance *instance,
struct vchiq_mmal_port *port)
{
@@ -923,7 +923,7 @@ static int port_info_get(struct vchiq_mmal_instance
*instance,
/* copy the values out of the message */
port->handle = rmsg->u.port_info_get_reply.port_handle;
- /* port type and index cached to use on port info set becuase
+ /* port type and index cached to use on port info set because
* it does not use a port handle
*/
port->type = rmsg->u.port_info_get_reply.port_type;
--
2.11.0
--
To unsubscribe from this list: send
[PATCH 5/6] staging: bcm2835-v4l2: Apply many whitespace fixes from checkpatch.
Generated with checkpatch.pl --fix-inplace, some manual fixes for
cases where checkpatch fixed one out of multiple lines of mis-indented
function parameters, and then git add -p out of the results.
I skipped some fixes that should probably instead be replaced with the
BIT() macro.
Signed-off-by: Eric Anholt
---
.../media/platform/bcm2835/bcm2835-camera.c| 90
drivers/staging/media/platform/bcm2835/controls.c | 236 ++---
.../staging/media/platform/bcm2835/mmal-vchiq.c| 13 +-
3 files changed, 167 insertions(+), 172 deletions(-)
diff --git a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
index 4f03949aecf3..4541a363905c 100644
--- a/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
+++ b/drivers/staging/media/platform/bcm2835/bcm2835-camera.c
@@ -38,7 +38,7 @@
#define BM2835_MMAL_MODULE_NAME "bcm2835-v4l2"
#define MIN_WIDTH 32
#define MIN_HEIGHT 32
-#define MIN_BUFFER_SIZE (80*1024)
+#define MIN_BUFFER_SIZE (80 * 1024)
#define MAX_VIDEO_MODE_WIDTH 1280
#define MAX_VIDEO_MODE_HEIGHT 720
@@ -420,6 +420,7 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
static int enable_camera(struct bm2835_mmal_dev *dev)
{
int ret;
+
if (!dev->camera_use_count) {
ret = vchiq_mmal_port_parameter_set(
dev->instance,
@@ -451,6 +452,7 @@ static int enable_camera(struct bm2835_mmal_dev *dev)
static int disable_camera(struct bm2835_mmal_dev *dev)
{
int ret;
+
if (!dev->camera_use_count) {
v4l2_err(&dev->v4l2_dev,
"Disabled the camera when already disabled\n");
@@ -459,6 +461,7 @@ static int disable_camera(struct bm2835_mmal_dev *dev)
dev->camera_use_count--;
if (!dev->camera_use_count) {
unsigned int i = 0x;
+
v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
"Disabling camera\n");
ret =
@@ -643,12 +646,14 @@ static void stop_streaming(struct vb2_queue *vq)
static void bm2835_mmal_lock(struct vb2_queue *vq)
{
struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vq);
+
mutex_lock(&dev->mutex);
}
static void bm2835_mmal_unlock(struct vb2_queue *vq)
{
struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vq);
+
mutex_unlock(&dev->mutex);
}
@@ -737,7 +742,7 @@ static int vidioc_try_fmt_vid_overlay(struct file *file,
void *priv,
1, 0);
v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
- "Overlay: Now w/h %dx%d l/t %dx%d\n",
+"Overlay: Now w/h %dx%d l/t %dx%d\n",
f->fmt.win.w.width, f->fmt.win.w.height,
f->fmt.win.w.left, f->fmt.win.w.top);
@@ -759,7 +764,7 @@ static int vidioc_s_fmt_vid_overlay(struct file *file, void
*priv,
dev->overlay = f->fmt.win;
if (dev->component[MMAL_COMPONENT_PREVIEW]->enabled) {
set_overlay_params(dev,
- &dev->component[MMAL_COMPONENT_PREVIEW]->input[0]);
+
&dev->component[MMAL_COMPONENT_PREVIEW]->input[0]);
}
return 0;
@@ -771,6 +776,7 @@ static int vidioc_overlay(struct file *file, void *f,
unsigned int on)
struct bm2835_mmal_dev *dev = video_drvdata(file);
struct vchiq_mmal_port *src;
struct vchiq_mmal_port *dst;
+
if ((on && dev->component[MMAL_COMPONENT_PREVIEW]->enabled) ||
(!on && !dev->component[MMAL_COMPONENT_PREVIEW]->enabled))
return 0; /* already in requested state */
@@ -842,7 +848,7 @@ static int vidioc_g_fbuf(struct file *file, void *fh,
a->fmt.pixelformat = V4L2_PIX_FMT_YUV420;
a->fmt.bytesperline = preview_port->es.video.width;
a->fmt.sizeimage = (preview_port->es.video.width *
- preview_port->es.video.height * 3)>>1;
+ preview_port->es.video.height * 3) >> 1;
a->fmt.colorspace = V4L2_COLORSPACE_SMPTE170M;
return 0;
@@ -958,8 +964,8 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void
*priv,
f->fmt.pix.field = V4L2_FIELD_NONE;
v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
- "Clipping/aligning %dx%d format %08X\n",
- f->fmt.pix.width, f->fmt.pix.height, f->fmt.pix.pixelformat);
+"Clipping/aligning %dx%d format %08X\n",
+f->fmt.pix.width, f->fmt.pix.height, f->fmt.pix.pixelformat);
v4l_bound_align_image(&f->fmt.pix.width, MIN_WIDTH, dev->max_width, 1,
&f
Re: [PATCH 6/6] staging: bcm2835-v4l2: Apply spelling fixes from checkpatch.
Joe Perches writes:
> On Fri, 2017-01-27 at 13:55 -0800, Eric Anholt wrote:
>> Generated with checkpatch.pl --fix-inplace and git add -p out of the
>> results.
>
> Maybe another.
>
>> diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
>> b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
> []
>> @@ -239,7 +239,7 @@ static int bulk_receive(struct vchiq_mmal_instance
>> *instance,
>> pr_err("buffer list empty trying to submit bulk receive\n");
>>
>> /* todo: this is a serious error, we should never have
>> - * commited a buffer_to_host operation to the mmal
>> + * committed a buffer_to_host operation to the mmal
>> * port without the buffer to back it up (underflow
>> * handling) and there is no obvious way to deal with
>> * this - how is the mmal servie going to react when
>
> Perhaps s/servie/service/ ?
I was trying to restrict this patch to just the fixes from checkpatch.
signature.asc
Description: PGP signature
Re: [PATCH 6/6] staging: bcm2835-v4l2: Apply spelling fixes from checkpatch.
Joe Perches writes:
> On Mon, 2017-01-30 at 12:05 -0800, Eric Anholt wrote:
>> Joe Perches writes:
>>
>> > On Fri, 2017-01-27 at 13:55 -0800, Eric Anholt wrote:
>> > > Generated with checkpatch.pl --fix-inplace and git add -p out of the
>> > > results.
>> >
>> > Maybe another.
>> >
>> > > diff --git a/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
>> > > b/drivers/staging/media/platform/bcm2835/mmal-vchiq.c
>> >
>> > []
>> > > @@ -239,7 +239,7 @@ static int bulk_receive(struct vchiq_mmal_instance
>> > > *instance,
>> > > pr_err("buffer list empty trying to submit bulk
>> > > receive\n");
>> > >
>> > > /* todo: this is a serious error, we should never have
>> > > - * commited a buffer_to_host operation to the mmal
>> > > + * committed a buffer_to_host operation to the mmal
>> > > * port without the buffer to back it up (underflow
>> > > * handling) and there is no obvious way to deal with
>> > > * this - how is the mmal servie going to react when
>> >
>> > Perhaps s/servie/service/ ?
>>
>> I was trying to restrict this patch to just the fixes from checkpatch.
>
> That's the wrong thing to do if you're fixing
> spelling defects. checkpatch is just one mechanism
> to identify some, and definitely not all, typos and
> spelling defects.
>
> If you fixing, fix. Don't just rely on the brainless
> tools, use your decidedly non-mechanical brain.
"if you touch anything, you must fix everything." If that's how things
work, I would just retract the patch.
signature.asc
Description: PGP signature
Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver
Mauro Carvalho Chehab writes:
> Em Fri, 27 Jan 2017 13:54:57 -0800
> Eric Anholt escreveu:
>
>> Here's my first pass at importing the camera driver. There's a bunch
>> of TODO left to it, most of which is documented, and the rest being
>> standard checkpatch fare.
>>
>> Unfortunately, when I try modprobing it on my pi3, the USB network
>> device dies, consistently. I'm not sure what's going on here yet, but
>> I'm going to keep working on some debug of it. I've unfortunately
>> changed a lot of variables (pi3 vs pi2, upstream vs downstream, vchi's
>> updates while in staging, 4.9 vs 4.4), so I probably won't figure it
>> out today.
>>
>> Note that the "Update the driver to the current VCHI API" patch will
>> conflict with the outstanding "Add vchi_queue_kernel_message and
>> vchi_queue_user_message" series, but the fix should be pretty obvious
>> when that lands.
>>
>> I built this against 4.10-rc1, but a merge with staging-next was clean
>> and still built fine.
>
> I'm trying it, building from the linux-next branch of the staging
> tree. No joy.
>
> That's what happens when I modprobe it:
>
> [ 991.841549] bcm2835_v4l2: module is from the staging directory, the
> quality is unknown, you have been warned.
> [ 991.842931] vchiq_get_state: g_state.remote == NULL
> [ 991.843437] vchiq_get_state: g_state.remote == NULL
> [ 991.843940] vchiq_get_state: g_state.remote == NULL
> [ 991.84] vchiq_get_state: g_state.remote == NULL
> [ 991.844947] vchiq_get_state: g_state.remote == NULL
> [ 991.845451] vchiq_get_state: g_state.remote == NULL
> [ 991.845954] vchiq_get_state: g_state.remote == NULL
> [ 991.846457] vchiq_get_state: g_state.remote == NULL
> [ 991.846961] vchiq_get_state: g_state.remote == NULL
> [ 991.847464] vchiq_get_state: g_state.remote == NULL
> [ 991.847969] vchiq: vchiq_initialise: videocore not initialized
>
> [ 991.847973] mmal_vchiq: Failed to initialise VCHI instance (status=-1)
Yeah, this failure mode sucks. I'm guessing you don't have a VCHI node
in the DT? Patch attached.
I haven't followed up on getting the DT documented so that it can be
merged, and it sounds like Michael has some plans for changing how VCHI
and VCHI's consumers get attached to each other so that it's not
DT-based anyway.
From 9488974b836b1fba7d32af34d612151872f9ce0d Mon Sep 17 00:00:00 2001
From: Eric Anholt
Date: Mon, 3 Oct 2016 11:23:34 -0700
Subject: [PATCH] ARM: bcm2835: Add VCHIQ to the DT.
Signed-off-by: Eric Anholt
---
arch/arm/boot/dts/bcm2835-rpi.dtsi | 8
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi
b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index caf2707680c1..f5fb5c5aa07a 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -26,6 +26,14 @@
firmware = <&firmware>;
#power-domain-cells = <1>;
};
+
+ vchiq {
+ compatible = "brcm,bcm2835-vchiq";
+ reg = <0x7e00b840 0xf>;
+ interrupts = <0 2>;
+ cache-line-size = <32>;
+ firmware = <&firmware>;
+ };
};
};
--
2.11.0
signature.asc
Description: PGP signature
Re: [PATCH 0/6] staging: BCM2835 MMAL V4L2 camera driver
Mauro Carvalho Chehab writes: > Em Wed, 15 Mar 2017 18:46:24 -0700 > Michael Zoran escreveu: > >> On Wed, 2017-03-15 at 22:08 -0300, Mauro Carvalho Chehab wrote: >> >> > No, I didn't. Thanks! Applied it but, unfortunately, didn't work. >> > Perhaps I'm missing some other patch. I'm compiling it from >> > the Greg's staging tree (branch staging-next): >> >https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging. >> > git/log/?h=staging-next >> > >> > Btw, as I'm running Raspbian, and didn't want to use compat32 bits, >> > I'm compiling the Kernel as an arm32 bits Kernel. >> > >> > I did a small trick to build the DTB on arm32: >> > >> >ln -sf ../../../arm64/boot/dts/broadcom/bcm2837-rpi-3-b.dts >> > arch/arm/boot/dts/bcm2837-rpi-3-b.dts >> >ln -sf ../../../arm64/boot/dts/broadcom/bcm2837.dtsi >> > arch/arm/boot/dts/bcm2837.dtsi >> >git checkout arch/arm/boot/dts/Makefile >> >sed "s,bcm2835-rpi-zero.dtb,bcm2835-rpi-zero.dtb bcm2837-rpi-3- >> > b.dtb," a && mv a arch/arm/boot/dts/Makefile >> > >> >> Two other hacks are currently needed to get the camera to work: >> >> 1. Add this to config.txt(This required to get the firmware to detect >> the camera) >> >> start_x=1 >> gpu_mem=128 > > I had this already. > >> >> 2. VC4 is incompatible with the firmware at this time, so you need >> to presently munge the build configuration. What you do is leave >> simplefb in the build config(I'm assuming you already have that), but >> you will need to remove VC4 from the config. >> >> The firmware currently adds a node for a simplefb for debugging >> purposes to show the boot log. Surprisingly, this is still good enough >> for basic usage and testing. > > That solved the issue. Thanks! It would be good to add a notice > about that at the TODO, not let it build if DRM_VC4. > > Please consider applying the enclosed path. The VC4 incompatibility (camera firmware's AWB ends up briefly using the GPU, without coordinating with the Linux driver) is supposed to be fixed in current firmware (https://github.com/raspberrypi/firmware/issues/760#issuecomment-287391025) signature.asc Description: PGP signature
Re: DRM device memory writeback (Mali-DP)
Ville Syrjälä writes: > On Fri, Jul 15, 2016 at 10:09:19AM +0100, Brian Starkey wrote: >> Hi Daniel, >> >> Thanks for taking a look. >> >> (+Cc Laurent) >> >> On Fri, Jul 15, 2016 at 09:33:34AM +0200, Daniel Vetter wrote: >> >On Thu, Jul 14, 2016 at 06:03:40PM +0100, Brian Starkey wrote: >> >> Hi, >> >> >> >> The Mali-DP display processors have a memory-writeback engine which >> >> can write the result of the composition (CRTC output) to a memory >> >> buffer in a variety of formats. >> >> >> >> We're looking for feedback/suggestions on how to expose this in the >> >> mali-dp DRM kernel driver - possibly via V4L2. >> >> >> >> We've got a few use cases where writeback is useful: >> >>- testing, to check the displayed image >> > >> >This might or might not need a separate interface. There are efforts to >> >make the intel kms validation tests in i-g-t generic (well under way >> >already), and part of that is creating a generic infrastructure to capture >> >display CRCs for functional tests (still in progress). >> > >> >But it might be better if userspace abstracts between full readback and >> >display CRC, assuming we can make full writeback cross-vendor enough for >> >that use-case. >> > >> >> I'd lean towards the userspace abstraction. >> Encumbering a simple CRC interface with all the complexity of >> full-writeback (size, scaling, pixel format, multi-planar etc.) sounds >> a bit unnecessary. >> >> Of course, if v4l2 isn't going to be the cross-vendor full-writeback >> implementation, then we need to be aiming to use whatever _is_ in >> the mali-dp driver. >> >> >>- screen recording >> >>- wireless display (e.g. Miracast) >> >>- dual-display clone mode >> >>- memory-to-memory composition >> >> Note that the HW is capable of writing one of the input planes instead >> >> of the CRTC output, but we've no good use-case for wanting to expose >> >> that. >> >> >> >> In our Android ADF driver[1] we exposed the memory write engine as >> >> part of the ADF device using ADF's "MEMORY" interface type. DRM/KMS >> >> doesn't have any similar support for memory output from CRTCs, but we >> >> want to expose the functionality in the mainline Mali-DP DRM driver. >> >> >> >> A previous discussion on the topic went towards exposing the >> >> memory-write engine via V4L2[2]. >> >> >> >> I'm thinking to userspace this would look like two distinct devices: >> >>- A DRM KMS display controller. >> >>- A V4L2 video source. >> >> They'd both exist in the same kernel driver. >> >> A V4L2 client can queue up (CAPTURE) buffers in the normal way, and >> >> the DRM driver would see if there's a buffer to dequeue every time a >> >> new modeset is received via the DRM API - if so, it can configure the >> >> HW to dump into it (one-shot operation). >> >> >> >> An implication of this is that if the screen is actively displaying a >> >> static scene and the V4L2 client queues up a buffer, it won't get >> >> filled until the DRM scene changes. This seems best, otherwise the >> >> V4L2 driver has to change the HW configuration out-of-band from the >> >> DRM device which sounds horribly racy. >> >> >> >> One further complication is scaling. Our HW has a scaler which can >> >> tasked with either scaling an input plane or the buffer being written >> >> to memory, but not both at the same time. This means we need to >> >> arbitrate the scaler between the DRM device (scaling input planes) and >> >> the V4L2 device (scaling output buffers). >> >> >> >> I think the simplest approach here is to allow V4L2 to "claim" the >> >> scaler by setting the image size (VIDIOC_S_FMT) to something other >> >> than the CRTC's current resolution. After that, any attempt to use the >> >> scaler on an input plane via DRM should fail atomic_check(). >> > >> >That's perfectly fine atomic_check behaviour. Only trouble is that the v4l >> >locking must integrate into the drm locking, but that should be doable. >> >Worst case you must shadow all v4l locks with a wait/wound >> >drm_modeset_lock to avoid deadlocks (since you could try to grab locks >> >from either end). >> > >> >> Yes, I haven't looked at the details of the locking but I'm hoping >> it's manageable. >> >> >> If the V4L2 client goes away or sets the image size to the CRTC's >> >> native resolution, then the DRM device is allowed to use the scaler. >> >> I don't know if/how the DRM device should communicate to userspace >> >> that the scaler is or isn't available for use. >> >> >> >> Any thoughts on this approach? >> >> Is it acceptable to both V4L2 and DRM folks? >> > >> >For streaming a V4L2 capture device seems like the right interface. But if >> >you want to use writeback in your compositor you must know which atomic >> >kms update results in which frame, since if you don't you can't use that >> >composited buffer for the next frame reliable. >> > >> >For that case I think a drm-only solution would be better, to make sure >> >you can do an atomic update and writeback
Re: [RFC PATCH 00/11] Introduce writeback connectors
Brian Starkey writes: > Hi, > > This RFC series introduces a new connector type: > DRM_MODE_CONNECTOR_WRITEBACK > It is a follow-on from a previous discussion: [1] > > Writeback connectors are used to expose the memory writeback engines > found in some display controllers, which can write a CRTC's > composition result to a memory buffer. > This is useful e.g. for testing, screen-recording, screenshots, > wireless display, display cloning, memory-to-memory composition. > > Patches 1-7 include the core framework changes required, and patches > 8-11 implement a writeback connector for the Mali-DP writeback engine. > The Mali-DP patches depend on this other series: [2]. > > The connector is given the FB_ID property for the output framebuffer, > and two new read-only properties: PIXEL_FORMATS and > PIXEL_FORMATS_SIZE, which expose the supported framebuffer pixel > formats of the engine. > > The EDID property is not exposed for writeback connectors. > > Writeback connector usage: > -- > Due to connector routing changes being treated as "full modeset" > operations, any client which wishes to use a writeback connector > should include the connector in every modeset. The writeback will not > actually become active until a framebuffer is attached. > > The writeback itself is enabled by attaching a framebuffer to the > FB_ID property of the connector. The driver must then ensure that the > CRTC content of that atomic commit is written into the framebuffer. > > The writeback works in a one-shot mode with each atomic commit. This > prevents the same content from being written multiple times. > In some cases (front-buffer rendering) there might be a desire for > continuous operation - I think a property could be added later for > this kind of control. > > Writeback can be disabled by setting FB_ID to zero. I think this sounds great, and the interface is just right IMO. I don't really see a use for continuous mode -- a sequence of one-shots makes a lot more sense because then you can know what data has changed, which anyone trying to use the writeback buffer would need to know. > Known issues: > - > * I'm not sure what "DPMS" should mean for writeback connectors. >It could be used to disable writeback (even when a framebuffer is >attached), or it could be hidden entirely (which would break the >legacy DPMS call for writeback connectors). > * With Daniel's recent re-iteration of the userspace API rules, I >fully expect to provide some userspace code to support this. The >question is what, and where? We want to use writeback for testing, >so perhaps some tests in igt is suitable. > * Documentation. Probably some portion of this cover letter needs to >make it into Documentation/ > * Synchronisation. Our hardware will finish the writeback by the next >vsync. I've not implemented fence support here, but it would be an >obvious addition. My hardware won't necessarily finish by the next vsync -- it trickles out at whatever rate it can find memory bandwidth to get the job done, and fires an interrupt when it's finished. So I would like some definition for how syncing works. One answer would be that these flips don't trigger their pageflip events until the writeback is done (so I need to collect both the vsync irq and the writeback irq before sending). Another would be that manage an independent fence for the writeback fb, so that you still immediately know when framebuffers from the previous scanout-only frame are idle. Also, tests for this in igt, please. Writeback in igt will give us so much more ability to cover KMS functionality on non-Intel hardware. signature.asc Description: PGP signature
Re: [RFC PATCH 00/11] Introduce writeback connectors
Brian Starkey writes: > Hi Eric, > > On Tue, Oct 11, 2016 at 12:01:14PM -0700, Eric Anholt wrote: >>Brian Starkey writes: >> >>> Hi, >>> >>> This RFC series introduces a new connector type: >>> DRM_MODE_CONNECTOR_WRITEBACK >>> It is a follow-on from a previous discussion: [1] >>> >>> Writeback connectors are used to expose the memory writeback engines >>> found in some display controllers, which can write a CRTC's >>> composition result to a memory buffer. >>> This is useful e.g. for testing, screen-recording, screenshots, >>> wireless display, display cloning, memory-to-memory composition. >>> >>> Patches 1-7 include the core framework changes required, and patches >>> 8-11 implement a writeback connector for the Mali-DP writeback engine. >>> The Mali-DP patches depend on this other series: [2]. >>> >>> The connector is given the FB_ID property for the output framebuffer, >>> and two new read-only properties: PIXEL_FORMATS and >>> PIXEL_FORMATS_SIZE, which expose the supported framebuffer pixel >>> formats of the engine. >>> >>> The EDID property is not exposed for writeback connectors. >>> >>> Writeback connector usage: >>> -- >>> Due to connector routing changes being treated as "full modeset" >>> operations, any client which wishes to use a writeback connector >>> should include the connector in every modeset. The writeback will not >>> actually become active until a framebuffer is attached. >>> >>> The writeback itself is enabled by attaching a framebuffer to the >>> FB_ID property of the connector. The driver must then ensure that the >>> CRTC content of that atomic commit is written into the framebuffer. >>> >>> The writeback works in a one-shot mode with each atomic commit. This >>> prevents the same content from being written multiple times. >>> In some cases (front-buffer rendering) there might be a desire for >>> continuous operation - I think a property could be added later for >>> this kind of control. >>> >>> Writeback can be disabled by setting FB_ID to zero. >> >>I think this sounds great, and the interface is just right IMO. >> > > Thanks, glad you like it! Hopefully you're equally agreeable with the > changes Daniel has been suggesting. Haven't seen anything objectionable there. >>> Known issues: >>> - >>> * I'm not sure what "DPMS" should mean for writeback connectors. >>>It could be used to disable writeback (even when a framebuffer is >>>attached), or it could be hidden entirely (which would break the >>>legacy DPMS call for writeback connectors). >>> * With Daniel's recent re-iteration of the userspace API rules, I >>>fully expect to provide some userspace code to support this. The >>>question is what, and where? We want to use writeback for testing, >>>so perhaps some tests in igt is suitable. >>> * Documentation. Probably some portion of this cover letter needs to >>>make it into Documentation/ >>> * Synchronisation. Our hardware will finish the writeback by the next >>>vsync. I've not implemented fence support here, but it would be an >>>obvious addition. >> >>My hardware won't necessarily finish by the next vsync -- it trickles >>out at whatever rate it can find memory bandwidth to get the job done, >>and fires an interrupt when it's finished. >> > > Is it bounded? You presumably have to finish the write-out before you > can change any input buffers? Yeah, I'm not sure what it would mean to try to swap my display list while write-out was happening. Each CRTC (each of which can only support one encoder at a time) has its own display list, though, so it could avoid blocking other modesets. >>So I would like some definition for how syncing works. One answer would >>be that these flips don't trigger their pageflip events until the >>writeback is done (so I need to collect both the vsync irq and the >>writeback irq before sending). Another would be that manage an >>independent fence for the writeback fb, so that you still immediately >>know when framebuffers from the previous scanout-only frame are idle. >> > > I much prefer the sound of the explicit fence approach. > > Hopefully we can agree that a new atomic commit can't be completed > whilst there's a writeback ongoing, otherwise managing the fence and > framebuffer lifetime sounds really tricky - they'd need to be decoupled > from the atomic_state and outlive the commit that spawned them. Oh, good point. I'm fine with that, but then my anticipated usecases for writeback are testing (don't care about performance) and fallback plane-squashing when a complicated modeset exceeds limits (in which case you have no simpler plane config to modeset to until the writeback is completed, anyway). signature.asc Description: PGP signature
Re: [PATCH v2 3/4] [media] bcm2835-unicam: Driver for CCP2/CSI2 camera interface
Dave Stevenson writes:
> diff --git a/drivers/media/platform/bcm2835/bcm2835-unicam.c
> b/drivers/media/platform/bcm2835/bcm2835-unicam.c
> new file mode 100644
> index 000..5b1adc3
> --- /dev/null
> +++ b/drivers/media/platform/bcm2835/bcm2835-unicam.c
> @@ -0,0 +1,2192 @@
> +/*
> + * BCM2835 Unicam capture Driver
> + *
> + * Copyright (C) 2017 - Raspberry Pi (Trading) Ltd.
> + *
> + * Dave Stevenson
> + *
> + * Based on TI am437x driver by Benoit Parrot and Lad, Prabhakar and
> + * TI CAL camera interface driver by Benoit Parrot.
> + *
Possible future improvement: this description of the driver is really
nice and could be turned into kernel-doc.
> + * There are two camera drivers in the kernel for BCM283x - this one
> + * and bcm2835-camera (currently in staging).
> + *
> + * This driver is purely the kernel control the Unicam peripheral - there
Maybe "This driver directly controls..."?
> + * is no involvement with the VideoCore firmware. Unicam receives CSI-2
> + * or CCP2 data and writes it into SDRAM. The only processing options are
> + * to repack Bayer data into an alternate format, and applying windowing
> + * (currently not implemented).
> + * It should be possible to connect it to any sensor with a
> + * suitable output interface and V4L2 subdevice driver.
> + *
> + * bcm2835-camera uses with the VideoCore firmware to control the sensor,
"uses the"
> + * Unicam, ISP, and all tuner control loops. Fully processed frames are
> + * delivered to the driver by the firmware. It only has sensor drivers
> + * for Omnivision OV5647, and Sony IMX219 sensors.
> + *
> + * The two drivers are mutually exclusive for the same Unicam instance.
> + * The VideoCore firmware checks the device tree configuration during boot.
> + * If it finds device tree nodes called csi0 or csi1 it will block the
> + * firmware from accessing the peripheral, and bcm2835-camera will
> + * not be able to stream data.
Thanks for describing this here!
> +/*
> + * The peripheral can unpack and repack between several of
> + * the Bayer raw formats, so any Bayer format can be advertised
> + * as the same Bayer order in each of the supported bit depths.
> + * Use lower case to avoid clashing with V4L2_PIX_FMT_SGBRG8
> + * formats.
> + */
> +#define PIX_FMT_ALL_BGGR v4l2_fourcc('b', 'g', 'g', 'r')
> +#define PIX_FMT_ALL_RGGB v4l2_fourcc('r', 'g', 'g', 'b')
> +#define PIX_FMT_ALL_GBRG v4l2_fourcc('g', 'b', 'r', 'g')
> +#define PIX_FMT_ALL_GRBG v4l2_fourcc('g', 'r', 'b', 'g')
Should thes fourccs be defined in a common v4l2 header, to reserve it
from clashing with others later?
This is really the only question I have about this driver before seeing
it merged. As far as me wearing my platform maintainer hat, I'm happy
with the driver, and my other little notes are optional.
> +static int unicam_probe(struct platform_device *pdev)
> +{
> + struct unicam_cfg *unicam_cfg;
> + struct unicam_device *unicam;
> + struct v4l2_ctrl_handler *hdl;
> + struct resource *res;
> + int ret;
> +
> + unicam = devm_kzalloc(&pdev->dev, sizeof(*unicam), GFP_KERNEL);
> + if (!unicam)
> + return -ENOMEM;
> +
> + unicam->pdev = pdev;
> + unicam_cfg = &unicam->cfg;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + unicam_cfg->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(unicam_cfg->base)) {
> + unicam_err(unicam, "Failed to get main io block\n");
> + return PTR_ERR(unicam_cfg->base);
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + unicam_cfg->clk_gate_base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(unicam_cfg->clk_gate_base)) {
> + unicam_err(unicam, "Failed to get 2nd io block\n");
> + return PTR_ERR(unicam_cfg->clk_gate_base);
> + }
> +
> + unicam->clock = devm_clk_get(&pdev->dev, "lp_clock");
> + if (IS_ERR(unicam->clock)) {
> + unicam_err(unicam, "Failed to get clock\n");
> + return PTR_ERR(unicam->clock);
> + }
> +
> + ret = platform_get_irq(pdev, 0);
> + if (ret <= 0) {
> + dev_err(&pdev->dev, "No IRQ resource\n");
> + return -ENODEV;
> + }
> + unicam_cfg->irq = ret;
> +
> + ret = devm_request_irq(&pdev->dev, unicam_cfg->irq, unicam_isr, 0,
> +"unicam_capture0", unicam);
Looks like there's no need to keep "irq" in the device private struct.
> + if (ret) {
> + dev_err(&pdev->dev, "Unable to request interrupt\n");
> + return -EINVAL;
> + }
> +
> + ret = v4l2_device_register(&pdev->dev, &unicam->v4l2_dev);
> + if (ret) {
> + unicam_err(unicam,
> +"Unable to register v4l2 device.\n");
> + return ret;
> + }
> +
> + /* Reserve space for the controls */
> + hdl = &unicam->ctrl_handler;
> + ret = v4l2_ctrl_handler_init(hdl, 16);
> + if (ret
Re: [PATCH v3 2/4] [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver
Dave Stevenson writes:
> Hi Stefan
>
> On 22 September 2017 at 07:45, Stefan Wahren wrote:
>> Hi Dave,
>>
>>> Dave Stevenson hat am 20. September 2017
>>> um 18:07 geschrieben:
>>>
>>>
>>> Document the DT bindings for the CSI2/CCP2 receiver peripheral
>>> (known as Unicam) on BCM283x SoCs.
>>>
>>> Signed-off-by: Dave Stevenson
>>> ---
>>>
>>> Changes since v2
>>> - Removed all references to Linux drivers.
>>> - Reworded section about disabling the firmware driver.
>>> - Renamed clock from "lp_clock" to "lp" in description and example.
>>> - Referred to video-interfaces.txt and stated requirements on
>>> remote-endpoint
>>> and data-lanes.
>>> - Corrected typo in example from csi to csi1.
>>> - Removed unnecessary #address-cells and #size-cells in example.
>>> - Removed setting of status from the example.
>>>
>>> .../devicetree/bindings/media/bcm2835-unicam.txt | 85
>>> ++
>>> 1 file changed, 85 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>>> b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>>> new file mode 100644
>>> index 000..7714fb3
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>>> @@ -0,0 +1,85 @@
>>> +Broadcom BCM283x Camera Interface (Unicam)
>>> +--
>>> +
>>> +The Unicam block on BCM283x SoCs is the receiver for either
>>> +CSI-2 or CCP2 data from image sensors or similar devices.
>>> +
>>> +The main platform using this SoC is the Raspberry Pi family of boards.
>>> +On the Pi the VideoCore firmware can also control this hardware block,
>>> +and driving it from two different processors will cause issues.
>>> +To avoid this, the firmware checks the device tree configuration
>>> +during boot. If it finds device tree nodes called csi0 or csi1 then
>>> +it will stop the firmware accessing the block, and it can then
>>> +safely be used via the device tree binding.
>>> +
>>> +Required properties:
>>> +===
>>> +- compatible : must be "brcm,bcm2835-unicam".
>>> +- reg: physical base address and length of the register
>>> sets for the
>>> + device.
>>> +- interrupts : should contain the IRQ line for this Unicam instance.
>>> +- clocks : list of clock specifiers, corresponding to entries in
>>> + clock-names property.
>>> +- clock-names: must contain an "lp" entry, matching entries in the
>>> + clocks property.
>>> +
>>> +Unicam supports a single port node. It should contain one 'port' child node
>>> +with child 'endpoint' node. Please refer to the bindings defined in
>>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>>> +
>>> +Within the endpoint node the "remote-endpoint" and "data-lanes" properties
>>> +are mandatory.
>>> +Data lane reordering is not supported so the data lanes must be in order,
>>> +starting at 1. The number of data lanes should represent the number of
>>> +usable lanes for the hardware block. That may be limited by either the SoC
>>> or
>>> +how the platform presents the interface, and the lower value must be used.
>>> +
>>> +Lane reordering is not supported on the clock lane either, so the optional
>>> +property "clock-lane" will implicitly be <0>.
>>> +Similarly lane inversion is not supported, therefore "lane-polarities" will
>>> +implicitly be <0 0 0 0 0>.
>>> +Neither of these values will be checked.
>>> +
>>> +Example:
>>> + csi1: csi1@7e801000 {
>>> + compatible = "brcm,bcm2835-unicam";
>>> + reg = <0x7e801000 0x800>,
>>> + <0x7e802004 0x4>;
>>
>> sorry, i didn't noticed this before. I'm afraid this is using a small range
>> of the CMI. Are there possible other users of this range? Does it make sense
>> to handle this by a separate clock driver?
>
> CMI (Clock Manager Image) consists of a total of 4 registers.
> 0x7e802000 is CMI_CAM0, with only bits 0-5 used for gating and
> inversion of the clock and data lanes (2 data lanes available on
> CAM0).
> 0x7e802004 is CMI_CAM1, with only bits 0-9 used for gating and
> inversion of the clock and data lanes (4 data lanes available on
> CAM1).
> 0x7e802008 is CMI_CAMTEST which I have no documentation or drivers for.
> 0x7e802010 is CMI_USBCTL. Only bit 6 is documented and is a reset. The
> default value is the required value. Nothing touches it that I can
> find.
Yeah, my conclusion previously was that it's appropriate to consider
this register as part of the unicam instance. No other HW block could
want to talk to it.
signature.asc
Description: PGP signature
Re: [Intel-gfx] [PATCH 01/17] dma-fence: Some kerneldoc polish for dma-fence.h
Daniel Vetter writes: > + /** > + * @fill_driver_data: > + * > + * Callback to fill in free-form debug info Returns amount of bytes > + * filled, or negative error on failure. Maybe this "Returns" should be on a new line? Or at least a '.' in between. Other than that, Reviewed-by: Eric Anholt Thanks! signature.asc Description: PGP signature
Re: [PATCH v3 2/4] [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver
Dave Stevenson writes:
> Hi Rob
>
> On 27 September 2017 at 22:51, Rob Herring wrote:
>> On Fri, Sep 22, 2017 at 05:07:22PM +0100, Dave Stevenson wrote:
>>> Hi Stefan
>>>
>>> On 22 September 2017 at 07:45, Stefan Wahren wrote:
>>> > Hi Dave,
>>> >
>>> >> Dave Stevenson hat am 20. September
>>> >> 2017 um 18:07 geschrieben:
>>> >>
>>> >>
>>> >> Document the DT bindings for the CSI2/CCP2 receiver peripheral
>>> >> (known as Unicam) on BCM283x SoCs.
>>> >>
>>> >> Signed-off-by: Dave Stevenson
>>> >> ---
>>> >>
>>> >> Changes since v2
>>> >> - Removed all references to Linux drivers.
>>> >> - Reworded section about disabling the firmware driver.
>>> >> - Renamed clock from "lp_clock" to "lp" in description and example.
>>> >> - Referred to video-interfaces.txt and stated requirements on
>>> >> remote-endpoint
>>> >> and data-lanes.
>>> >> - Corrected typo in example from csi to csi1.
>>> >> - Removed unnecessary #address-cells and #size-cells in example.
>>> >> - Removed setting of status from the example.
>>> >>
>>> >> .../devicetree/bindings/media/bcm2835-unicam.txt | 85
>>> >> ++
>>> >> 1 file changed, 85 insertions(+)
>>> >> create mode 100644
>>> >> Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>>> >>
>>> >> diff --git a/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>>> >> b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>>> >> new file mode 100644
>>> >> index 000..7714fb3
>>> >> --- /dev/null
>>> >> +++ b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
>>> >> @@ -0,0 +1,85 @@
>>> >> +Broadcom BCM283x Camera Interface (Unicam)
>>> >> +--
>>> >> +
>>> >> +The Unicam block on BCM283x SoCs is the receiver for either
>>> >> +CSI-2 or CCP2 data from image sensors or similar devices.
>>> >> +
>>> >> +The main platform using this SoC is the Raspberry Pi family of boards.
>>> >> +On the Pi the VideoCore firmware can also control this hardware block,
>>> >> +and driving it from two different processors will cause issues.
>>> >> +To avoid this, the firmware checks the device tree configuration
>>> >> +during boot. If it finds device tree nodes called csi0 or csi1 then
>>> >> +it will stop the firmware accessing the block, and it can then
>>> >> +safely be used via the device tree binding.
>>> >> +
>>> >> +Required properties:
>>> >> +===
>>> >> +- compatible : must be "brcm,bcm2835-unicam".
>>> >> +- reg: physical base address and length of the register
>>> >> sets for the
>>> >> + device.
>>> >> +- interrupts : should contain the IRQ line for this Unicam instance.
>>> >> +- clocks : list of clock specifiers, corresponding to entries in
>>> >> + clock-names property.
>>> >> +- clock-names: must contain an "lp" entry, matching entries in
>>> >> the
>>> >> + clocks property.
>>> >> +
>>> >> +Unicam supports a single port node. It should contain one 'port' child
>>> >> node
>>> >> +with child 'endpoint' node. Please refer to the bindings defined in
>>> >> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>>> >> +
>>> >> +Within the endpoint node the "remote-endpoint" and "data-lanes"
>>> >> properties
>>> >> +are mandatory.
>>> >> +Data lane reordering is not supported so the data lanes must be in
>>> >> order,
>>> >> +starting at 1. The number of data lanes should represent the number of
>>> >> +usable lanes for the hardware block. That may be limited by either the
>>> >> SoC or
>>> >> +how the platform presents the interface, and the lower value must be
>>> >> used.
>>> >> +
>>> >> +Lane reordering is not supported on the clock lane either, so the
>>> >> optional
>>> >> +property "clock-lane" will implicitly be <0>.
>>> >> +Similarly lane inversion is not supported, therefore "lane-polarities"
>>> >> will
>>> >> +implicitly be <0 0 0 0 0>.
>>> >> +Neither of these values will be checked.
>>> >> +
>>> >> +Example:
>>> >> + csi1: csi1@7e801000 {
>>> >> + compatible = "brcm,bcm2835-unicam";
>>> >> + reg = <0x7e801000 0x800>,
>>> >> + <0x7e802004 0x4>;
>>> >
>>> > sorry, i didn't noticed this before. I'm afraid this is using a small
>>> > range of the CMI. Are there possible other users of this range? Does it
>>> > make sense to handle this by a separate clock driver?
>>>
>>> CMI (Clock Manager Image) consists of a total of 4 registers.
>>> 0x7e802000 is CMI_CAM0, with only bits 0-5 used for gating and
>>> inversion of the clock and data lanes (2 data lanes available on
>>> CAM0).
>>> 0x7e802004 is CMI_CAM1, with only bits 0-9 used for gating and
>>> inversion of the clock and data lanes (4 data lanes available on
>>> CAM1).
>>> 0x7e802008 is CMI_CAMTEST which I have no documentation or drivers for.
>>> 0x7e802010 is CMI_USBCTL. Only bit 6 is documented and is a reset. The
>>> default value is the required value. Nothing touches it that
Re: [PATCH v3 2/4] [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver
Rob Herring writes: > On Tue, Nov 21, 2017 at 1:26 PM, Eric Anholt wrote: >> Dave Stevenson writes: >> >>> Hi Rob >>> >>> On 27 September 2017 at 22:51, Rob Herring wrote: >>>> On Fri, Sep 22, 2017 at 05:07:22PM +0100, Dave Stevenson wrote: >>>>> Hi Stefan >>>>> >>>>> On 22 September 2017 at 07:45, Stefan Wahren >>>>> wrote: >>>>> > Hi Dave, >>>>> > >>>>> >> Dave Stevenson hat am 20. September >>>>> >> 2017 um 18:07 geschrieben: >>>>> >> >>>>> >> >>>>> >> Document the DT bindings for the CSI2/CCP2 receiver peripheral >>>>> >> (known as Unicam) on BCM283x SoCs. >>>>> >> >>>>> >> Signed-off-by: Dave Stevenson >>>>> >> --- >>>>> >> >>>>> >> Changes since v2 >>>>> >> - Removed all references to Linux drivers. >>>>> >> - Reworded section about disabling the firmware driver. >>>>> >> - Renamed clock from "lp_clock" to "lp" in description and example. >>>>> >> - Referred to video-interfaces.txt and stated requirements on >>>>> >> remote-endpoint >>>>> >> and data-lanes. >>>>> >> - Corrected typo in example from csi to csi1. >>>>> >> - Removed unnecessary #address-cells and #size-cells in example. >>>>> >> - Removed setting of status from the example. >>>>> >> >>>>> >> .../devicetree/bindings/media/bcm2835-unicam.txt | 85 >>>>> >> ++ >>>>> >> 1 file changed, 85 insertions(+) >>>>> >> create mode 100644 >>>>> >> Documentation/devicetree/bindings/media/bcm2835-unicam.txt >>>>> >> >>>>> >> diff --git >>>>> >> a/Documentation/devicetree/bindings/media/bcm2835-unicam.txt >>>>> >> b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt >>>>> >> new file mode 100644 >>>>> >> index 000..7714fb3 >>>>> >> --- /dev/null >>>>> >> +++ b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt >>>>> >> @@ -0,0 +1,85 @@ >>>>> >> +Broadcom BCM283x Camera Interface (Unicam) >>>>> >> +-- >>>>> >> + >>>>> >> +The Unicam block on BCM283x SoCs is the receiver for either >>>>> >> +CSI-2 or CCP2 data from image sensors or similar devices. >>>>> >> + >>>>> >> +The main platform using this SoC is the Raspberry Pi family of boards. >>>>> >> +On the Pi the VideoCore firmware can also control this hardware block, >>>>> >> +and driving it from two different processors will cause issues. >>>>> >> +To avoid this, the firmware checks the device tree configuration >>>>> >> +during boot. If it finds device tree nodes called csi0 or csi1 then >>>>> >> +it will stop the firmware accessing the block, and it can then >>>>> >> +safely be used via the device tree binding. >>>>> >> + >>>>> >> +Required properties: >>>>> >> +=== >>>>> >> +- compatible : must be "brcm,bcm2835-unicam". >>>>> >> +- reg: physical base address and length of the >>>>> >> register sets for the >>>>> >> + device. >>>>> >> +- interrupts : should contain the IRQ line for this Unicam instance. >>>>> >> +- clocks : list of clock specifiers, corresponding to entries in >>>>> >> + clock-names property. >>>>> >> +- clock-names: must contain an "lp" entry, matching entries >>>>> >> in the >>>>> >> + clocks property. >>>>> >> + >>>>> >> +Unicam supports a single port node. It should contain one 'port' >>>>> >> child node >>>>> >> +with child 'endpoint' node. Please refer to the bindings defined in >>>>> >> +Documentation/devicetree/bindings/me
