On 14.11.2022 11:57, Marek Szyprowski wrote:
> On 10.11.2022 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@xxxxxxxxxxxxxxxxxxxx>
>> ---
>> 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[] = {
>> + 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;
>
>
> Please add support for MEDIA_BUS_FMT_FIXED and maybe default to
> MEDIA_BUS_FMT_RGB888_1X24 if requested format is not matched.
>
> Otherwise the above check breaks all current clients of the Samsung
> DSIM/Exynos DSI. I didn't dig into the bus matching code yet, but all
> DSI panels requests such format on my test systems...
I've checked a bit more the bus format related code and it looks that
there are more issues. The DSI panels don't use the MEDIA_BUS_FMT_*
formats thus the bridge negotiation code selects MEDIA_BUS_FMT_FIXED for
them. On Arndale board with Toshiba tc358764 bridge the
MEDIA_BUS_FMT_RGB888_1X7X4_SPWG output_fmt is requested in
samsung_dsim_atomic_get_input_bus_fmts() (forwarded from the LVDS panel,
but this doesn't look like a format really needed on that bridge). A bit
more logic seems to be needed there to make it working properly.
I suggest to move all this bus format related changes into a separate
patchset and adjust other bridges/panels/etc as needed in it.
>
>> +
>> + *num_input_fmts = 1;
>> +
>> + input_fmts = kmalloc(sizeof(*input_fmts), GFP_KERNEL);
>> + if (!input_fmts)
>> + return NULL;
>> +
>> + input_fmts[0] = output_fmt;
>> +
>> + 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,
>> @@ -1385,6 +1437,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,
>
> Best regards
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
