Hello Linus,
thank you for the comments.
čet, 15. lip 2023. u 21:55 Linus Walleij <linus.walleij@xxxxxxxxxx> napisao je:
>
> Hi Paulo,
>
> thanks for your patch!
>
> Overall this looks very good.
>
> I doubt that the display controller is actually by Fannal, but I guess
> you tried to find out? We usually try to identify the underlying display
> controller so the driver can be named after it and reused for more
> display panels.
Yes, of course, the controller is ST7701S.
>
> Some minor questions/nitpicks below.
>
> On Wed, Jun 7, 2023 at 5:11 PM Paulo Pavacic <pavacic.p@xxxxxxxxx> wrote:
>
> > +static int fannal_panel_enable(struct drm_panel *panel)
> > +{
> > + struct mipi_dsi_device *dsi = to_mipi_dsi_device(panel->dev);
> > +
> > + mipi_dsi_generic_write_seq(dsi, 0xFF, 0x77, 0x01, 0x00, 0x00, 0x13);
> > + mipi_dsi_generic_write_seq(dsi, 0xEF, 0x08);
>
> Why is everything using mipi_dsi_generic_write_seq() instead of
> mipi_dsi_dcs_write_seq()?
Okay, I will replace it.
>
> Especially here, because 0x11 is certainly a command:
>
> > + mipi_dsi_generic_write_seq(dsi, 0x11); //MIPI_DCS_EXIT_SLEEP_MODE
>
> Instead use:
>
> ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> if (ret)
> return ret;
>
>
> > + mipi_dsi_generic_write_seq(dsi, 0x29); //MIPI_DCS_SET_DISPLAY_ON
>
> Instead use:
>
> ret = mipi_dsi_dcs_set_display_on(dsi);
> if (ret)
> return ret;
>
Okay I will replace all commands with a functions found here:
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_mipi_dsi.c#L995
> Yours,
> Linus Walleij
Best regards,
Paulo
