| Message ID | 1431020329-11414-12-git-send-email-damien.lespiau@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2015-05-07 14:38 GMT-03:00 Damien Lespiau <damien.lespiau@intel.com>: > The orignal code started by storing the actual central frequency (in Hz, > using a uint64_t) in a uint32_t which codes for the register value. That > can't be right. > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index f3c5b49..a99fee1 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1117,8 +1117,6 @@ static void skl_wrpll_params_populate(struct skl_wrpll_params *params, > { > uint64_t dco_freq; > > - params->central_freq = central_freq; > - > switch (central_freq) { While at it, since it's related, can I ask you to add a default case to this switch statement? You know, just to be sure... It can be in a separate patch. And I guess the discussion of patch 9 will define how that default case will look like. With or without it: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > case 9600000000ULL: > params->central_freq = 0; > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, May 27, 2015 at 04:58:46PM -0300, Paulo Zanoni wrote: > 2015-05-07 14:38 GMT-03:00 Damien Lespiau <damien.lespiau@intel.com>: > > The orignal code started by storing the actual central frequency (in Hz, > > using a uint64_t) in a uint32_t which codes for the register value. That > > can't be right. > > > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index f3c5b49..a99fee1 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1117,8 +1117,6 @@ static void skl_wrpll_params_populate(struct skl_wrpll_params *params, > > { > > uint64_t dco_freq; > > > > - params->central_freq = central_freq; > > - > > switch (central_freq) { > > While at it, since it's related, can I ask you to add a default case > to this switch statement? You know, just to be sure... It can be in a > separate patch. And I guess the discussion of patch 9 will define how > that default case will look like. > > With or without it: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> Merged the reviewed patches to dinq, thanks. -Daniel > > > case 9600000000ULL: > > params->central_freq = 0; > > -- > > 2.1.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index f3c5b49..a99fee1 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1117,8 +1117,6 @@ static void skl_wrpll_params_populate(struct skl_wrpll_params *params, { uint64_t dco_freq; - params->central_freq = central_freq; - switch (central_freq) { case 9600000000ULL: params->central_freq = 0;
The orignal code started by storing the actual central frequency (in Hz, using a uint64_t) in a uint32_t which codes for the register value. That can't be right. Signed-off-by: Damien Lespiau <damien.lespiau@intel.com> --- drivers/gpu/drm/i915/intel_ddi.c | 2 -- 1 file changed, 2 deletions(-)