[PATCH v8 09/14] drm: bridge: samsung-dsim: Add atomic_get_input_bus_fmts
Jagan Teki
jagan at amarulasolutions.com
Wed Nov 16 03:30:20 PST 2022
On Wed, Nov 16, 2022 at 4:37 PM Marek Szyprowski
<m.szyprowski at samsung.com> wrote:
>
> On 16.11.2022 11:49, Marek Vasut wrote:
> > 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 <marex at 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 <jagan at 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 ?
> >
> Yes, if I got the bus format negotiation code right, but I didn't check
> the details.
It Looks like if the number of formats returned by the array is more
than 1, then the input_fmts array needs to return by listing formats
in decreasing preference order so that the bridge core will try all
formats until it finds one that works. But the question here is how
much is the array number, how to decide? I can see dw-hdmi return a
maximum of 3 possible input formats for an output format so the array
seems to be 3 here, but the rest of drivers like imx8qxp-pixel-link do
return 1 from the list of supported - dsim doing the same here.
If we are sure dsim can support or feature 2 then order RGB888 and
YUV422 decreasing preference and return them otherwise stick with a
single return of checking supported list and return desired one.
Jagan.
More information about the linux-arm-kernel
mailing list