On Tue, Jul 11, 2017 at 12:56 AM, Daniel Vetter <[email protected]> wrote: > On Mon, Jul 10, 2017 at 03:47:54PM -0700, John Stultz wrote: >> On Mon, Jul 10, 2017 at 2:18 PM, Sean Paul <[email protected]> wrote: >> > On Mon, Jul 10, 2017 at 01:48:02PM -0700, John Stultz wrote: >> >> Currently the hikey dsi logic cannot generate accurate byte >> >> clocks values for all pixel clock values. Thus if a mode clock >> >> is selected that cannot match the calculated byte clock, the >> >> device will boot with a blank screen. >> >> >> >> This patch uses the new mode_valid callback (many thanks to >> >> Jose Abreu for upstreaming it!) to enforces known good mode >> >> clocks for well known resolutions, which should allow the >> >> display to work from given EDID options, and ensures for a given >> >> resolution & refresh, the right mode clock is selected. >> >> >> >> Cc: Daniel Vetter <[email protected]> >> >> Cc: Jani Nikula <[email protected]> >> >> Cc: Sean Paul <[email protected]> >> >> Cc: David Airlie <[email protected]> >> >> Cc: Rob Clark <[email protected]> >> >> Cc: Xinliang Liu <[email protected]> >> >> Cc: Xinliang Liu <[email protected]> >> >> Cc: Rongrong Zou <[email protected]> >> >> Cc: Xinwei Kong <[email protected]> >> >> Cc: Chen Feng <[email protected]> >> >> Cc: Jose Abreu <[email protected]> >> >> Cc: Archit Taneja <[email protected]> >> >> Cc: [email protected] >> >> Signed-off-by: John Stultz <[email protected]> >> >> --- >> >> drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 37 >> >> ++++++++++++++++++++++++++++ >> >> 1 file changed, 37 insertions(+) >> >> >> >> diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c >> >> b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c >> >> index f77dcfa..a84f4bb 100644 >> >> --- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c >> >> +++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c >> >> @@ -603,6 +603,42 @@ static void dsi_encoder_enable(struct drm_encoder >> >> *encoder) >> >> dsi->enable = true; >> >> } >> >> >> >> +static enum drm_mode_status dsi_encoder_mode_valid(struct drm_encoder >> >> *crtc, >> >> + const struct drm_display_mode *mode) >> >> +{ >> >> + /* >> >> + * kirin cannot generate all modes, so use the whitelist below >> >> + */ >> >> + DRM_DEBUG("%s: Checking mode %ix%i@%i clock: %i...", __func__, >> >> + mode->hdisplay, mode->vdisplay, drm_mode_vrefresh(mode), >> >> mode->clock); >> >> + if ((mode->hdisplay == 1920 && mode->vdisplay == 1080 && >> >> mode->clock == 148500) || >> >> + (mode->hdisplay == 1920 && mode->vdisplay == 1080 && >> >> mode->clock == 80192) || >> >> + (mode->hdisplay == 1920 && mode->vdisplay == 1080 && >> >> mode->clock == 74250) || >> >> + (mode->hdisplay == 1920 && mode->vdisplay == 1080 && >> >> mode->clock == 61855) || >> >> + (mode->hdisplay == 1680 && mode->vdisplay == 1050 && >> >> mode->clock == 147116) || >> >> + (mode->hdisplay == 1680 && mode->vdisplay == 1050 && >> >> mode->clock == 146250) || >> >> + (mode->hdisplay == 1680 && mode->vdisplay == 1050 && >> >> mode->clock == 144589) || >> >> + (mode->hdisplay == 1600 && mode->vdisplay == 1200 && >> >> mode->clock == 160961) || >> >> + (mode->hdisplay == 1600 && mode->vdisplay == 900 && >> >> mode->clock == 118963) || >> >> + (mode->hdisplay == 1440 && mode->vdisplay == 900 && >> >> mode->clock == 126991) || >> >> + (mode->hdisplay == 1280 && mode->vdisplay == 1024 && >> >> mode->clock == 128946) || >> >> + (mode->hdisplay == 1280 && mode->vdisplay == 1024 && >> >> mode->clock == 98619) || >> >> + (mode->hdisplay == 1280 && mode->vdisplay == 960 && >> >> mode->clock == 102081) || >> >> + (mode->hdisplay == 1280 && mode->vdisplay == 800 && >> >> mode->clock == 83496) || >> >> + (mode->hdisplay == 1280 && mode->vdisplay == 720 && >> >> mode->clock == 74440) || >> >> + (mode->hdisplay == 1280 && mode->vdisplay == 720 && >> >> mode->clock == 74250) || >> >> + (mode->hdisplay == 1024 && mode->vdisplay == 768 && >> >> mode->clock == 78800) || >> >> + (mode->hdisplay == 1024 && mode->vdisplay == 768 && >> >> mode->clock == 75000) || >> >> + (mode->hdisplay == 1024 && mode->vdisplay == 768 && >> >> mode->clock == 81833) || >> >> + (mode->hdisplay == 800 && mode->vdisplay == 600 && >> >> mode->clock == 48907) || >> >> + (mode->hdisplay == 800 && mode->vdisplay == 600 && >> >> mode->clock == 40000)) { >> > >> > Bikeshed incoming: >> > >> > Can you break this out into a lookup table? It's kind of long-winded >> > as-is. It'd >> >> That's fair. The list has slowly grown from 4 or so modes to what it >> is now, so it is a bit longer then it was originally. >> >> I'll spin the patches with that change. >> >> > be even better if you could calculate whether the mode is valid, but I >> > didn't >> > spend enough time to figure out if this is possible. >> >> Theoretically that might be possible, checking if the requested freq >> matches the calculated freq, and I've tried that but so far I've not >> been able to get it to work, as in some cases the freq on the current >> whitelist don't exactly match but do work on the large majority of >> monitors tested (while other freq have a similar error but don't work >> on most monitors). >> >> I'd like to spend some more time to try to refine and tune this, but >> having the current whitelist works fairly well, so I'm not sure its >> worth sitting on (this is basically the last functional patch >> outstanding for HiKey to fully work upstream - except the mali gpu of >> course), while I try to tinker and tune it. >> >> Thanks so much for the feedback! > > Yeah the proper approach is to compute your pll/clock settings and bail > out if those don't work. That's generally a magic spreadsheet supplied by > the hw validation engineers, and I honestly don't want to know how they > create it. Explicit modelist in the kernel sounds like a very bad hack.
So without such a magic spreadsheet, how would you suggest I move this forward? Not having the whitelist hack and picking modes the device can't generate is a fairly major usability issue. thanks -john _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
