Hi, 2018-02-16 21:54 GMT+01:00 Doug Anderson <dianders at chromium.org>: > Hi, > > On Fri, Feb 16, 2018 at 4:34 AM, Enric Balletbo Serra > <eballetbo at gmail.com> wrote: >> Hi, >> >> 2018-01-31 17:52 GMT+01:00 Doug Anderson <dianders at chromium.org>: >>> Hi, >>> >>> >>> On Wed, Jan 31, 2018 at 7:16 AM, Sean Paul <seanpaul at chromium.org> wrote: >>>> On Wed, Jan 31, 2018 at 7:54 AM, Lucas Stach <l.stach at pengutronix.de> wrote: >>>>> Am Dienstag, den 30.01.2018, 21:29 +0100 schrieb Thierry Escande: >>>>>> From: Sean Paul <seanpaul at chromium.org> >>>>>> >>>>>> 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> >>> >> >> As Thierry no longer has the hardware to test these patch series, I'll >> take care of these and follow the upstreaming process. I think that >> doesn't make sense send a v4 version of all 43 patches for this >> change. Right now, only this patch received comments so I'll wait a >> bit more for if we can get the other patches reviewed. If the others >> are fine just and I don't need to send a new version just don't apply >> this one and I will send a second version of that specific patch. Or >> even better, is really trivial what needs to be changed, so maybe the >> maintainer can do it? ;) > > Just as a heads up, Sean Paul has a series of patches to replace this > patch. The following are IDs from patchwork.kernel.org: > > 10207583 New [v3,1/6] dt-bindings: Clarify timing subnode use > as panel-timing > 10207585 New [v3,2/6] dt-bindings: Add headings to > simple-panel bindings > 10207591 New [v3,3/6] dt-bindings: Add panel-timing subnode > to simple-panel > 10207593 New [v3,4/6] drm/panel: simple: Add ability to > override typical timing > 10207595 New [v3,5/6] drm/panel: simple: Use display_timing > for lq123p1jx31 > 10207603 New [v3,6/6] arm64: dts: rockchip: Specify override > mode for kevin panel > > -Doug Nice, I was not aware of these, I'll test. That means that this patch can be removed from these series as the Sean solution is a lot better. Just a note that this patch can be removed without any collateral impact on the other patches, so just ignore it. Regards, Enric