Re: [PATCH 1/2] drm/panel: sitronix-st7789v: add panel orientation support

[neil . armstrong]

On 02/08/2023 15:19, Michael Riesch wrote:
> Hi Neil,
>
> On 8/2/23 14:39, Neil Armstrong wrote:
> > On 18/07/2023 17:12, Michael Riesch wrote:
> > > Determine the orientation of the display based on the device tree and
> > > propagate it.
> > >
> > > While at it, fix the indentation in the struct drm_panel_funcs.
> > >
> > > Signed-off-by: Michael Riesch <michael.riesch@xxxxxxxxxxxxxx>
> > > ---
> > >    drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 28
> > > +++++++++++++++++++++-----
> > >    1 file changed, 23 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > index bbc4569cbcdc..6575f07d49e3 100644
> > > --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
> > > @@ -116,6 +116,7 @@ struct st7789v {
> > >        struct spi_device *spi;
> > >        struct gpio_desc *reset;
> > >        struct regulator *power;
> > > +    enum drm_panel_orientation orientation;
> > >    };
> > >      enum st7789v_prefix {
> > > @@ -170,6 +171,7 @@ static const struct drm_display_mode default_mode = {
> > >    static int st7789v_get_modes(struct drm_panel *panel,
> > >                     struct drm_connector *connector)
> > >    {
> > > +    struct st7789v *ctx = panel_to_st7789v(panel);
> > >        struct drm_display_mode *mode;
> > >          mode = drm_mode_duplicate(connector->dev, &default_mode);
> > > @@ -188,9 +190,22 @@ static int st7789v_get_modes(struct drm_panel
> > > *panel,
> > >        connector->display_info.width_mm = 61;
> > >        connector->display_info.height_mm = 103;
> > >    +    /*
> > > +     * TODO: Remove once all drm drivers call
> > > +     * drm_connector_set_orientation_from_panel()
> > > +     */
> > > +    drm_connector_set_panel_orientation(connector, ctx->orientation);
> > > +
> > >        return 1;
> > >    }
> > >    +static enum drm_panel_orientation st7789v_get_orientation(struct
> > > drm_panel *p)
> > > +{
> > > +    struct st7789v *ctx = panel_to_st7789v(p);
> > > +
> > > +    return ctx->orientation;
> > > +}
> > > +
> > >    static int st7789v_prepare(struct drm_panel *panel)
> > >    {
> > >        struct st7789v *ctx = panel_to_st7789v(panel);
> > > @@ -346,11 +361,12 @@ static int st7789v_unprepare(struct drm_panel
> > > *panel)
> > >    }
> > >      static const struct drm_panel_funcs st7789v_drm_funcs = {
> > > -    .disable    = st7789v_disable,
> > > -    .enable        = st7789v_enable,
> > > -    .get_modes    = st7789v_get_modes,
> > > -    .prepare    = st7789v_prepare,
> > > -    .unprepare    = st7789v_unprepare,
> > > +    .disable = st7789v_disable,
> > > +    .enable    = st7789v_enable,
> > > +    .get_modes = st7789v_get_modes,
> > > +    .get_orientation = st7789v_get_orientation,
> > > +    .prepare = st7789v_prepare,
> > > +    .unprepare = st7789v_unprepare,
> >
> > Changing the indentation of the whole block is a spurious change,
> > either change it in a separate patch or use the current indentation
> > style...
>
> OK, if we agree that the indentation should be changed I'll be happy to
> move the change to an extra patch.

Sure,

Neil

> > >    };
> > >      static int st7789v_probe(struct spi_device *spi)
> > > @@ -382,6 +398,8 @@ static int st7789v_probe(struct spi_device *spi)
> > >        if (ret)
> > >            return ret;
> > >    +    of_drm_get_panel_orientation(spi->dev.of_node, &ctx->orientation);
> > > +
> > >        drm_panel_add(&ctx->panel);
> > >          return 0;
> >
> > With this changed:
> >
> > Reviewed-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
>
> Thanks!
>
> Best regards,
> Michael
>
> > Thanks,
> > Neil