Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: Make sure we have enough memory bandwidth on ICL
Nit: Commit message V4 and Patch Subject V3
Acked-by: Clint Taylor
-Clint
On 5/3/19 12:08 PM, Ville Syrjala wrote:
From: Ville Syrjälä
ICL has so many planes that it can easily exceed the maximum
effective memory bandwidth of the system. We must therefore check
that we don't exceed that limit.
The algorithm is very magic number heavy and lacks sufficient
explanation for now. We also have no sane way to query the
memory clock and timings, so we must rely on a combination of
raw readout from the memory controller and hardcoded assumptions.
The memory controller values obviously change as the system
jumps between the different SAGV points, so we try to stabilize
it first by disabling SAGV for the duration of the readout.
The utilized bandwidth is tracked via a device wide atomic
private object. That is actually not robust because we can't
afford to enforce strict global ordering between the pipes.
Thus I think I'll need to change this to simply chop up the
available bandwidth between all the active pipes. Each pipe
can then do whatever it wants as long as it doesn't exceed
its budget. That scheme will also require that we assume that
any number of planes could be active at any time.
TODO: make it robust and deal with all the open questions
v2: Sleep longer after disabling SAGV
v3: Poll for the dclk to get raised (seen it take 250ms!)
If the system has 2133MT/s memory then we pointlessly
wait one full second :(
v4: Use the new pcode interface to get the qgv points rather
that using hardcoded numbers
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.c | 229 ++
drivers/gpu/drm/i915/i915_drv.h | 10 +
drivers/gpu/drm/i915/i915_reg.h | 3 +
drivers/gpu/drm/i915/intel_atomic_plane.c | 20 ++
drivers/gpu/drm/i915/intel_atomic_plane.h | 2 +
drivers/gpu/drm/i915/intel_bw.c | 181 +
drivers/gpu/drm/i915/intel_bw.h | 46 +
drivers/gpu/drm/i915/intel_display.c | 40 +++-
drivers/gpu/drm/i915/intel_drv.h | 2 +
10 files changed, 533 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/i915/intel_bw.c
create mode 100644 drivers/gpu/drm/i915/intel_bw.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 68106fe35a04..139a0fc19390 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -138,6 +138,7 @@ i915-y += intel_audio.o \
intel_atomic.o \
intel_atomic_plane.o \
intel_bios.o \
+ intel_bw.o \
intel_cdclk.o \
intel_color.o \
intel_combo_phy.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5ed864752c7b..b7fa7b51c2e2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -70,6 +70,7 @@
#include "intel_overlay.h"
#include "intel_pipe_crc.h"
#include "intel_pm.h"
+#include "intel_sideband.h"
#include "intel_sprite.h"
#include "intel_uc.h"
@@ -1435,6 +1436,232 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv)
return 0;
}
+struct intel_qgv_point {
+ u16 dclk, t_rp, t_rdpre, t_rc, t_ras, t_rcd;
+};
+
+struct intel_sagv_info {
+ struct intel_qgv_point points[3];
+ u8 num_points;
+ u8 num_channels;
+ u8 t_bl;
+ enum intel_dram_type dram_type;
+};
+
+static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv,
+ struct intel_sagv_info *si)
+{
+ u32 val = 0;
+ int ret;
+
+ ret = sandybridge_pcode_read(dev_priv,
+ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
+ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
+&val, NULL);
+ if (ret)
+ return ret;
+
+ switch (val & 0xf) {
+ case 0:
+ si->dram_type = INTEL_DRAM_DDR4;
+ break;
+ case 1:
+ si->dram_type = INTEL_DRAM_DDR3;
+ break;
+ case 2:
+ si->dram_type = INTEL_DRAM_LPDDR3;
+ break;
+ case 3:
+ si->dram_type = INTEL_DRAM_LPDDR3;
+ break;
+ default:
+ MISSING_CASE(val & 0xf);
+ break;
+ }
+
+ si->num_channels = (val & 0xf0) >> 4;
+ si->num_points = (val & 0xf00) >> 8;
+
+ si->t_bl = si->dram_type == INTEL_DRAM_DDR4 ? 4 : 8;
+
+ return 0;
+}
+
+static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
+struct intel_qgv_point *sp,
+int point)
+{
+ u32 val = 0, val2;
+ int ret;
+
+ ret = sandybridge_pcode_read(dev_priv,
+ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
+
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: Make sure we have enough memory bandwidth on ICL
On Mon, May 13, 2019 at 05:13:10PM +0300, Ville Syrjälä wrote:
> On Fri, May 10, 2019 at 05:42:09PM -0700, Matt Roper wrote:
> > On Fri, May 03, 2019 at 10:08:31PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä
> > >
> > > ICL has so many planes that it can easily exceed the maximum
> > > effective memory bandwidth of the system. We must therefore check
> > > that we don't exceed that limit.
> > >
> > > The algorithm is very magic number heavy and lacks sufficient
> > > explanation for now. We also have no sane way to query the
> > > memory clock and timings, so we must rely on a combination of
> > > raw readout from the memory controller and hardcoded assumptions.
> > > The memory controller values obviously change as the system
> > > jumps between the different SAGV points, so we try to stabilize
> > > it first by disabling SAGV for the duration of the readout.
> > >
> > > The utilized bandwidth is tracked via a device wide atomic
> > > private object. That is actually not robust because we can't
> > > afford to enforce strict global ordering between the pipes.
> > > Thus I think I'll need to change this to simply chop up the
> > > available bandwidth between all the active pipes. Each pipe
> > > can then do whatever it wants as long as it doesn't exceed
> > > its budget. That scheme will also require that we assume that
> > > any number of planes could be active at any time.
> > >
> > > TODO: make it robust and deal with all the open questions
> > >
> > > v2: Sleep longer after disabling SAGV
> > > v3: Poll for the dclk to get raised (seen it take 250ms!)
> > > If the system has 2133MT/s memory then we pointlessly
> > > wait one full second :(
> > > v4: Use the new pcode interface to get the qgv points rather
> > > that using hardcoded numbers
> > >
> > > Signed-off-by: Ville Syrjälä
> > > ---
> > > drivers/gpu/drm/i915/Makefile | 1 +
> > > drivers/gpu/drm/i915/i915_drv.c | 229 ++
> > > drivers/gpu/drm/i915/i915_drv.h | 10 +
> > > drivers/gpu/drm/i915/i915_reg.h | 3 +
> > > drivers/gpu/drm/i915/intel_atomic_plane.c | 20 ++
> > > drivers/gpu/drm/i915/intel_atomic_plane.h | 2 +
> > > drivers/gpu/drm/i915/intel_bw.c | 181 +
> > > drivers/gpu/drm/i915/intel_bw.h | 46 +
> > > drivers/gpu/drm/i915/intel_display.c | 40 +++-
> > > drivers/gpu/drm/i915/intel_drv.h | 2 +
> > > 10 files changed, 533 insertions(+), 1 deletion(-)
> > > create mode 100644 drivers/gpu/drm/i915/intel_bw.c
> > > create mode 100644 drivers/gpu/drm/i915/intel_bw.h
> > >
> > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > > index 68106fe35a04..139a0fc19390 100644
> > > --- a/drivers/gpu/drm/i915/Makefile
> > > +++ b/drivers/gpu/drm/i915/Makefile
> > > @@ -138,6 +138,7 @@ i915-y += intel_audio.o \
> > > intel_atomic.o \
> > > intel_atomic_plane.o \
> > > intel_bios.o \
> > > + intel_bw.o \
> > > intel_cdclk.o \
> > > intel_color.o \
> > > intel_combo_phy.o \
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index 5ed864752c7b..b7fa7b51c2e2 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -70,6 +70,7 @@
> > > #include "intel_overlay.h"
> > > #include "intel_pipe_crc.h"
> > > #include "intel_pm.h"
> > > +#include "intel_sideband.h"
> > > #include "intel_sprite.h"
> > > #include "intel_uc.h"
> > >
> > > @@ -1435,6 +1436,232 @@ bxt_get_dram_info(struct drm_i915_private
> > > *dev_priv)
> > > return 0;
> > > }
> > >
> > > +struct intel_qgv_point {
> > > + u16 dclk, t_rp, t_rdpre, t_rc, t_ras, t_rcd;
> > > +};
> > > +
> > > +struct intel_sagv_info {
> > > + struct intel_qgv_point points[3];
> > > + u8 num_points;
> > > + u8 num_channels;
> > > + u8 t_bl;
> > > + enum intel_dram_type dram_type;
> > > +};
> > > +
> > > +static int icl_pcode_read_mem_global_info(struct drm_i915_private
> > > *dev_priv,
> > > + struct intel_sagv_info *si)
> > > +{
> > > + u32 val = 0;
> > > + int ret;
> > > +
> > > + ret = sandybridge_pcode_read(dev_priv,
> > > + ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> > > + ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
> > > + &val, NULL);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + switch (val & 0xf) {
> > > + case 0:
> > > + si->dram_type = INTEL_DRAM_DDR4;
> > > + break;
> > > + case 1:
> > > + si->dram_type = INTEL_DRAM_DDR3;
> > > + break;
> > > + case 2:
> > > + si->dram_type = INTEL_DRAM_LPDDR3;
> > > + break;
> > > + case 3:
> > > + si->dram_type = INTEL_DRAM_LPDDR3;
> > > + break;
> > > + default:
> > > + MISSING_CASE(val & 0xf);
> > > + break;
> > > + }
> > > +
> > > + si->num_channels
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: Make sure we have enough memory bandwidth on ICL
On Mon, 2019-05-13 at 17:16 +0300, Ville Syrjälä wrote:
> On Wed, May 08, 2019 at 09:05:06PM +, Sripada, Radhakrishna
> wrote:
> > On Fri, 2019-05-03 at 22:08 +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä
> > >
> > > ICL has so many planes that it can easily exceed the maximum
> > > effective memory bandwidth of the system. We must therefore check
> > > that we don't exceed that limit.
> > >
> > > The algorithm is very magic number heavy and lacks sufficient
> > > explanation for now. We also have no sane way to query the
> > > memory clock and timings, so we must rely on a combination of
> > > raw readout from the memory controller and hardcoded assumptions.
> > > The memory controller values obviously change as the system
> > > jumps between the different SAGV points, so we try to stabilize
> > > it first by disabling SAGV for the duration of the readout.
> > >
> > > The utilized bandwidth is tracked via a device wide atomic
> > > private object. That is actually not robust because we can't
> > > afford to enforce strict global ordering between the pipes.
> > > Thus I think I'll need to change this to simply chop up the
> > > available bandwidth between all the active pipes. Each pipe
> > > can then do whatever it wants as long as it doesn't exceed
> > > its budget. That scheme will also require that we assume that
> > > any number of planes could be active at any time.
> > >
> > > TODO: make it robust and deal with all the open questions
> > >
> > > v2: Sleep longer after disabling SAGV
> > > v3: Poll for the dclk to get raised (seen it take 250ms!)
> > > If the system has 2133MT/s memory then we pointlessly
> > > wait one full second :(
> > > v4: Use the new pcode interface to get the qgv points rather
> > > that using hardcoded numbers
> > >
> > > Signed-off-by: Ville Syrjälä
> > > ---
> > > drivers/gpu/drm/i915/Makefile | 1 +
> > > drivers/gpu/drm/i915/i915_drv.c | 229
> > > ++
> > > drivers/gpu/drm/i915/i915_drv.h | 10 +
> > > drivers/gpu/drm/i915/i915_reg.h | 3 +
> > > drivers/gpu/drm/i915/intel_atomic_plane.c | 20 ++
> > > drivers/gpu/drm/i915/intel_atomic_plane.h | 2 +
> > > drivers/gpu/drm/i915/intel_bw.c | 181
> > > +
> > > drivers/gpu/drm/i915/intel_bw.h | 46 +
> > > drivers/gpu/drm/i915/intel_display.c | 40 +++-
> > > drivers/gpu/drm/i915/intel_drv.h | 2 +
> > > 10 files changed, 533 insertions(+), 1 deletion(-)
> > > create mode 100644 drivers/gpu/drm/i915/intel_bw.c
> > > create mode 100644 drivers/gpu/drm/i915/intel_bw.h
> > >
> > > diff --git a/drivers/gpu/drm/i915/Makefile
> > > b/drivers/gpu/drm/i915/Makefile
> > > index 68106fe35a04..139a0fc19390 100644
> > > --- a/drivers/gpu/drm/i915/Makefile
> > > +++ b/drivers/gpu/drm/i915/Makefile
> > > @@ -138,6 +138,7 @@ i915-y += intel_audio.o \
> > > intel_atomic.o \
> > > intel_atomic_plane.o \
> > > intel_bios.o \
> > > + intel_bw.o \
> > > intel_cdclk.o \
> > > intel_color.o \
> > > intel_combo_phy.o \
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index 5ed864752c7b..b7fa7b51c2e2 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -70,6 +70,7 @@
> > > #include "intel_overlay.h"
> > > #include "intel_pipe_crc.h"
> > > #include "intel_pm.h"
> > > +#include "intel_sideband.h"
> > > #include "intel_sprite.h"
> > > #include "intel_uc.h"
> > >
> > > @@ -1435,6 +1436,232 @@ bxt_get_dram_info(struct drm_i915_private
> > > *dev_priv)
> > > return 0;
> > > }
> > >
> > > +struct intel_qgv_point {
> > > + u16 dclk, t_rp, t_rdpre, t_rc, t_ras, t_rcd;
> > > +};
> > > +
> > > +struct intel_sagv_info {
> > > + struct intel_qgv_point points[3];
> > > + u8 num_points;
> > > + u8 num_channels;
> > > + u8 t_bl;
> > > + enum intel_dram_type dram_type;
> > > +};
> > > +
> > > +static int icl_pcode_read_mem_global_info(struct
> > > drm_i915_private
> > > *dev_priv,
> > > + struct intel_sagv_info *si)
> > > +{
> > > + u32 val = 0;
> > > + int ret;
> > > +
> > > + ret = sandybridge_pcode_read(dev_priv,
> > > + ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> > > + ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
> > > + &val, NULL);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + switch (val & 0xf) {
> > > + case 0:
> > > + si->dram_type = INTEL_DRAM_DDR4;
> > > + break;
> > > + case 1:
> > > + si->dram_type = INTEL_DRAM_DDR3;
> > > + break;
> > > + case 2:
> > > + si->dram_type = INTEL_DRAM_LPDDR3;
> > > + break;
> > > + case 3:
> > > + si->dram_type = INTEL_DRAM_LPDDR3;
> > > + break;
> > > + default:
> > > + MISSING_CASE(val & 0xf);
> > > + break;
> > > + }
> > > +
> > >
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: Make sure we have enough memory bandwidth on ICL
On Wed, May 08, 2019 at 09:05:06PM +, Sripada, Radhakrishna wrote:
> On Fri, 2019-05-03 at 22:08 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > ICL has so many planes that it can easily exceed the maximum
> > effective memory bandwidth of the system. We must therefore check
> > that we don't exceed that limit.
> >
> > The algorithm is very magic number heavy and lacks sufficient
> > explanation for now. We also have no sane way to query the
> > memory clock and timings, so we must rely on a combination of
> > raw readout from the memory controller and hardcoded assumptions.
> > The memory controller values obviously change as the system
> > jumps between the different SAGV points, so we try to stabilize
> > it first by disabling SAGV for the duration of the readout.
> >
> > The utilized bandwidth is tracked via a device wide atomic
> > private object. That is actually not robust because we can't
> > afford to enforce strict global ordering between the pipes.
> > Thus I think I'll need to change this to simply chop up the
> > available bandwidth between all the active pipes. Each pipe
> > can then do whatever it wants as long as it doesn't exceed
> > its budget. That scheme will also require that we assume that
> > any number of planes could be active at any time.
> >
> > TODO: make it robust and deal with all the open questions
> >
> > v2: Sleep longer after disabling SAGV
> > v3: Poll for the dclk to get raised (seen it take 250ms!)
> > If the system has 2133MT/s memory then we pointlessly
> > wait one full second :(
> > v4: Use the new pcode interface to get the qgv points rather
> > that using hardcoded numbers
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/Makefile | 1 +
> > drivers/gpu/drm/i915/i915_drv.c | 229
> > ++
> > drivers/gpu/drm/i915/i915_drv.h | 10 +
> > drivers/gpu/drm/i915/i915_reg.h | 3 +
> > drivers/gpu/drm/i915/intel_atomic_plane.c | 20 ++
> > drivers/gpu/drm/i915/intel_atomic_plane.h | 2 +
> > drivers/gpu/drm/i915/intel_bw.c | 181 +
> > drivers/gpu/drm/i915/intel_bw.h | 46 +
> > drivers/gpu/drm/i915/intel_display.c | 40 +++-
> > drivers/gpu/drm/i915/intel_drv.h | 2 +
> > 10 files changed, 533 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/gpu/drm/i915/intel_bw.c
> > create mode 100644 drivers/gpu/drm/i915/intel_bw.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile
> > b/drivers/gpu/drm/i915/Makefile
> > index 68106fe35a04..139a0fc19390 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -138,6 +138,7 @@ i915-y += intel_audio.o \
> > intel_atomic.o \
> > intel_atomic_plane.o \
> > intel_bios.o \
> > + intel_bw.o \
> > intel_cdclk.o \
> > intel_color.o \
> > intel_combo_phy.o \
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 5ed864752c7b..b7fa7b51c2e2 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -70,6 +70,7 @@
> > #include "intel_overlay.h"
> > #include "intel_pipe_crc.h"
> > #include "intel_pm.h"
> > +#include "intel_sideband.h"
> > #include "intel_sprite.h"
> > #include "intel_uc.h"
> >
> > @@ -1435,6 +1436,232 @@ bxt_get_dram_info(struct drm_i915_private
> > *dev_priv)
> > return 0;
> > }
> >
> > +struct intel_qgv_point {
> > + u16 dclk, t_rp, t_rdpre, t_rc, t_ras, t_rcd;
> > +};
> > +
> > +struct intel_sagv_info {
> > + struct intel_qgv_point points[3];
> > + u8 num_points;
> > + u8 num_channels;
> > + u8 t_bl;
> > + enum intel_dram_type dram_type;
> > +};
> > +
> > +static int icl_pcode_read_mem_global_info(struct drm_i915_private
> > *dev_priv,
> > + struct intel_sagv_info *si)
> > +{
> > + u32 val = 0;
> > + int ret;
> > +
> > + ret = sandybridge_pcode_read(dev_priv,
> > +ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> > +ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
> > +&val, NULL);
> > + if (ret)
> > + return ret;
> > +
> > + switch (val & 0xf) {
> > + case 0:
> > + si->dram_type = INTEL_DRAM_DDR4;
> > + break;
> > + case 1:
> > + si->dram_type = INTEL_DRAM_DDR3;
> > + break;
> > + case 2:
> > + si->dram_type = INTEL_DRAM_LPDDR3;
> > + break;
> > + case 3:
> > + si->dram_type = INTEL_DRAM_LPDDR3;
> > + break;
> > + default:
> > + MISSING_CASE(val & 0xf);
> > + break;
> > + }
> > +
> > + si->num_channels = (val & 0xf0) >> 4;
> > + si->num_points = (val & 0xf00) >> 8;
> > +
> > + si->t_bl = si->dram_type == INTEL_DRAM_DDR4 ? 4 : 8;
> > +
> > + return 0;
> > +}
> > +
> > +static int icl_pcode_read_qgv_point_info(struct
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: Make sure we have enough memory bandwidth on ICL
On Fri, May 10, 2019 at 05:42:09PM -0700, Matt Roper wrote:
> On Fri, May 03, 2019 at 10:08:31PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > ICL has so many planes that it can easily exceed the maximum
> > effective memory bandwidth of the system. We must therefore check
> > that we don't exceed that limit.
> >
> > The algorithm is very magic number heavy and lacks sufficient
> > explanation for now. We also have no sane way to query the
> > memory clock and timings, so we must rely on a combination of
> > raw readout from the memory controller and hardcoded assumptions.
> > The memory controller values obviously change as the system
> > jumps between the different SAGV points, so we try to stabilize
> > it first by disabling SAGV for the duration of the readout.
> >
> > The utilized bandwidth is tracked via a device wide atomic
> > private object. That is actually not robust because we can't
> > afford to enforce strict global ordering between the pipes.
> > Thus I think I'll need to change this to simply chop up the
> > available bandwidth between all the active pipes. Each pipe
> > can then do whatever it wants as long as it doesn't exceed
> > its budget. That scheme will also require that we assume that
> > any number of planes could be active at any time.
> >
> > TODO: make it robust and deal with all the open questions
> >
> > v2: Sleep longer after disabling SAGV
> > v3: Poll for the dclk to get raised (seen it take 250ms!)
> > If the system has 2133MT/s memory then we pointlessly
> > wait one full second :(
> > v4: Use the new pcode interface to get the qgv points rather
> > that using hardcoded numbers
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/Makefile | 1 +
> > drivers/gpu/drm/i915/i915_drv.c | 229 ++
> > drivers/gpu/drm/i915/i915_drv.h | 10 +
> > drivers/gpu/drm/i915/i915_reg.h | 3 +
> > drivers/gpu/drm/i915/intel_atomic_plane.c | 20 ++
> > drivers/gpu/drm/i915/intel_atomic_plane.h | 2 +
> > drivers/gpu/drm/i915/intel_bw.c | 181 +
> > drivers/gpu/drm/i915/intel_bw.h | 46 +
> > drivers/gpu/drm/i915/intel_display.c | 40 +++-
> > drivers/gpu/drm/i915/intel_drv.h | 2 +
> > 10 files changed, 533 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/gpu/drm/i915/intel_bw.c
> > create mode 100644 drivers/gpu/drm/i915/intel_bw.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 68106fe35a04..139a0fc19390 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -138,6 +138,7 @@ i915-y += intel_audio.o \
> > intel_atomic.o \
> > intel_atomic_plane.o \
> > intel_bios.o \
> > + intel_bw.o \
> > intel_cdclk.o \
> > intel_color.o \
> > intel_combo_phy.o \
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 5ed864752c7b..b7fa7b51c2e2 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -70,6 +70,7 @@
> > #include "intel_overlay.h"
> > #include "intel_pipe_crc.h"
> > #include "intel_pm.h"
> > +#include "intel_sideband.h"
> > #include "intel_sprite.h"
> > #include "intel_uc.h"
> >
> > @@ -1435,6 +1436,232 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv)
> > return 0;
> > }
> >
> > +struct intel_qgv_point {
> > + u16 dclk, t_rp, t_rdpre, t_rc, t_ras, t_rcd;
> > +};
> > +
> > +struct intel_sagv_info {
> > + struct intel_qgv_point points[3];
> > + u8 num_points;
> > + u8 num_channels;
> > + u8 t_bl;
> > + enum intel_dram_type dram_type;
> > +};
> > +
> > +static int icl_pcode_read_mem_global_info(struct drm_i915_private
> > *dev_priv,
> > + struct intel_sagv_info *si)
> > +{
> > + u32 val = 0;
> > + int ret;
> > +
> > + ret = sandybridge_pcode_read(dev_priv,
> > +ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> > +ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
> > +&val, NULL);
> > + if (ret)
> > + return ret;
> > +
> > + switch (val & 0xf) {
> > + case 0:
> > + si->dram_type = INTEL_DRAM_DDR4;
> > + break;
> > + case 1:
> > + si->dram_type = INTEL_DRAM_DDR3;
> > + break;
> > + case 2:
> > + si->dram_type = INTEL_DRAM_LPDDR3;
> > + break;
> > + case 3:
> > + si->dram_type = INTEL_DRAM_LPDDR3;
> > + break;
> > + default:
> > + MISSING_CASE(val & 0xf);
> > + break;
> > + }
> > +
> > + si->num_channels = (val & 0xf0) >> 4;
> > + si->num_points = (val & 0xf00) >> 8;
> > +
> > + si->t_bl = si->dram_type == INTEL_DRAM_DDR4 ? 4 : 8;
> > +
> > + return 0;
> > +}
> > +
> > +static int icl_pcode_read_qgv_point_info(struct drm_i915_priv
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: Make sure we have enough memory bandwidth on ICL
Op 03-05-2019 om 21:08 schreef Ville Syrjala:
> From: Ville Syrjälä
>
> ICL has so many planes that it can easily exceed the maximum
> effective memory bandwidth of the system. We must therefore check
> that we don't exceed that limit.
>
> The algorithm is very magic number heavy and lacks sufficient
> explanation for now. We also have no sane way to query the
> memory clock and timings, so we must rely on a combination of
> raw readout from the memory controller and hardcoded assumptions.
> The memory controller values obviously change as the system
> jumps between the different SAGV points, so we try to stabilize
> it first by disabling SAGV for the duration of the readout.
>
> The utilized bandwidth is tracked via a device wide atomic
> private object. That is actually not robust because we can't
> afford to enforce strict global ordering between the pipes.
> Thus I think I'll need to change this to simply chop up the
> available bandwidth between all the active pipes. Each pipe
> can then do whatever it wants as long as it doesn't exceed
> its budget. That scheme will also require that we assume that
> any number of planes could be active at any time.
>
> TODO: make it robust and deal with all the open questions
>
> v2: Sleep longer after disabling SAGV
> v3: Poll for the dclk to get raised (seen it take 250ms!)
> If the system has 2133MT/s memory then we pointlessly
> wait one full second :(
> v4: Use the new pcode interface to get the qgv points rather
> that using hardcoded numbers
>
> Signed-off-by: Ville Syrjälä
> ---
Splitting out the HW readout would be nice, it would make it easier to review
the separate parts that this patch tries to accomplish. :)
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_drv.c | 229 ++
> drivers/gpu/drm/i915/i915_drv.h | 10 +
> drivers/gpu/drm/i915/i915_reg.h | 3 +
> drivers/gpu/drm/i915/intel_atomic_plane.c | 20 ++
> drivers/gpu/drm/i915/intel_atomic_plane.h | 2 +
> drivers/gpu/drm/i915/intel_bw.c | 181 +
> drivers/gpu/drm/i915/intel_bw.h | 46 +
> drivers/gpu/drm/i915/intel_display.c | 40 +++-
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> 10 files changed, 533 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/i915/intel_bw.c
> create mode 100644 drivers/gpu/drm/i915/intel_bw.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 68106fe35a04..139a0fc19390 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -138,6 +138,7 @@ i915-y += intel_audio.o \
> intel_atomic.o \
> intel_atomic_plane.o \
> intel_bios.o \
> + intel_bw.o \
> intel_cdclk.o \
> intel_color.o \
> intel_combo_phy.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 5ed864752c7b..b7fa7b51c2e2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -70,6 +70,7 @@
> #include "intel_overlay.h"
> #include "intel_pipe_crc.h"
> #include "intel_pm.h"
> +#include "intel_sideband.h"
> #include "intel_sprite.h"
> #include "intel_uc.h"
>
> @@ -1435,6 +1436,232 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv)
> return 0;
> }
>
> +struct intel_qgv_point {
> + u16 dclk, t_rp, t_rdpre, t_rc, t_ras, t_rcd;
> +};
> +
> +struct intel_sagv_info {
> + struct intel_qgv_point points[3];
> + u8 num_points;
> + u8 num_channels;
> + u8 t_bl;
> + enum intel_dram_type dram_type;
> +};
> +
> +static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv,
> + struct intel_sagv_info *si)
> +{
> + u32 val = 0;
> + int ret;
> +
> + ret = sandybridge_pcode_read(dev_priv,
> + ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> + ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
> + &val, NULL);
> + if (ret)
> + return ret;
> +
> + switch (val & 0xf) {
> + case 0:
> + si->dram_type = INTEL_DRAM_DDR4;
> + break;
> + case 1:
> + si->dram_type = INTEL_DRAM_DDR3;
> + break;
> + case 2:
> + si->dram_type = INTEL_DRAM_LPDDR3;
> + break;
> + case 3:
> + si->dram_type = INTEL_DRAM_LPDDR3;
> + break;
> + default:
> + MISSING_CASE(val & 0xf);
> + break;
> + }
> +
> + si->num_channels = (val & 0xf0) >> 4;
> + si->num_points = (val & 0xf00) >> 8;
> +
> + si->t_bl = si->dram_type == INTEL_DRAM_DDR4 ? 4 : 8;
> +
> + return 0;
> +}
> +
> +static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
> + struct intel_qgv_point *sp,
> + i
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: Make sure we have enough memory bandwidth on ICL
On Fri, May 03, 2019 at 10:08:31PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> ICL has so many planes that it can easily exceed the maximum
> effective memory bandwidth of the system. We must therefore check
> that we don't exceed that limit.
>
> The algorithm is very magic number heavy and lacks sufficient
> explanation for now. We also have no sane way to query the
> memory clock and timings, so we must rely on a combination of
> raw readout from the memory controller and hardcoded assumptions.
> The memory controller values obviously change as the system
> jumps between the different SAGV points, so we try to stabilize
> it first by disabling SAGV for the duration of the readout.
>
> The utilized bandwidth is tracked via a device wide atomic
> private object. That is actually not robust because we can't
> afford to enforce strict global ordering between the pipes.
> Thus I think I'll need to change this to simply chop up the
> available bandwidth between all the active pipes. Each pipe
> can then do whatever it wants as long as it doesn't exceed
> its budget. That scheme will also require that we assume that
> any number of planes could be active at any time.
>
> TODO: make it robust and deal with all the open questions
>
> v2: Sleep longer after disabling SAGV
> v3: Poll for the dclk to get raised (seen it take 250ms!)
> If the system has 2133MT/s memory then we pointlessly
> wait one full second :(
> v4: Use the new pcode interface to get the qgv points rather
> that using hardcoded numbers
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_drv.c | 229 ++
> drivers/gpu/drm/i915/i915_drv.h | 10 +
> drivers/gpu/drm/i915/i915_reg.h | 3 +
> drivers/gpu/drm/i915/intel_atomic_plane.c | 20 ++
> drivers/gpu/drm/i915/intel_atomic_plane.h | 2 +
> drivers/gpu/drm/i915/intel_bw.c | 181 +
> drivers/gpu/drm/i915/intel_bw.h | 46 +
> drivers/gpu/drm/i915/intel_display.c | 40 +++-
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> 10 files changed, 533 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/i915/intel_bw.c
> create mode 100644 drivers/gpu/drm/i915/intel_bw.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 68106fe35a04..139a0fc19390 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -138,6 +138,7 @@ i915-y += intel_audio.o \
> intel_atomic.o \
> intel_atomic_plane.o \
> intel_bios.o \
> + intel_bw.o \
> intel_cdclk.o \
> intel_color.o \
> intel_combo_phy.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 5ed864752c7b..b7fa7b51c2e2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -70,6 +70,7 @@
> #include "intel_overlay.h"
> #include "intel_pipe_crc.h"
> #include "intel_pm.h"
> +#include "intel_sideband.h"
> #include "intel_sprite.h"
> #include "intel_uc.h"
>
> @@ -1435,6 +1436,232 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv)
> return 0;
> }
>
> +struct intel_qgv_point {
> + u16 dclk, t_rp, t_rdpre, t_rc, t_ras, t_rcd;
> +};
> +
> +struct intel_sagv_info {
> + struct intel_qgv_point points[3];
> + u8 num_points;
> + u8 num_channels;
> + u8 t_bl;
> + enum intel_dram_type dram_type;
> +};
> +
> +static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv,
> + struct intel_sagv_info *si)
> +{
> + u32 val = 0;
> + int ret;
> +
> + ret = sandybridge_pcode_read(dev_priv,
> + ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> + ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
> + &val, NULL);
> + if (ret)
> + return ret;
> +
> + switch (val & 0xf) {
> + case 0:
> + si->dram_type = INTEL_DRAM_DDR4;
> + break;
> + case 1:
> + si->dram_type = INTEL_DRAM_DDR3;
> + break;
> + case 2:
> + si->dram_type = INTEL_DRAM_LPDDR3;
> + break;
> + case 3:
> + si->dram_type = INTEL_DRAM_LPDDR3;
> + break;
> + default:
> + MISSING_CASE(val & 0xf);
> + break;
> + }
> +
> + si->num_channels = (val & 0xf0) >> 4;
> + si->num_points = (val & 0xf00) >> 8;
> +
> + si->t_bl = si->dram_type == INTEL_DRAM_DDR4 ? 4 : 8;
> +
> + return 0;
> +}
> +
> +static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
> + struct intel_qgv_point *sp,
> + int point)
> +{
> + u32 val = 0, val2;
> + int ret;
> +
> + ret = sandybridge_pcode_read(dev_priv,
> +
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: Make sure we have enough memory bandwidth on ICL
On Fri, 2019-05-03 at 22:08 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> ICL has so many planes that it can easily exceed the maximum
> effective memory bandwidth of the system. We must therefore check
> that we don't exceed that limit.
>
> The algorithm is very magic number heavy and lacks sufficient
> explanation for now. We also have no sane way to query the
> memory clock and timings, so we must rely on a combination of
> raw readout from the memory controller and hardcoded assumptions.
> The memory controller values obviously change as the system
> jumps between the different SAGV points, so we try to stabilize
> it first by disabling SAGV for the duration of the readout.
>
> The utilized bandwidth is tracked via a device wide atomic
> private object. That is actually not robust because we can't
> afford to enforce strict global ordering between the pipes.
> Thus I think I'll need to change this to simply chop up the
> available bandwidth between all the active pipes. Each pipe
> can then do whatever it wants as long as it doesn't exceed
> its budget. That scheme will also require that we assume that
> any number of planes could be active at any time.
>
> TODO: make it robust and deal with all the open questions
>
> v2: Sleep longer after disabling SAGV
> v3: Poll for the dclk to get raised (seen it take 250ms!)
> If the system has 2133MT/s memory then we pointlessly
> wait one full second :(
> v4: Use the new pcode interface to get the qgv points rather
> that using hardcoded numbers
>
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/i915_drv.c | 229
> ++
> drivers/gpu/drm/i915/i915_drv.h | 10 +
> drivers/gpu/drm/i915/i915_reg.h | 3 +
> drivers/gpu/drm/i915/intel_atomic_plane.c | 20 ++
> drivers/gpu/drm/i915/intel_atomic_plane.h | 2 +
> drivers/gpu/drm/i915/intel_bw.c | 181 +
> drivers/gpu/drm/i915/intel_bw.h | 46 +
> drivers/gpu/drm/i915/intel_display.c | 40 +++-
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> 10 files changed, 533 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/i915/intel_bw.c
> create mode 100644 drivers/gpu/drm/i915/intel_bw.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile
> b/drivers/gpu/drm/i915/Makefile
> index 68106fe35a04..139a0fc19390 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -138,6 +138,7 @@ i915-y += intel_audio.o \
> intel_atomic.o \
> intel_atomic_plane.o \
> intel_bios.o \
> + intel_bw.o \
> intel_cdclk.o \
> intel_color.o \
> intel_combo_phy.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> index 5ed864752c7b..b7fa7b51c2e2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -70,6 +70,7 @@
> #include "intel_overlay.h"
> #include "intel_pipe_crc.h"
> #include "intel_pm.h"
> +#include "intel_sideband.h"
> #include "intel_sprite.h"
> #include "intel_uc.h"
>
> @@ -1435,6 +1436,232 @@ bxt_get_dram_info(struct drm_i915_private
> *dev_priv)
> return 0;
> }
>
> +struct intel_qgv_point {
> + u16 dclk, t_rp, t_rdpre, t_rc, t_ras, t_rcd;
> +};
> +
> +struct intel_sagv_info {
> + struct intel_qgv_point points[3];
> + u8 num_points;
> + u8 num_channels;
> + u8 t_bl;
> + enum intel_dram_type dram_type;
> +};
> +
> +static int icl_pcode_read_mem_global_info(struct drm_i915_private
> *dev_priv,
> + struct intel_sagv_info *si)
> +{
> + u32 val = 0;
> + int ret;
> +
> + ret = sandybridge_pcode_read(dev_priv,
> + ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> + ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
> + &val, NULL);
> + if (ret)
> + return ret;
> +
> + switch (val & 0xf) {
> + case 0:
> + si->dram_type = INTEL_DRAM_DDR4;
> + break;
> + case 1:
> + si->dram_type = INTEL_DRAM_DDR3;
> + break;
> + case 2:
> + si->dram_type = INTEL_DRAM_LPDDR3;
> + break;
> + case 3:
> + si->dram_type = INTEL_DRAM_LPDDR3;
> + break;
> + default:
> + MISSING_CASE(val & 0xf);
> + break;
> + }
> +
> + si->num_channels = (val & 0xf0) >> 4;
> + si->num_points = (val & 0xf00) >> 8;
> +
> + si->t_bl = si->dram_type == INTEL_DRAM_DDR4 ? 4 : 8;
> +
> + return 0;
> +}
> +
> +static int icl_pcode_read_qgv_point_info(struct drm_i915_private
> *dev_priv,
> + struct intel_qgv_point *sp,
> + int point)
Are we trying to retrieve the dram timing parameters to calculate the
latency? If so can that be se
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: Make sure we have enough memory bandwidth on ICL
On Mon, May 06, 2019 at 03:38:43PM -0700, Clinton Taylor wrote:
>
> On 5/3/19 12:08 PM, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > ICL has so many planes that it can easily exceed the maximum
> > effective memory bandwidth of the system. We must therefore check
> > that we don't exceed that limit.
> >
> > The algorithm is very magic number heavy and lacks sufficient
> > explanation for now. We also have no sane way to query the
> > memory clock and timings, so we must rely on a combination of
> > raw readout from the memory controller and hardcoded assumptions.
> > The memory controller values obviously change as the system
> > jumps between the different SAGV points, so we try to stabilize
> > it first by disabling SAGV for the duration of the readout.
> >
> > The utilized bandwidth is tracked via a device wide atomic
> > private object. That is actually not robust because we can't
> > afford to enforce strict global ordering between the pipes.
> > Thus I think I'll need to change this to simply chop up the
> > available bandwidth between all the active pipes. Each pipe
> > can then do whatever it wants as long as it doesn't exceed
> > its budget. That scheme will also require that we assume that
> > any number of planes could be active at any time.
> >
> > TODO: make it robust and deal with all the open questions
>
> TODO: Add comments detailing structures
>
> >
> > v2: Sleep longer after disabling SAGV
> > v3: Poll for the dclk to get raised (seen it take 250ms!)
> > If the system has 2133MT/s memory then we pointlessly
> > wait one full second :(
> > v4: Use the new pcode interface to get the qgv points rather
> > that using hardcoded numbers
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/Makefile | 1 +
> > drivers/gpu/drm/i915/i915_drv.c | 229 ++
> > drivers/gpu/drm/i915/i915_drv.h | 10 +
> > drivers/gpu/drm/i915/i915_reg.h | 3 +
> > drivers/gpu/drm/i915/intel_atomic_plane.c | 20 ++
> > drivers/gpu/drm/i915/intel_atomic_plane.h | 2 +
> > drivers/gpu/drm/i915/intel_bw.c | 181 +
> > drivers/gpu/drm/i915/intel_bw.h | 46 +
> > drivers/gpu/drm/i915/intel_display.c | 40 +++-
> > drivers/gpu/drm/i915/intel_drv.h | 2 +
> > 10 files changed, 533 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/gpu/drm/i915/intel_bw.c
> > create mode 100644 drivers/gpu/drm/i915/intel_bw.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 68106fe35a04..139a0fc19390 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -138,6 +138,7 @@ i915-y += intel_audio.o \
> > intel_atomic.o \
> > intel_atomic_plane.o \
> > intel_bios.o \
> > + intel_bw.o \
> > intel_cdclk.o \
> > intel_color.o \
> > intel_combo_phy.o \
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 5ed864752c7b..b7fa7b51c2e2 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -70,6 +70,7 @@
> > #include "intel_overlay.h"
> > #include "intel_pipe_crc.h"
> > #include "intel_pm.h"
> > +#include "intel_sideband.h"
> > #include "intel_sprite.h"
> > #include "intel_uc.h"
> >
> > @@ -1435,6 +1436,232 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv)
> > return 0;
> > }
> >
> > +struct intel_qgv_point {
> > + u16 dclk, t_rp, t_rdpre, t_rc, t_ras, t_rcd;
> > +};
> > +
> > +struct intel_sagv_info {
> > + struct intel_qgv_point points[3];
> > + u8 num_points;
> > + u8 num_channels;
> > + u8 t_bl;
> > + enum intel_dram_type dram_type;
> > +};
> > +
> > +static int icl_pcode_read_mem_global_info(struct drm_i915_private
> > *dev_priv,
> > + struct intel_sagv_info *si)
> > +{
> > + u32 val = 0;
> > + int ret;
> > +
> > + ret = sandybridge_pcode_read(dev_priv,
> > +ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
> > +ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
> > +&val, NULL);
> > + if (ret)
> > + return ret;
> > +
> > + switch (val & 0xf) {
> > + case 0:
> > + si->dram_type = INTEL_DRAM_DDR4;
> > + break;
> > + case 1:
> > + si->dram_type = INTEL_DRAM_DDR3;
> > + break;
> > + case 2:
> > + si->dram_type = INTEL_DRAM_LPDDR3;
> > + break;
> > + case 3:
> > + si->dram_type = INTEL_DRAM_LPDDR3;
> > + break;
> > + default:
> > + MISSING_CASE(val & 0xf);
> > + break;
> > + }
> > +
> > + si->num_channels = (val & 0xf0) >> 4;
> > + si->num_points = (val & 0xf00) >> 8;
> > +
> > + si->t_bl = si->dram_type == INTEL_DRAM_DDR4 ? 4 : 8;
> > +
> > + return 0;
> > +}
> > +
> > +static int
Re: [Intel-gfx] [PATCH v3 2/2] drm/i915: Make sure we have enough memory bandwidth on ICL
On 5/3/19 12:08 PM, Ville Syrjala wrote:
From: Ville Syrjälä
ICL has so many planes that it can easily exceed the maximum
effective memory bandwidth of the system. We must therefore check
that we don't exceed that limit.
The algorithm is very magic number heavy and lacks sufficient
explanation for now. We also have no sane way to query the
memory clock and timings, so we must rely on a combination of
raw readout from the memory controller and hardcoded assumptions.
The memory controller values obviously change as the system
jumps between the different SAGV points, so we try to stabilize
it first by disabling SAGV for the duration of the readout.
The utilized bandwidth is tracked via a device wide atomic
private object. That is actually not robust because we can't
afford to enforce strict global ordering between the pipes.
Thus I think I'll need to change this to simply chop up the
available bandwidth between all the active pipes. Each pipe
can then do whatever it wants as long as it doesn't exceed
its budget. That scheme will also require that we assume that
any number of planes could be active at any time.
TODO: make it robust and deal with all the open questions
TODO: Add comments detailing structures
v2: Sleep longer after disabling SAGV
v3: Poll for the dclk to get raised (seen it take 250ms!)
If the system has 2133MT/s memory then we pointlessly
wait one full second :(
v4: Use the new pcode interface to get the qgv points rather
that using hardcoded numbers
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.c | 229 ++
drivers/gpu/drm/i915/i915_drv.h | 10 +
drivers/gpu/drm/i915/i915_reg.h | 3 +
drivers/gpu/drm/i915/intel_atomic_plane.c | 20 ++
drivers/gpu/drm/i915/intel_atomic_plane.h | 2 +
drivers/gpu/drm/i915/intel_bw.c | 181 +
drivers/gpu/drm/i915/intel_bw.h | 46 +
drivers/gpu/drm/i915/intel_display.c | 40 +++-
drivers/gpu/drm/i915/intel_drv.h | 2 +
10 files changed, 533 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/i915/intel_bw.c
create mode 100644 drivers/gpu/drm/i915/intel_bw.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 68106fe35a04..139a0fc19390 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -138,6 +138,7 @@ i915-y += intel_audio.o \
intel_atomic.o \
intel_atomic_plane.o \
intel_bios.o \
+ intel_bw.o \
intel_cdclk.o \
intel_color.o \
intel_combo_phy.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5ed864752c7b..b7fa7b51c2e2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -70,6 +70,7 @@
#include "intel_overlay.h"
#include "intel_pipe_crc.h"
#include "intel_pm.h"
+#include "intel_sideband.h"
#include "intel_sprite.h"
#include "intel_uc.h"
@@ -1435,6 +1436,232 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv)
return 0;
}
+struct intel_qgv_point {
+ u16 dclk, t_rp, t_rdpre, t_rc, t_ras, t_rcd;
+};
+
+struct intel_sagv_info {
+ struct intel_qgv_point points[3];
+ u8 num_points;
+ u8 num_channels;
+ u8 t_bl;
+ enum intel_dram_type dram_type;
+};
+
+static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv,
+ struct intel_sagv_info *si)
+{
+ u32 val = 0;
+ int ret;
+
+ ret = sandybridge_pcode_read(dev_priv,
+ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
+ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
+&val, NULL);
+ if (ret)
+ return ret;
+
+ switch (val & 0xf) {
+ case 0:
+ si->dram_type = INTEL_DRAM_DDR4;
+ break;
+ case 1:
+ si->dram_type = INTEL_DRAM_DDR3;
+ break;
+ case 2:
+ si->dram_type = INTEL_DRAM_LPDDR3;
+ break;
+ case 3:
+ si->dram_type = INTEL_DRAM_LPDDR3;
+ break;
+ default:
+ MISSING_CASE(val & 0xf);
+ break;
+ }
+
+ si->num_channels = (val & 0xf0) >> 4;
+ si->num_points = (val & 0xf00) >> 8;
+
+ si->t_bl = si->dram_type == INTEL_DRAM_DDR4 ? 4 : 8;
+
+ return 0;
+}
+
+static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
+struct intel_qgv_point *sp,
+int point)
+{
+ u32 val = 0, val2;
+ int ret;
+
+ ret = sandybridge_pcode_read(dev_priv,
+ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
+
ICL_PCODE_MEM_SS_
[Intel-gfx] [PATCH v3 2/2] drm/i915: Make sure we have enough memory bandwidth on ICL
From: Ville Syrjälä
ICL has so many planes that it can easily exceed the maximum
effective memory bandwidth of the system. We must therefore check
that we don't exceed that limit.
The algorithm is very magic number heavy and lacks sufficient
explanation for now. We also have no sane way to query the
memory clock and timings, so we must rely on a combination of
raw readout from the memory controller and hardcoded assumptions.
The memory controller values obviously change as the system
jumps between the different SAGV points, so we try to stabilize
it first by disabling SAGV for the duration of the readout.
The utilized bandwidth is tracked via a device wide atomic
private object. That is actually not robust because we can't
afford to enforce strict global ordering between the pipes.
Thus I think I'll need to change this to simply chop up the
available bandwidth between all the active pipes. Each pipe
can then do whatever it wants as long as it doesn't exceed
its budget. That scheme will also require that we assume that
any number of planes could be active at any time.
TODO: make it robust and deal with all the open questions
v2: Sleep longer after disabling SAGV
v3: Poll for the dclk to get raised (seen it take 250ms!)
If the system has 2133MT/s memory then we pointlessly
wait one full second :(
v4: Use the new pcode interface to get the qgv points rather
that using hardcoded numbers
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/Makefile | 1 +
drivers/gpu/drm/i915/i915_drv.c | 229 ++
drivers/gpu/drm/i915/i915_drv.h | 10 +
drivers/gpu/drm/i915/i915_reg.h | 3 +
drivers/gpu/drm/i915/intel_atomic_plane.c | 20 ++
drivers/gpu/drm/i915/intel_atomic_plane.h | 2 +
drivers/gpu/drm/i915/intel_bw.c | 181 +
drivers/gpu/drm/i915/intel_bw.h | 46 +
drivers/gpu/drm/i915/intel_display.c | 40 +++-
drivers/gpu/drm/i915/intel_drv.h | 2 +
10 files changed, 533 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/i915/intel_bw.c
create mode 100644 drivers/gpu/drm/i915/intel_bw.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 68106fe35a04..139a0fc19390 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -138,6 +138,7 @@ i915-y += intel_audio.o \
intel_atomic.o \
intel_atomic_plane.o \
intel_bios.o \
+ intel_bw.o \
intel_cdclk.o \
intel_color.o \
intel_combo_phy.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5ed864752c7b..b7fa7b51c2e2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -70,6 +70,7 @@
#include "intel_overlay.h"
#include "intel_pipe_crc.h"
#include "intel_pm.h"
+#include "intel_sideband.h"
#include "intel_sprite.h"
#include "intel_uc.h"
@@ -1435,6 +1436,232 @@ bxt_get_dram_info(struct drm_i915_private *dev_priv)
return 0;
}
+struct intel_qgv_point {
+ u16 dclk, t_rp, t_rdpre, t_rc, t_ras, t_rcd;
+};
+
+struct intel_sagv_info {
+ struct intel_qgv_point points[3];
+ u8 num_points;
+ u8 num_channels;
+ u8 t_bl;
+ enum intel_dram_type dram_type;
+};
+
+static int icl_pcode_read_mem_global_info(struct drm_i915_private *dev_priv,
+ struct intel_sagv_info *si)
+{
+ u32 val = 0;
+ int ret;
+
+ ret = sandybridge_pcode_read(dev_priv,
+ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
+ICL_PCODE_MEM_SS_READ_GLOBAL_INFO,
+&val, NULL);
+ if (ret)
+ return ret;
+
+ switch (val & 0xf) {
+ case 0:
+ si->dram_type = INTEL_DRAM_DDR4;
+ break;
+ case 1:
+ si->dram_type = INTEL_DRAM_DDR3;
+ break;
+ case 2:
+ si->dram_type = INTEL_DRAM_LPDDR3;
+ break;
+ case 3:
+ si->dram_type = INTEL_DRAM_LPDDR3;
+ break;
+ default:
+ MISSING_CASE(val & 0xf);
+ break;
+ }
+
+ si->num_channels = (val & 0xf0) >> 4;
+ si->num_points = (val & 0xf00) >> 8;
+
+ si->t_bl = si->dram_type == INTEL_DRAM_DDR4 ? 4 : 8;
+
+ return 0;
+}
+
+static int icl_pcode_read_qgv_point_info(struct drm_i915_private *dev_priv,
+struct intel_qgv_point *sp,
+int point)
+{
+ u32 val = 0, val2;
+ int ret;
+
+ ret = sandybridge_pcode_read(dev_priv,
+ICL_PCODE_MEM_SUBSYSYSTEM_INFO |
+
ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point),
+&val, &val2);
+ if (ret)
+ re
