diff mbox series

[4/4] drm/sun4i: rgb: Change the pixel clock validation check

Message ID ec2dc2a7b3d4bd44f7a2a6e1c1813f92449a7310.1551191081.git-series.maxime.ripard@bootlin.com (mailing list archive)
State New, archived
Headers show
Series drm/sun4i: rgb: Relax the pixel clock check | expand

Commit Message

Maxime Ripard Feb. 26, 2019, 2:25 p.m. UTC 
The current code, since commit bb43d40d7c83 ("drm/sun4i: rgb: Validate the
clock rate"), perform some validation on the pixel clock to filter out the
EDID modes provided by monitors (through bridges) that we wouldn't be able
to reach. For the usual modes, we're able to generate a perfect clock rate,
so a strict check was enough.

However, this had the side effect of preventing displays that would work
otherwise to operate properly, since we would pretty much never be able to
generate an exact rate for those displays, even though we would fall within
that panel tolerance.

This was also shown to happen for unusual modes exposed through EDIDs, for
example on eDP panels.

We can work around this by simplifying a bit the problem: no panels we've
encountered so far actually needed that check. All of them are tied to a
particular board when it is produced, and made to work with the Allwinner
BSP. That pretty much guarantees that we never have a pixel clock out of
reach.

On the other hand, the EDIDs modes that needed to be validated have always
been exposed through bridges.

Let's just use that metric to instead of validating all modes, only
validate modes when we have a bridge attached. It should be good enough for
now, while we still have room for improvements or refinements using the
display_timings structure for example for panels.

We also add a tolerance for EDID-based modes instead of doing a strict
check. This tolerance is of 0.5% which is the one advertised in the VESA
DVT and CVT specs. If that needed to be extended in the future, we can add
a custom module parameter to relax it a bit.

Fixes: bb43d40d7c83 ("drm/sun4i: rgb: Validate the clock rate")
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/gpu/drm/sun4i/sun4i_rgb.c | 37 ++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

Comments

kernel test robot Feb. 27, 2019, 3:55 a.m. UTC | #1 
Hi Maxime,

I love your patch! Yet something to improve:

[auto build test ERROR on ]

url:    https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-sun4i-rgb-Relax-the-pixel-clock-check/20190227-012757
base:    
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.2.0 make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

>> drivers/gpu//drm/sun4i/sun4i_rgb.c:62:1: error: unknown type name 'DEFINE'; did you mean 'EMFILE'?
    DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE 5
    ^~~~~~
    EMFILE
   drivers/gpu//drm/sun4i/sun4i_rgb.c:62:37: error: expected '=', ',', ';', 'asm' or '__attribute__' before numeric constant
    DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE 5
                                        ^
   drivers/gpu//drm/sun4i/sun4i_rgb.c:191:16: error: 'sun4i_rgb_mode_valid' undeclared here (not in a function); did you mean 'sun4i_rgb_con_funcs'?
     .mode_valid = sun4i_rgb_mode_valid,
                   ^~~~~~~~~~~~~~~~~~~~
                   sun4i_rgb_con_funcs

vim +62 drivers/gpu//drm/sun4i/sun4i_rgb.c

    55	
    56	/*
    57	 * VESA DMT defines a tolerance of 0.5% on the pixel clock, while the
    58	 * CVT spec reuses that tolerance in its examples, so it looks to be a
    59	 * good default tolerance for the EDID-based modes. Define it to 5 per
    60	 * mille to avoid floating point operations.
    61	 */
  > 62	DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE	5
    63	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Feb. 27, 2019, 4:55 a.m. UTC | #2 
Hi Maxime,

I love your patch! Yet something to improve:

[auto build test ERROR on ]

url:    https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-sun4i-rgb-Relax-the-pixel-clock-check/20190227-012757
base:    
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> drivers/gpu//drm/sun4i/sun4i_rgb.c:62:1: error: unknown type name 'DEFINE'; did you mean 'EPIPE'?
    DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE 5
    ^~~~~~
    EPIPE
>> drivers/gpu//drm/sun4i/sun4i_rgb.c:62:37: error: expected '=', ',', ';', 'asm' or '__attribute__' before numeric constant
    DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE 5
                                        ^
>> drivers/gpu//drm/sun4i/sun4i_rgb.c:191:16: error: 'sun4i_rgb_mode_valid' undeclared here (not in a function); did you mean 'sun4i_rgb_con_funcs'?
     .mode_valid = sun4i_rgb_mode_valid,
                   ^~~~~~~~~~~~~~~~~~~~
                   sun4i_rgb_con_funcs

vim +191 drivers/gpu//drm/sun4i/sun4i_rgb.c

29e57fab9 Maxime Ripard  2015-10-29   55  
3c3671edd Maxime Ripard  2019-02-26   56  /*
3c3671edd Maxime Ripard  2019-02-26   57   * VESA DMT defines a tolerance of 0.5% on the pixel clock, while the
3c3671edd Maxime Ripard  2019-02-26   58   * CVT spec reuses that tolerance in its examples, so it looks to be a
3c3671edd Maxime Ripard  2019-02-26   59   * good default tolerance for the EDID-based modes. Define it to 5 per
3c3671edd Maxime Ripard  2019-02-26   60   * mille to avoid floating point operations.
3c3671edd Maxime Ripard  2019-02-26   61   */
3c3671edd Maxime Ripard  2019-02-26  @62  DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE	5
3c3671edd Maxime Ripard  2019-02-26   63  
cde8b7548 Giulio Benetti 2018-03-13   64  static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
cde8b7548 Giulio Benetti 2018-03-13   65  						 const struct drm_display_mode *mode)
29e57fab9 Maxime Ripard  2015-10-29   66  {
cde8b7548 Giulio Benetti 2018-03-13   67  	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(crtc);
b9c8506cb Chen-Yu Tsai   2017-02-23   68  	struct sun4i_tcon *tcon = rgb->tcon;
29e57fab9 Maxime Ripard  2015-10-29   69  	u32 hsync = mode->hsync_end - mode->hsync_start;
29e57fab9 Maxime Ripard  2015-10-29   70  	u32 vsync = mode->vsync_end - mode->vsync_start;
7659a82a3 Maxime Ripard  2019-02-26   71  	unsigned long long rate = mode->clock * 1000;
3c3671edd Maxime Ripard  2019-02-26   72  	unsigned long long lowest, highest;
7659a82a3 Maxime Ripard  2019-02-26   73  	unsigned long long rounded_rate;
29e57fab9 Maxime Ripard  2015-10-29   74  
29e57fab9 Maxime Ripard  2015-10-29   75  	DRM_DEBUG_DRIVER("Validating modes...\n");
29e57fab9 Maxime Ripard  2015-10-29   76  
29e57fab9 Maxime Ripard  2015-10-29   77  	if (hsync < 1)
29e57fab9 Maxime Ripard  2015-10-29   78  		return MODE_HSYNC_NARROW;
29e57fab9 Maxime Ripard  2015-10-29   79  
29e57fab9 Maxime Ripard  2015-10-29   80  	if (hsync > 0x3ff)
29e57fab9 Maxime Ripard  2015-10-29   81  		return MODE_HSYNC_WIDE;
29e57fab9 Maxime Ripard  2015-10-29   82  
29e57fab9 Maxime Ripard  2015-10-29   83  	if ((mode->hdisplay < 1) || (mode->htotal < 1))
29e57fab9 Maxime Ripard  2015-10-29   84  		return MODE_H_ILLEGAL;
29e57fab9 Maxime Ripard  2015-10-29   85  
29e57fab9 Maxime Ripard  2015-10-29   86  	if ((mode->hdisplay > 0x7ff) || (mode->htotal > 0xfff))
29e57fab9 Maxime Ripard  2015-10-29   87  		return MODE_BAD_HVALUE;
29e57fab9 Maxime Ripard  2015-10-29   88  
29e57fab9 Maxime Ripard  2015-10-29   89  	DRM_DEBUG_DRIVER("Horizontal parameters OK\n");
29e57fab9 Maxime Ripard  2015-10-29   90  
29e57fab9 Maxime Ripard  2015-10-29   91  	if (vsync < 1)
29e57fab9 Maxime Ripard  2015-10-29   92  		return MODE_VSYNC_NARROW;
29e57fab9 Maxime Ripard  2015-10-29   93  
29e57fab9 Maxime Ripard  2015-10-29   94  	if (vsync > 0x3ff)
29e57fab9 Maxime Ripard  2015-10-29   95  		return MODE_VSYNC_WIDE;
29e57fab9 Maxime Ripard  2015-10-29   96  
29e57fab9 Maxime Ripard  2015-10-29   97  	if ((mode->vdisplay < 1) || (mode->vtotal < 1))
29e57fab9 Maxime Ripard  2015-10-29   98  		return MODE_V_ILLEGAL;
29e57fab9 Maxime Ripard  2015-10-29   99  
29e57fab9 Maxime Ripard  2015-10-29  100  	if ((mode->vdisplay > 0x7ff) || (mode->vtotal > 0xfff))
29e57fab9 Maxime Ripard  2015-10-29  101  		return MODE_BAD_VVALUE;
29e57fab9 Maxime Ripard  2015-10-29  102  
29e57fab9 Maxime Ripard  2015-10-29  103  	DRM_DEBUG_DRIVER("Vertical parameters OK\n");
29e57fab9 Maxime Ripard  2015-10-29  104  
3c3671edd Maxime Ripard  2019-02-26  105  	/*
3c3671edd Maxime Ripard  2019-02-26  106  	 * TODO: We should use the struct display_timing if available
3c3671edd Maxime Ripard  2019-02-26  107  	 * and / or trying to stretch the timings within that
3c3671edd Maxime Ripard  2019-02-26  108  	 * tolerancy to take care of panels that we wouldn't be able
3c3671edd Maxime Ripard  2019-02-26  109  	 * to have a exact match for.
3c3671edd Maxime Ripard  2019-02-26  110  	 */
3c3671edd Maxime Ripard  2019-02-26  111  	if (rgb->panel) {
3c3671edd Maxime Ripard  2019-02-26  112  		DRM_DEBUG_DRIVER("RGB panel used, skipping clock rate checks");
3c3671edd Maxime Ripard  2019-02-26  113  		goto out;
3c3671edd Maxime Ripard  2019-02-26  114  	}
3c3671edd Maxime Ripard  2019-02-26  115  
3c3671edd Maxime Ripard  2019-02-26  116  	/*
3c3671edd Maxime Ripard  2019-02-26  117  	 * That shouldn't ever happen unless something is really wrong, but it
3c3671edd Maxime Ripard  2019-02-26  118  	 * doesn't harm to check.
3c3671edd Maxime Ripard  2019-02-26  119  	 */
3c3671edd Maxime Ripard  2019-02-26  120  	if (!rgb->bridge)
3c3671edd Maxime Ripard  2019-02-26  121  		goto out;
3c3671edd Maxime Ripard  2019-02-26  122  
5af894bd2 Maxime Ripard  2018-02-21  123  	tcon->dclk_min_div = 6;
5af894bd2 Maxime Ripard  2018-02-21  124  	tcon->dclk_max_div = 127;
bb43d40d7 Maxime Ripard  2016-04-21  125  	rounded_rate = clk_round_rate(tcon->dclk, rate);
3c3671edd Maxime Ripard  2019-02-26  126  
3c3671edd Maxime Ripard  2019-02-26  127  	lowest = rate * (1000 - SUN4I_RGB_DOTCLOCK_TOLERANCE);
3c3671edd Maxime Ripard  2019-02-26  128  	do_div(lowest, 1000);
3c3671edd Maxime Ripard  2019-02-26  129  	if (rounded_rate < lowest)
bb43d40d7 Maxime Ripard  2016-04-21  130  		return MODE_CLOCK_LOW;
bb43d40d7 Maxime Ripard  2016-04-21  131  
3c3671edd Maxime Ripard  2019-02-26  132  	highest = rate * (1000 + SUN4I_RGB_DOTCLOCK_TOLERANCE);
3c3671edd Maxime Ripard  2019-02-26  133  	do_div(highest, 1000);
3c3671edd Maxime Ripard  2019-02-26  134  	if (rounded_rate > highest)
bb43d40d7 Maxime Ripard  2016-04-21  135  		return MODE_CLOCK_HIGH;
bb43d40d7 Maxime Ripard  2016-04-21  136  
3c3671edd Maxime Ripard  2019-02-26  137  out:
bb43d40d7 Maxime Ripard  2016-04-21  138  	DRM_DEBUG_DRIVER("Clock rate OK\n");
bb43d40d7 Maxime Ripard  2016-04-21  139  
29e57fab9 Maxime Ripard  2015-10-29  140  	return MODE_OK;
29e57fab9 Maxime Ripard  2015-10-29  141  }
29e57fab9 Maxime Ripard  2015-10-29  142  
29e57fab9 Maxime Ripard  2015-10-29  143  static struct drm_connector_helper_funcs sun4i_rgb_con_helper_funcs = {
29e57fab9 Maxime Ripard  2015-10-29  144  	.get_modes	= sun4i_rgb_get_modes,
29e57fab9 Maxime Ripard  2015-10-29  145  };
29e57fab9 Maxime Ripard  2015-10-29  146  
29e57fab9 Maxime Ripard  2015-10-29  147  static void
29e57fab9 Maxime Ripard  2015-10-29  148  sun4i_rgb_connector_destroy(struct drm_connector *connector)
29e57fab9 Maxime Ripard  2015-10-29  149  {
29e57fab9 Maxime Ripard  2015-10-29  150  	struct sun4i_rgb *rgb = drm_connector_to_sun4i_rgb(connector);
29e57fab9 Maxime Ripard  2015-10-29  151  
b413dde1a Maxime Ripard  2019-02-26  152  	drm_panel_detach(rgb->panel);
29e57fab9 Maxime Ripard  2015-10-29  153  	drm_connector_cleanup(connector);
29e57fab9 Maxime Ripard  2015-10-29  154  }
29e57fab9 Maxime Ripard  2015-10-29  155  
32b4d5756 Bhumika Goyal  2017-08-08  156  static const struct drm_connector_funcs sun4i_rgb_con_funcs = {
29e57fab9 Maxime Ripard  2015-10-29  157  	.fill_modes		= drm_helper_probe_single_connector_modes,
29e57fab9 Maxime Ripard  2015-10-29  158  	.destroy		= sun4i_rgb_connector_destroy,
29e57fab9 Maxime Ripard  2015-10-29  159  	.reset			= drm_atomic_helper_connector_reset,
29e57fab9 Maxime Ripard  2015-10-29  160  	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
29e57fab9 Maxime Ripard  2015-10-29  161  	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
29e57fab9 Maxime Ripard  2015-10-29  162  };
29e57fab9 Maxime Ripard  2015-10-29  163  
29e57fab9 Maxime Ripard  2015-10-29  164  static void sun4i_rgb_encoder_enable(struct drm_encoder *encoder)
29e57fab9 Maxime Ripard  2015-10-29  165  {
29e57fab9 Maxime Ripard  2015-10-29  166  	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
29e57fab9 Maxime Ripard  2015-10-29  167  
29e57fab9 Maxime Ripard  2015-10-29  168  	DRM_DEBUG_DRIVER("Enabling RGB output\n");
29e57fab9 Maxime Ripard  2015-10-29  169  
b413dde1a Maxime Ripard  2019-02-26  170  	if (rgb->panel) {
b413dde1a Maxime Ripard  2019-02-26  171  		drm_panel_prepare(rgb->panel);
b413dde1a Maxime Ripard  2019-02-26  172  		drm_panel_enable(rgb->panel);
29e57fab9 Maxime Ripard  2015-10-29  173  	}
45e88f994 Maxime Ripard  2017-10-17  174  }
29e57fab9 Maxime Ripard  2015-10-29  175  
29e57fab9 Maxime Ripard  2015-10-29  176  static void sun4i_rgb_encoder_disable(struct drm_encoder *encoder)
29e57fab9 Maxime Ripard  2015-10-29  177  {
29e57fab9 Maxime Ripard  2015-10-29  178  	struct sun4i_rgb *rgb = drm_encoder_to_sun4i_rgb(encoder);
29e57fab9 Maxime Ripard  2015-10-29  179  
29e57fab9 Maxime Ripard  2015-10-29  180  	DRM_DEBUG_DRIVER("Disabling RGB output\n");
29e57fab9 Maxime Ripard  2015-10-29  181  
b413dde1a Maxime Ripard  2019-02-26  182  	if (rgb->panel) {
b413dde1a Maxime Ripard  2019-02-26  183  		drm_panel_disable(rgb->panel);
b413dde1a Maxime Ripard  2019-02-26  184  		drm_panel_unprepare(rgb->panel);
4b3095025 Jonathan Liu   2016-08-30  185  	}
45e88f994 Maxime Ripard  2017-10-17  186  }
29e57fab9 Maxime Ripard  2015-10-29  187  
29e57fab9 Maxime Ripard  2015-10-29  188  static struct drm_encoder_helper_funcs sun4i_rgb_enc_helper_funcs = {
29e57fab9 Maxime Ripard  2015-10-29  189  	.disable	= sun4i_rgb_encoder_disable,
29e57fab9 Maxime Ripard  2015-10-29  190  	.enable		= sun4i_rgb_encoder_enable,
cde8b7548 Giulio Benetti 2018-03-13 @191  	.mode_valid	= sun4i_rgb_mode_valid,
29e57fab9 Maxime Ripard  2015-10-29  192  };
29e57fab9 Maxime Ripard  2015-10-29  193  

:::::: The code at line 191 was first introduced by commit
:::::: cde8b7548272640437d2fa691ae211f940066f6b drm/sun4i: move rgb mode_valid from connector to encoder

:::::: TO: Giulio Benetti <giulio.benetti@micronovasrl.com>
:::::: CC: Maxime Ripard <maxime.ripard@bootlin.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Paul Kocialkowski March 7, 2019, 1:36 p.m. UTC | #3 
Hi,

On Tue, 2019-02-26 at 15:25 +0100, Maxime Ripard wrote:
> The current code, since commit bb43d40d7c83 ("drm/sun4i: rgb: Validate the
> clock rate"), perform some validation on the pixel clock to filter out the
> EDID modes provided by monitors (through bridges) that we wouldn't be able
> to reach. For the usual modes, we're able to generate a perfect clock rate,
> so a strict check was enough.
> 
> However, this had the side effect of preventing displays that would work
> otherwise to operate properly, since we would pretty much never be able to
> generate an exact rate for those displays, even though we would fall within
> that panel tolerance.
> 
> This was also shown to happen for unusual modes exposed through EDIDs, for
> example on eDP panels.
> 
> We can work around this by simplifying a bit the problem: no panels we've
> encountered so far actually needed that check. All of them are tied to a
> particular board when it is produced, and made to work with the Allwinner
> BSP. That pretty much guarantees that we never have a pixel clock out of
> reach.
> 
> On the other hand, the EDIDs modes that needed to be validated have always
> been exposed through bridges.
> 
> Let's just use that metric to instead of validating all modes, only
> validate modes when we have a bridge attached. It should be good enough for
> now, while we still have room for improvements or refinements using the
> display_timings structure for example for panels.
> 
> We also add a tolerance for EDID-based modes instead of doing a strict
> check. This tolerance is of 0.5% which is the one advertised in the VESA
> DVT and CVT specs. If that needed to be extended in the future, we can add
> a custom module parameter to relax it a bit.
> 
> Fixes: bb43d40d7c83 ("drm/sun4i: rgb: Validate the clock rate")
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>

With the define fixed and given the comment below, this is:

Reviewed-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

> ---
>  drivers/gpu/drm/sun4i/sun4i_rgb.c | 37 ++++++++++++++++++++++++++++++--
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> index 893b6e6d4d85..05beefe93989 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
> @@ -53,6 +53,14 @@ static int sun4i_rgb_get_modes(struct drm_connector *connector)
>  	return drm_panel_get_modes(rgb->panel);
>  }
>  
> +/*
> + * VESA DMT defines a tolerance of 0.5% on the pixel clock, while the
> + * CVT spec reuses that tolerance in its examples, so it looks to be a
> + * good default tolerance for the EDID-based modes. Define it to 5 per
> + * mille to avoid floating point operations.
> + */
> +DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE	5

It could be nice to have some kind of unit made explicit in the define.
Maybe something like SUN4I_RGB_DOTCLOCK_TOLERANCE_PER_THOUSAND.

That's just a suggestion, feel free to take it or leave it :)

Cheers,

Paul

> +
>  static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
>  						 const struct drm_display_mode *mode)
>  {
> @@ -61,6 +69,7 @@ static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
>  	u32 hsync = mode->hsync_end - mode->hsync_start;
>  	u32 vsync = mode->vsync_end - mode->vsync_start;
>  	unsigned long long rate = mode->clock * 1000;
> +	unsigned long long lowest, highest;
>  	unsigned long long rounded_rate;
>  
>  	DRM_DEBUG_DRIVER("Validating modes...\n");
> @@ -93,15 +102,39 @@ static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
>  
>  	DRM_DEBUG_DRIVER("Vertical parameters OK\n");
>  
> +	/*
> +	 * TODO: We should use the struct display_timing if available
> +	 * and / or trying to stretch the timings within that
> +	 * tolerancy to take care of panels that we wouldn't be able
> +	 * to have a exact match for.
> +	 */
> +	if (rgb->panel) {
> +		DRM_DEBUG_DRIVER("RGB panel used, skipping clock rate checks");
> +		goto out;
> +	}
> +
> +	/*
> +	 * That shouldn't ever happen unless something is really wrong, but it
> +	 * doesn't harm to check.
> +	 */
> +	if (!rgb->bridge)
> +		goto out;
> +
>  	tcon->dclk_min_div = 6;
>  	tcon->dclk_max_div = 127;
>  	rounded_rate = clk_round_rate(tcon->dclk, rate);
> -	if (rounded_rate < rate)
> +
> +	lowest = rate * (1000 - SUN4I_RGB_DOTCLOCK_TOLERANCE);
> +	do_div(lowest, 1000);
> +	if (rounded_rate < lowest)
>  		return MODE_CLOCK_LOW;
>  
> -	if (rounded_rate > rate)
> +	highest = rate * (1000 + SUN4I_RGB_DOTCLOCK_TOLERANCE);
> +	do_div(highest, 1000);
> +	if (rounded_rate > highest)
>  		return MODE_CLOCK_HIGH;
>  
> +out:
>  	DRM_DEBUG_DRIVER("Clock rate OK\n");
>  
>  	return MODE_OK;
diff mbox series

Patch

diff --git a/drivers/gpu/drm/sun4i/sun4i_rgb.c b/drivers/gpu/drm/sun4i/sun4i_rgb.c
index 893b6e6d4d85..05beefe93989 100644
--- a/drivers/gpu/drm/sun4i/sun4i_rgb.c
+++ b/drivers/gpu/drm/sun4i/sun4i_rgb.c
@@ -53,6 +53,14 @@  static int sun4i_rgb_get_modes(struct drm_connector *connector)
 	return drm_panel_get_modes(rgb->panel);
 }
 
+/*
+ * VESA DMT defines a tolerance of 0.5% on the pixel clock, while the
+ * CVT spec reuses that tolerance in its examples, so it looks to be a
+ * good default tolerance for the EDID-based modes. Define it to 5 per
+ * mille to avoid floating point operations.
+ */
+DEFINE SUN4I_RGB_DOTCLOCK_TOLERANCE	5
+
 static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
 						 const struct drm_display_mode *mode)
 {
@@ -61,6 +69,7 @@  static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
 	u32 hsync = mode->hsync_end - mode->hsync_start;
 	u32 vsync = mode->vsync_end - mode->vsync_start;
 	unsigned long long rate = mode->clock * 1000;
+	unsigned long long lowest, highest;
 	unsigned long long rounded_rate;
 
 	DRM_DEBUG_DRIVER("Validating modes...\n");
@@ -93,15 +102,39 @@  static enum drm_mode_status sun4i_rgb_mode_valid(struct drm_encoder *crtc,
 
 	DRM_DEBUG_DRIVER("Vertical parameters OK\n");
 
+	/*
+	 * TODO: We should use the struct display_timing if available
+	 * and / or trying to stretch the timings within that
+	 * tolerancy to take care of panels that we wouldn't be able
+	 * to have a exact match for.
+	 */
+	if (rgb->panel) {
+		DRM_DEBUG_DRIVER("RGB panel used, skipping clock rate checks");
+		goto out;
+	}
+
+	/*
+	 * That shouldn't ever happen unless something is really wrong, but it
+	 * doesn't harm to check.
+	 */
+	if (!rgb->bridge)
+		goto out;
+
 	tcon->dclk_min_div = 6;
 	tcon->dclk_max_div = 127;
 	rounded_rate = clk_round_rate(tcon->dclk, rate);
-	if (rounded_rate < rate)
+
+	lowest = rate * (1000 - SUN4I_RGB_DOTCLOCK_TOLERANCE);
+	do_div(lowest, 1000);
+	if (rounded_rate < lowest)
 		return MODE_CLOCK_LOW;
 
-	if (rounded_rate > rate)
+	highest = rate * (1000 + SUN4I_RGB_DOTCLOCK_TOLERANCE);
+	do_div(highest, 1000);
+	if (rounded_rate > highest)
 		return MODE_CLOCK_HIGH;
 
+out:
 	DRM_DEBUG_DRIVER("Clock rate OK\n");
 
 	return MODE_OK;