Re: [PATCH v3 03/13] drm/bridge: samsung-dsim: add flag to control header FIFO wait

[Inki Dae <daeinki@gmail.com>]

2025년 7월 7일 (월) 오전 3:26, Kaustabh Chakraborty <kauschluss@disroot.org>님이 작성:
>
> Exynos7870's DSIM device doesn't require waiting for the header FIFO
> during a MIPI DSI transfer. Add a flag in the driver data in order to
> control said behavior.

I will merge this patch as-is and just sharing the following comments
purely for reference (for other Exynos DRM contributors).

This patch likely needs follow-up improvements in conjunction with
Marek’s earlier patch (commit-id:
15f389da11257b806da75a070cfa41ca0cc15aae). I’m unable to verify this
at the moment because the SoC Technical Reference Manual (TRM)
documentation I have is incomplete.

Based on these two patches, we should plan for future code
improvements that account for two scenarios:

- The header FIFO must be waited for.
- The header FIFO does not need to be waited for.

Currently, this is handled via driver data using the
has_broken_fifoctrl_emptyhdr and wait_for_hdr_fifo flags. If the
handling of this behavior changes in newer Exynos SoCs, this approach
may become confusing or lead to inconsistencies.

Thanks,
Inki Dae

>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
>  drivers/gpu/drm/bridge/samsung-dsim.c | 15 ++++++++++++---
>  include/drm/bridge/samsung-dsim.h     |  1 +
>  2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index dca3939dd99fa07aee309067b93e652bc9a9b78f..84af24171b6856cbcf95b9077b997ee587fc0409 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -418,6 +418,7 @@ static const struct samsung_dsim_driver_data exynos3_dsi_driver_data = {
>         .has_clklane_stop = 1,
>         .num_clks = 2,
>         .max_freq = 1000,
> +       .wait_for_hdr_fifo = 1,
>         .wait_for_reset = 1,
>         .num_bits_resol = 11,
>         .pll_p_offset = 13,
> @@ -438,6 +439,7 @@ static const struct samsung_dsim_driver_data exynos4_dsi_driver_data = {
>         .has_clklane_stop = 1,
>         .num_clks = 2,
>         .max_freq = 1000,
> +       .wait_for_hdr_fifo = 1,
>         .wait_for_reset = 1,
>         .num_bits_resol = 11,
>         .pll_p_offset = 13,
> @@ -456,6 +458,7 @@ static const struct samsung_dsim_driver_data exynos5_dsi_driver_data = {
>         .has_legacy_status_reg = 1,
>         .num_clks = 2,
>         .max_freq = 1000,
> +       .wait_for_hdr_fifo = 1,
>         .wait_for_reset = 1,
>         .num_bits_resol = 11,
>         .pll_p_offset = 13,
> @@ -474,6 +477,7 @@ static const struct samsung_dsim_driver_data exynos5433_dsi_driver_data = {
>         .has_clklane_stop = 1,
>         .num_clks = 5,
>         .max_freq = 1500,
> +       .wait_for_hdr_fifo = 1,
>         .wait_for_reset = 0,
>         .num_bits_resol = 12,
>         .pll_p_offset = 13,
> @@ -492,6 +496,7 @@ static const struct samsung_dsim_driver_data exynos5422_dsi_driver_data = {
>         .has_clklane_stop = 1,
>         .num_clks = 2,
>         .max_freq = 1500,
> +       .wait_for_hdr_fifo = 1,
>         .wait_for_reset = 1,
>         .num_bits_resol = 12,
>         .pll_p_offset = 13,
> @@ -510,6 +515,7 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
>         .has_clklane_stop = 1,
>         .num_clks = 2,
>         .max_freq = 2100,
> +       .wait_for_hdr_fifo = 1,
>         .wait_for_reset = 0,
>         .num_bits_resol = 12,
>         /*
> @@ -1117,6 +1123,7 @@ static void samsung_dsim_send_to_fifo(struct samsung_dsim *dsi,
>  {
>         struct device *dev = dsi->dev;
>         struct mipi_dsi_packet *pkt = &xfer->packet;
> +       const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
>         const u8 *payload = pkt->payload + xfer->tx_done;
>         u16 length = pkt->payload_length - xfer->tx_done;
>         bool first = !xfer->tx_done;
> @@ -1157,9 +1164,11 @@ static void samsung_dsim_send_to_fifo(struct samsung_dsim *dsi,
>                 return;
>
>         reg = get_unaligned_le32(pkt->header);
> -       if (samsung_dsim_wait_for_hdr_fifo(dsi)) {
> -               dev_err(dev, "waiting for header FIFO timed out\n");
> -               return;
> +       if (driver_data->wait_for_hdr_fifo) {
> +               if (samsung_dsim_wait_for_hdr_fifo(dsi)) {
> +                       dev_err(dev, "waiting for header FIFO timed out\n");
> +                       return;
> +               }
>         }
>
>         if (NEQV(xfer->flags & MIPI_DSI_MSG_USE_LPM,
> diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h
> index f0c1e5c5ed490afe0bcfd06830f52471710b29ea..62c07952bd00f9c2c287a6a998f0e243dd4032a9 100644
> --- a/include/drm/bridge/samsung-dsim.h
> +++ b/include/drm/bridge/samsung-dsim.h
> @@ -61,6 +61,7 @@ struct samsung_dsim_driver_data {
>         unsigned int num_clks;
>         unsigned int min_freq;
>         unsigned int max_freq;
> +       unsigned int wait_for_hdr_fifo;
>         unsigned int wait_for_reset;
>         unsigned int num_bits_resol;
>         unsigned int pll_p_offset;
>
> --
> 2.49.0
>
>