Re: [V3, 4/4] media: platform: dwc: Add MIPI CSI-2 controller driver
On 11.01.2019 10:11, Luis de Oliveira wrote:
>
>
> On 11-Jan-19 7:25, eugen.hris...@microchip.com wrote:
>>
>>
>> On 10.01.2019 18:18, Luis de Oliveira wrote:
>>>
>>>
>>> On 09-Jan-19 13:07, eugen.hris...@microchip.com wrote:
On 19.10.2018 15:52, Luis Oliveira wrote:
> Add the Synopsys MIPI CSI-2 controller driver. This
> controller driver is divided in platform dependent functions
> and core functions. It also includes a platform for future
> DesignWare drivers.
>
> Signed-off-by: Luis Oliveira
> ---
> Changelog
> v2-V3
> - exposed IPI settings to userspace
> - fixed headers
>
> MAINTAINERS | 11 +
> drivers/media/platform/dwc/Kconfig | 30 +-
> drivers/media/platform/dwc/Makefile | 2 +
> drivers/media/platform/dwc/dw-csi-plat.c | 699
> +++
> drivers/media/platform/dwc/dw-csi-plat.h | 77
> drivers/media/platform/dwc/dw-mipi-csi.c | 494 ++
> drivers/media/platform/dwc/dw-mipi-csi.h | 202 +
> include/media/dwc/dw-mipi-csi-pltfrm.h | 102 +
> 8 files changed, 1616 insertions(+), 1 deletion(-)
> create mode 100644 drivers/media/platform/dwc/dw-csi-plat.c
> create mode 100644 drivers/media/platform/dwc/dw-csi-plat.h
> create mode 100644 drivers/media/platform/dwc/dw-mipi-csi.c
> create mode 100644 drivers/media/platform/dwc/dw-mipi-csi.h
> create mode 100644 include/media/dwc/dw-mipi-csi-pltfrm.h
>
[snip]
> +/* Video formats supported by the MIPI CSI-2 */
> +const struct mipi_fmt dw_mipi_csi_formats[] = {
> + {
> + /* RAW 8 */
> + .code = MEDIA_BUS_FMT_SBGGR8_1X8,
> + .depth = 8,
> + },
> + {
> + /* RAW 10 */
> + .code = MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE,
> + .depth = 10,
> + },
Hi Luis,
Any reason why RAW12 format is not handled here ?
Here, namely MEDIA_BUS_FMT_SBGGR12_1X12 etc.
>>> Hi Eugen,
>>>
>>> My Hw testing setup currently does not support it, so I didn't add it.
>>>
> + {
> + /* RGB 565 */
> + .code = MEDIA_BUS_FMT_RGB565_2X8_BE,
> + .depth = 16,
> + },
> + {
> + /* BGR 565 */
> + .code = MEDIA_BUS_FMT_RGB565_2X8_LE,
> + .depth = 16,
> + },
> + {
> + /* RGB 888 */
> + .code = MEDIA_BUS_FMT_RGB888_2X12_LE,
> + .depth = 24,
> + },
> + {
> + /* BGR 888 */
> + .code = MEDIA_BUS_FMT_RGB888_2X12_BE,
> + .depth = 24,
> + },
> +};
[snip]
> +
> +void dw_mipi_csi_set_ipi_fmt(struct mipi_csi_dev *csi_dev)
> +{
> + struct device *dev = csi_dev->dev;
> +
> + if (csi_dev->ipi_dt)
> + dw_mipi_csi_write(csi_dev, reg.IPI_DATA_TYPE, csi_dev->ipi_dt);
> + else {
> + switch (csi_dev->fmt->code) {
> + case MEDIA_BUS_FMT_RGB565_2X8_BE:
> + case MEDIA_BUS_FMT_RGB565_2X8_LE:
> + dw_mipi_csi_write(csi_dev,
> + reg.IPI_DATA_TYPE, CSI_2_RGB565);
> + dev_dbg(dev, "DT: RGB 565");
> + break;
> +
> + case MEDIA_BUS_FMT_RGB888_2X12_LE:
> + case MEDIA_BUS_FMT_RGB888_2X12_BE:
> + dw_mipi_csi_write(csi_dev,
> + reg.IPI_DATA_TYPE, CSI_2_RGB888);
> + dev_dbg(dev, "DT: RGB 888");
> + break;
> + case MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE:
> + dw_mipi_csi_write(csi_dev,
> + reg.IPI_DATA_TYPE, CSI_2_RAW10);
> + dev_dbg(dev, "DT: RAW 10");
> + break;
> + case MEDIA_BUS_FMT_SBGGR8_1X8:
> + dw_mipi_csi_write(csi_dev,
> + reg.IPI_DATA_TYPE, CSI_2_RAW8);
> + dev_dbg(dev, "DT: RAW 8");
> + break;
Same here, in CSI_2_RAW12 case it will default to a RGB565 case.
Thanks,
Eugen
>>> I will try to add the support for this data type in my next patchset if not
>>> I
>>> will flag it as unsupported for now in the commit message and code.
>>
>> Hi Luis,
>>
>> I am currently trying to see if your driver works , and I need the RAW12
>> type, that's why I am asking about it.
>>
>> One question related to the subdevice you create here, how do you
>> register this subdev into the media subsystem ? sync or async, or not at
>> all ?
>> After the driver probes, there is no call to the set format functions, I
>> added a node inside the Device tree, I see you are registering media
>> pads, but the o
Re: [V3, 4/4] media: platform: dwc: Add MIPI CSI-2 controller driver
On 11-Jan-19 7:25, eugen.hris...@microchip.com wrote:
>
>
> On 10.01.2019 18:18, Luis de Oliveira wrote:
>>
>>
>> On 09-Jan-19 13:07, eugen.hris...@microchip.com wrote:
>>>
>>>
>>> On 19.10.2018 15:52, Luis Oliveira wrote:
Add the Synopsys MIPI CSI-2 controller driver. This
controller driver is divided in platform dependent functions
and core functions. It also includes a platform for future
DesignWare drivers.
Signed-off-by: Luis Oliveira
---
Changelog
v2-V3
- exposed IPI settings to userspace
- fixed headers
MAINTAINERS | 11 +
drivers/media/platform/dwc/Kconfig | 30 +-
drivers/media/platform/dwc/Makefile | 2 +
drivers/media/platform/dwc/dw-csi-plat.c | 699
+++
drivers/media/platform/dwc/dw-csi-plat.h | 77
drivers/media/platform/dwc/dw-mipi-csi.c | 494 ++
drivers/media/platform/dwc/dw-mipi-csi.h | 202 +
include/media/dwc/dw-mipi-csi-pltfrm.h | 102 +
8 files changed, 1616 insertions(+), 1 deletion(-)
create mode 100644 drivers/media/platform/dwc/dw-csi-plat.c
create mode 100644 drivers/media/platform/dwc/dw-csi-plat.h
create mode 100644 drivers/media/platform/dwc/dw-mipi-csi.c
create mode 100644 drivers/media/platform/dwc/dw-mipi-csi.h
create mode 100644 include/media/dwc/dw-mipi-csi-pltfrm.h
>>>
>>> [snip]
>>>
+/* Video formats supported by the MIPI CSI-2 */
+const struct mipi_fmt dw_mipi_csi_formats[] = {
+ {
+ /* RAW 8 */
+ .code = MEDIA_BUS_FMT_SBGGR8_1X8,
+ .depth = 8,
+ },
+ {
+ /* RAW 10 */
+ .code = MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE,
+ .depth = 10,
+ },
>>>
>>> Hi Luis,
>>>
>>> Any reason why RAW12 format is not handled here ?
>>>
>>> Here, namely MEDIA_BUS_FMT_SBGGR12_1X12 etc.
>>>
>> Hi Eugen,
>>
>> My Hw testing setup currently does not support it, so I didn't add it.
>>
+ {
+ /* RGB 565 */
+ .code = MEDIA_BUS_FMT_RGB565_2X8_BE,
+ .depth = 16,
+ },
+ {
+ /* BGR 565 */
+ .code = MEDIA_BUS_FMT_RGB565_2X8_LE,
+ .depth = 16,
+ },
+ {
+ /* RGB 888 */
+ .code = MEDIA_BUS_FMT_RGB888_2X12_LE,
+ .depth = 24,
+ },
+ {
+ /* BGR 888 */
+ .code = MEDIA_BUS_FMT_RGB888_2X12_BE,
+ .depth = 24,
+ },
+};
>>>
>>> [snip]
>>>
+
+void dw_mipi_csi_set_ipi_fmt(struct mipi_csi_dev *csi_dev)
+{
+ struct device *dev = csi_dev->dev;
+
+ if (csi_dev->ipi_dt)
+ dw_mipi_csi_write(csi_dev, reg.IPI_DATA_TYPE, csi_dev->ipi_dt);
+ else {
+ switch (csi_dev->fmt->code) {
+ case MEDIA_BUS_FMT_RGB565_2X8_BE:
+ case MEDIA_BUS_FMT_RGB565_2X8_LE:
+ dw_mipi_csi_write(csi_dev,
+ reg.IPI_DATA_TYPE, CSI_2_RGB565);
+ dev_dbg(dev, "DT: RGB 565");
+ break;
+
+ case MEDIA_BUS_FMT_RGB888_2X12_LE:
+ case MEDIA_BUS_FMT_RGB888_2X12_BE:
+ dw_mipi_csi_write(csi_dev,
+ reg.IPI_DATA_TYPE, CSI_2_RGB888);
+ dev_dbg(dev, "DT: RGB 888");
+ break;
+ case MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE:
+ dw_mipi_csi_write(csi_dev,
+ reg.IPI_DATA_TYPE, CSI_2_RAW10);
+ dev_dbg(dev, "DT: RAW 10");
+ break;
+ case MEDIA_BUS_FMT_SBGGR8_1X8:
+ dw_mipi_csi_write(csi_dev,
+ reg.IPI_DATA_TYPE, CSI_2_RAW8);
+ dev_dbg(dev, "DT: RAW 8");
+ break;
>>>
>>> Same here, in CSI_2_RAW12 case it will default to a RGB565 case.
>>>
>>> Thanks,
>>>
>>> Eugen
>>>
>>>
>> I will try to add the support for this data type in my next patchset if not I
>> will flag it as unsupported for now in the commit message and code.
>
> Hi Luis,
>
> I am currently trying to see if your driver works , and I need the RAW12
> type, that's why I am asking about it.
>
> One question related to the subdevice you create here, how do you
> register this subdev into the media subsystem ? sync or async, or not at
> all ?
> After the driver probes, there is no call to the set format functions, I
> added a node inside the Device tree, I see you are registering media
> pads, but the other endpoint needs to be an async waiting for completion
> node or your subdev registers in some other way ? (maybe some link
> validation req
Re: [V3, 4/4] media: platform: dwc: Add MIPI CSI-2 controller driver
On 10.01.2019 18:18, Luis de Oliveira wrote:
>
>
> On 09-Jan-19 13:07, eugen.hris...@microchip.com wrote:
>>
>>
>> On 19.10.2018 15:52, Luis Oliveira wrote:
>>> Add the Synopsys MIPI CSI-2 controller driver. This
>>> controller driver is divided in platform dependent functions
>>> and core functions. It also includes a platform for future
>>> DesignWare drivers.
>>>
>>> Signed-off-by: Luis Oliveira
>>> ---
>>> Changelog
>>> v2-V3
>>> - exposed IPI settings to userspace
>>> - fixed headers
>>>
>>>MAINTAINERS | 11 +
>>>drivers/media/platform/dwc/Kconfig | 30 +-
>>>drivers/media/platform/dwc/Makefile | 2 +
>>>drivers/media/platform/dwc/dw-csi-plat.c | 699
>>> +++
>>>drivers/media/platform/dwc/dw-csi-plat.h | 77
>>>drivers/media/platform/dwc/dw-mipi-csi.c | 494 ++
>>>drivers/media/platform/dwc/dw-mipi-csi.h | 202 +
>>>include/media/dwc/dw-mipi-csi-pltfrm.h | 102 +
>>>8 files changed, 1616 insertions(+), 1 deletion(-)
>>>create mode 100644 drivers/media/platform/dwc/dw-csi-plat.c
>>>create mode 100644 drivers/media/platform/dwc/dw-csi-plat.h
>>>create mode 100644 drivers/media/platform/dwc/dw-mipi-csi.c
>>>create mode 100644 drivers/media/platform/dwc/dw-mipi-csi.h
>>>create mode 100644 include/media/dwc/dw-mipi-csi-pltfrm.h
>>>
>>
>> [snip]
>>
>>> +/* Video formats supported by the MIPI CSI-2 */
>>> +const struct mipi_fmt dw_mipi_csi_formats[] = {
>>> + {
>>> + /* RAW 8 */
>>> + .code = MEDIA_BUS_FMT_SBGGR8_1X8,
>>> + .depth = 8,
>>> + },
>>> + {
>>> + /* RAW 10 */
>>> + .code = MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE,
>>> + .depth = 10,
>>> + },
>>
>> Hi Luis,
>>
>> Any reason why RAW12 format is not handled here ?
>>
>> Here, namely MEDIA_BUS_FMT_SBGGR12_1X12 etc.
>>
> Hi Eugen,
>
> My Hw testing setup currently does not support it, so I didn't add it.
>
>>> + {
>>> + /* RGB 565 */
>>> + .code = MEDIA_BUS_FMT_RGB565_2X8_BE,
>>> + .depth = 16,
>>> + },
>>> + {
>>> + /* BGR 565 */
>>> + .code = MEDIA_BUS_FMT_RGB565_2X8_LE,
>>> + .depth = 16,
>>> + },
>>> + {
>>> + /* RGB 888 */
>>> + .code = MEDIA_BUS_FMT_RGB888_2X12_LE,
>>> + .depth = 24,
>>> + },
>>> + {
>>> + /* BGR 888 */
>>> + .code = MEDIA_BUS_FMT_RGB888_2X12_BE,
>>> + .depth = 24,
>>> + },
>>> +};
>>
>> [snip]
>>
>>> +
>>> +void dw_mipi_csi_set_ipi_fmt(struct mipi_csi_dev *csi_dev)
>>> +{
>>> + struct device *dev = csi_dev->dev;
>>> +
>>> + if (csi_dev->ipi_dt)
>>> + dw_mipi_csi_write(csi_dev, reg.IPI_DATA_TYPE, csi_dev->ipi_dt);
>>> + else {
>>> + switch (csi_dev->fmt->code) {
>>> + case MEDIA_BUS_FMT_RGB565_2X8_BE:
>>> + case MEDIA_BUS_FMT_RGB565_2X8_LE:
>>> + dw_mipi_csi_write(csi_dev,
>>> + reg.IPI_DATA_TYPE, CSI_2_RGB565);
>>> + dev_dbg(dev, "DT: RGB 565");
>>> + break;
>>> +
>>> + case MEDIA_BUS_FMT_RGB888_2X12_LE:
>>> + case MEDIA_BUS_FMT_RGB888_2X12_BE:
>>> + dw_mipi_csi_write(csi_dev,
>>> + reg.IPI_DATA_TYPE, CSI_2_RGB888);
>>> + dev_dbg(dev, "DT: RGB 888");
>>> + break;
>>> + case MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE:
>>> + dw_mipi_csi_write(csi_dev,
>>> + reg.IPI_DATA_TYPE, CSI_2_RAW10);
>>> + dev_dbg(dev, "DT: RAW 10");
>>> + break;
>>> + case MEDIA_BUS_FMT_SBGGR8_1X8:
>>> + dw_mipi_csi_write(csi_dev,
>>> + reg.IPI_DATA_TYPE, CSI_2_RAW8);
>>> + dev_dbg(dev, "DT: RAW 8");
>>> + break;
>>
>> Same here, in CSI_2_RAW12 case it will default to a RGB565 case.
>>
>> Thanks,
>>
>> Eugen
>>
>>
> I will try to add the support for this data type in my next patchset if not I
> will flag it as unsupported for now in the commit message and code.
Hi Luis,
I am currently trying to see if your driver works , and I need the RAW12
type, that's why I am asking about it.
One question related to the subdevice you create here, how do you
register this subdev into the media subsystem ? sync or async, or not at
all ?
After the driver probes, there is no call to the set format functions, I
added a node inside the Device tree, I see you are registering media
pads, but the other endpoint needs to be an async waiting for completion
node or your subdev registers in some other way ? (maybe some link
validation required ?)
Thanks for your help,
Eugen
>
> Thanks for your review,
> Luis
>
>>
>>> + default:
>>> + dw_mipi_csi_write(csi_dev
Re: [V3, 4/4] media: platform: dwc: Add MIPI CSI-2 controller driver
Hi Laurent, Sorry for taking so long to answer. I was stuck in other projects and lost track of this. My answers inline, On 14-Nov-18 20:22, Laurent Pinchart wrote: > Hi Luis, > > Thank you for the patch. > > On Friday, 19 October 2018 15:52:26 EET Luis Oliveira wrote: >> Add the Synopsys MIPI CSI-2 controller driver. This >> controller driver is divided in platform dependent functions >> and core functions. It also includes a platform for future >> DesignWare drivers. >> >> Signed-off-by: Luis Oliveira >> --- >> Changelog >> v2-V3 >> - exposed IPI settings to userspace > > Could you please explain why you need this and can't use standard APIs ? > Custom sysfs attributes are needed, they need to be documented in > Documentation/ABI/. > IPI - Image Pixel Interface enables to access direct video stream in our CSI-2 Host IP and needs to be configured to match the video source. I can't hard code it. But maybe I can think of a way to configure it using the video configuration. I will try do that in the next patchset. >> - fixed headers >> >> MAINTAINERS | 11 + >> drivers/media/platform/dwc/Kconfig | 30 +- >> drivers/media/platform/dwc/Makefile | 2 + >> drivers/media/platform/dwc/dw-csi-plat.c | 699 >> drivers/media/platform/dwc/dw-csi-plat.h | 77 >> drivers/media/platform/dwc/dw-mipi-csi.c | 494 ++ >> drivers/media/platform/dwc/dw-mipi-csi.h | 202 + >> include/media/dwc/dw-mipi-csi-pltfrm.h | 102 + >> 8 files changed, 1616 insertions(+), 1 deletion(-) >> create mode 100644 drivers/media/platform/dwc/dw-csi-plat.c >> create mode 100644 drivers/media/platform/dwc/dw-csi-plat.h >> create mode 100644 drivers/media/platform/dwc/dw-mipi-csi.c >> create mode 100644 drivers/media/platform/dwc/dw-mipi-csi.h >> create mode 100644 include/media/dwc/dw-mipi-csi-pltfrm.h >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index da2e509..fd5f1fc 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -14032,6 +14032,16 @@ S: Maintained >> F: drivers/dma/dwi-axi-dmac/ >> F: Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.txt >> >> +SYNOPSYS DESIGNWARE MIPI CSI-2 HOST VIDEO PLATFORM >> +M: Luis Oliveira >> +L: linux-me...@vger.kernel.org >> +T: git git://linuxtv.org/media_tree.git >> +S: Maintained >> +F: drivers/media/platform/dwc >> +F: include/media/dwc/dw-mipi-csi-pltfrm.h >> +F: Documentation/devicetree/bindings/media/snps,dw-csi-plat.txt >> +F: Documentation/devicetree/bindings/phy/snps,dphy-rx.txt > > These two lines belong to patches 1/4 and 3/4. Doesn't checkpatch.pl warn > about this ? Now that I mentioned checkpatch.pl, I tried running it on this > series, and it generates a fair number of warnings and errors. Could you > please fix them ? > I did, but maybe - or probably - i did not fix them all. Sorry. I am preparing a new patchset and I will make sure everything goes right this time. >> + >> SYNOPSYS DESIGNWARE DMAC DRIVER >> M: Viresh Kumar >> R: Andy Shevchenko >> @@ -16217,3 +16227,4 @@ T: git >> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git S: Buried >> alive in reporters >> F: * >> F: */ >> + > > Stray new line. > Noted. > [snip] > Thank you, Luis
Re: [V3, 4/4] media: platform: dwc: Add MIPI CSI-2 controller driver
On 09-Jan-19 13:07, eugen.hris...@microchip.com wrote:
>
>
> On 19.10.2018 15:52, Luis Oliveira wrote:
>> Add the Synopsys MIPI CSI-2 controller driver. This
>> controller driver is divided in platform dependent functions
>> and core functions. It also includes a platform for future
>> DesignWare drivers.
>>
>> Signed-off-by: Luis Oliveira
>> ---
>> Changelog
>> v2-V3
>> - exposed IPI settings to userspace
>> - fixed headers
>>
>> MAINTAINERS | 11 +
>> drivers/media/platform/dwc/Kconfig | 30 +-
>> drivers/media/platform/dwc/Makefile | 2 +
>> drivers/media/platform/dwc/dw-csi-plat.c | 699
>> +++
>> drivers/media/platform/dwc/dw-csi-plat.h | 77
>> drivers/media/platform/dwc/dw-mipi-csi.c | 494 ++
>> drivers/media/platform/dwc/dw-mipi-csi.h | 202 +
>> include/media/dwc/dw-mipi-csi-pltfrm.h | 102 +
>> 8 files changed, 1616 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/media/platform/dwc/dw-csi-plat.c
>> create mode 100644 drivers/media/platform/dwc/dw-csi-plat.h
>> create mode 100644 drivers/media/platform/dwc/dw-mipi-csi.c
>> create mode 100644 drivers/media/platform/dwc/dw-mipi-csi.h
>> create mode 100644 include/media/dwc/dw-mipi-csi-pltfrm.h
>>
>
> [snip]
>
>> +/* Video formats supported by the MIPI CSI-2 */
>> +const struct mipi_fmt dw_mipi_csi_formats[] = {
>> +{
>> +/* RAW 8 */
>> +.code = MEDIA_BUS_FMT_SBGGR8_1X8,
>> +.depth = 8,
>> +},
>> +{
>> +/* RAW 10 */
>> +.code = MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE,
>> +.depth = 10,
>> +},
>
> Hi Luis,
>
> Any reason why RAW12 format is not handled here ?
>
> Here, namely MEDIA_BUS_FMT_SBGGR12_1X12 etc.
>
Hi Eugen,
My Hw testing setup currently does not support it, so I didn't add it.
>> +{
>> +/* RGB 565 */
>> +.code = MEDIA_BUS_FMT_RGB565_2X8_BE,
>> +.depth = 16,
>> +},
>> +{
>> +/* BGR 565 */
>> +.code = MEDIA_BUS_FMT_RGB565_2X8_LE,
>> +.depth = 16,
>> +},
>> +{
>> +/* RGB 888 */
>> +.code = MEDIA_BUS_FMT_RGB888_2X12_LE,
>> +.depth = 24,
>> +},
>> +{
>> +/* BGR 888 */
>> +.code = MEDIA_BUS_FMT_RGB888_2X12_BE,
>> +.depth = 24,
>> +},
>> +};
>
> [snip]
>
>> +
>> +void dw_mipi_csi_set_ipi_fmt(struct mipi_csi_dev *csi_dev)
>> +{
>> +struct device *dev = csi_dev->dev;
>> +
>> +if (csi_dev->ipi_dt)
>> +dw_mipi_csi_write(csi_dev, reg.IPI_DATA_TYPE, csi_dev->ipi_dt);
>> +else {
>> +switch (csi_dev->fmt->code) {
>> +case MEDIA_BUS_FMT_RGB565_2X8_BE:
>> +case MEDIA_BUS_FMT_RGB565_2X8_LE:
>> +dw_mipi_csi_write(csi_dev,
>> +reg.IPI_DATA_TYPE, CSI_2_RGB565);
>> +dev_dbg(dev, "DT: RGB 565");
>> +break;
>> +
>> +case MEDIA_BUS_FMT_RGB888_2X12_LE:
>> +case MEDIA_BUS_FMT_RGB888_2X12_BE:
>> +dw_mipi_csi_write(csi_dev,
>> +reg.IPI_DATA_TYPE, CSI_2_RGB888);
>> +dev_dbg(dev, "DT: RGB 888");
>> +break;
>> +case MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE:
>> +dw_mipi_csi_write(csi_dev,
>> +reg.IPI_DATA_TYPE, CSI_2_RAW10);
>> +dev_dbg(dev, "DT: RAW 10");
>> +break;
>> +case MEDIA_BUS_FMT_SBGGR8_1X8:
>> +dw_mipi_csi_write(csi_dev,
>> +reg.IPI_DATA_TYPE, CSI_2_RAW8);
>> +dev_dbg(dev, "DT: RAW 8");
>> +break;
>
> Same here, in CSI_2_RAW12 case it will default to a RGB565 case.
>
> Thanks,
>
> Eugen
>
>
I will try to add the support for this data type in my next patchset if not I
will flag it as unsupported for now in the commit message and code.
Thanks for your review,
Luis
>
>> +default:
>> +dw_mipi_csi_write(csi_dev,
>> +reg.IPI_DATA_TYPE, CSI_2_RGB565);
>> +dev_dbg(dev, "Error");
>> +break;
>> +}
>> +}
>> +}
>> +
>
> [snip]
>
Re: [V3, 4/4] media: platform: dwc: Add MIPI CSI-2 controller driver
On 10-Dec-18 12:39, eugen.hris...@microchip.com wrote:
>
>
> On 19.10.2018 15:52, Luis Oliveira wrote:
>> Add the Synopsys MIPI CSI-2 controller driver. This
>> controller driver is divided in platform dependent functions
>> and core functions. It also includes a platform for future
>> DesignWare drivers.
>>
>> Signed-off-by: Luis Oliveira
>> ---
>> Changelog
>> v2-V3
>> - exposed IPI settings to userspace
>> - fixed headers
> [...]
>
> snip
>
>
>> +
>> +static int
>> +dw_mipi_csi_parse_dt(struct platform_device *pdev, struct mipi_csi_dev *dev)
>> +{
>> +struct device_node *node = pdev->dev.of_node;
>> +struct v4l2_fwnode_endpoint endpoint;
>
> Hello Luis,
>
> I believe you have to initialize "endpoint" here correctly, otherwise
> the parsing mechanism (fwnode_endpoint_parse) will consider you have a
> specific mbus type and fail to probe the endpoint: bail out with debug
> message "expecting bus type not found "
>
> (namely, initialize to zero which is the UNKNOWN mbus type, or , to a
> specific mbus (from DT or whatever source))
>
> Eugen
>
Hi Eugen,
Sorry for my late reply, I was on Christmas break.
You are right, I will add that fix.
>
>> +int ret;
>> +
>> +node = of_graph_get_next_endpoint(node, NULL);
>> +if (!node) {
>> +dev_err(&pdev->dev, "No port node at %s\n",
>> +pdev->dev.of_node->full_name);
>> +return -EINVAL;
>> +}
>> +
>> +ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(node), &endpoint);
>> +if (ret)
>> +goto err;
>> +
>> +dev->index = endpoint.base.port - 1;
>> +if (dev->index >= CSI_MAX_ENTITIES) {
>> +ret = -ENXIO;
>> +goto err;
>> +}
>> +dev->hw.num_lanes = endpoint.bus.mipi_csi2.num_data_lanes;
>> +
>> +err:
>> +of_node_put(node);
>> +return ret;
>> +}
>> +
>
> snip
>
Thank you.
Re: [V3, 4/4] media: platform: dwc: Add MIPI CSI-2 controller driver
On 19.10.2018 15:52, Luis Oliveira wrote:
> Add the Synopsys MIPI CSI-2 controller driver. This
> controller driver is divided in platform dependent functions
> and core functions. It also includes a platform for future
> DesignWare drivers.
>
> Signed-off-by: Luis Oliveira
> ---
> Changelog
> v2-V3
> - exposed IPI settings to userspace
> - fixed headers
>
> MAINTAINERS | 11 +
> drivers/media/platform/dwc/Kconfig | 30 +-
> drivers/media/platform/dwc/Makefile | 2 +
> drivers/media/platform/dwc/dw-csi-plat.c | 699
> +++
> drivers/media/platform/dwc/dw-csi-plat.h | 77
> drivers/media/platform/dwc/dw-mipi-csi.c | 494 ++
> drivers/media/platform/dwc/dw-mipi-csi.h | 202 +
> include/media/dwc/dw-mipi-csi-pltfrm.h | 102 +
> 8 files changed, 1616 insertions(+), 1 deletion(-)
> create mode 100644 drivers/media/platform/dwc/dw-csi-plat.c
> create mode 100644 drivers/media/platform/dwc/dw-csi-plat.h
> create mode 100644 drivers/media/platform/dwc/dw-mipi-csi.c
> create mode 100644 drivers/media/platform/dwc/dw-mipi-csi.h
> create mode 100644 include/media/dwc/dw-mipi-csi-pltfrm.h
>
[snip]
> +/* Video formats supported by the MIPI CSI-2 */
> +const struct mipi_fmt dw_mipi_csi_formats[] = {
> + {
> + /* RAW 8 */
> + .code = MEDIA_BUS_FMT_SBGGR8_1X8,
> + .depth = 8,
> + },
> + {
> + /* RAW 10 */
> + .code = MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE,
> + .depth = 10,
> + },
Hi Luis,
Any reason why RAW12 format is not handled here ?
Here, namely MEDIA_BUS_FMT_SBGGR12_1X12 etc.
> + {
> + /* RGB 565 */
> + .code = MEDIA_BUS_FMT_RGB565_2X8_BE,
> + .depth = 16,
> + },
> + {
> + /* BGR 565 */
> + .code = MEDIA_BUS_FMT_RGB565_2X8_LE,
> + .depth = 16,
> + },
> + {
> + /* RGB 888 */
> + .code = MEDIA_BUS_FMT_RGB888_2X12_LE,
> + .depth = 24,
> + },
> + {
> + /* BGR 888 */
> + .code = MEDIA_BUS_FMT_RGB888_2X12_BE,
> + .depth = 24,
> + },
> +};
[snip]
> +
> +void dw_mipi_csi_set_ipi_fmt(struct mipi_csi_dev *csi_dev)
> +{
> + struct device *dev = csi_dev->dev;
> +
> + if (csi_dev->ipi_dt)
> + dw_mipi_csi_write(csi_dev, reg.IPI_DATA_TYPE, csi_dev->ipi_dt);
> + else {
> + switch (csi_dev->fmt->code) {
> + case MEDIA_BUS_FMT_RGB565_2X8_BE:
> + case MEDIA_BUS_FMT_RGB565_2X8_LE:
> + dw_mipi_csi_write(csi_dev,
> + reg.IPI_DATA_TYPE, CSI_2_RGB565);
> + dev_dbg(dev, "DT: RGB 565");
> + break;
> +
> + case MEDIA_BUS_FMT_RGB888_2X12_LE:
> + case MEDIA_BUS_FMT_RGB888_2X12_BE:
> + dw_mipi_csi_write(csi_dev,
> + reg.IPI_DATA_TYPE, CSI_2_RGB888);
> + dev_dbg(dev, "DT: RGB 888");
> + break;
> + case MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE:
> + dw_mipi_csi_write(csi_dev,
> + reg.IPI_DATA_TYPE, CSI_2_RAW10);
> + dev_dbg(dev, "DT: RAW 10");
> + break;
> + case MEDIA_BUS_FMT_SBGGR8_1X8:
> + dw_mipi_csi_write(csi_dev,
> + reg.IPI_DATA_TYPE, CSI_2_RAW8);
> + dev_dbg(dev, "DT: RAW 8");
> + break;
Same here, in CSI_2_RAW12 case it will default to a RGB565 case.
Thanks,
Eugen
> + default:
> + dw_mipi_csi_write(csi_dev,
> + reg.IPI_DATA_TYPE, CSI_2_RGB565);
> + dev_dbg(dev, "Error");
> + break;
> + }
> + }
> +}
> +
[snip]
Re: [V3, 4/4] media: platform: dwc: Add MIPI CSI-2 controller driver
On 19.10.2018 15:52, Luis Oliveira wrote:
> Add the Synopsys MIPI CSI-2 controller driver. This
> controller driver is divided in platform dependent functions
> and core functions. It also includes a platform for future
> DesignWare drivers.
>
> Signed-off-by: Luis Oliveira
> ---
> Changelog
> v2-V3
> - exposed IPI settings to userspace
> - fixed headers
[...]
snip
> +
> +static int
> +dw_mipi_csi_parse_dt(struct platform_device *pdev, struct mipi_csi_dev *dev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct v4l2_fwnode_endpoint endpoint;
Hello Luis,
I believe you have to initialize "endpoint" here correctly, otherwise
the parsing mechanism (fwnode_endpoint_parse) will consider you have a
specific mbus type and fail to probe the endpoint: bail out with debug
message "expecting bus type not found "
(namely, initialize to zero which is the UNKNOWN mbus type, or , to a
specific mbus (from DT or whatever source))
Eugen
> + int ret;
> +
> + node = of_graph_get_next_endpoint(node, NULL);
> + if (!node) {
> + dev_err(&pdev->dev, "No port node at %s\n",
> + pdev->dev.of_node->full_name);
> + return -EINVAL;
> + }
> +
> + ret = v4l2_fwnode_endpoint_parse(of_fwnode_handle(node), &endpoint);
> + if (ret)
> + goto err;
> +
> + dev->index = endpoint.base.port - 1;
> + if (dev->index >= CSI_MAX_ENTITIES) {
> + ret = -ENXIO;
> + goto err;
> + }
> + dev->hw.num_lanes = endpoint.bus.mipi_csi2.num_data_lanes;
> +
> +err:
> + of_node_put(node);
> + return ret;
> +}
> +
snip
