| Message ID | 1409830075-11139-58-git-send-email-damien.lespiau@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 04 Sep 2014, Damien Lespiau <damien.lespiau@intel.com> wrote: > The new version of the macro does a few things better: > - protect the arguments, > - only evaluate the arguments once, > - check that the arguments are of the same type, > > Change LC_FREQ_2K to be a unsigned 64bit constant and removed the '()' > from the caller as a result. > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> Reviewed-by: Jani Nikula <jani.nikula@intel.com> Can be picked up for dinq separate from this series, right? > --- > drivers/gpu/drm/i915/intel_ddi.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 64eda53..ba1103f 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -460,7 +460,7 @@ intel_ddi_get_crtc_encoder(struct drm_crtc *crtc) > } > > #define LC_FREQ 2700 > -#define LC_FREQ_2K (LC_FREQ * 2000) > +#define LC_FREQ_2K U64_C(LC_FREQ * 2000) > > #define P_MIN 2 > #define P_MAX 64 > @@ -472,7 +472,11 @@ intel_ddi_get_crtc_encoder(struct drm_crtc *crtc) > #define VCO_MIN 2400 > #define VCO_MAX 4800 > > -#define ABS_DIFF(a, b) ((a > b) ? (a - b) : (b - a)) > +#define abs_diff(a, b) ({ \ > + typeof(a) __a = (a); \ > + typeof(b) __b = (b); \ > + (void) (&__a == &__b); \ > + __a > __b ? (__a - __b) : (__b - __a); }) > > struct wrpll_rnp { > unsigned p, n2, r2; > @@ -582,9 +586,9 @@ static void wrpll_update_rnp(uint64_t freq2k, unsigned budget, > */ > a = freq2k * budget * p * r2; > b = freq2k * budget * best->p * best->r2; > - diff = ABS_DIFF((freq2k * p * r2), (LC_FREQ_2K * n2)); > - diff_best = ABS_DIFF((freq2k * best->p * best->r2), > - (LC_FREQ_2K * best->n2)); > + diff = abs_diff(freq2k * p * r2, LC_FREQ_2K * n2); > + diff_best = abs_diff(freq2k * best->p * best->r2, > + LC_FREQ_2K * best->n2); > c = 1000000 * diff; > d = 1000000 * diff_best; > > -- > 1.8.3.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, Sep 04, 2014 at 03:11:24PM +0300, Jani Nikula wrote: > On Thu, 04 Sep 2014, Damien Lespiau <damien.lespiau@intel.com> wrote: > > The new version of the macro does a few things better: > > - protect the arguments, > > - only evaluate the arguments once, > > - check that the arguments are of the same type, > > > > Change LC_FREQ_2K to be a unsigned 64bit constant and removed the '()' > > from the caller as a result. > > > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > Can be picked up for dinq separate from this series, right? Yes, it can indeed.
On Thu, Sep 04, 2014 at 01:32:24PM +0100, Damien Lespiau wrote: > On Thu, Sep 04, 2014 at 03:11:24PM +0300, Jani Nikula wrote: > > On Thu, 04 Sep 2014, Damien Lespiau <damien.lespiau@intel.com> wrote: > > > The new version of the macro does a few things better: > > > - protect the arguments, > > > - only evaluate the arguments once, > > > - check that the arguments are of the same type, > > > > > > Change LC_FREQ_2K to be a unsigned 64bit constant and removed the '()' > > > from the caller as a result. > > > > > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > > > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > > > Can be picked up for dinq separate from this series, right? > > Yes, it can indeed. s/can/must/ If the platform enabling changes shared code, that part must go in through the normal merge process and has the same merge window cutoff deadlines like everything else. Queued for -next, thanks for the patch. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 64eda53..ba1103f 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -460,7 +460,7 @@ intel_ddi_get_crtc_encoder(struct drm_crtc *crtc) } #define LC_FREQ 2700 -#define LC_FREQ_2K (LC_FREQ * 2000) +#define LC_FREQ_2K U64_C(LC_FREQ * 2000) #define P_MIN 2 #define P_MAX 64 @@ -472,7 +472,11 @@ intel_ddi_get_crtc_encoder(struct drm_crtc *crtc) #define VCO_MIN 2400 #define VCO_MAX 4800 -#define ABS_DIFF(a, b) ((a > b) ? (a - b) : (b - a)) +#define abs_diff(a, b) ({ \ + typeof(a) __a = (a); \ + typeof(b) __b = (b); \ + (void) (&__a == &__b); \ + __a > __b ? (__a - __b) : (__b - __a); }) struct wrpll_rnp { unsigned p, n2, r2; @@ -582,9 +586,9 @@ static void wrpll_update_rnp(uint64_t freq2k, unsigned budget, */ a = freq2k * budget * p * r2; b = freq2k * budget * best->p * best->r2; - diff = ABS_DIFF((freq2k * p * r2), (LC_FREQ_2K * n2)); - diff_best = ABS_DIFF((freq2k * best->p * best->r2), - (LC_FREQ_2K * best->n2)); + diff = abs_diff(freq2k * p * r2, LC_FREQ_2K * n2); + diff_best = abs_diff(freq2k * best->p * best->r2, + LC_FREQ_2K * best->n2); c = 1000000 * diff; d = 1000000 * diff_best;
The new version of the macro does a few things better: - protect the arguments, - only evaluate the arguments once, - check that the arguments are of the same type, Change LC_FREQ_2K to be a unsigned 64bit constant and removed the '()' from the caller as a result. Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> --- drivers/gpu/drm/i915/intel_ddi.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)