Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver

[Neil Armstrong <narmstrong@baylibre.com>]

Hi,

On 22/11/2021 14:16, Jagan Teki wrote:
> Hi Neil,
> 
> On Mon, Nov 22, 2021 at 6:22 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> On 22/11/2021 07:52, Jagan Teki wrote:
>>> Some display panels would come up with a non-DSI output, those
>>> can have an option to connect the DSI host by means of interface
>>> bridge converter.
>>>
>>> This DSI to non-DSI interface bridge converter would requires
>>> DSI Host to handle drm bridge functionalities in order to DSI
>>> Host to Interface bridge.
>>>
>>> This patch convert the existing to a drm bridge driver with a
>>> built-in encoder support for compatibility with existing
>>> component drivers.
>>>
>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>> ---
>>> Changes for v5:
>>> - add atomic APIs
>>> - find host and device variant DSI devices.
>>> Changes for v4, v3:
>>> - none
>>>
>>>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 112 ++++++++++++++++++++-----
>>>  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |   7 ++
>>>  2 files changed, 96 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>>> index 43d9c9e5198d..a6a272b55f77 100644
>>> --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>>> +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
>>> @@ -21,6 +21,7 @@
>>>
>>>  #include <drm/drm_atomic_helper.h>
>>>  #include <drm/drm_mipi_dsi.h>
>>> +#include <drm/drm_of.h>
>>>  #include <drm/drm_panel.h>
>>>  #include <drm/drm_print.h>
>>>  #include <drm/drm_probe_helper.h>
>>> @@ -713,10 +714,11 @@ static int sun6i_dsi_start(struct sun6i_dsi *dsi,
>>>       return 0;
>>>  }
>>>
>>> -static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
>>> +static void sun6i_dsi_bridge_atomic_enable(struct drm_bridge *bridge,
>>> +                                        struct drm_bridge_state *old_bridge_state)
>>>  {
>>> -     struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
>>> -     struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
>>> +     struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
>>> +     struct drm_display_mode *mode = &bridge->encoder->crtc->state->adjusted_mode;
>>>       struct mipi_dsi_device *device = dsi->device;
>>>       union phy_configure_opts opts = { };
>>>       struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy;
>>> @@ -772,6 +774,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
>>>       if (dsi->panel)
>>>               drm_panel_prepare(dsi->panel);
>>>
>>> +     if (dsi->next_bridge)
>>> +             dsi->next_bridge->funcs->atomic_pre_enable(dsi->next_bridge, old_bridge_state);
>>> +
>>>       /*
>>>        * FIXME: This should be moved after the switch to HS mode.
>>>        *
>>> @@ -787,6 +792,9 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder *encoder)
>>>       if (dsi->panel)
>>>               drm_panel_enable(dsi->panel);
>>>
>>> +     if (dsi->next_bridge)
>>> +             dsi->next_bridge->funcs->atomic_enable(dsi->next_bridge, old_bridge_state);
>>> +
>>
>>
>> No need to call the next bridge atomic pre_enable/enable/disable/post_disable since they will
>> be called automatically on the bridge chain.
> 
> Correct, but the existing bridge chain (stack) is not compatible with
> sun6i DSI start sequence. We cannot send any DCS once we start HS
> mode.
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c#n775

It's a classical DSI sequence init issue, look at dw-mipi-dsi:
https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c

We setup the "command-mode" (low-speed) withing mode_set so when the next bridge dsi_pre_enable is called,
low-speed DCS can be sent, then the bridge enable() sets video mode (high-speed).
The disable still needs to call the next_bridge post_disable :
https://elixir.bootlin.com/linux/v5.16-rc2/source/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L893

You can send any low-speed DCS once HS mode is started if the HW supports it and the driver handles it, look
at the https://elixir.bootlin.com/linux/v5.16-rc2/source/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L397
The MIPI_DSI_MODE_LPM and MIPI_DSI_MSG_USE_LPM is used for that.

Neil

> 
> This specific problem can be fixed only if we change the bridge chain
> from stack to queue. Please check this series
> https://patchwork.kernel.org/project/dri-devel/patch/20210214194102.126146-6-jagan@amarulasolutions.com/
> 
> Jagan.
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel