Re: [PATCH 2/2] drm/i915: Make sure we have enough memory bandwidth on ICL

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx