diff mbox series

[v7,07/10] drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts

Message ID 20221005151309.7278-8-jagan@amarulasolutions.com
State New
Headers
Series drm: bridge: Add Samsung MIPI DSIM bridge |

Commit Message

Jagan Teki Oct. 5, 2022, 3:13 p.m. UTC 
Finding the right input bus format throughout the pipeline is hard
so add atomic_get_input_bus_fmts callback and initialize with the
default RGB888_1X24 bus format on DSI-end.

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.

v7, v6, v5, v4:
* none

v3:
* include media-bus-format.h

v2:
* none

v1:
* new patch

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

Comments

Marek Vasut Oct. 15, 2022, 10:01 p.m. UTC | #1 
On 10/5/22 17:13, Jagan Teki wrote:

[...]

> @@ -1321,6 +1322,32 @@ static void samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
>   	pm_runtime_put_sync(dsi->dev);
>   }
>   
> +#define MAX_INPUT_SEL_FORMATS	1
> +
> +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;
> +
> +	*num_input_fmts = 0;
> +
> +	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
> +			     GFP_KERNEL);
> +	if (!input_fmts)
> +		return NULL;
> +
> +	/* This is the DSI-end bus format */
> +	input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> +	*num_input_fmts = 1;

Is this the only supported format ? NXP AN13573 lists the following:

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

Loosely Packed Pixel Stream, 20-bit YCbCr, 4:2:2
Packed Pixel Stream, 24-bit YCbCr, 4:2:2
Packed Pixel Stream, 16-bit YCbCr, 4:2:2
Packed Pixel Stream, 30-bit RGB, 10-10-10
Packed Pixel Stream, 36-bit RGB, 12-12-12
Packed Pixel Stream, 12-bit YCbCr, 4:2:0
Packed Pixel Stream, 16-bit RGB, 5-6-5
Packed Pixel Stream, 18-bit RGB, 6-6-6
Loosely Packed Pixel Stream, 18-bit RGB, 6-6-6
Packed Pixel Stream, 24-bit RGB, 8-8-8 Format

The MX8MM/MN LCDIF can generate all of those RGB formats , the MX8MP 
LCDIFv3 can also generate the 16bit YCbCr .

It seems there should be more formats here.
Jagan Teki Oct. 17, 2022, 3:58 a.m. UTC | #2 
On Sun, Oct 16, 2022 at 3:31 AM Marek Vasut <marex@denx.de> wrote:
>
> On 10/5/22 17:13, Jagan Teki wrote:
>
> [...]
>
> > @@ -1321,6 +1322,32 @@ static void samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
> >       pm_runtime_put_sync(dsi->dev);
> >   }
> >
> > +#define MAX_INPUT_SEL_FORMATS        1
> > +
> > +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;
> > +
> > +     *num_input_fmts = 0;
> > +
> > +     input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
> > +                          GFP_KERNEL);
> > +     if (!input_fmts)
> > +             return NULL;
> > +
> > +     /* This is the DSI-end bus format */
> > +     input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> > +     *num_input_fmts = 1;
>
> Is this the only supported format ? NXP AN13573 lists the following:

At least it only formats I have tested on my panel.

>
> i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
> 3.7.4 Pixel formats
> Table 14. DSI pixel packing formats
>
> Loosely Packed Pixel Stream, 20-bit YCbCr, 4:2:2
> Packed Pixel Stream, 24-bit YCbCr, 4:2:2
> Packed Pixel Stream, 16-bit YCbCr, 4:2:2
> Packed Pixel Stream, 30-bit RGB, 10-10-10
> Packed Pixel Stream, 36-bit RGB, 12-12-12
> Packed Pixel Stream, 12-bit YCbCr, 4:2:0
> Packed Pixel Stream, 16-bit RGB, 5-6-5
> Packed Pixel Stream, 18-bit RGB, 6-6-6
> Loosely Packed Pixel Stream, 18-bit RGB, 6-6-6
> Packed Pixel Stream, 24-bit RGB, 8-8-8 Format
>
> The MX8MM/MN LCDIF can generate all of those RGB formats , the MX8MP
> LCDIFv3 can also generate the 16bit YCbCr .
>
> It seems there should be more formats here.

The idea of this patch is to support the default format first, and can
possibly add future patches with the addition of new formats.

Thanks,
Jagan.
Marek Vasut Oct. 17, 2022, 7:24 a.m. UTC | #3 
On 10/17/22 05:58, Jagan Teki wrote:
> On Sun, Oct 16, 2022 at 3:31 AM Marek Vasut <marex@denx.de> wrote:
>>
>> On 10/5/22 17:13, Jagan Teki wrote:
>>
>> [...]
>>
>>> @@ -1321,6 +1322,32 @@ static void samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
>>>        pm_runtime_put_sync(dsi->dev);
>>>    }
>>>
>>> +#define MAX_INPUT_SEL_FORMATS        1
>>> +
>>> +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;
>>> +
>>> +     *num_input_fmts = 0;
>>> +
>>> +     input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
>>> +                          GFP_KERNEL);
>>> +     if (!input_fmts)
>>> +             return NULL;
>>> +
>>> +     /* This is the DSI-end bus format */
>>> +     input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
>>> +     *num_input_fmts = 1;
>>
>> Is this the only supported format ? NXP AN13573 lists the following:
> 
> At least it only formats I have tested on my panel.
> 
>>
>> i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
>> 3.7.4 Pixel formats
>> Table 14. DSI pixel packing formats
>>
>> Loosely Packed Pixel Stream, 20-bit YCbCr, 4:2:2
>> Packed Pixel Stream, 24-bit YCbCr, 4:2:2
>> Packed Pixel Stream, 16-bit YCbCr, 4:2:2
>> Packed Pixel Stream, 30-bit RGB, 10-10-10
>> Packed Pixel Stream, 36-bit RGB, 12-12-12
>> Packed Pixel Stream, 12-bit YCbCr, 4:2:0
>> Packed Pixel Stream, 16-bit RGB, 5-6-5
>> Packed Pixel Stream, 18-bit RGB, 6-6-6
>> Loosely Packed Pixel Stream, 18-bit RGB, 6-6-6
>> Packed Pixel Stream, 24-bit RGB, 8-8-8 Format
>>
>> The MX8MM/MN LCDIF can generate all of those RGB formats , the MX8MP
>> LCDIFv3 can also generate the 16bit YCbCr .
>>
>> It seems there should be more formats here.
> 
> The idea of this patch is to support the default format first, and can
> possibly add future patches with the addition of new formats.

Since you already know about the list, please add all the formats, so we 
won't be adding known broken code first, only to fix it later.
Jagan Teki Nov. 3, 2022, 7:40 a.m. UTC | #4 
On Mon, Oct 17, 2022 at 12:54 PM Marek Vasut <marex@denx.de> wrote:
>
> On 10/17/22 05:58, Jagan Teki wrote:
> > On Sun, Oct 16, 2022 at 3:31 AM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 10/5/22 17:13, Jagan Teki wrote:
> >>
> >> [...]
> >>
> >>> @@ -1321,6 +1322,32 @@ static void samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
> >>>        pm_runtime_put_sync(dsi->dev);
> >>>    }
> >>>
> >>> +#define MAX_INPUT_SEL_FORMATS        1
> >>> +
> >>> +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;
> >>> +
> >>> +     *num_input_fmts = 0;
> >>> +
> >>> +     input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
> >>> +                          GFP_KERNEL);
> >>> +     if (!input_fmts)
> >>> +             return NULL;
> >>> +
> >>> +     /* This is the DSI-end bus format */
> >>> +     input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> >>> +     *num_input_fmts = 1;
> >>
> >> Is this the only supported format ? NXP AN13573 lists the following:
> >
> > At least it only formats I have tested on my panel.
> >
> >>
> >> i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
> >> 3.7.4 Pixel formats
> >> Table 14. DSI pixel packing formats
> >>
> >> Loosely Packed Pixel Stream, 20-bit YCbCr, 4:2:2
> >> Packed Pixel Stream, 24-bit YCbCr, 4:2:2
> >> Packed Pixel Stream, 16-bit YCbCr, 4:2:2
> >> Packed Pixel Stream, 30-bit RGB, 10-10-10
> >> Packed Pixel Stream, 36-bit RGB, 12-12-12
> >> Packed Pixel Stream, 12-bit YCbCr, 4:2:0
> >> Packed Pixel Stream, 16-bit RGB, 5-6-5
> >> Packed Pixel Stream, 18-bit RGB, 6-6-6
> >> Loosely Packed Pixel Stream, 18-bit RGB, 6-6-6
> >> Packed Pixel Stream, 24-bit RGB, 8-8-8 Format
> >>
> >> The MX8MM/MN LCDIF can generate all of those RGB formats , the MX8MP
> >> LCDIFv3 can also generate the 16bit YCbCr .
> >>
> >> It seems there should be more formats here.
> >
> > The idea of this patch is to support the default format first, and can
> > possibly add future patches with the addition of new formats.
>
> Since you already know about the list, please add all the formats, so we
> won't be adding known broken code first, only to fix it later.

Okay. I can see the DSI section Mini TRM shown below formats. (13.6.2 Features)

Supports pixel format: 16bpp, 18bpp packed, 18bpp loosely packed (3 byte
format), and 24bpp

I will try to add these 4 formats. let me know.

Jagan.
Jagan Teki Nov. 3, 2022, 9:39 a.m. UTC | #5 
On Sun, Oct 16, 2022 at 3:31 AM Marek Vasut <marex@denx.de> wrote:
>
> On 10/5/22 17:13, Jagan Teki wrote:
>
> [...]
>
> > @@ -1321,6 +1322,32 @@ static void samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
> >       pm_runtime_put_sync(dsi->dev);
> >   }
> >
> > +#define MAX_INPUT_SEL_FORMATS        1
> > +
> > +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;
> > +
> > +     *num_input_fmts = 0;
> > +
> > +     input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
> > +                          GFP_KERNEL);
> > +     if (!input_fmts)
> > +             return NULL;
> > +
> > +     /* This is the DSI-end bus format */
> > +     input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> > +     *num_input_fmts = 1;
>
> Is this the only supported format ? NXP AN13573 lists the following:
>
> i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
> 3.7.4 Pixel formats
> Table 14. DSI pixel packing formats
>
> Loosely Packed Pixel Stream, 20-bit YCbCr, 4:2:2
> Packed Pixel Stream, 24-bit YCbCr, 4:2:2
> Packed Pixel Stream, 16-bit YCbCr, 4:2:2

Look like these are unsupported in media-bus-format.h list.

> Packed Pixel Stream, 30-bit RGB, 10-10-10
> Packed Pixel Stream, 36-bit RGB, 12-12-12
> Packed Pixel Stream, 12-bit YCbCr, 4:2:0

Same issue, unsupported.

> Packed Pixel Stream, 16-bit RGB, 5-6-5

MEDIA_BUS_FMT_RGB565_1X16

> Packed Pixel Stream, 18-bit RGB, 6-6-6

Same issue, unsupported.

> Loosely Packed Pixel Stream, 18-bit RGB, 6-6-6
> Packed Pixel Stream, 24-bit RGB, 8-8-8 Format

MEDIA_BUS_FMT_RGB666_1X18
MEDIA_BUS_FMT_RGB888_1X24

>
> The MX8MM/MN LCDIF can generate all of those RGB formats , the MX8MP
> LCDIFv3 can also generate the 16bit YCbCr .

Is YCbCr denoted as UYVY in media-bus-format.h ?

Thanks,
Jagan.
Marek Vasut Nov. 3, 2022, 4:02 p.m. UTC | #6 
On 11/3/22 10:39, Jagan Teki wrote:
> On Sun, Oct 16, 2022 at 3:31 AM Marek Vasut <marex@denx.de> wrote:
>>
>> On 10/5/22 17:13, Jagan Teki wrote:
>>
>> [...]
>>
>>> @@ -1321,6 +1322,32 @@ static void samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
>>>        pm_runtime_put_sync(dsi->dev);
>>>    }
>>>
>>> +#define MAX_INPUT_SEL_FORMATS        1
>>> +
>>> +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;
>>> +
>>> +     *num_input_fmts = 0;
>>> +
>>> +     input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
>>> +                          GFP_KERNEL);
>>> +     if (!input_fmts)
>>> +             return NULL;
>>> +
>>> +     /* This is the DSI-end bus format */
>>> +     input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
>>> +     *num_input_fmts = 1;
>>
>> Is this the only supported format ? NXP AN13573 lists the following:
>>
>> i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
>> 3.7.4 Pixel formats
>> Table 14. DSI pixel packing formats
>>
>> Loosely Packed Pixel Stream, 20-bit YCbCr, 4:2:2
>> Packed Pixel Stream, 24-bit YCbCr, 4:2:2
>> Packed Pixel Stream, 16-bit YCbCr, 4:2:2
> 
> Look like these are unsupported in media-bus-format.h list.

Aren't those:

MEDIA_BUS_FMT_UYVY12_1X24
MEDIA_BUS_FMT_UYVY8_1X16

?

Those are packed, and subsampled 4:2:2

>> Packed Pixel Stream, 30-bit RGB, 10-10-10

MEDIA_BUS_FMT_RGB101010_1X30

>> Packed Pixel Stream, 36-bit RGB, 12-12-12

MEDIA_BUS_FMT_RGB121212_1X36

>> Packed Pixel Stream, 12-bit YCbCr, 4:2:0
> 
> Same issue, unsupported.

The 12-bit packed 4:2:0 might be something along the lines of

drivers/media/platform/rockchip/rkisp1/rkisp1-resizer.c
MEDIA_BUS_FMT_YUYV8_1_5X8, /* YUV420 */

>> Packed Pixel Stream, 16-bit RGB, 5-6-5
> 
> MEDIA_BUS_FMT_RGB565_1X16
> 
>> Packed Pixel Stream, 18-bit RGB, 6-6-6
> 
> Same issue, unsupported.

MEDIA_BUS_FMT_RGB666_1X18

>> Loosely Packed Pixel Stream, 18-bit RGB, 6-6-6
>> Packed Pixel Stream, 24-bit RGB, 8-8-8 Format
> 
> MEDIA_BUS_FMT_RGB666_1X18
> MEDIA_BUS_FMT_RGB888_1X24
> 
>>
>> The MX8MM/MN LCDIF can generate all of those RGB formats , the MX8MP
>> LCDIFv3 can also generate the 16bit YCbCr .
> 
> Is YCbCr denoted as UYVY in media-bus-format.h ?
I think this applies:

https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/colorspaces.html
"
Sometimes people confuse Y’CbCr as being a colorspace. This is not 
correct, it is just an encoding of an R’G’B’ color into luma and chroma 
values.
"

And esp. this:

"
In order to correctly interpret a color you need to know the 
quantization range, whether it is R’G’B’ or Y’CbCr, the used Y’CbCr 
encoding and the colorspace. From that information you can calculate the 
corresponding CIE XYZ color and map that again to whatever colorspace 
your display device uses.
"

Which means that in order to properly describe or interpret the data, 
you need the entire v4l2_mbus_framefmt content, not just the pixel code:

https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/subdev-formats.html

But this information is not passed across the bus, that's metadata 
internal to the kernel.
Marek Vasut Nov. 3, 2022, 4:03 p.m. UTC | #7 
On 11/3/22 08:40, Jagan Teki wrote:
> On Mon, Oct 17, 2022 at 12:54 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 10/17/22 05:58, Jagan Teki wrote:
>>> On Sun, Oct 16, 2022 at 3:31 AM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 10/5/22 17:13, Jagan Teki wrote:
>>>>
>>>> [...]
>>>>
>>>>> @@ -1321,6 +1322,32 @@ static void samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
>>>>>         pm_runtime_put_sync(dsi->dev);
>>>>>     }
>>>>>
>>>>> +#define MAX_INPUT_SEL_FORMATS        1
>>>>> +
>>>>> +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;
>>>>> +
>>>>> +     *num_input_fmts = 0;
>>>>> +
>>>>> +     input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
>>>>> +                          GFP_KERNEL);
>>>>> +     if (!input_fmts)
>>>>> +             return NULL;
>>>>> +
>>>>> +     /* This is the DSI-end bus format */
>>>>> +     input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
>>>>> +     *num_input_fmts = 1;
>>>>
>>>> Is this the only supported format ? NXP AN13573 lists the following:
>>>
>>> At least it only formats I have tested on my panel.
>>>
>>>>
>>>> i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
>>>> 3.7.4 Pixel formats
>>>> Table 14. DSI pixel packing formats
>>>>
>>>> Loosely Packed Pixel Stream, 20-bit YCbCr, 4:2:2
>>>> Packed Pixel Stream, 24-bit YCbCr, 4:2:2
>>>> Packed Pixel Stream, 16-bit YCbCr, 4:2:2
>>>> Packed Pixel Stream, 30-bit RGB, 10-10-10
>>>> Packed Pixel Stream, 36-bit RGB, 12-12-12
>>>> Packed Pixel Stream, 12-bit YCbCr, 4:2:0
>>>> Packed Pixel Stream, 16-bit RGB, 5-6-5
>>>> Packed Pixel Stream, 18-bit RGB, 6-6-6
>>>> Loosely Packed Pixel Stream, 18-bit RGB, 6-6-6
>>>> Packed Pixel Stream, 24-bit RGB, 8-8-8 Format
>>>>
>>>> The MX8MM/MN LCDIF can generate all of those RGB formats , the MX8MP
>>>> LCDIFv3 can also generate the 16bit YCbCr .
>>>>
>>>> It seems there should be more formats here.
>>>
>>> The idea of this patch is to support the default format first, and can
>>> possibly add future patches with the addition of new formats.
>>
>> Since you already know about the list, please add all the formats, so we
>> won't be adding known broken code first, only to fix it later.
> 
> Okay. I can see the DSI section Mini TRM shown below formats. (13.6.2 Features)
> 
> Supports pixel format: 16bpp, 18bpp packed, 18bpp loosely packed (3 byte
> format), and 24bpp
> 
> I will try to add these 4 formats. let me know.

You should be able to add all but the 'Packed Pixel Stream, 12-bit 
YCbCr, 4:2:0' which would have to be defined (that's a few lines patch?).
Jagan Teki Nov. 3, 2022, 5:27 p.m. UTC | #8 
On Thu, Nov 3, 2022 at 9:56 PM Marek Vasut <marex@denx.de> wrote:
>
> On 11/3/22 10:39, Jagan Teki wrote:
> > On Sun, Oct 16, 2022 at 3:31 AM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 10/5/22 17:13, Jagan Teki wrote:
> >>
> >> [...]
> >>
> >>> @@ -1321,6 +1322,32 @@ static void samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
> >>>        pm_runtime_put_sync(dsi->dev);
> >>>    }
> >>>
> >>> +#define MAX_INPUT_SEL_FORMATS        1
> >>> +
> >>> +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;
> >>> +
> >>> +     *num_input_fmts = 0;
> >>> +
> >>> +     input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
> >>> +                          GFP_KERNEL);
> >>> +     if (!input_fmts)
> >>> +             return NULL;
> >>> +
> >>> +     /* This is the DSI-end bus format */
> >>> +     input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> >>> +     *num_input_fmts = 1;
> >>
> >> Is this the only supported format ? NXP AN13573 lists the following:
> >>
> >> i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
> >> 3.7.4 Pixel formats
> >> Table 14. DSI pixel packing formats
> >>
> >> Loosely Packed Pixel Stream, 20-bit YCbCr, 4:2:2
> >> Packed Pixel Stream, 24-bit YCbCr, 4:2:2
> >> Packed Pixel Stream, 16-bit YCbCr, 4:2:2
> >
> > Look like these are unsupported in media-bus-format.h list.
>
> Aren't those:
>
> MEDIA_BUS_FMT_UYVY12_1X24

Why is UYVY12 - YCbCr, 4:2:2 is 4+2+2 = 8 then it has UYVY8 ?

> MEDIA_BUS_FMT_UYVY8_1X16

If YCbCr is UYVY (I still don't get this notation, sorry) then Packed
Pixel Stream, 24-bit YCbCr, 4:2:2 with 2 Pixels per packet from Table
14 can be

MEDIA_BUS_FMT_UYVY8_2X24
(YCbCr 4:2:2 is UYVY8)

 " based on a reference example from media bus format doc
4.13.3.4.1.1.3. Packed YUV Formats, For instance, a format where
pixels are encoded as 8-bit YUV values downsampled to 4:2:2 and
transferred as 2 8-bit bus samples per pixel in the U, Y, V, Y order
will be named MEDIA_BUS_FMT_UYVY8_2X8."

https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/subdev-formats.html

_2X24 here 2 Pixels per packet is the exact packets to consider or we
can consider 1 Pixel per packet also. If later is true then _1X24 from
your notation is correct.

Thanks,
Jagan.
Marek Vasut Nov. 3, 2022, 6:58 p.m. UTC | #9 
On 11/3/22 18:27, Jagan Teki wrote:
> On Thu, Nov 3, 2022 at 9:56 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 11/3/22 10:39, Jagan Teki wrote:
>>> On Sun, Oct 16, 2022 at 3:31 AM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 10/5/22 17:13, Jagan Teki wrote:
>>>>
>>>> [...]
>>>>
>>>>> @@ -1321,6 +1322,32 @@ static void samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
>>>>>         pm_runtime_put_sync(dsi->dev);
>>>>>     }
>>>>>
>>>>> +#define MAX_INPUT_SEL_FORMATS        1
>>>>> +
>>>>> +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;
>>>>> +
>>>>> +     *num_input_fmts = 0;
>>>>> +
>>>>> +     input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
>>>>> +                          GFP_KERNEL);
>>>>> +     if (!input_fmts)
>>>>> +             return NULL;
>>>>> +
>>>>> +     /* This is the DSI-end bus format */
>>>>> +     input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
>>>>> +     *num_input_fmts = 1;
>>>>
>>>> Is this the only supported format ? NXP AN13573 lists the following:
>>>>
>>>> i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
>>>> 3.7.4 Pixel formats
>>>> Table 14. DSI pixel packing formats
>>>>
>>>> Loosely Packed Pixel Stream, 20-bit YCbCr, 4:2:2
>>>> Packed Pixel Stream, 24-bit YCbCr, 4:2:2
>>>> Packed Pixel Stream, 16-bit YCbCr, 4:2:2
>>>
>>> Look like these are unsupported in media-bus-format.h list.
>>
>> Aren't those:
>>
>> MEDIA_BUS_FMT_UYVY12_1X24
> 
> Why is UYVY12 - YCbCr, 4:2:2 is 4+2+2 = 8 then it has UYVY8 ?

(someone please correct me if I'm totally wrong here)

The 12 is channel width (12 bit for each Y1/Y2/U/V channel sample).
The 4:2:2 is subsampling (where are the color components sampled 
relative to brightness component).

Picture is here:
https://upload.wikimedia.org/wikipedia/commons/f/f2/Common_chroma_subsampling_ratios.svg

Each Y square of the left is 12bit sample.
Each U+V square is 12bit sample for U and 12bit sample for V.

In case of 4:4:4 subsampling, each luminance (brightness) component has 
matching chrominance (color) components.

In case of the 4:2:2 subsampling, two neighboring luminance components 
share two chrominance components. To transfer one pixel including color 
information, you have to transfer two pixels, Y0+U as 2x12bit sample in 
one cycle of 24bit bus, and then Y1+V as 2x12bit sample in another cycle 
of 24bit bus (2 clock cycles total, 4 samples total). From that you can 
reconstruct the two top-left squares (purple pixels) in the rightmost 
YUV column of 4:2:2 row.

The entire trick is that because eye is less sensitive to color than it 
is to light, you can transfer less color information and thus save 
bandwidth without anyone noticing (much of it).

>> MEDIA_BUS_FMT_UYVY8_1X16
> 
> If YCbCr is UYVY (I still don't get this notation, sorry) then Packed
> Pixel Stream, 24-bit YCbCr, 4:2:2 with 2 Pixels per packet from Table
> 14 can be
> 
> MEDIA_BUS_FMT_UYVY8_2X24
> (YCbCr 4:2:2 is UYVY8)
> 
>   " based on a reference example from media bus format doc
> 4.13.3.4.1.1.3. Packed YUV Formats, For instance, a format where
> pixels are encoded as 8-bit YUV values downsampled to 4:2:2 and
> transferred as 2 8-bit bus samples per pixel in the U, Y, V, Y order
> will be named MEDIA_BUS_FMT_UYVY8_2X8."

The way I read the above is that the channel width of each channel is 
8-bit , so you start with two pixels Y0/U/Y1/V which add up to 32bit 
total. That is transferred over 8-bit bus, in 4 bus cycles total. One 
pixel therefore takes 2 cycles of the 8 bit bus to transfer, even if you 
cannot transfer one pixel separately .

> https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/subdev-formats.html
> 
> _2X24 here 2 Pixels per packet is the exact packets to consider or we
> can consider 1 Pixel per packet also. If later is true then _1X24 from
> your notation is correct.

Since the DSIM input bus is 32bit wide, to transfer one such 4:2:2 
pixel, you need 1 bus cycle (2x12 bits per half of two pixels).

[...]
Jagan Teki Nov. 7, 2022, 5 p.m. UTC | #10 
Hi Marek,

On Fri, Nov 4, 2022 at 12:28 AM Marek Vasut <marex@denx.de> wrote:
>
> On 11/3/22 18:27, Jagan Teki wrote:
> > On Thu, Nov 3, 2022 at 9:56 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 11/3/22 10:39, Jagan Teki wrote:
> >>> On Sun, Oct 16, 2022 at 3:31 AM Marek Vasut <marex@denx.de> wrote:
> >>>>
> >>>> On 10/5/22 17:13, Jagan Teki wrote:
> >>>>
> >>>> [...]
> >>>>
> >>>>> @@ -1321,6 +1322,32 @@ static void samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
> >>>>>         pm_runtime_put_sync(dsi->dev);
> >>>>>     }
> >>>>>
> >>>>> +#define MAX_INPUT_SEL_FORMATS        1
> >>>>> +
> >>>>> +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;
> >>>>> +
> >>>>> +     *num_input_fmts = 0;
> >>>>> +
> >>>>> +     input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
> >>>>> +                          GFP_KERNEL);
> >>>>> +     if (!input_fmts)
> >>>>> +             return NULL;
> >>>>> +
> >>>>> +     /* This is the DSI-end bus format */
> >>>>> +     input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
> >>>>> +     *num_input_fmts = 1;
> >>>>
> >>>> Is this the only supported format ? NXP AN13573 lists the following:
> >>>>
> >>>> i.MX 8/RT MIPI DSI/CSI-2, Rev. 0, 21 March 2022
> >>>> 3.7.4 Pixel formats
> >>>> Table 14. DSI pixel packing formats
> >>>>
> >>>> Loosely Packed Pixel Stream, 20-bit YCbCr, 4:2:2
> >>>> Packed Pixel Stream, 24-bit YCbCr, 4:2:2
> >>>> Packed Pixel Stream, 16-bit YCbCr, 4:2:2
> >>>
> >>> Look like these are unsupported in media-bus-format.h list.
> >>
> >> Aren't those:
> >>
> >> MEDIA_BUS_FMT_UYVY12_1X24
> >
> > Why is UYVY12 - YCbCr, 4:2:2 is 4+2+2 = 8 then it has UYVY8 ?
>
> (someone please correct me if I'm totally wrong here)
>
> The 12 is channel width (12 bit for each Y1/Y2/U/V channel sample).
> The 4:2:2 is subsampling (where are the color components sampled
> relative to brightness component).
>
> Picture is here:
> https://upload.wikimedia.org/wikipedia/commons/f/f2/Common_chroma_subsampling_ratios.svg
>
> Each Y square of the left is 12bit sample.
> Each U+V square is 12bit sample for U and 12bit sample for V.
>
> In case of 4:4:4 subsampling, each luminance (brightness) component has
> matching chrominance (color) components.
>
> In case of the 4:2:2 subsampling, two neighboring luminance components
> share two chrominance components. To transfer one pixel including color
> information, you have to transfer two pixels, Y0+U as 2x12bit sample in
> one cycle of 24bit bus, and then Y1+V as 2x12bit sample in another cycle
> of 24bit bus (2 clock cycles total, 4 samples total). From that you can
> reconstruct the two top-left squares (purple pixels) in the rightmost
> YUV column of 4:2:2 row.
>
> The entire trick is that because eye is less sensitive to color than it
> is to light, you can transfer less color information and thus save
> bandwidth without anyone noticing (much of it).
>
> >> MEDIA_BUS_FMT_UYVY8_1X16
> >
> > If YCbCr is UYVY (I still don't get this notation, sorry) then Packed
> > Pixel Stream, 24-bit YCbCr, 4:2:2 with 2 Pixels per packet from Table
> > 14 can be
> >
> > MEDIA_BUS_FMT_UYVY8_2X24
> > (YCbCr 4:2:2 is UYVY8)
> >
> >   " based on a reference example from media bus format doc
> > 4.13.3.4.1.1.3. Packed YUV Formats, For instance, a format where
> > pixels are encoded as 8-bit YUV values downsampled to 4:2:2 and
> > transferred as 2 8-bit bus samples per pixel in the U, Y, V, Y order
> > will be named MEDIA_BUS_FMT_UYVY8_2X8."
>
> The way I read the above is that the channel width of each channel is
> 8-bit , so you start with two pixels Y0/U/Y1/V which add up to 32bit
> total. That is transferred over 8-bit bus, in 4 bus cycles total. One
> pixel therefore takes 2 cycles of the 8 bit bus to transfer, even if you
> cannot transfer one pixel separately .
>
> > https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/subdev-formats.html
> >
> > _2X24 here 2 Pixels per packet is the exact packets to consider or we
> > can consider 1 Pixel per packet also. If later is true then _1X24 from
> > your notation is correct.
>
> Since the DSIM input bus is 32bit wide, to transfer one such 4:2:2
> pixel, you need 1 bus cycle (2x12 bits per half of two pixels).

Thanks for your explanation. I need some time to understand and it
looks worth waiting for others to comment on this.

Meanwhile, I'm planning to send subsequent version patches with
possible supported formats like,

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,

Let me know.

Jagan.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 41970e794a7c..f714e49c1eab 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,32 @@  static void samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
 	pm_runtime_put_sync(dsi->dev);
 }
 
+#define MAX_INPUT_SEL_FORMATS	1
+
+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;
+
+	*num_input_fmts = 0;
+
+	input_fmts = kcalloc(MAX_INPUT_SEL_FORMATS, sizeof(*input_fmts),
+			     GFP_KERNEL);
+	if (!input_fmts)
+		return NULL;
+
+	/* This is the DSI-end bus format */
+	input_fmts[0] = MEDIA_BUS_FMT_RGB888_1X24;
+	*num_input_fmts = 1;
+
+	return input_fmts;
+}
+
 static int samsung_dsim_atomic_check(struct drm_bridge *bridge,
 				     struct drm_bridge_state *bridge_state,
 				     struct drm_crtc_state *crtc_state,
@@ -1384,6 +1411,7 @@  static const struct drm_bridge_funcs samsung_dsim_bridge_funcs = {
 	.atomic_duplicate_state		= drm_atomic_helper_bridge_duplicate_state,
 	.atomic_destroy_state		= drm_atomic_helper_bridge_destroy_state,
 	.atomic_reset			= drm_atomic_helper_bridge_reset,
+	.atomic_get_input_bus_fmts	= samsung_dsim_atomic_get_input_bus_fmts,
 	.atomic_check			= samsung_dsim_atomic_check,
 	.atomic_pre_enable		= samsung_dsim_atomic_pre_enable,
 	.atomic_enable			= samsung_dsim_atomic_enable,