On the Arm Juno platform, the HDLCD pixel clock is constrained to 250KHz resolution in order to avoid the tiny System Control Processor spending aeons trying to calculate exact PLL coefficients. This means that modes like my oddball 1600x1200 with 130.89MHz clock get rejected since the rate cannot be matched exactly. In practice, though, this mode works quite happily with the clock at 131MHz, so let's relax the check to allow a little bit of slop.

Signed-off-by: Robin Murphy robin.murphy@arm.com --- drivers/gpu/drm/arm/hdlcd_crtc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c index ecac6fe0b213..a3efa28436ea 100644 --- a/drivers/gpu/drm/arm/hdlcd_crtc.c +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c @@ -193,7 +193,8 @@ static enum drm_mode_status hdlcd_crtc_mode_valid(struct drm_crtc *crtc, long rate, clk_rate = mode->clock * 1000;

rate = clk_round_rate(hdlcd->clk, clk_rate); - if (rate != clk_rate) { + /* 0.1% seems a close enough tolerance for the TDA19988 on Juno */ + if (abs(rate - clk_rate) * 1000 > clk_rate) { /* clock required by mode not supported by hardware */ return MODE_NOCLOCK; }

On Fri, May 17, 2019 at 05:37:21PM +0100, Robin Murphy wrote:

Rather than allowing any old mode through, then subsequently refusing unmatchable clock rates in atomic_check when it's too late to back out and pick a different mode, let's do that validation up-front where it will cause unsupported modes to be correctly pruned in the first place.

This also eliminates an issue whereby a perceived clock rate of 0 would cause atomic disable to fail and prevent the module from being unloaded.

Signed-off-by: Robin Murphy robin.murphy@arm.com

Acked-by: Liviu Dudau liviu.dudau@arm.com

Thanks for the patch!

Best regards, Liviu

---

This supersedes my previous patch here: https://patchwork.freedesktop.org/patch/288553/

---

drivers/gpu/drm/arm/hdlcd_crtc.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c index 0b2b62f8fa3c..ecac6fe0b213 100644 --- a/drivers/gpu/drm/arm/hdlcd_crtc.c +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c @@ -186,20 +186,19 @@ static void hdlcd_crtc_atomic_disable(struct drm_crtc *crtc, clk_disable_unprepare(hdlcd->clk); }

-static int hdlcd_crtc_atomic_check(struct drm_crtc *crtc,

• 		   struct drm_crtc_state *state)
  

+static enum drm_mode_status hdlcd_crtc_mode_valid(struct drm_crtc *crtc,

• const struct drm_display_mode *mode)
  

{ struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);

• struct drm_display_mode *mode = &state->adjusted_mode; long rate, clk_rate = mode->clock * 1000;

  rate = clk_round_rate(hdlcd->clk, clk_rate); if (rate != clk_rate) { /* clock required by mode not supported by hardware */

• return -EINVAL;
  

• return MODE_NOCLOCK;
  

  }

• return 0;

• return MODE_OK;

}

static void hdlcd_crtc_atomic_begin(struct drm_crtc *crtc, @@ -220,7 +219,7 @@ static void hdlcd_crtc_atomic_begin(struct drm_crtc *crtc, }

static const struct drm_crtc_helper_funcs hdlcd_crtc_helper_funcs = {

• .atomic_check = hdlcd_crtc_atomic_check,

• .mode_valid = hdlcd_crtc_mode_valid, .atomic_begin = hdlcd_crtc_atomic_begin, .atomic_enable = hdlcd_crtc_atomic_enable, .atomic_disable = hdlcd_crtc_atomic_disable,

-- 2.21.0.dirty