From: Ville Syrjälä ville.syrjala@linux.intel.com
No need to store the return value in a variable since we don't have to do any unwinding.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_modes.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 5a8033fda4e3..4157250140b0 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1596,12 +1596,8 @@ int drm_mode_convert_umode(struct drm_device *dev, struct drm_display_mode *out, const struct drm_mode_modeinfo *in) { - int ret = -EINVAL; - - if (in->clock > INT_MAX || in->vrefresh > INT_MAX) { - ret = -ERANGE; - goto out; - } + if (in->clock > INT_MAX || in->vrefresh > INT_MAX) + return -ERANGE;
out->clock = in->clock; out->hdisplay = in->hdisplay; @@ -1622,14 +1618,11 @@ int drm_mode_convert_umode(struct drm_device *dev,
out->status = drm_mode_validate_driver(dev, out); if (out->status != MODE_OK) - goto out; + return -EINVAL;
drm_mode_set_crtcinfo(out, CRTC_INTERLACE_HALVE_V);
- ret = 0; - -out: - return ret; + return 0; }
/**
From: Ville Syrjälä ville.syrjala@linux.intel.com
Do the refresh rate calculation with a single division. This gives us slightly more accurate results, especially for interlaced since we don't just double the final truncated result.
We do lose one bit compared to the old way, so with an interlaced mode the new code can only handle ~2GHz instead of the ~4GHz the old code handeled.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_modes.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 4157250140b0..f6b7c0e36a1a 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -773,24 +773,23 @@ EXPORT_SYMBOL(drm_mode_hsync); int drm_mode_vrefresh(const struct drm_display_mode *mode) { int refresh = 0; - unsigned int calc_val;
if (mode->vrefresh > 0) refresh = mode->vrefresh; else if (mode->htotal > 0 && mode->vtotal > 0) { - int vtotal; - vtotal = mode->vtotal; - /* work out vrefresh the value will be x1000 */ - calc_val = (mode->clock * 1000); - calc_val /= mode->htotal; - refresh = (calc_val + vtotal / 2) / vtotal; + unsigned int num, den; + + num = mode->clock * 1000; + den = mode->htotal * mode->vtotal;
if (mode->flags & DRM_MODE_FLAG_INTERLACE) - refresh *= 2; + num *= 2; if (mode->flags & DRM_MODE_FLAG_DBLSCAN) - refresh /= 2; + den *= 2; if (mode->vscan > 1) - refresh /= mode->vscan; + den *= mode->vscan; + + refresh = DIV_ROUND_CLOSEST(num, den); } return refresh; }
On Tue, Mar 13, 2018 at 05:07:58PM +0200, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Do the refresh rate calculation with a single division. This gives us slightly more accurate results, especially for interlaced since we don't just double the final truncated result.
We do lose one bit compared to the old way, so with an interlaced mode the new code can only handle ~2GHz instead of the ~4GHz the old code handeled.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
I kinda want special integers here that Oops on overflow, I thought they're coming. That aside:
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_modes.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 4157250140b0..f6b7c0e36a1a 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -773,24 +773,23 @@ EXPORT_SYMBOL(drm_mode_hsync); int drm_mode_vrefresh(const struct drm_display_mode *mode) { int refresh = 0;
unsigned int calc_val;
if (mode->vrefresh > 0) refresh = mode->vrefresh; else if (mode->htotal > 0 && mode->vtotal > 0) {
int vtotal;vtotal = mode->vtotal;/* work out vrefresh the value will be x1000 */calc_val = (mode->clock * 1000);calc_val /= mode->htotal;refresh = (calc_val + vtotal / 2) / vtotal;
unsigned int num, den;num = mode->clock * 1000;den = mode->htotal * mode->vtotal;if (mode->flags & DRM_MODE_FLAG_INTERLACE)
refresh *= 2;
num *= 2;
refresh /= 2;
den *= 2;
refresh /= mode->vscan;
den *= mode->vscan;refresh = DIV_ROUND_CLOSEST(num, den);}
2.16.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Mar 14, 2018 at 02:56:28PM +0100, Daniel Vetter wrote:
On Tue, Mar 13, 2018 at 05:07:58PM +0200, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Do the refresh rate calculation with a single division. This gives us slightly more accurate results, especially for interlaced since we don't just double the final truncated result.
We do lose one bit compared to the old way, so with an interlaced mode the new code can only handle ~2GHz instead of the ~4GHz the old code handeled.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
I kinda want special integers here that Oops on overflow, I thought they're coming.
Would be nice indeed.
That aside:
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_modes.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 4157250140b0..f6b7c0e36a1a 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -773,24 +773,23 @@ EXPORT_SYMBOL(drm_mode_hsync); int drm_mode_vrefresh(const struct drm_display_mode *mode) { int refresh = 0;
unsigned int calc_val;
if (mode->vrefresh > 0) refresh = mode->vrefresh; else if (mode->htotal > 0 && mode->vtotal > 0) {
int vtotal;vtotal = mode->vtotal;/* work out vrefresh the value will be x1000 */calc_val = (mode->clock * 1000);calc_val /= mode->htotal;refresh = (calc_val + vtotal / 2) / vtotal;
unsigned int num, den;num = mode->clock * 1000;den = mode->htotal * mode->vtotal;if (mode->flags & DRM_MODE_FLAG_INTERLACE)
refresh *= 2;
num *= 2;
refresh /= 2;
den *= 2;
refresh /= mode->vscan;
den *= mode->vscan;refresh = DIV_ROUND_CLOSEST(num, den);}
2.16.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Wed, Mar 14, 2018 at 02:56:28PM +0100, Daniel Vetter wrote:
On Tue, Mar 13, 2018 at 05:07:58PM +0200, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Do the refresh rate calculation with a single division. This gives us slightly more accurate results, especially for interlaced since we don't just double the final truncated result.
We do lose one bit compared to the old way, so with an interlaced mode the new code can only handle ~2GHz instead of the ~4GHz the old code handeled.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
I kinda want special integers here that Oops on overflow, I thought they're coming. That aside:
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Thanks. Patches 1-2 pushed to drm-misc-next.
drivers/gpu/drm/drm_modes.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 4157250140b0..f6b7c0e36a1a 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -773,24 +773,23 @@ EXPORT_SYMBOL(drm_mode_hsync); int drm_mode_vrefresh(const struct drm_display_mode *mode) { int refresh = 0;
unsigned int calc_val;
if (mode->vrefresh > 0) refresh = mode->vrefresh; else if (mode->htotal > 0 && mode->vtotal > 0) {
int vtotal;vtotal = mode->vtotal;/* work out vrefresh the value will be x1000 */calc_val = (mode->clock * 1000);calc_val /= mode->htotal;refresh = (calc_val + vtotal / 2) / vtotal;
unsigned int num, den;num = mode->clock * 1000;den = mode->htotal * mode->vtotal;if (mode->flags & DRM_MODE_FLAG_INTERLACE)
refresh *= 2;
num *= 2;
refresh /= 2;
den *= 2;
refresh /= mode->vscan;
den *= mode->vscan;refresh = DIV_ROUND_CLOSEST(num, den);}
2.16.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
From: Ville Syrjälä ville.syrjala@linux.intel.com
Ignore the vrefresh in the mode the user passed in and instead calculate the value based on the actual timings. This way we can actually trust mode->vrefresh to some degree.
Or should we compare the user's idea of vrefresh with the one we get from the timings and return an error if they differ? We can't really be sure what the user is asking in that case.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_modes.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index f6b7c0e36a1a..021526ec6fa0 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1609,7 +1609,11 @@ int drm_mode_convert_umode(struct drm_device *dev, out->vsync_end = in->vsync_end; out->vtotal = in->vtotal; out->vscan = in->vscan; - out->vrefresh = in->vrefresh; + /* + * Ignore what the user is saying here and instead + * calculate vrefresh based on the actual timings. + */ + out->vrefresh = 0; out->flags = in->flags; out->type = in->type; strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN); @@ -1619,6 +1623,8 @@ int drm_mode_convert_umode(struct drm_device *dev, if (out->status != MODE_OK) return -EINVAL;
+ out->vrefresh = drm_mode_vrefresh(out); + drm_mode_set_crtcinfo(out, CRTC_INTERLACE_HALVE_V);
return 0;
Op 13-03-18 om 16:07 schreef Ville Syrjala:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Ignore the vrefresh in the mode the user passed in and instead calculate the value based on the actual timings. This way we can actually trust mode->vrefresh to some degree.
Or should we compare the user's idea of vrefresh with the one we get from the timings and return an error if they differ? We can't really be sure what the user is asking in that case.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_modes.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index f6b7c0e36a1a..021526ec6fa0 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1609,7 +1609,11 @@ int drm_mode_convert_umode(struct drm_device *dev, out->vsync_end = in->vsync_end; out->vtotal = in->vtotal; out->vscan = in->vscan;
- out->vrefresh = in->vrefresh;
/** Ignore what the user is saying here and instead* calculate vrefresh based on the actual timings.*/- out->vrefresh = 0; out->flags = in->flags; out->type = in->type; strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
@@ -1619,6 +1623,8 @@ int drm_mode_convert_umode(struct drm_device *dev, if (out->status != MODE_OK) return -EINVAL;
out->vrefresh = drm_mode_vrefresh(out);
drm_mode_set_crtcinfo(out, CRTC_INTERLACE_HALVE_V);
return 0;
Could we also get this with dim fixes tags dc911f5bd8aa, so we can backport the alt mode handling patch?
And update the documentation about vrefresh, that you can retrieve it with drm_mode_vrefresh?
~Maarten
On Tue, Mar 13, 2018 at 08:04:03PM +0100, Maarten Lankhorst wrote:
Op 13-03-18 om 16:07 schreef Ville Syrjala:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Ignore the vrefresh in the mode the user passed in and instead calculate the value based on the actual timings. This way we can actually trust mode->vrefresh to some degree.
Or should we compare the user's idea of vrefresh with the one we get from the timings and return an error if they differ? We can't really be sure what the user is asking in that case.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/drm_modes.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index f6b7c0e36a1a..021526ec6fa0 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1609,7 +1609,11 @@ int drm_mode_convert_umode(struct drm_device *dev, out->vsync_end = in->vsync_end; out->vtotal = in->vtotal; out->vscan = in->vscan;
- out->vrefresh = in->vrefresh;
/** Ignore what the user is saying here and instead* calculate vrefresh based on the actual timings.*/- out->vrefresh = 0; out->flags = in->flags; out->type = in->type; strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
@@ -1619,6 +1623,8 @@ int drm_mode_convert_umode(struct drm_device *dev, if (out->status != MODE_OK) return -EINVAL;
out->vrefresh = drm_mode_vrefresh(out);
drm_mode_set_crtcinfo(out, CRTC_INTERLACE_HALVE_V);
return 0;
Could we also get this with dim fixes tags dc911f5bd8aa, so we can backport the alt mode handling patch?
Do we want/need to backport it actually?
And update the documentation about vrefresh, that you can retrieve it with drm_mode_vrefresh?
The whole "return cached value if present, otherwise calculate but don't update the cached value" apporach always seemed off to me. Might be a good idea to change it somehow. Maybe just always calculate it, or do the cached value updates in sensible places so that you only have to calculate once. But I haven't actually checked how much work that would entail.
On Tue, Mar 13, 2018 at 05:07:57PM +0200, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
No need to store the return value in a variable since we don't have to do any unwinding.
Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
drivers/gpu/drm/drm_modes.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 5a8033fda4e3..4157250140b0 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1596,12 +1596,8 @@ int drm_mode_convert_umode(struct drm_device *dev, struct drm_display_mode *out, const struct drm_mode_modeinfo *in) {
- int ret = -EINVAL;
- if (in->clock > INT_MAX || in->vrefresh > INT_MAX) {
ret = -ERANGE;goto out;- }
if (in->clock > INT_MAX || in->vrefresh > INT_MAX)
return -ERANGE;out->clock = in->clock; out->hdisplay = in->hdisplay;
@@ -1622,14 +1618,11 @@ int drm_mode_convert_umode(struct drm_device *dev,
out->status = drm_mode_validate_driver(dev, out); if (out->status != MODE_OK)
goto out;
return -EINVAL;drm_mode_set_crtcinfo(out, CRTC_INTERLACE_HALVE_V);
- ret = 0;
-out:
- return ret;
- return 0;
}
/**
2.16.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Maarten Lankhorst -
Ville Syrjala -
Ville Syrjälä