On 11/16/22 09:07, Marek Szyprowski wrote:
On 15.11.2022 13:00, Marek Vasut wrote:
On 11/14/22 08:49, Jagan Teki wrote:
On Sun, Nov 13, 2022 at 5:51 AM Marek Vasut <ma...@denx.de> wrote:

On 11/10/22 19:38, Jagan Teki wrote:
Finding the right input bus format throughout the pipeline is hard
so add atomic_get_input_bus_fmts callback and initialize with the
proper input format from list of supported output formats.

This format can be used in pipeline for negotiating bus format between
the DSI-end of this bridge and the other component closer to pipeline
components.

List of Pixel formats are taken from,
AN13573 i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
3.7.4 Pixel formats
Table 14. DSI pixel packing formats

v8:
* added pixel formats supported by NXP AN13573 i.MX 8/RT MIPI
DSI/CSI-2

v7, v6, v5, v4:
* none

v3:
* include media-bus-format.h

v2:
* none

v1:
* new patch

Signed-off-by: Jagan Teki <ja...@amarulasolutions.com>
---
    drivers/gpu/drm/bridge/samsung-dsim.c | 53
+++++++++++++++++++++++++++
    1 file changed, 53 insertions(+)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 0fe153b29e4f..33e5ae9c865f 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -15,6 +15,7 @@
    #include <linux/clk.h>
    #include <linux/delay.h>
    #include <linux/irq.h>
+#include <linux/media-bus-format.h>
    #include <linux/of_device.h>
    #include <linux/phy/phy.h>

@@ -1321,6 +1322,57 @@ static void
samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
        pm_runtime_put_sync(dsi->dev);
    }

+/*
+ * This pixel output formats list referenced from,
+ * AN13573 i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
+ * 3.7.4 Pixel formats
+ * Table 14. DSI pixel packing formats
+ */
+static const u32 samsung_dsim_pixel_output_fmts[] = {

You can also add :

MEDIA_BUS_FMT_YUYV10_1X20

MEDIA_BUS_FMT_YUYV12_1X24

Are these for the below formats?

"Loosely Packed Pixel Stream, 20-bit YCbCr, 4:2:2
   Packed Pixel Stream, 24-bit YCbCr, 4:2:2"

+     MEDIA_BUS_FMT_UYVY8_1X16,
+     MEDIA_BUS_FMT_RGB101010_1X30,
+     MEDIA_BUS_FMT_RGB121212_1X36,
+     MEDIA_BUS_FMT_RGB565_1X16,
+     MEDIA_BUS_FMT_RGB666_1X18,
+     MEDIA_BUS_FMT_RGB888_1X24,
+};
+
+static bool samsung_dsim_pixel_output_fmt_supported(u32 fmt)
+{
+     int i;
+
+     for (i = 0; i < ARRAY_SIZE(samsung_dsim_pixel_output_fmts);
i++) {
+             if (samsung_dsim_pixel_output_fmts[i] == fmt)
+                     return true;
+     }
+
+     return false;
+}
+
+static u32 *
+samsung_dsim_atomic_get_input_bus_fmts(struct drm_bridge *bridge,
+                                    struct drm_bridge_state
*bridge_state,
+                                    struct drm_crtc_state
*crtc_state,
+                                    struct drm_connector_state
*conn_state,
+                                    u32 output_fmt,
+                                    unsigned int *num_input_fmts)
+{
+     u32 *input_fmts;
+
+     if (!samsung_dsim_pixel_output_fmt_supported(output_fmt))
+             return NULL;
+
+     *num_input_fmts = 1;

Shouldn't this be 6 ?

+     input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL);
+     if (!input_fmts)
+             return NULL;
+
+     input_fmts[0] = output_fmt;

Shouldn't this be a list of all 6 supported pixel formats ?

Negotiation would settle to return one input_fmt from the list of
supporting output_fmts. so the num_input_fmts would be 1 rather than
the number of fmts in the supporting list. This is what I understood
from the atomic_get_input_bus_fmts API. let me know if I miss
something here.

How does the negotiation work for this kind of pipeline:

LCDIFv3<->DSIM<->HDMI bridge<->HDMI connector

where all elements (LCDIFv3, DSIM, HDMI bridge) can support either
RGB888 or packed YUV422 ?

Who decides the format used by such pipeline ?

Why should it be the DSIM bridge and not e.g. the HDMI bridge or the
LCDIFv3 ?


If I got it right, the drivers reports their preference for the returned
formats. The format that is first in the reported array is the most
preferred one. This probably means that in your case the HDMI bridge
decides by reporting RGB or YUV first (if all elements supports both).

But in that case, we need to list input_fmts[0]...input_fmts[n-1] in samsung_dsim_atomic_get_input_bus_fmts() and also set *num_input_fmts = n, correct ?

Reply via email to