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.
>
>>
>> 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.
