[PATCH 2/2] drm/panel: Add driver for Samsung S6D16D0 panel

[Linus Walleij]

On Tue, Oct 9, 2018 at 10:29 AM Andrzej Hajda <a.hajda at samsung.com> wrote:
> On 08.10.2018 12:58, Linus Walleij wrote:

> > +static const struct drm_display_mode samsung_s6d16d0_mode = {
> > +     .clock = 420160, /* HS 420160kHz, LP 19200kHz */
> > +     .hdisplay = 864,
> > +     .hsync_start = 864 + 154,
> > +     .hsync_end = 864 + 154 + 16,
> > +     .htotal = 864 + 154 + 16 + 32,
> > +     .vdisplay = 480,
> > +     .vsync_start = 480 + 1,
> > +     .vsync_end = 480 + 1 + 1,
> > +     .vtotal = 480 + 1 + 1 + 1,
> > +     .vrefresh = 60,
> > +};
>
> htotal*vtotal*vrefresh should be equal to clock * 1000, even in command
> mode, and they should be equal to TearingEffect signal frequency.

OK I had no clue. I'll look closer at this. I suspect the .clock is just
wrong and should instead be based on what you say.

Hm what about making a macro for that or something...
it seems the clock should simply be calculated from the
other values.

> > +static int s6d16d0_unprepare(struct drm_panel *panel)
> > +{
> > +     struct s6d16d0 *s6 = panel_to_s6d16d0(panel);
> > +     struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
> > +     int ret;
> > +
> > +     /* Exit sleep mode and power on */
> > +     ret = mipi_dsi_dcs_set_display_off(dsi);
> > +     if (ret) {
> > +             dev_err(s6->dev, "failed to turn display off\n");
> > +             return ret;
> > +     }
> > +     ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
> > +     if (ret) {
> > +             dev_err(s6->dev, "failed to enter sleep mode\n");
> > +             return ret;
> > +     }
>
> Usually display_off should be called from _disable callback and enter
> sleep mode and power off from _unprepare callback, symmetrically on
> power-on path.

OK I try that.

> Moreover usually there should be some delays between these commands to
> allow panel to enter appropriate states.

There are none of these in the out-of-tree driver, and this is one of
those Samsung panels without any datasheet (known to me at least).
And it works...

> > +     /* Enabe tearing mode: send TE (tearing effect) at VBLANK */
> > +     ret = mipi_dsi_dcs_set_tear_on(dsi,
> > +                                    MIPI_DSI_DCS_TEAR_MODE_VBLANK);
>
> So the panel does not signal TE to DisplayController via dedicated
> line/gpio.

Well there is a dedicated line, but in my case (at least) it is not
routed to a GPIO but to a dedicated line on the DSI host.

> I guess then it is up to DSI-master to deliver this signal to
> DisplayController? Am I right?

Yes this hardware has a combined display controller and three
DSI masters (also DPI output) in the same register range.
They are jotted together in a display complex.

> What DSI-master driver is used with this
> panel?

This:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-stericsson.git/commit/?h=ux500-mcde

I am in the uncomfortable big upfront design phase that is so typical
for DRM drivers. Everything has to be built up from zero. :/

Yours,
Linus Walleij