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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andrzej,

I was just looking at this (which confused me and was not
fixed in v2):

On Tue, Oct 9, 2018 at 8:46 PM Linus Walleij <linus.walleij@xxxxxxxxxx> 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.

Isn't the case such that the clock we use which will be =< hs_rate
or lp_rate respectively, controls vrefresh?

So actually, the sync cycles are constant, the clock comes from
(usually) the best fit from the clock framework, and then vrefresh
depends on what frequency we get from there, and it will be
considerably lower in lp mode than in hs mode?

It feels a bit silly to return two modes for the display that are
going to be identical depending on whether hs or lp is used,
but in some sense it would be the right thing, but I think then
the display driver needs to be smart enough to identify
that "OK now KMS is asking for HS mode" and "OK now
KMA is asking for LP mode".

Further, switching the display into the "LP mode" should
probably have effects on the host other than just switching to
the LP clock.

I think due to the crudeness of the frameworks graphics of course
comes out as it should but if userspace would do something
creative like starting to inspect the vrefresh it gets and try to
switch modes depending on that, I guess we have a bit of a
problem. I guess currently userspace can't choose between
HS or LP mode anyway, but I guess we should
support that.

I've changed this to clocking matching the HS mode right now
but I feel a bit uncertain as to what the real fix is :/

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux