Re: [PATCH v3 33/43] drm/panel: simple: Change mode for Sharp lq123p1jx31

[Doug Anderson]

Hi,


On Wed, Jan 31, 2018 at 7:16 AM, Sean Paul <seanpaul@xxxxxxxxxxxx> wrote:
> On Wed, Jan 31, 2018 at 7:54 AM, Lucas Stach <l.stach@xxxxxxxxxxxxxx> wrote:
>> Am Dienstag, den 30.01.2018, 21:29 +0100 schrieb Thierry Escande:
>>> From: Sean Paul <seanpaul@xxxxxxxxxxxx>
>>>
>>> Change the mode for Sharp lq123p1jx31 panel to something more
>>> rockchip-friendly such that we can use the fixed PLLs to
>>> generate the pixel clock
>>
>> This should really switch to a display timing instead of exposing a
>> single mode. The display timing has min, typical, max tuples for all
>> the timings values, which would allow the attached driver to vary the
>> timings inside the allowed bounds if it makes sense.
>>
>> Trying to hit a specific pixel clock to free up a PLL is exactly one of
>> the use cases envisioned for the display timings stuff.
>>
>
> Agreed, I think we had this discussion the first time around. We
> should drop this patch.
>
> Thanks for catching this!

Are you sure we should drop this?  In order for things to work
properly (not generate noise on the digitizer or other EMI), this
needs to run at a very specific pixel clock with very specific
blanking times.  I know that earlier we had slightly different
blanking times and Samsung came back and said that there was noise on
the digitizer.  I could be wrong, but I don't think there's any way
currently to be able to specify exactly what timings should be used on
a particular board.

Don't get be wrong--I think a patch such as this one that claims a
single board's timings as the "right" ones for a generic panel is a
bit of a hack.  ...but at the same time there are no other users of
this panel (that I know of) in mainline and the timings presented here
are certainly sane timings for this panel.

In any case, previous discussion at: https://patchwork.kernel.org/patch/9614603/


...oh, and looking at the previous discussion reminds me that the
timings presented in this here patch are actually not the right ones
(they have the right PLL, but the wrong blankings to avoid the noise
issues).  See <//chromium-review.googlesource.com/381015>



-Doug