Hi Sakari,
I have questions about "bus-type" handling below.
On 10/15/20 5:41 PM, Hugues FRUCHET wrote:
> Hi Sakari,
>
> Thanks for reviewing,
>
> On 10/13/20 11:07 AM, Sakari Ailus wrote:
>> Hi Hugues,
>>
>> On Wed, Oct 07, 2020 at 06:14:50PM +0200, Hugues Fruchet wrote:
>>> Add support of BT656 embedded synchronization bus.
>>> This mode allows to save hardware synchro lines hsync & vsync
>>> by replacing them with synchro codes embedded in data stream.
>>> This bus type is only compatible with 8 bits width data bus.
>>> Due to reserved values 0x00 & 0xff used for synchro codes,
>>> valid data only vary from 0x1 to 0xfe, this is up to sensor
>>> to clip accordingly pixel data. As a consequence of this
>>> clipping, JPEG is not supported when using this bus type.
>>> DCMI crop feature is also not available with this bus type.
>>
>> You can have more than 62 characters per line. In fact, 75 is the
>> recommended maximum.
>>
>> You should also amend the bindings to cover BT.656 mode. Also bus-type
>> should probably be made mandatory, too.
> Will do both.
>
My understanding was that parallel BT656 bus is handled by the absence
of hsync/vsync-active, as stated in
Documentation/devicetree/bindings/media/video-interfaces.txt:
" Note, that if HSYNC and VSYNC polarities are not specified, embedded
synchronization may be required, where supported. "
Do you want to enforce usage of "bus-type" now in order to be more
explicit on parallel or bt656 ?
If I change binding to make "bus-type" required, I have to change the
current board devicetree files to add support of it, and I would prefer
to not do that. Is there a way to put "bus-type" as not mandatory and
rely on absence of hsync/vysnc to trig BT656 ? I will nevertheless amend
bindings in order to document that.
What do you suggest ?
>>
>>>
>>> Signed-off-by: Hugues Fruchet <hugues.fruchet@xxxxxx>
>>> ---
>>> drivers/media/platform/stm32/stm32-dcmi.c | 37
>>> +++++++++++++++++++++++++++++--
>>> 1 file changed, 35 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/stm32/stm32-dcmi.c
>>> b/drivers/media/platform/stm32/stm32-dcmi.c
>>> index fd1c41c..d7d7cdb 100644
>>> --- a/drivers/media/platform/stm32/stm32-dcmi.c
>>> +++ b/drivers/media/platform/stm32/stm32-dcmi.c
>>> @@ -157,6 +157,7 @@ struct stm32_dcmi {
>>> struct vb2_queue queue;
>>> struct v4l2_fwnode_bus_parallel bus;
>>> + enum v4l2_mbus_type bus_type;
>>> struct completion complete;
>>> struct clk *mclk;
>>> enum state state;
>>> @@ -777,6 +778,23 @@ static int dcmi_start_streaming(struct vb2_queue
>>> *vq, unsigned int count)
>>> if (dcmi->bus.flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
>>> val |= CR_PCKPOL;
>>> + /*
>>> + * BT656 embedded synchronisation bus mode.
>>> + *
>>> + * Default SAV/EAV mode is supported here with default codes
>>> + * SAV=0xff000080 & EAV=0xff00009d.
>>> + * With DCMI this means LSC=SAV=0x80 & LEC=EAV=0x9d.
>>> + */
>>> + if (dcmi->bus_type == V4L2_MBUS_BT656) {
>>> + val |= CR_ESS;
>>> +
>>> + /* Unmask all codes */
>>> + reg_write(dcmi->regs, DCMI_ESUR, 0xffffffff);/*
>>> FEC:LEC:LSC:FSC */
>>> +
>>> + /* Trig on LSC=0x80 & LEC=0x9d codes, ignore FSC and FEC */
>>> + reg_write(dcmi->regs, DCMI_ESCR, 0xff9d80ff);/*
>>> FEC:LEC:LSC:FSC */
>>> + }
>>> +
>>> reg_write(dcmi->regs, DCMI_CR, val);
>>> /* Set crop */
>>> @@ -1067,8 +1085,9 @@ static int dcmi_set_fmt(struct stm32_dcmi
>>> *dcmi, struct v4l2_format *f)
>>> if (ret)
>>> return ret;
>>> - /* Disable crop if JPEG is requested */
>>> - if (pix->pixelformat == V4L2_PIX_FMT_JPEG)
>>> + /* Disable crop if JPEG is requested or BT656 bus is selected */
>>> + if (pix->pixelformat == V4L2_PIX_FMT_JPEG &&
>>> + dcmi->bus_type != V4L2_MBUS_BT656)
>>> dcmi->do_crop = false;
>>> /* pix to mbus format */
>>> @@ -1592,6 +1611,11 @@ static int dcmi_formats_init(struct stm32_dcmi
>>> *dcmi)
>>> if (dcmi_formats[i].mbus_code != mbus_code.code)
>>> continue;
>>> + /* Exclude JPEG if BT656 bus is selected */
>>> + if (dcmi_formats[i].fourcc == V4L2_PIX_FMT_JPEG &&
>>> + dcmi->bus_type == V4L2_MBUS_BT656)
>>> + continue;
>>> +
>>> /* Code supported, have we got this fourcc yet? */
>>> for (j = 0; j < num_fmts; j++)
>>> if (sd_fmts[j]->fourcc ==
>>> @@ -1873,9 +1897,18 @@ static int dcmi_probe(struct platform_device
>>> *pdev)
>>> dev_err(&pdev->dev, "CSI bus not supported\n");
>>> return -ENODEV;
>>> }
>>> +
>>> + if (ep.bus_type == V4L2_MBUS_BT656 &&
>>> + ep.bus.parallel.bus_width != 8) {
>>> + dev_err(&pdev->dev, "BT656 bus conflicts with %d bits bus
>>> width (8 bits required)\n",
>>> + ep.bus.parallel.bus_width);
>>
>> bus_width is unsigned here.
> I will fix it.
>
>>
>>> + return -ENODEV;
>>> + }
>>> +
>>> dcmi->bus.flags = ep.bus.parallel.flags;
>>> dcmi->bus.bus_width = ep.bus.parallel.bus_width;
>>> dcmi->bus.data_shift = ep.bus.parallel.data_shift;
>>> + dcmi->bus_type = ep.bus_type;
>>> irq = platform_get_irq(pdev, 0);
>>> if (irq <= 0)
>>
>
> BR,
> Hugues.
BR,
Hugues.
