[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
Hi Jagan
@@ -1503,28 +1506,18 @@ static int samsung_dsim_panel_or_bridge(struct
samsung_dsim *dsi,
{
struct drm_bridge *panel_bridge;
struct drm_panel *panel;
- struct device_node *remote;
-
- if (of_graph_is_present(node)) {
- remote = of_graph_get_remote_node(node, DSI_PORT_OUT, 0);
- if (!remote)
- return -ENODEV;
+ int ret;
- node = remote;
- }
+ ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, ,
+ _bridge);
+ if (ret)
+ return ret;
- panel_bridge = of_drm_find_bridge(node);
- if (!panel_bridge) {
- panel = of_drm_find_panel(node);
- if (!IS_ERR(panel)) {
- panel_bridge = drm_panel_bridge_add(panel);
- if (IS_ERR(panel_bridge))
- return PTR_ERR(panel_bridge);
- }
+ if (panel) {
+ panel_bridge = drm_panel_bridge_add(panel);
+ if (IS_ERR(panel_bridge))
+ return PTR_ERR(panel_bridge);
}
-
- of_node_put(node);
-
dsi->out_bridge = panel_bridge;
I need to apply this change to register my panel on imx8mn even mode I
found that
@@ -1594,11 +1587,15 @@ static int samsung_dsim_host_attach(struct
mipi_dsi_host *host,
return ret;
}
- mutex_lock(>mode_config.mutex);
dsi->lanes = device->lanes;
dsi->format = device->format;
dsi->mode_flags = device->mode_flags;
+
+ if (!drm)
+ return 0;
+
+ mutex_lock(>mode_config.mutex);
mode_config is not initialized in this path.
Michael
On Tue, Nov 30, 2021 at 8:39 AM Jagan Teki wrote:
>
> On Fri, Nov 26, 2021 at 9:34 PM Maxime Ripard wrote:
> >
> > On Thu, Nov 25, 2021 at 09:44:14PM +0530, Jagan Teki wrote:
> > > On Thu, Nov 25, 2021 at 9:40 PM Maxime Ripard wrote:
> > > >
> > > > On Thu, Nov 25, 2021 at 07:55:41PM +0530, Jagan Teki wrote:
> > > > > Hi,
> > > > >
> > > > > On Thu, Nov 25, 2021 at 7:45 PM Maxime Ripard
> > > > > wrote:
> > > > > >
> > > > > > On Wed, Nov 24, 2021 at 12:02:47AM +0530, Jagan Teki wrote:
> > > > > > > > > > > > + dsi->panel = of_drm_find_panel(remote);
> > > > > > > > > > > > + if (IS_ERR(dsi->panel)) {
> > > > > > > > > > > > + dsi->panel = NULL;
> > > > > > > > > > > > +
> > > > > > > > > > > > + dsi->next_bridge =
> > > > > > > > > > > > of_drm_find_bridge(remote);
> > > > > > > > > > > > + if (IS_ERR(dsi->next_bridge)) {
> > > > > > > > > > > > + dev_err(dsi->dev, "failed to find
> > > > > > > > > > > > bridge\n");
> > > > > > > > > > > > + return PTR_ERR(dsi->next_bridge);
> > > > > > > > > > > > + }
> > > > > > > > > > > > + } else {
> > > > > > > > > > > > + dsi->next_bridge = NULL;
> > > > > > > > > > > > + }
> > > > > > > > > > > > +
> > > > > > > > > > > > + of_node_put(remote);
> > > > > > > > > > >
> > > > > > > > > > > Using devm_drm_of_get_bridge would greatly simplify the
> > > > > > > > > > > driver
> > > > > > > > > >
> > > > > > > > > > I'm aware of this and this would break the existing sunxi
> > > > > > > > > > dsi binding,
> > > > > > > > > > we are not using ports based pipeline in dsi node.
> > > > > > > > > > Of-course you have
> > > > > > > > > > pointed the same before, please check below
> > > > > > > > > > https://patchwork.kernel.org/project/dri-devel/patch/20210322140152.101709-2-ja...@amarulasolutions.com/
> > > > > > > > >
> > > > > > > > > Then drm_of_find_panel_or_bridge needs to be adjusted to
> > > > > > > > > handle the DSI
> > > > > > > > > bindings and look for a panel or bridge not only through the
> > > > > > > > > OF graph,
> > > > > > > > > but also on the child nodes
> > > > > > > >
> > > > > > > > Okay. I need to check this.
> > > > > > >
> > > > > > > devm_drm_of_get_bridge is not working with legacy binding like
> > > > > > > the one
> > > > > > > used in sun6i dsi
> > > > > >
> > > > > > There's nothing legacy about it.
> > > > >
> > > > > What I'm mean legacy here with current binding used in sun6i-dsi like
> > > > > this.
> > > > >
> > > > > {
> > > > > vcc-dsi-supply = <_dcdc1>; /* VCC-DSI */
> > > > > status = "okay";
> > > > >
> > > > > panel@0 {
> > > > >compatible = "bananapi,s070wv20-ct16-icn6211";
> > > > >reg = <0>;
> > > > >reset-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /*
> > > > > LCD-RST: PL5 */
> > > > > enable-gpios = < 1 7 GPIO_ACTIVE_HIGH>; /*
> > > > > LCD-PWR-EN: PB7 */
> > > > > backlight = <>;
> > > > > };
> > > > > };
> > > >
> > > > Yes, I know, it's the generic DSI binding. It's still not legacy.
> > > >
> > > > > devm_drm_of_get_bridge
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
On Fri, Nov 26, 2021 at 9:34 PM Maxime Ripard wrote:
>
> On Thu, Nov 25, 2021 at 09:44:14PM +0530, Jagan Teki wrote:
> > On Thu, Nov 25, 2021 at 9:40 PM Maxime Ripard wrote:
> > >
> > > On Thu, Nov 25, 2021 at 07:55:41PM +0530, Jagan Teki wrote:
> > > > Hi,
> > > >
> > > > On Thu, Nov 25, 2021 at 7:45 PM Maxime Ripard wrote:
> > > > >
> > > > > On Wed, Nov 24, 2021 at 12:02:47AM +0530, Jagan Teki wrote:
> > > > > > > > > > > + dsi->panel = of_drm_find_panel(remote);
> > > > > > > > > > > + if (IS_ERR(dsi->panel)) {
> > > > > > > > > > > + dsi->panel = NULL;
> > > > > > > > > > > +
> > > > > > > > > > > + dsi->next_bridge =
> > > > > > > > > > > of_drm_find_bridge(remote);
> > > > > > > > > > > + if (IS_ERR(dsi->next_bridge)) {
> > > > > > > > > > > + dev_err(dsi->dev, "failed to find
> > > > > > > > > > > bridge\n");
> > > > > > > > > > > + return PTR_ERR(dsi->next_bridge);
> > > > > > > > > > > + }
> > > > > > > > > > > + } else {
> > > > > > > > > > > + dsi->next_bridge = NULL;
> > > > > > > > > > > + }
> > > > > > > > > > > +
> > > > > > > > > > > + of_node_put(remote);
> > > > > > > > > >
> > > > > > > > > > Using devm_drm_of_get_bridge would greatly simplify the
> > > > > > > > > > driver
> > > > > > > > >
> > > > > > > > > I'm aware of this and this would break the existing sunxi dsi
> > > > > > > > > binding,
> > > > > > > > > we are not using ports based pipeline in dsi node. Of-course
> > > > > > > > > you have
> > > > > > > > > pointed the same before, please check below
> > > > > > > > > https://patchwork.kernel.org/project/dri-devel/patch/20210322140152.101709-2-ja...@amarulasolutions.com/
> > > > > > > >
> > > > > > > > Then drm_of_find_panel_or_bridge needs to be adjusted to handle
> > > > > > > > the DSI
> > > > > > > > bindings and look for a panel or bridge not only through the OF
> > > > > > > > graph,
> > > > > > > > but also on the child nodes
> > > > > > >
> > > > > > > Okay. I need to check this.
> > > > > >
> > > > > > devm_drm_of_get_bridge is not working with legacy binding like the
> > > > > > one
> > > > > > used in sun6i dsi
> > > > >
> > > > > There's nothing legacy about it.
> > > >
> > > > What I'm mean legacy here with current binding used in sun6i-dsi like
> > > > this.
> > > >
> > > > {
> > > > vcc-dsi-supply = <_dcdc1>; /* VCC-DSI */
> > > > status = "okay";
> > > >
> > > > panel@0 {
> > > >compatible = "bananapi,s070wv20-ct16-icn6211";
> > > >reg = <0>;
> > > >reset-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /*
> > > > LCD-RST: PL5 */
> > > > enable-gpios = < 1 7 GPIO_ACTIVE_HIGH>; /*
> > > > LCD-PWR-EN: PB7 */
> > > > backlight = <>;
> > > > };
> > > > };
> > >
> > > Yes, I know, it's the generic DSI binding. It's still not legacy.
> > >
> > > > devm_drm_of_get_bridge cannot find the device with above binding and
> > > > able to find the device with below binding.
> > > >
> > > > {
> > > >vcc-dsi-supply = <_dcdc1>; /* VCC-DSI */
> > > >status = "okay";
> > > >
> > > > ports {
> > > > #address-cells = <1>;
> > > > #size-cells = <0>;
> > > >
> > > >dsi_out: port@0 {
> > > >reg = <0>;
> > > >
> > > > dsi_out_bridge: endpoint {
> > > > remote-endpoint = <_out_dsi>;
> > > > };
> > > >};
> > > > };
> > > >
> > > > panel@0 {
> > > > compatible = "bananapi,s070wv20-ct16-icn6211";
> > > > reg = <0>;
> > > > reset-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /* LCD-RST:
> > > > PL5 */
> > > > enable-gpios = < 1 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN:
> > > > PB7 */
> > > > backlight = <>;
> > > >
> > > > port {
> > > > bridge_out_dsi: endpoint {
> > > > remote-endpoint = <_out_bridge>;
> > > > };
> > > > };
> > > >};
> > > > };
> > >
> > > Yes, I know, and that's because ...
> >
> > Okay. I will use find panel and bridge separately instead of
> > devm_drm_of_get_bridge in version patches.
>
> That's not been my point, at all?
>
> I mean, that whole discussion has been because you shouldn't do that.
>
> > >
> > > > >
> > > > > > https://patchwork.kernel.org/project/dri-devel/patch/20211122065223.88059-6-ja...@amarulasolutions.com/
> > > > > >
> > > > > > dsi->next_bridge = devm_drm_of_get_bridge(dsi->dev,
> > > > > > dsi->dev->of_node, 0, 0);
> > > > > > if (IS_ERR(dsi->next_bridge))
> > > > > >return PTR_ERR(dsi->next_bridge);
> > > > > >
> > > > > > It is only working if we have ports on the pipeline, something like
> > > > > > this
> > > > > >
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
On Thu, Nov 25, 2021 at 09:44:14PM +0530, Jagan Teki wrote:
> On Thu, Nov 25, 2021 at 9:40 PM Maxime Ripard wrote:
> >
> > On Thu, Nov 25, 2021 at 07:55:41PM +0530, Jagan Teki wrote:
> > > Hi,
> > >
> > > On Thu, Nov 25, 2021 at 7:45 PM Maxime Ripard wrote:
> > > >
> > > > On Wed, Nov 24, 2021 at 12:02:47AM +0530, Jagan Teki wrote:
> > > > > > > > > > + dsi->panel = of_drm_find_panel(remote);
> > > > > > > > > > + if (IS_ERR(dsi->panel)) {
> > > > > > > > > > + dsi->panel = NULL;
> > > > > > > > > > +
> > > > > > > > > > + dsi->next_bridge = of_drm_find_bridge(remote);
> > > > > > > > > > + if (IS_ERR(dsi->next_bridge)) {
> > > > > > > > > > + dev_err(dsi->dev, "failed to find
> > > > > > > > > > bridge\n");
> > > > > > > > > > + return PTR_ERR(dsi->next_bridge);
> > > > > > > > > > + }
> > > > > > > > > > + } else {
> > > > > > > > > > + dsi->next_bridge = NULL;
> > > > > > > > > > + }
> > > > > > > > > > +
> > > > > > > > > > + of_node_put(remote);
> > > > > > > > >
> > > > > > > > > Using devm_drm_of_get_bridge would greatly simplify the driver
> > > > > > > >
> > > > > > > > I'm aware of this and this would break the existing sunxi dsi
> > > > > > > > binding,
> > > > > > > > we are not using ports based pipeline in dsi node. Of-course
> > > > > > > > you have
> > > > > > > > pointed the same before, please check below
> > > > > > > > https://patchwork.kernel.org/project/dri-devel/patch/20210322140152.101709-2-ja...@amarulasolutions.com/
> > > > > > >
> > > > > > > Then drm_of_find_panel_or_bridge needs to be adjusted to handle
> > > > > > > the DSI
> > > > > > > bindings and look for a panel or bridge not only through the OF
> > > > > > > graph,
> > > > > > > but also on the child nodes
> > > > > >
> > > > > > Okay. I need to check this.
> > > > >
> > > > > devm_drm_of_get_bridge is not working with legacy binding like the one
> > > > > used in sun6i dsi
> > > >
> > > > There's nothing legacy about it.
> > >
> > > What I'm mean legacy here with current binding used in sun6i-dsi like
> > > this.
> > >
> > > {
> > > vcc-dsi-supply = <_dcdc1>; /* VCC-DSI */
> > > status = "okay";
> > >
> > > panel@0 {
> > >compatible = "bananapi,s070wv20-ct16-icn6211";
> > >reg = <0>;
> > >reset-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /*
> > > LCD-RST: PL5 */
> > > enable-gpios = < 1 7 GPIO_ACTIVE_HIGH>; /*
> > > LCD-PWR-EN: PB7 */
> > > backlight = <>;
> > > };
> > > };
> >
> > Yes, I know, it's the generic DSI binding. It's still not legacy.
> >
> > > devm_drm_of_get_bridge cannot find the device with above binding and
> > > able to find the device with below binding.
> > >
> > > {
> > >vcc-dsi-supply = <_dcdc1>; /* VCC-DSI */
> > >status = "okay";
> > >
> > > ports {
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > >dsi_out: port@0 {
> > >reg = <0>;
> > >
> > > dsi_out_bridge: endpoint {
> > > remote-endpoint = <_out_dsi>;
> > > };
> > >};
> > > };
> > >
> > > panel@0 {
> > > compatible = "bananapi,s070wv20-ct16-icn6211";
> > > reg = <0>;
> > > reset-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /* LCD-RST: PL5
> > > */
> > > enable-gpios = < 1 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN:
> > > PB7 */
> > > backlight = <>;
> > >
> > > port {
> > > bridge_out_dsi: endpoint {
> > > remote-endpoint = <_out_bridge>;
> > > };
> > > };
> > >};
> > > };
> >
> > Yes, I know, and that's because ...
>
> Okay. I will use find panel and bridge separately instead of
> devm_drm_of_get_bridge in version patches.
That's not been my point, at all?
I mean, that whole discussion has been because you shouldn't do that.
> >
> > > >
> > > > > https://patchwork.kernel.org/project/dri-devel/patch/20211122065223.88059-6-ja...@amarulasolutions.com/
> > > > >
> > > > > dsi->next_bridge = devm_drm_of_get_bridge(dsi->dev,
> > > > > dsi->dev->of_node, 0, 0);
> > > > > if (IS_ERR(dsi->next_bridge))
> > > > >return PTR_ERR(dsi->next_bridge);
> > > > >
> > > > > It is only working if we have ports on the pipeline, something like
> > > > > this
> > > > > https://patchwork.kernel.org/project/dri-devel/patch/20210214194102.126146-8-ja...@amarulasolutions.com/
> > > > >
> > > > > Please have a look and let me know if I miss anything?
> > > >
> > > > Yes, you're missing the answer you quoted earlier:
> > >
> > > Yes, I'm trying to resolve the comment one after another. Will get back.
> >
> > ... You've ignored that comment.
>
> Not
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
On Mon, Nov 22, 2021 at 8:34 PM Maxime Ripard wrote:
>
> On Mon, Nov 22, 2021 at 07:49:26PM +0530, Jagan Teki wrote:
> > On Mon, Nov 22, 2021 at 7:35 PM Maxime Ripard wrote:
> > > On Mon, Nov 22, 2021 at 07:18:13PM +0530, Jagan Teki wrote:
> > > > On Mon, Nov 22, 2021 at 3:37 PM Maxime Ripard wrote:
> > > > >
> > > > > On Mon, Nov 22, 2021 at 12:22:19PM +0530, 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.
> > > > >
> > > > > In order to do this you would need to use the DRM bridge API...
> > > >
> > > > Sorry, which bridge API do you mean?
> > >
> > > Any variant of of_drm_find_bridge, and drm_bridge_attach. Just like
> > > we're doing in sun4i_rgb.c
> >
> > Yes, we have drm_bridge_attach in bind and bridge_function.attach
> > calls in this patch and of_drm_find_bridge in sun6i_mipi_dsi_attach.
> > Not sure which API's I've missed.
>
> None, that's my point, you don't need anything else in order to do what
> you wanted to achieve.
Correct, the order is some how confused in this patch. I will fix it
in next version.
>
> > >
> > > > > > This patch convert the existing to a drm bridge driver with a
> > > > > > built-in encoder support for compatibility with existing
> > > > > > component drivers.
> > > > >
> > > > > ... but changing the encoder driver to a bridge is completely
> > > > > unnecessary to do so. Why did you need to make that change?
> > > >
> > > > Idea of this series is to convert the driver to bridge and use the
> > > > latest bridge function from the v1 series.
> > >
> > > Ok, but it's not at all what you mention in your commit log? You don't
> > > need any of that in order to support a bridge downstream.
> >
> > I've mentioned "Converting to bridge driver" and thought it has
> > meaning of converting encoder related function to bridge functions as
> > well. Not think about specific description to describe on commit
> > message. Will update this.
>
> But you provided no reason to do so. The only one you did mention was
> that you wanted to support downstream bridges, but you don't need to
> convert the DSI driver to a bridge in order to do that.
Okay. Look like I've combined both downstream bridge support and
converting bridge together. This what it totally confused, I will fix
it.
>
> > > > > > Signed-off-by: Jagan Teki
> > > > > >
> > > > > > ---
> > > > > > 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
> > > > > > #include
> > > > > > +#include
> > > > > > #include
> > > > > > #include
> > > > > > #include
> > > > > > @@ -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 =
> > > > > > >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 =
> > > > > > >encoder->crtc->state->adjusted_mode;
> > > > > > struct mipi_dsi_device *device = dsi->device;
> > > > > > union phy_configure_opts opts = { };
> > > > > > struct phy_configure_opts_mipi_dphy *cfg = _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);
> > > > > > +
> > > > >
> > > > > Please use the proper helpers.
> > > >
> > > > If we use
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
On Thu, Nov 25, 2021 at 9:40 PM Maxime Ripard wrote:
>
> On Thu, Nov 25, 2021 at 07:55:41PM +0530, Jagan Teki wrote:
> > Hi,
> >
> > On Thu, Nov 25, 2021 at 7:45 PM Maxime Ripard wrote:
> > >
> > > On Wed, Nov 24, 2021 at 12:02:47AM +0530, Jagan Teki wrote:
> > > > > > > > > + dsi->panel = of_drm_find_panel(remote);
> > > > > > > > > + if (IS_ERR(dsi->panel)) {
> > > > > > > > > + dsi->panel = NULL;
> > > > > > > > > +
> > > > > > > > > + dsi->next_bridge = of_drm_find_bridge(remote);
> > > > > > > > > + if (IS_ERR(dsi->next_bridge)) {
> > > > > > > > > + dev_err(dsi->dev, "failed to find
> > > > > > > > > bridge\n");
> > > > > > > > > + return PTR_ERR(dsi->next_bridge);
> > > > > > > > > + }
> > > > > > > > > + } else {
> > > > > > > > > + dsi->next_bridge = NULL;
> > > > > > > > > + }
> > > > > > > > > +
> > > > > > > > > + of_node_put(remote);
> > > > > > > >
> > > > > > > > Using devm_drm_of_get_bridge would greatly simplify the driver
> > > > > > >
> > > > > > > I'm aware of this and this would break the existing sunxi dsi
> > > > > > > binding,
> > > > > > > we are not using ports based pipeline in dsi node. Of-course you
> > > > > > > have
> > > > > > > pointed the same before, please check below
> > > > > > > https://patchwork.kernel.org/project/dri-devel/patch/20210322140152.101709-2-ja...@amarulasolutions.com/
> > > > > >
> > > > > > Then drm_of_find_panel_or_bridge needs to be adjusted to handle the
> > > > > > DSI
> > > > > > bindings and look for a panel or bridge not only through the OF
> > > > > > graph,
> > > > > > but also on the child nodes
> > > > >
> > > > > Okay. I need to check this.
> > > >
> > > > devm_drm_of_get_bridge is not working with legacy binding like the one
> > > > used in sun6i dsi
> > >
> > > There's nothing legacy about it.
> >
> > What I'm mean legacy here with current binding used in sun6i-dsi like this.
> >
> > {
> > vcc-dsi-supply = <_dcdc1>; /* VCC-DSI */
> > status = "okay";
> >
> > panel@0 {
> >compatible = "bananapi,s070wv20-ct16-icn6211";
> >reg = <0>;
> >reset-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /*
> > LCD-RST: PL5 */
> > enable-gpios = < 1 7 GPIO_ACTIVE_HIGH>; /*
> > LCD-PWR-EN: PB7 */
> > backlight = <>;
> > };
> > };
>
> Yes, I know, it's the generic DSI binding. It's still not legacy.
>
> > devm_drm_of_get_bridge cannot find the device with above binding and
> > able to find the device with below binding.
> >
> > {
> >vcc-dsi-supply = <_dcdc1>; /* VCC-DSI */
> >status = "okay";
> >
> > ports {
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> >dsi_out: port@0 {
> >reg = <0>;
> >
> > dsi_out_bridge: endpoint {
> > remote-endpoint = <_out_dsi>;
> > };
> >};
> > };
> >
> > panel@0 {
> > compatible = "bananapi,s070wv20-ct16-icn6211";
> > reg = <0>;
> > reset-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /* LCD-RST: PL5 */
> > enable-gpios = < 1 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN: PB7
> > */
> > backlight = <>;
> >
> > port {
> > bridge_out_dsi: endpoint {
> > remote-endpoint = <_out_bridge>;
> > };
> > };
> >};
> > };
>
> Yes, I know, and that's because ...
Okay. I will use find panel and bridge separately instead of
devm_drm_of_get_bridge in version patches.
>
> > >
> > > > https://patchwork.kernel.org/project/dri-devel/patch/20211122065223.88059-6-ja...@amarulasolutions.com/
> > > >
> > > > dsi->next_bridge = devm_drm_of_get_bridge(dsi->dev, dsi->dev->of_node,
> > > > 0, 0);
> > > > if (IS_ERR(dsi->next_bridge))
> > > >return PTR_ERR(dsi->next_bridge);
> > > >
> > > > It is only working if we have ports on the pipeline, something like this
> > > > https://patchwork.kernel.org/project/dri-devel/patch/20210214194102.126146-8-ja...@amarulasolutions.com/
> > > >
> > > > Please have a look and let me know if I miss anything?
> > >
> > > Yes, you're missing the answer you quoted earlier:
> >
> > Yes, I'm trying to resolve the comment one after another. Will get back.
>
> ... You've ignored that comment.
Not understand which comment you mean. There are few about bridge
conversion details, I will send my comments.
Thanks,
Jagan.
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
On Thu, Nov 25, 2021 at 07:55:41PM +0530, Jagan Teki wrote:
> Hi,
>
> On Thu, Nov 25, 2021 at 7:45 PM Maxime Ripard wrote:
> >
> > On Wed, Nov 24, 2021 at 12:02:47AM +0530, Jagan Teki wrote:
> > > > > > > > + dsi->panel = of_drm_find_panel(remote);
> > > > > > > > + if (IS_ERR(dsi->panel)) {
> > > > > > > > + dsi->panel = NULL;
> > > > > > > > +
> > > > > > > > + dsi->next_bridge = of_drm_find_bridge(remote);
> > > > > > > > + if (IS_ERR(dsi->next_bridge)) {
> > > > > > > > + dev_err(dsi->dev, "failed to find
> > > > > > > > bridge\n");
> > > > > > > > + return PTR_ERR(dsi->next_bridge);
> > > > > > > > + }
> > > > > > > > + } else {
> > > > > > > > + dsi->next_bridge = NULL;
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + of_node_put(remote);
> > > > > > >
> > > > > > > Using devm_drm_of_get_bridge would greatly simplify the driver
> > > > > >
> > > > > > I'm aware of this and this would break the existing sunxi dsi
> > > > > > binding,
> > > > > > we are not using ports based pipeline in dsi node. Of-course you
> > > > > > have
> > > > > > pointed the same before, please check below
> > > > > > https://patchwork.kernel.org/project/dri-devel/patch/20210322140152.101709-2-ja...@amarulasolutions.com/
> > > > >
> > > > > Then drm_of_find_panel_or_bridge needs to be adjusted to handle the
> > > > > DSI
> > > > > bindings and look for a panel or bridge not only through the OF graph,
> > > > > but also on the child nodes
> > > >
> > > > Okay. I need to check this.
> > >
> > > devm_drm_of_get_bridge is not working with legacy binding like the one
> > > used in sun6i dsi
> >
> > There's nothing legacy about it.
>
> What I'm mean legacy here with current binding used in sun6i-dsi like this.
>
> {
> vcc-dsi-supply = <_dcdc1>; /* VCC-DSI */
> status = "okay";
>
> panel@0 {
>compatible = "bananapi,s070wv20-ct16-icn6211";
>reg = <0>;
>reset-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /*
> LCD-RST: PL5 */
> enable-gpios = < 1 7 GPIO_ACTIVE_HIGH>; /*
> LCD-PWR-EN: PB7 */
> backlight = <>;
> };
> };
Yes, I know, it's the generic DSI binding. It's still not legacy.
> devm_drm_of_get_bridge cannot find the device with above binding and
> able to find the device with below binding.
>
> {
>vcc-dsi-supply = <_dcdc1>; /* VCC-DSI */
>status = "okay";
>
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
>dsi_out: port@0 {
>reg = <0>;
>
> dsi_out_bridge: endpoint {
> remote-endpoint = <_out_dsi>;
> };
>};
> };
>
> panel@0 {
> compatible = "bananapi,s070wv20-ct16-icn6211";
> reg = <0>;
> reset-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /* LCD-RST: PL5 */
> enable-gpios = < 1 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN: PB7 */
> backlight = <>;
>
> port {
> bridge_out_dsi: endpoint {
> remote-endpoint = <_out_bridge>;
> };
> };
>};
> };
Yes, I know, and that's because ...
> >
> > > https://patchwork.kernel.org/project/dri-devel/patch/20211122065223.88059-6-ja...@amarulasolutions.com/
> > >
> > > dsi->next_bridge = devm_drm_of_get_bridge(dsi->dev, dsi->dev->of_node, 0,
> > > 0);
> > > if (IS_ERR(dsi->next_bridge))
> > >return PTR_ERR(dsi->next_bridge);
> > >
> > > It is only working if we have ports on the pipeline, something like this
> > > https://patchwork.kernel.org/project/dri-devel/patch/20210214194102.126146-8-ja...@amarulasolutions.com/
> > >
> > > Please have a look and let me know if I miss anything?
> >
> > Yes, you're missing the answer you quoted earlier:
>
> Yes, I'm trying to resolve the comment one after another. Will get back.
... You've ignored that comment.
Maxime
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20211125161026.ndfygaa6c4nnst4i%40gilmour.
signature.asc
Description: PGP signature
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
Hi,
On Thu, Nov 25, 2021 at 7:45 PM Maxime Ripard wrote:
>
> On Wed, Nov 24, 2021 at 12:02:47AM +0530, Jagan Teki wrote:
> > > > > > > + dsi->panel = of_drm_find_panel(remote);
> > > > > > > + if (IS_ERR(dsi->panel)) {
> > > > > > > + dsi->panel = NULL;
> > > > > > > +
> > > > > > > + dsi->next_bridge = of_drm_find_bridge(remote);
> > > > > > > + if (IS_ERR(dsi->next_bridge)) {
> > > > > > > + dev_err(dsi->dev, "failed to find
> > > > > > > bridge\n");
> > > > > > > + return PTR_ERR(dsi->next_bridge);
> > > > > > > + }
> > > > > > > + } else {
> > > > > > > + dsi->next_bridge = NULL;
> > > > > > > + }
> > > > > > > +
> > > > > > > + of_node_put(remote);
> > > > > >
> > > > > > Using devm_drm_of_get_bridge would greatly simplify the driver
> > > > >
> > > > > I'm aware of this and this would break the existing sunxi dsi binding,
> > > > > we are not using ports based pipeline in dsi node. Of-course you have
> > > > > pointed the same before, please check below
> > > > > https://patchwork.kernel.org/project/dri-devel/patch/20210322140152.101709-2-ja...@amarulasolutions.com/
> > > >
> > > > Then drm_of_find_panel_or_bridge needs to be adjusted to handle the DSI
> > > > bindings and look for a panel or bridge not only through the OF graph,
> > > > but also on the child nodes
> > >
> > > Okay. I need to check this.
> >
> > devm_drm_of_get_bridge is not working with legacy binding like the one
> > used in sun6i dsi
>
> There's nothing legacy about it.
What I'm mean legacy here with current binding used in sun6i-dsi like this.
{
vcc-dsi-supply = <_dcdc1>; /* VCC-DSI */
status = "okay";
panel@0 {
compatible = "bananapi,s070wv20-ct16-icn6211";
reg = <0>;
reset-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /*
LCD-RST: PL5 */
enable-gpios = < 1 7 GPIO_ACTIVE_HIGH>; /*
LCD-PWR-EN: PB7 */
backlight = <>;
};
};
devm_drm_of_get_bridge cannot find the device with above binding and
able to find the device with below binding.
{
vcc-dsi-supply = <_dcdc1>; /* VCC-DSI */
status = "okay";
ports {
#address-cells = <1>;
#size-cells = <0>;
dsi_out: port@0 {
reg = <0>;
dsi_out_bridge: endpoint {
remote-endpoint = <_out_dsi>;
};
};
};
panel@0 {
compatible = "bananapi,s070wv20-ct16-icn6211";
reg = <0>;
reset-gpios = <_pio 0 5 GPIO_ACTIVE_HIGH>; /* LCD-RST: PL5 */
enable-gpios = < 1 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN: PB7 */
backlight = <>;
port {
bridge_out_dsi: endpoint {
remote-endpoint = <_out_bridge>;
};
};
};
};
>
> > https://patchwork.kernel.org/project/dri-devel/patch/20211122065223.88059-6-ja...@amarulasolutions.com/
> >
> > dsi->next_bridge = devm_drm_of_get_bridge(dsi->dev, dsi->dev->of_node, 0,
> > 0);
> > if (IS_ERR(dsi->next_bridge))
> >return PTR_ERR(dsi->next_bridge);
> >
> > It is only working if we have ports on the pipeline, something like this
> > https://patchwork.kernel.org/project/dri-devel/patch/20210214194102.126146-8-ja...@amarulasolutions.com/
> >
> > Please have a look and let me know if I miss anything?
>
> Yes, you're missing the answer you quoted earlier:
Yes, I'm trying to resolve the comment one after another. Will get back.
Thanks,
Jagan.
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/CAMty3ZC0KOUxr2rComOCfC70wGS_aSXzjFGS4f%3DpEB6MQHRGFw%40mail.gmail.com.
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
On Wed, Nov 24, 2021 at 12:02:47AM +0530, Jagan Teki wrote:
> > > > > > + dsi->panel = of_drm_find_panel(remote);
> > > > > > + if (IS_ERR(dsi->panel)) {
> > > > > > + dsi->panel = NULL;
> > > > > > +
> > > > > > + dsi->next_bridge = of_drm_find_bridge(remote);
> > > > > > + if (IS_ERR(dsi->next_bridge)) {
> > > > > > + dev_err(dsi->dev, "failed to find bridge\n");
> > > > > > + return PTR_ERR(dsi->next_bridge);
> > > > > > + }
> > > > > > + } else {
> > > > > > + dsi->next_bridge = NULL;
> > > > > > + }
> > > > > > +
> > > > > > + of_node_put(remote);
> > > > >
> > > > > Using devm_drm_of_get_bridge would greatly simplify the driver
> > > >
> > > > I'm aware of this and this would break the existing sunxi dsi binding,
> > > > we are not using ports based pipeline in dsi node. Of-course you have
> > > > pointed the same before, please check below
> > > > https://patchwork.kernel.org/project/dri-devel/patch/20210322140152.101709-2-ja...@amarulasolutions.com/
> > >
> > > Then drm_of_find_panel_or_bridge needs to be adjusted to handle the DSI
> > > bindings and look for a panel or bridge not only through the OF graph,
> > > but also on the child nodes
> >
> > Okay. I need to check this.
>
> devm_drm_of_get_bridge is not working with legacy binding like the one
> used in sun6i dsi
There's nothing legacy about it.
> https://patchwork.kernel.org/project/dri-devel/patch/20211122065223.88059-6-ja...@amarulasolutions.com/
>
> dsi->next_bridge = devm_drm_of_get_bridge(dsi->dev, dsi->dev->of_node, 0, 0);
> if (IS_ERR(dsi->next_bridge))
>return PTR_ERR(dsi->next_bridge);
>
> It is only working if we have ports on the pipeline, something like this
> https://patchwork.kernel.org/project/dri-devel/patch/20210214194102.126146-8-ja...@amarulasolutions.com/
>
> Please have a look and let me know if I miss anything?
Yes, you're missing the answer you quoted earlier:
> > > Then drm_of_find_panel_or_bridge needs to be adjusted to handle the DSI
> > > bindings and look for a panel or bridge not only through the OF graph,
> > > but also on the child nodes
Maxime
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/20211125141516.oymscgs3xcjqmgce%40gilmour.
signature.asc
Description: PGP signature
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
Hi Maxime,
On Mon, Nov 22, 2021 at 7:49 PM Jagan Teki wrote:
>
> Hi Maxime,
>
> On Mon, Nov 22, 2021 at 7:35 PM Maxime Ripard wrote:
> >
> > On Mon, Nov 22, 2021 at 07:18:13PM +0530, Jagan Teki wrote:
> > > Hi Maxime,
> > >
> > > On Mon, Nov 22, 2021 at 3:37 PM Maxime Ripard wrote:
> > > >
> > > > On Mon, Nov 22, 2021 at 12:22:19PM +0530, 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.
> > > >
> > > > In order to do this you would need to use the DRM bridge API...
> > >
> > > Sorry, which bridge API do you mean?
> >
> > Any variant of of_drm_find_bridge, and drm_bridge_attach. Just like
> > we're doing in sun4i_rgb.c
>
> Yes, we have drm_bridge_attach in bind and bridge_function.attach
> calls in this patch and of_drm_find_bridge in sun6i_mipi_dsi_attach.
> Not sure which API's I've missed.
>
> >
> > > > > This patch convert the existing to a drm bridge driver with a
> > > > > built-in encoder support for compatibility with existing
> > > > > component drivers.
> > > >
> > > > ... but changing the encoder driver to a bridge is completely
> > > > unnecessary to do so. Why did you need to make that change?
> > >
> > > Idea of this series is to convert the driver to bridge and use the
> > > latest bridge function from the v1 series.
> >
> > Ok, but it's not at all what you mention in your commit log? You don't
> > need any of that in order to support a bridge downstream.
>
> I've mentioned "Converting to bridge driver" and thought it has
> meaning of converting encoder related function to bridge functions as
> well. Not think about specific description to describe on commit
> message. Will update this.
>
> >
> > > >
> > > > > Signed-off-by: Jagan Teki
> > > > >
> > > > > ---
> > > > > 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
> > > > > #include
> > > > > +#include
> > > > > #include
> > > > > #include
> > > > > #include
> > > > > @@ -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 =
> > > > > >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 =
> > > > > >encoder->crtc->state->adjusted_mode;
> > > > > struct mipi_dsi_device *device = dsi->device;
> > > > > union phy_configure_opts opts = { };
> > > > > struct phy_configure_opts_mipi_dphy *cfg = _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);
> > > > > +
> > > >
> > > > Please use the proper helpers.
> > >
> > > If we use bridge_functions we need to take atomic functions as
> > > precedence as the next bridge functions might convert atomic calls.
> >
> > We've had this discussion over and over again, but this is something
> > that needs to be documented and / or in your commit log.
> >
> > You must not deviate from the standard (and expected) behavior without
> > any kind of justification.
>
> Not exactly sure about what you mean, sorry. All these atomic bridge
> functions are already documented if I'm not wrong and Laurent have
> patches to switch the normal functions to atomic. Not sure what else
> need to document here and need justification for it if the driver is
> converting to bridge.
>
> >
> > > >
> > > > > /*
> > > > >* FIXME: This should be
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
On Mon, Nov 22, 2021 at 07:49:26PM +0530, Jagan Teki wrote:
> On Mon, Nov 22, 2021 at 7:35 PM Maxime Ripard wrote:
> > On Mon, Nov 22, 2021 at 07:18:13PM +0530, Jagan Teki wrote:
> > > On Mon, Nov 22, 2021 at 3:37 PM Maxime Ripard wrote:
> > > >
> > > > On Mon, Nov 22, 2021 at 12:22:19PM +0530, 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.
> > > >
> > > > In order to do this you would need to use the DRM bridge API...
> > >
> > > Sorry, which bridge API do you mean?
> >
> > Any variant of of_drm_find_bridge, and drm_bridge_attach. Just like
> > we're doing in sun4i_rgb.c
>
> Yes, we have drm_bridge_attach in bind and bridge_function.attach
> calls in this patch and of_drm_find_bridge in sun6i_mipi_dsi_attach.
> Not sure which API's I've missed.
None, that's my point, you don't need anything else in order to do what
you wanted to achieve.
> >
> > > > > This patch convert the existing to a drm bridge driver with a
> > > > > built-in encoder support for compatibility with existing
> > > > > component drivers.
> > > >
> > > > ... but changing the encoder driver to a bridge is completely
> > > > unnecessary to do so. Why did you need to make that change?
> > >
> > > Idea of this series is to convert the driver to bridge and use the
> > > latest bridge function from the v1 series.
> >
> > Ok, but it's not at all what you mention in your commit log? You don't
> > need any of that in order to support a bridge downstream.
>
> I've mentioned "Converting to bridge driver" and thought it has
> meaning of converting encoder related function to bridge functions as
> well. Not think about specific description to describe on commit
> message. Will update this.
But you provided no reason to do so. The only one you did mention was
that you wanted to support downstream bridges, but you don't need to
convert the DSI driver to a bridge in order to do that.
> > > > > Signed-off-by: Jagan Teki
> > > > >
> > > > > ---
> > > > > 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
> > > > > #include
> > > > > +#include
> > > > > #include
> > > > > #include
> > > > > #include
> > > > > @@ -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 =
> > > > > >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 =
> > > > > >encoder->crtc->state->adjusted_mode;
> > > > > struct mipi_dsi_device *device = dsi->device;
> > > > > union phy_configure_opts opts = { };
> > > > > struct phy_configure_opts_mipi_dphy *cfg = _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);
> > > > > +
> > > >
> > > > Please use the proper helpers.
> > >
> > > If we use bridge_functions we need to take atomic functions as
> > > precedence as the next bridge functions might convert atomic calls.
> >
> > We've had this discussion over and over again, but this is something
> > that needs to be documented and / or in your commit log.
> >
> > You must not deviate from the standard (and expected) behavior without
> > any kind of justification.
>
> Not exactly sure about what you mean, sorry. All these atomic bridge
> functions are already documented if I'm not wrong and Laurent have
>
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
Hi Maxime,
On Mon, Nov 22, 2021 at 7:35 PM Maxime Ripard wrote:
>
> On Mon, Nov 22, 2021 at 07:18:13PM +0530, Jagan Teki wrote:
> > Hi Maxime,
> >
> > On Mon, Nov 22, 2021 at 3:37 PM Maxime Ripard wrote:
> > >
> > > On Mon, Nov 22, 2021 at 12:22:19PM +0530, 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.
> > >
> > > In order to do this you would need to use the DRM bridge API...
> >
> > Sorry, which bridge API do you mean?
>
> Any variant of of_drm_find_bridge, and drm_bridge_attach. Just like
> we're doing in sun4i_rgb.c
Yes, we have drm_bridge_attach in bind and bridge_function.attach
calls in this patch and of_drm_find_bridge in sun6i_mipi_dsi_attach.
Not sure which API's I've missed.
>
> > > > This patch convert the existing to a drm bridge driver with a
> > > > built-in encoder support for compatibility with existing
> > > > component drivers.
> > >
> > > ... but changing the encoder driver to a bridge is completely
> > > unnecessary to do so. Why did you need to make that change?
> >
> > Idea of this series is to convert the driver to bridge and use the
> > latest bridge function from the v1 series.
>
> Ok, but it's not at all what you mention in your commit log? You don't
> need any of that in order to support a bridge downstream.
I've mentioned "Converting to bridge driver" and thought it has
meaning of converting encoder related function to bridge functions as
well. Not think about specific description to describe on commit
message. Will update this.
>
> > >
> > > > Signed-off-by: Jagan Teki
> > > >
> > > > ---
> > > > 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
> > > > #include
> > > > +#include
> > > > #include
> > > > #include
> > > > #include
> > > > @@ -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 =
> > > > >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 =
> > > > >encoder->crtc->state->adjusted_mode;
> > > > struct mipi_dsi_device *device = dsi->device;
> > > > union phy_configure_opts opts = { };
> > > > struct phy_configure_opts_mipi_dphy *cfg = _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);
> > > > +
> > >
> > > Please use the proper helpers.
> >
> > If we use bridge_functions we need to take atomic functions as
> > precedence as the next bridge functions might convert atomic calls.
>
> We've had this discussion over and over again, but this is something
> that needs to be documented and / or in your commit log.
>
> You must not deviate from the standard (and expected) behavior without
> any kind of justification.
Not exactly sure about what you mean, sorry. All these atomic bridge
functions are already documented if I'm not wrong and Laurent have
patches to switch the normal functions to atomic. Not sure what else
need to document here and need justification for it if the driver is
converting to bridge.
>
> > >
> > > > /*
> > > >* 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)
> > > > +
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
On Mon, Nov 22, 2021 at 07:18:13PM +0530, Jagan Teki wrote:
> Hi Maxime,
>
> On Mon, Nov 22, 2021 at 3:37 PM Maxime Ripard wrote:
> >
> > On Mon, Nov 22, 2021 at 12:22:19PM +0530, 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.
> >
> > In order to do this you would need to use the DRM bridge API...
>
> Sorry, which bridge API do you mean?
Any variant of of_drm_find_bridge, and drm_bridge_attach. Just like
we're doing in sun4i_rgb.c
> > > This patch convert the existing to a drm bridge driver with a
> > > built-in encoder support for compatibility with existing
> > > component drivers.
> >
> > ... but changing the encoder driver to a bridge is completely
> > unnecessary to do so. Why did you need to make that change?
>
> Idea of this series is to convert the driver to bridge and use the
> latest bridge function from the v1 series.
Ok, but it's not at all what you mention in your commit log? You don't
need any of that in order to support a bridge downstream.
> >
> > > Signed-off-by: Jagan Teki
> > >
> > > ---
> > > 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
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -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 =
> > > >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 =
> > > >encoder->crtc->state->adjusted_mode;
> > > struct mipi_dsi_device *device = dsi->device;
> > > union phy_configure_opts opts = { };
> > > struct phy_configure_opts_mipi_dphy *cfg = _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);
> > > +
> >
> > Please use the proper helpers.
>
> If we use bridge_functions we need to take atomic functions as
> precedence as the next bridge functions might convert atomic calls.
We've had this discussion over and over again, but this is something
that needs to be documented and / or in your commit log.
You must not deviate from the standard (and expected) behavior without
any kind of justification.
> >
> > > /*
> > >* 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);
> > > +
> >
> > Ditto
> >
> > > sun6i_dsi_start(dsi, DSI_START_HSC);
> > >
> > > udelay(1000);
> > > @@ -794,15 +802,19 @@ static void sun6i_dsi_encoder_enable(struct
> > > drm_encoder *encoder)
> > > sun6i_dsi_start(dsi, DSI_START_HSD);
> > > }
> > >
> > > -static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
> > > +static void sun6i_dsi_bridge_atomic_disable(struct drm_bridge *bridge,
> > > + struct drm_bridge_state
> > > *old_bridge_state)
> > > {
> > > - struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
> > > + struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
> > >
> > > DRM_DEBUG_DRIVER("Disabling DSI output\n");
> > >
> > > if (dsi->panel) {
> > > drm_panel_disable(dsi->panel);
> > > drm_panel_unprepare(dsi->panel);
> > > + } else if (dsi->next_bridge) {
> > > +
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
Hi Maxime,
On Mon, Nov 22, 2021 at 3:37 PM Maxime Ripard wrote:
>
> On Mon, Nov 22, 2021 at 12:22:19PM +0530, 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.
>
> In order to do this you would need to use the DRM bridge API...
Sorry, which bridge API do you mean?
>
> > This patch convert the existing to a drm bridge driver with a
> > built-in encoder support for compatibility with existing
> > component drivers.
>
> ... but changing the encoder driver to a bridge is completely
> unnecessary to do so. Why did you need to make that change?
Idea of this series is to convert the driver to bridge and use the
latest bridge function from the v1 series.
>
> > Signed-off-by: Jagan Teki
> >
> > ---
> > 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
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -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 = >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 =
> > >encoder->crtc->state->adjusted_mode;
> > struct mipi_dsi_device *device = dsi->device;
> > union phy_configure_opts opts = { };
> > struct phy_configure_opts_mipi_dphy *cfg = _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);
> > +
>
> Please use the proper helpers.
If we use bridge_functions we need to take atomic functions as
precedence as the next bridge functions might convert atomic calls.
>
> > /*
> >* 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);
> > +
>
> Ditto
>
> > sun6i_dsi_start(dsi, DSI_START_HSC);
> >
> > udelay(1000);
> > @@ -794,15 +802,19 @@ static void sun6i_dsi_encoder_enable(struct
> > drm_encoder *encoder)
> > sun6i_dsi_start(dsi, DSI_START_HSD);
> > }
> >
> > -static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
> > +static void sun6i_dsi_bridge_atomic_disable(struct drm_bridge *bridge,
> > + struct drm_bridge_state
> > *old_bridge_state)
> > {
> > - struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
> > + struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
> >
> > DRM_DEBUG_DRIVER("Disabling DSI output\n");
> >
> > if (dsi->panel) {
> > drm_panel_disable(dsi->panel);
> > drm_panel_unprepare(dsi->panel);
> > + } else if (dsi->next_bridge) {
> > + dsi->next_bridge->funcs->atomic_disable(dsi->next_bridge,
> > old_bridge_state);
> > +
> > dsi->next_bridge->funcs->atomic_post_disable(dsi->next_bridge,
> > old_bridge_state);
>
> Ditto
>
> > }
> >
> > phy_power_off(dsi->dphy);
> > @@ -842,9 +854,25 @@ static const struct drm_connector_funcs
> > sun6i_dsi_connector_funcs = {
> > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> > };
> >
> > -static const struct drm_encoder_helper_funcs sun6i_dsi_enc_helper_funcs = {
> > - .disable= sun6i_dsi_encoder_disable,
> > - .enable = sun6i_dsi_encoder_enable,
> > +static int sun6i_dsi_bridge_attach(struct drm_bridge *bridge,
> > +enum
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
Hi Neil,
On Mon, Nov 22, 2021 at 6:22 PM Neil Armstrong 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
> > ---
> > 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
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -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 = >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 =
> > >encoder->crtc->state->adjusted_mode;
> > struct mipi_dsi_device *device = dsi->device;
> > union phy_configure_opts opts = { };
> > struct phy_configure_opts_mipi_dphy *cfg = _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
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-ja...@amarulasolutions.com/
Jagan.
--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to linux-sunxi+unsubscr...@googlegroups.com.
To view this discussion on the web, visit
https://groups.google.com/d/msgid/linux-sunxi/CAMty3ZDATTKoJq7aLOe5i%3DRPo2UHzqnLs8j8sT-EBNdpC7%3D3DQ%40mail.gmail.com.
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
Hi Maxime,
On Mon, Nov 22, 2021 at 11:07:12AM +0100, Maxime Ripard wrote:
> On Mon, Nov 22, 2021 at 12:22:19PM +0530, 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.
>
> In order to do this you would need to use the DRM bridge API...
>
> > This patch convert the existing to a drm bridge driver with a
> > built-in encoder support for compatibility with existing
> > component drivers.
>
> ... but changing the encoder driver to a bridge is completely
> unnecessary to do so. Why did you need to make that change?
>
> > Signed-off-by: Jagan Teki
> >
> > ---
> > 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
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -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 = >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 =
> > >encoder->crtc->state->adjusted_mode;
> > struct mipi_dsi_device *device = dsi->device;
> > union phy_configure_opts opts = { };
> > struct phy_configure_opts_mipi_dphy *cfg = _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);
> > +
>
> Please use the proper helpers.
I don't know about this series in particular, but overall we try to move
encoders to bridge drivers in order to standardize on a single API. The
drm_encoder can't be removed as it's exposed to userspace, so it then
becomes a dumb encoder, without any operation implemented.
> > /*
> > * 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);
> > +
>
> Ditto
>
> > sun6i_dsi_start(dsi, DSI_START_HSC);
> >
> > udelay(1000);
> > @@ -794,15 +802,19 @@ static void sun6i_dsi_encoder_enable(struct
> > drm_encoder *encoder)
> > sun6i_dsi_start(dsi, DSI_START_HSD);
> > }
> >
> > -static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
> > +static void sun6i_dsi_bridge_atomic_disable(struct drm_bridge *bridge,
> > + struct drm_bridge_state
> > *old_bridge_state)
> > {
> > - struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
> > + struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
> >
> > DRM_DEBUG_DRIVER("Disabling DSI output\n");
> >
> > if (dsi->panel) {
> > drm_panel_disable(dsi->panel);
> > drm_panel_unprepare(dsi->panel);
> > + } else if (dsi->next_bridge) {
> > + dsi->next_bridge->funcs->atomic_disable(dsi->next_bridge,
> > old_bridge_state);
> > + dsi->next_bridge->funcs->atomic_post_disable(dsi->next_bridge,
> > old_bridge_state);
>
> Ditto
>
> > }
> >
> > phy_power_off(dsi->dphy);
> > @@ -842,9 +854,25 @@ static const struct drm_connector_funcs
> > sun6i_dsi_connector_funcs = {
> > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> > };
> >
> > -static const struct drm_encoder_helper_funcs sun6i_dsi_enc_helper_funcs = {
> > - .disable= sun6i_dsi_encoder_disable,
> > - .enable = sun6i_dsi_encoder_enable,
> > +static int sun6i_dsi_bridge_attach(struct drm_bridge *bridge,
> > + enum drm_bridge_attach_flags flags)
> > +{
> > + struct sun6i_dsi *dsi =
[linux-sunxi] Re: [PATCH v5 3/7] drm: sun4i: dsi: Convert to bridge driver
On Mon, Nov 22, 2021 at 12:22:19PM +0530, 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.
In order to do this you would need to use the DRM bridge API...
> This patch convert the existing to a drm bridge driver with a
> built-in encoder support for compatibility with existing
> component drivers.
... but changing the encoder driver to a bridge is completely
unnecessary to do so. Why did you need to make that change?
> Signed-off-by: Jagan Teki
>
> ---
> 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
> #include
> +#include
> #include
> #include
> #include
> @@ -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 = >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 =
> >encoder->crtc->state->adjusted_mode;
> struct mipi_dsi_device *device = dsi->device;
> union phy_configure_opts opts = { };
> struct phy_configure_opts_mipi_dphy *cfg = _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);
> +
Please use the proper helpers.
> /*
>* 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);
> +
Ditto
> sun6i_dsi_start(dsi, DSI_START_HSC);
>
> udelay(1000);
> @@ -794,15 +802,19 @@ static void sun6i_dsi_encoder_enable(struct drm_encoder
> *encoder)
> sun6i_dsi_start(dsi, DSI_START_HSD);
> }
>
> -static void sun6i_dsi_encoder_disable(struct drm_encoder *encoder)
> +static void sun6i_dsi_bridge_atomic_disable(struct drm_bridge *bridge,
> + struct drm_bridge_state
> *old_bridge_state)
> {
> - struct sun6i_dsi *dsi = encoder_to_sun6i_dsi(encoder);
> + struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
>
> DRM_DEBUG_DRIVER("Disabling DSI output\n");
>
> if (dsi->panel) {
> drm_panel_disable(dsi->panel);
> drm_panel_unprepare(dsi->panel);
> + } else if (dsi->next_bridge) {
> + dsi->next_bridge->funcs->atomic_disable(dsi->next_bridge,
> old_bridge_state);
> + dsi->next_bridge->funcs->atomic_post_disable(dsi->next_bridge,
> old_bridge_state);
Ditto
> }
>
> phy_power_off(dsi->dphy);
> @@ -842,9 +854,25 @@ static const struct drm_connector_funcs
> sun6i_dsi_connector_funcs = {
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> };
>
> -static const struct drm_encoder_helper_funcs sun6i_dsi_enc_helper_funcs = {
> - .disable= sun6i_dsi_encoder_disable,
> - .enable = sun6i_dsi_encoder_enable,
> +static int sun6i_dsi_bridge_attach(struct drm_bridge *bridge,
> +enum drm_bridge_attach_flags flags)
> +{
> + struct sun6i_dsi *dsi = bridge_to_sun6i_dsi(bridge);
> +
> + if (dsi->next_bridge)
> + return drm_bridge_attach(bridge->encoder, dsi->next_bridge,
> + NULL, 0);
> +
> + return 0;
> +}
> +
> +static const struct drm_bridge_funcs sun6i_dsi_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_enable
