On Mon, 2015-06-15 at 14:51 +0200, Daniel Vetter wrote:
> On Wed, Jun 10, 2015 at 09:12:16AM +0100, Peter Antoine wrote:
> > This change adds the programming of the MOCS registers to the gen 9+
> > platforms. This change set programs the MOCS register values to a set
> > of values that are defined to be optimal.
> >
> > It creates a fixed register set that is programmed across the different
> > engines so that all engines have the same table. This is done as the
> > main RCS context only holds the registers for itself and the shared
> > L3 values. By trying to keep the registers consistent across the
> > different engines it should make the programming for the registers
> > consistent.
> >
> > v2:
> > -'static const' for private data structures and style changes.(Matt Turner)
> > v3:
> > - Make the tables "slightly" more readable. (Damien Lespiau)
> > - Updated tables fix performance regression.
> >
> >
> > Signed-off-by: Peter Antoine <peter.antoine@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/Makefile | 3 +-
> > drivers/gpu/drm/i915/i915_reg.h | 9 ++
> > drivers/gpu/drm/i915/intel_lrc.c | 68 ++++++++++
> > drivers/gpu/drm/i915/intel_mocs.c | 252 ++++++++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_mocs.h | 119 ++++++++++++++++++
> > 5 files changed, 450 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/gpu/drm/i915/intel_mocs.c
> > create mode 100644 drivers/gpu/drm/i915/intel_mocs.h
> >
> > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index b7ddf48..cd7b910 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -36,7 +36,8 @@ i915-y += i915_cmd_parser.o \
> > i915_trace_points.o \
> > intel_lrc.o \
> > intel_ringbuffer.o \
> > - intel_uncore.o
> > + intel_uncore.o \
> > + intel_mocs.o
> >
> > # autogenerated null render state
> > i915-y += intel_renderstate_gen6.o \
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 7213224..3a435b5 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7829,4 +7829,13 @@ enum skl_disp_power_wells {
> > #define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
> > #define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
> >
> > +/* MOCS (Memory Object Control State) registers */
> > +#define GEN9_LNCFCMOCS0 (0xB020) /* L3 Cache Control base */
> > +
> > +#define GEN9_GFX_MOCS_0 (0xc800) /* Graphics MOCS base register*/
> > +#define GEN9_MFX0_MOCS_0 (0xc900) /* Media 0 MOCS base register*/
> > +#define GEN9_MFX1_MOCS_0 (0xcA00) /* Media 1 MOCS base register*/
> > +#define GEN9_VEBOX_MOCS_0 (0xcB00) /* Video MOCS base register*/
> > +#define GEN9_BLT_MOCS_0 (0xcc00) /* Blitter MOCS base register*/
> > +
> > #endif /* _I915_REG_H_ */
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index 9f5485d..c875569 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -135,6 +135,7 @@
> > #include <drm/drmP.h>
> > #include <drm/i915_drm.h>
> > #include "i915_drv.h"
> > +#include "intel_mocs.h"
> >
> > #define GEN9_LR_CONTEXT_RENDER_SIZE (22 * PAGE_SIZE)
> > #define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
> > @@ -1370,6 +1371,67 @@ out:
> > return ret;
> > }
> >
> > +/*
> > + * i915_gem_program_mocs() - program the MOCS register.
> > + *
> > + * ring: The ring that the programming batch will be run in.
> > + * ctx: The intel_context to be used.
> > + *
> > + * This function will emit a batch buffer with the values required for
> > + * programming the MOCS register values for all the currenly supported
> > + * rings.
> > + *
> > + * Return: 0 on success, otherwise the error status.
> > + */
> > +static int i915_gem_program_mocs(struct intel_engine_cs *ring,
> > + struct intel_context *ctx)
>
> If you go with a separate source file then imo this would be the function
> to move there. As a rule of thumb if your new C file is full of non-static
> functions used somewhere else and you need plenty of header definitions to
> make it all work then you've split things in a useless way. Separate
> source files are only useful if you actually manage to encapsulate things
> a bit. If that encapsulation isn't given then bothering with all the
> kerneldoc isn't worth it eithe since due to the tight coupling it'll
> outdate extremely fast.
Yup. Bad chose during the merge, intel_logical_ring_begin() became
static, and instead of reversing that change, I moved my function.
reverted and reverted.
> -Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
