mbox series

[0/5] ti-sn65dsi83: Finalize transition to atomic operations

Message ID 20210621125518.13715-1-laurent.pinchart@ideasonboard.com (mailing list archive)
Headers show
Series ti-sn65dsi83: Finalize transition to atomic operations | expand

Message

Laurent Pinchart June 21, 2021, 12:55 p.m. UTC
Hello,

This patch series is based on top of "[PATCH] drm/bridge: ti-sn65dsi83:
Replace connector format patching with atomic_get_input_bus_fmts". It
completes the transition to atomic operations in the ti-sn65dsi83
driver. The main reason for this change is patch 4/5 that fixes a few
issues in the driver (see the patch's commit message for details), but
overall it also brings the driver to the most recent API which is nice
in itself.

Laurent Pinchart (5):
  drm: bridge: ti-sn65dsi83: Move LVDS format selection to .mode_set()
  drm: bridge: ti-sn65dsi83: Pass mode explicitly to helper functions
  drm: bridge: ti-sn65dsi83: Switch to atomic operations
  drm: bridge: ti-sn65dsi83: Retrieve output format from bridge state
  drm: bridge: ti-sn65dsi83: Retrieve the display mode from the state

 drivers/gpu/drm/bridge/ti-sn65dsi83.c | 166 +++++++++++++-------------
 1 file changed, 82 insertions(+), 84 deletions(-)

Comments

Sam Ravnborg June 21, 2021, 6:49 p.m. UTC | #1
Hi Laurent,

On Mon, Jun 21, 2021 at 03:55:13PM +0300, Laurent Pinchart wrote:
> Hello,
> 
> This patch series is based on top of "[PATCH] drm/bridge: ti-sn65dsi83:
> Replace connector format patching with atomic_get_input_bus_fmts". It
> completes the transition to atomic operations in the ti-sn65dsi83
> driver. The main reason for this change is patch 4/5 that fixes a few
> issues in the driver (see the patch's commit message for details), but
> overall it also brings the driver to the most recent API which is nice
> in itself.
> 
> Laurent Pinchart (5):
>   drm: bridge: ti-sn65dsi83: Move LVDS format selection to .mode_set()
>   drm: bridge: ti-sn65dsi83: Pass mode explicitly to helper functions
>   drm: bridge: ti-sn65dsi83: Switch to atomic operations
>   drm: bridge: ti-sn65dsi83: Retrieve output format from bridge state
>   drm: bridge: ti-sn65dsi83: Retrieve the display mode from the state
> 
>  drivers/gpu/drm/bridge/ti-sn65dsi83.c | 166 +++++++++++++-------------
>  1 file changed, 82 insertions(+), 84 deletions(-)

I have browsed the series and it all looked good.
Acked-by: Sam Ravnborg <sam@ravnborg.org>

on them all.

It is news to me that the atomic ops are the way to go - but then I have
been off-line for a while so no suprise or maybe I just missed it
before.

It would be good if the comments in drm_bridge.h could point out what is
deprecated, so we know what to avoid in new and updated bridge drivers.
But this is all un-related to this series.

	Sam
Laurent Pinchart June 21, 2021, 7 p.m. UTC | #2
Hi Sam,

On Mon, Jun 21, 2021 at 08:49:53PM +0200, Sam Ravnborg wrote:
> On Mon, Jun 21, 2021 at 03:55:13PM +0300, Laurent Pinchart wrote:
> > Hello,
> > 
> > This patch series is based on top of "[PATCH] drm/bridge: ti-sn65dsi83:
> > Replace connector format patching with atomic_get_input_bus_fmts". It
> > completes the transition to atomic operations in the ti-sn65dsi83
> > driver. The main reason for this change is patch 4/5 that fixes a few
> > issues in the driver (see the patch's commit message for details), but
> > overall it also brings the driver to the most recent API which is nice
> > in itself.
> > 
> > Laurent Pinchart (5):
> >   drm: bridge: ti-sn65dsi83: Move LVDS format selection to .mode_set()
> >   drm: bridge: ti-sn65dsi83: Pass mode explicitly to helper functions
> >   drm: bridge: ti-sn65dsi83: Switch to atomic operations
> >   drm: bridge: ti-sn65dsi83: Retrieve output format from bridge state
> >   drm: bridge: ti-sn65dsi83: Retrieve the display mode from the state
> > 
> >  drivers/gpu/drm/bridge/ti-sn65dsi83.c | 166 +++++++++++++-------------
> >  1 file changed, 82 insertions(+), 84 deletions(-)
> 
> I have browsed the series and it all looked good.
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> 
> on them all.
> 
> It is news to me that the atomic ops are the way to go - but then I have
> been off-line for a while so no suprise or maybe I just missed it
> before.

They're not mandatory as such, but they give us access to the atomic
state, which is sometimes required. Overall I think it would be nice to
move to the atomic operations and drop the legacy ones, to avoid
maintaining two sets of operations. It will take time :-)

> It would be good if the comments in drm_bridge.h could point out what is
> deprecated, so we know what to avoid in new and updated bridge drivers.
> But this is all un-related to this series.

It's a good point. Would you like to submit a patch, or should I do so ?
Sam Ravnborg June 21, 2021, 7:34 p.m. UTC | #3
Hi Laurent,

> > 
> > It is news to me that the atomic ops are the way to go - but then I have
> > been off-line for a while so no suprise or maybe I just missed it
> > before.
> 
> They're not mandatory as such, but they give us access to the atomic
> state, which is sometimes required. Overall I think it would be nice to
> move to the atomic operations and drop the legacy ones, to avoid
> maintaining two sets of operations. It will take time :-)
Yeah, but if we can get more people working on the job..
> 
> > It would be good if the comments in drm_bridge.h could point out what is
> > deprecated, so we know what to avoid in new and updated bridge drivers.
> > But this is all un-related to this series.
> 
> It's a good point. Would you like to submit a patch, or should I do so ?
Please do as I would have to dig around to do it right as I have
fogotten most of the drm internals the last couple of months.

Just something simple like: "This is deprecated, do not use!" would do
the trick for me. Then I would know what to look for if I was reviewing
a new bridge driver or patching an existing one or just trying to gentle
push someone in the right direction.

For drm_drv.h this really helped me to understand what should not be
used.

	Sam
Robert Foss June 22, 2021, 8:28 a.m. UTC | #4
Applied to drm-misc-next


On Mon, 21 Jun 2021 at 20:49, Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Hi Laurent,
>
> On Mon, Jun 21, 2021 at 03:55:13PM +0300, Laurent Pinchart wrote:
> > Hello,
> >
> > This patch series is based on top of "[PATCH] drm/bridge: ti-sn65dsi83:
> > Replace connector format patching with atomic_get_input_bus_fmts". It
> > completes the transition to atomic operations in the ti-sn65dsi83
> > driver. The main reason for this change is patch 4/5 that fixes a few
> > issues in the driver (see the patch's commit message for details), but
> > overall it also brings the driver to the most recent API which is nice
> > in itself.
> >
> > Laurent Pinchart (5):
> >   drm: bridge: ti-sn65dsi83: Move LVDS format selection to .mode_set()
> >   drm: bridge: ti-sn65dsi83: Pass mode explicitly to helper functions
> >   drm: bridge: ti-sn65dsi83: Switch to atomic operations
> >   drm: bridge: ti-sn65dsi83: Retrieve output format from bridge state
> >   drm: bridge: ti-sn65dsi83: Retrieve the display mode from the state
> >
> >  drivers/gpu/drm/bridge/ti-sn65dsi83.c | 166 +++++++++++++-------------
> >  1 file changed, 82 insertions(+), 84 deletions(-)
>
> I have browsed the series and it all looked good.
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>
> on them all.
>
> It is news to me that the atomic ops are the way to go - but then I have
> been off-line for a while so no suprise or maybe I just missed it
> before.
>
> It would be good if the comments in drm_bridge.h could point out what is
> deprecated, so we know what to avoid in new and updated bridge drivers.
> But this is all un-related to this series.
>
>         Sam