On Thu, Mar 21, 2019 at 09:34:00AM +0000, Lisovskiy, Stanislav wrote:
> On Wed, 2019-03-20 at 23:46 +0200, Ville Syrjala wrote:
> > +static void cnl_get_dram_timings(struct drm_i915_private *dev_priv,
> > + const struct dram_info *dram,
> > + struct intel_dram_timings *t)
> > +{
> > + u32 val;
> > +
> > + if (dram->channels & BIT(0)) {
> > + val = I915_READ(MCHBAR_CH0_CR_TC_PRE_0_0_0_MCHBAR);
> > + cnl_get_dram_ch_timings(t, 0, dram->type, val);
> > + } else if (dram->channels & BIT(1)) {
> > + val = I915_READ(MCHBAR_CH1_CR_TC_PRE_0_0_0_MCHBAR);
> > + cnl_get_dram_ch_timings(t, 1, dram->type, val);
> > + }
> > +}
>
> So if have now dram->channels as a bit mask, can't it be so
> that we have both enabled and what will happen then?
>
> This code will always use channel 0 timings in that case as I
> understand, is it expected to work this way?
It seemed to be fine when I tested it (ie. it always picked the slower
timings when I had mixed DIMMs in the system). But I think I have to
retest that one more time to make sure SAGV wasn't playing tricks on me.
>
> Also, if it is always either one or another, then we probably don't
> need "else if" here.
>
> Best Regards,
>
> Lisovskiy Stanislav
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
