On 09.10.2018 20:46, Linus Walleij wrote:
> On Tue, Oct 9, 2018 at 10:29 AM Andrzej Hajda <a.hajda@xxxxxxxxxxx> 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.
In general .vrefresh is redundant, and it should be calculated from
clock and totals, otherwise rounding errors can be significant. On the
other side .vrefresh is much more readable than clock.
So maybe some initializer:
#define DRM_MODE_FROM_VM(hact, hfp, hsyn, hbp, vact, vfp, vsyn, vbp,
vrefresh) ...
would work, and will eliminate these pluses from current initializers.
Here vrefresh could be even float, as the initializer will evaluate and
convert to int in compile time.
>
>>> +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...
Yes, the panel is turned off so artifacts are hard to notice anyway. So
lets leave it as is.
Regards
Andrzej
>
>>> + /* 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
>
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
