From mboxrd@z Thu Jan 1 00:00:00 1970 From: Francisco Jerez Subject: Re: [PATCHv7] drm/i915: Added Programming of the MOCS Date: Thu, 09 Jul 2015 18:47:43 +0300 Message-ID: <87twtdqqn4.fsf@riseup.net> References: <1436296381-1174-1-git-send-email-currojerez@riseup.net> <559D3CE4.7010002@linux.intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0174554821==" Return-path: Received: from mx1.riseup.net (mx1.riseup.net [198.252.153.129]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2034C6E0B7 for ; Thu, 9 Jul 2015 08:48:07 -0700 (PDT) In-Reply-To: <559D3CE4.7010002@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "Siluvery, Arun" , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============0174554821== Content-Type: multipart/signed; boundary="==-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" --==-=-= Content-Type: multipart/mixed; boundary="=-=-=" --=-=-= Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable "Siluvery, Arun" writes: > On 07/07/2015 20:13, Francisco Jerez wrote: >> From: Peter Antoine >> >> 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 Turn= er) >> v3: >> - Make the tables "slightly" more readable. (Damien Lespiau) >> - Updated tables fix performance regression. >> v4: >> - Code formatting. (Chris Wilson) >> - re-privatised mocs code. (Daniel Vetter) >> v5: >> - Changed the name of a function. (Chris Wilson) >> v6: >> - re-based >> - Added Mesa table entry (skylake & broxton) (Francisco Jerez) >> - Tidied up the readability defines (Francisco Jerez) >> - NUMBER of entries defines wrong. (Jim Bish) >> - Added comments to clear up the meaning of the tables (Jim Bish) >> >> Signed-off-by: Peter Antoine >> >> v7 (Francisco Jerez): >> - Don't write L3-specific MOCS_ESC/SCC values into the e/LLC control >> tables. Prefix L3-specific defines consistently with L3_ and >> e/LLC-specific defines with LE_ to avoid this kind of confusion in >> the future. >> - Change L3CC WT define back to RESERVED (matches my hardware >> documentation and the original patch, probably a misunderstanding >> of my own previous comment). >> - Drop Android tables, define new minimal tables more suitable for the >> open source stack. >> - Add comment that the MOCS tables are part of the kernel ABI. >> - Move intel_logical_ring_begin() and _advance() calls one level down >> (Chris Wilson). >> - Minor formatting and style fixes. >> >> Signed-off-by: Francisco Jerez >> --- >> drivers/gpu/drm/i915/Makefile | 1 + >> drivers/gpu/drm/i915/i915_reg.h | 9 ++ >> drivers/gpu/drm/i915/intel_lrc.c | 11 +- >> drivers/gpu/drm/i915/intel_lrc.h | 1 + >> drivers/gpu/drm/i915/intel_mocs.c | 324 ++++++++++++++++++++++++++++++= ++++++++ >> drivers/gpu/drm/i915/intel_mocs.h | 57 +++++++ >> 6 files changed, 401 insertions(+), 2 deletions(-) >> 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/Makefi= le >> index de21965..e52e012 100644 >> --- a/drivers/gpu/drm/i915/Makefile >> +++ b/drivers/gpu/drm/i915/Makefile >> @@ -36,6 +36,7 @@ i915-y +=3D i915_cmd_parser.o \ >> i915_trace_points.o \ >> intel_hotplug.o \ >> intel_lrc.o \ >> + intel_mocs.o \ >> intel_ringbuffer.o \ >> intel_uncore.o >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915= _reg.h >> index 2a29bcc..9b17260 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -7906,4 +7906,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/int= el_lrc.c >> index d4f8b43..466d17c 100644 >> --- a/drivers/gpu/drm/i915/intel_lrc.c >> +++ b/drivers/gpu/drm/i915/intel_lrc.c >> @@ -135,6 +135,7 @@ >> #include >> #include >> #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) >> @@ -772,8 +773,7 @@ static int logical_ring_prepare(struct drm_i915_gem_= request *req, int bytes) >> * >> * Return: non-zero if the ringbuffer is not ready to be written to. >> */ >> -static int intel_logical_ring_begin(struct drm_i915_gem_request *req, >> - int num_dwords) >> +int intel_logical_ring_begin(struct drm_i915_gem_request *req, int num_= dwords) >> { >> struct drm_i915_private *dev_priv; >> int ret; >> @@ -1675,6 +1675,13 @@ static int gen8_init_rcs_context(struct drm_i915_= gem_request *req) >> if (ret) >> return ret; >> >> + /* >> + * Failing to program the MOCS is non-fatal.The system will not >> + * run at peak performance. So generate a warning and carry on. >> + */ >> + if (intel_rcs_context_init_mocs(req) !=3D 0) >> + DRM_ERROR("MOCS failed to program: expect performance issues."); > > we don't see this type of constructs, to be consistent why not, > ret =3D intel_rcs_context_init_mocs(req); > if (ret) > DRM_ERROR(); > > nitpick, comment says warning but DRM_ERROR is used. > '\n' missing in DRM_ERROR > Thanks, fixed. >> + >> return intel_lr_context_render_state_init(req); >> } >> >> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/int= el_lrc.h >> index e0299fb..64f89f99 100644 >> --- a/drivers/gpu/drm/i915/intel_lrc.h >> +++ b/drivers/gpu/drm/i915/intel_lrc.h >> @@ -42,6 +42,7 @@ int intel_logical_ring_reserve_space(struct drm_i915_g= em_request *request); >> void intel_logical_ring_stop(struct intel_engine_cs *ring); >> void intel_logical_ring_cleanup(struct intel_engine_cs *ring); >> int intel_logical_rings_init(struct drm_device *dev); >> +int intel_logical_ring_begin(struct drm_i915_gem_request *req, int num_= dwords); >> >> int logical_ring_flush_all_caches(struct drm_i915_gem_request *req); >> /** >> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/in= tel_mocs.c >> new file mode 100644 >> index 0000000..f7b93e9 >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/intel_mocs.c >> @@ -0,0 +1,324 @@ >> +/* >> + * Copyright (c) 2015 Intel Corporation >> + * >> + * Permission is hereby granted, free of charge, to any person obtainin= g a >> + * copy of this software and associated documentation files (the "Softw= are"), >> + * to deal in the Software without restriction, including without limit= ation >> + * the rights to use, copy, modify, merge, publish, distribute, sublice= nse, >> + * and/or sell copies of the Software, and to permit persons to whom the >> + * Software is furnished to do so, subject to the following conditions:= * >> + * The above copyright notice and this permission notice (including the= next >> + * paragraph) shall be included in all copies or substantial portions o= f the >> + * Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPR= ESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABIL= ITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT S= HALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR= OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARIS= ING FROM, >> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALIN= GS IN THE >> + * SOFTWARE. >> + */ >> + >> +#include "intel_mocs.h" >> +#include "intel_lrc.h" >> +#include "intel_ringbuffer.h" >> + >> +/* structures required */ >> +struct drm_i915_mocs_entry { >> + u32 control_value; >> + u16 l3cc_value; >> +}; >> + >> +struct drm_i915_mocs_table { >> + u32 size; >> + const struct drm_i915_mocs_entry *table; >> +}; >> + > too much spacing. > >> +/* Defines for the tables (XXX_MOCS_0 - XXX_MOCS_63) */ >> +#define LE_CACHEABILITY(value) (value << 0) >> +#define LE_TGT_CACHE(value) (value << 2) >> +#define LE_LRUM(value) (value << 4) >> +#define LE_AOM(value) (value << 6) >> +#define LE_RSC(value) (value << 7) >> +#define LE_SCC(value) (value << 8) >> +#define LE_PFM(value) (value << 11) >> +#define LE_SCF(value) (value << 14) >> + >> +/* Defines for the tables (LNCFMOCS0 - LNCFMOCS31) - two entries per wo= rd */ >> +#define L3_ESC(value) (value << 0) >> +#define L3_SCC(value) (value << 1) >> +#define L3_CACHEABILITY(value) (value << 4) >> + >> +/* Helper defines */ >> +#define GEN9_NUM_MOCS_ENTRIES (62) /* 62 out of 64 - 63 & 64 are reser= ved. */ >> + >> +/* (e)LLC caching options */ >> +#define LE_PAGETABLE (0) >> +#define LE_UC (1) >> +#define LE_WT (2) >> +#define LE_WB (3) >> + >> +/* L3 caching options */ >> +#define L3_DIRECT (0) >> +#define L3_UC (1) >> +#define L3_RESERVED (2) >> +#define L3_WB (3) >> + >> +/* Target cache */ >> +#define ELLC (0) >> +#define LLC (1) >> +#define LLC_ELLC (2) >> + >> +/* >> + * MOCS tables >> + * >> + * These are the MOCS tables that are programmed across all the rings. >> + * The control value is programmed to all the rings that support the >> + * MOCS registers. While the l3cc_values are only programmed to the >> + * LNCFCMOCS0 - LNCFCMOCS32 registers. >> + * >> + * These tables are intended to be kept reasonably consistent across >> + * platforms. However some of the fields are not applicable to all of >> + * them. >> + * >> + * NOTE: These tables MUST start with being uncached and the length >> + * MUST be less than 63 as the last two registers are reserved >> + * by the hardware. These tables are part of the kernel ABI and >> + * may only be updated incrementally by adding entries at the >> + * end. >> + */ >> +static const struct drm_i915_mocs_entry skylake_mocs_table[] =3D { >> + /* { 0x00000009, 0x0010 } */ >> + { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(0) | >> + LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), >> + (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, >> + /* { 0x00000038, 0x0030 } */ >> + { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3)= | >> + LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), >> + (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, >> + /* { 0x0000003b, 0x0030 } */ >> + { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | >> + LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), >> + (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } >> +}; >> + >> +/* NOTE: the LE_TGT_CACHE is not used on Broxton */ >> +static const struct drm_i915_mocs_entry broxton_mocs_table[] =3D { >> + /* { 0x00000009, 0x0010 } */ >> + { (LE_CACHEABILITY(LE_UC) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(0) | >> + LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), >> + (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_UC)) }, >> + /* { 0x00000038, 0x0030 } */ >> + { (LE_CACHEABILITY(LE_PAGETABLE) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3)= | >> + LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), >> + (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) }, >> + /* { 0x0000003b, 0x0030 } */ >> + { (LE_CACHEABILITY(LE_WB) | LE_TGT_CACHE(LLC_ELLC) | LE_LRUM(3) | >> + LE_AOM(0) | LE_RSC(0) | LE_SCC(0) | LE_PFM(0) | LE_SCF(0)), >> + (L3_ESC(0) | L3_SCC(0) | L3_CACHEABILITY(L3_WB)) } >> +}; >> + >> +/** >> + * get_mocs_settings >> + * >> + * This function will return the values of the MOCS table that needs to >> + * be programmed for the platform. It will return the values that need >> + * to be programmed and if they need to be programmed. >> + * >> + * If the return values is false then the registers do not need program= ming. >> + */ >> +static bool get_mocs_settings(struct drm_device *dev, >> + struct drm_i915_mocs_table *table) >> +{ > you will get a warning from kernel 0-day that description is missing for= =20 > arguments and return value, I think it is better to add now or new=20 > warning gets added to the current list of warnings. > Fixed. >> + bool result =3D false; >> + >> + if (IS_SKYLAKE(dev)) { >> + table->size =3D ARRAY_SIZE(skylake_mocs_table); >> + table->table =3D skylake_mocs_table; >> + result =3D true; >> + } else if (IS_BROXTON(dev)) { >> + table->size =3D ARRAY_SIZE(broxton_mocs_table); >> + table->table =3D broxton_mocs_table; >> + result =3D true; >> + } else { >> + /* Platform that should have a MOCS table does not */ >> + WARN_ON(INTEL_INFO(dev)->gen >=3D 9); > if we use this kernel on future platforms before MOCS support is added,=20 > this prints WARNING every time context is initialized, I faced similar=20 > issue with WA batch and the comment that I received was that WARNING is=20 > not very useful here hence changed it to DRM_ERROR but no strong=20 > opinion, perhaps WARN_ONCE if you want to retain the warning. > Right, I've changed it locally to use WARN_ONCE() instead. >> + } >> + >> + return result; >> +} >> + >> +/** >> + * emit_mocs_control_table() - emit the mocs control table >> + * @req: Request to set up the MOCS table for. >> + * @table: The values to program into the control regs. >> + * @reg_base: The base for the engine that needs to be programmed. >> + * >> + * This function simply emits a MI_LOAD_REGISTER_IMM command for the >> + * given table starting at the given address. >> + * >> + * @return 0 on success, otherwise the error status. >> + */ >> +static int emit_mocs_control_table(struct drm_i915_gem_request *req, >> + const struct drm_i915_mocs_table *table, >> + u32 reg_base) >> +{ >> + struct intel_ringbuffer *ringbuf =3D req->ringbuf; >> + unsigned int index; >> + int ret; >> + >> + ret =3D intel_logical_ring_begin(req, 2 + 2 * GEN9_NUM_MOCS_ENTRIES); >> + if (ret) { >> + DRM_DEBUG("intel_logical_ring_begin failed %d\n", ret); >> + return ret; >> + } >> + >> + intel_logical_ring_emit(ringbuf, >> + MI_LOAD_REGISTER_IMM(GEN9_NUM_MOCS_ENTRIES)); >> + >> + for (index =3D 0; index < table->size; index++) { >> + intel_logical_ring_emit(ringbuf, reg_base + index * 4); >> + intel_logical_ring_emit(ringbuf, >> + table->table[index].control_value); >> + } > > intel_logical_ring_emit(ringbuf, MI_NOOP); > after the loop to make number of commands even. > Count also need to be updated. > It would be illegal to emit a MI_NOOP here in the middle of another command. It's already being done a few lines later BTW. >> + >> + /* >> + * Ok, now set the unused entries to uncached. These entries >> + * are officially undefined and no contract for the contents >> + * and settings is given for these entries. >> + * >> + * Entry 0 in the table is uncached - so we are just writing >> + * that value to all the used entries. >> + */ >> + for (; index < GEN9_NUM_MOCS_ENTRIES; index++) { >> + intel_logical_ring_emit(ringbuf, reg_base + index * 4); >> + intel_logical_ring_emit(ringbuf, table->table[0].control_value); >> + } >> + >> + intel_logical_ring_emit(ringbuf, MI_NOOP); >> + intel_logical_ring_advance(ringbuf); >> + >> + return 0; >> +} >> + >> +/** >> + * emit_mocs_l3cc_table() - emit the mocs control table >> + * @req: Request to set up the MOCS table for. >> + * @table: The values to program into the control regs. >> + * >> + * This function simply emits a MI_LOAD_REGISTER_IMM command for the >> + * given table starting at the given address. This register set is >> + * programmed in pairs. >> + * >> + * @return 0 on success, otherwise the error status. >> + */ >> +static int emit_mocs_l3cc_table(struct drm_i915_gem_request *req, >> + const struct drm_i915_mocs_table *table) >> +{ >> + struct intel_ringbuffer *ringbuf =3D req->ringbuf; >> + unsigned int count; >> + unsigned int i; >> + u32 value; >> + u32 filler =3D (table->table[0].l3cc_value & 0xffff) | >> + ((table->table[0].l3cc_value & 0xffff) << 16); >> + int ret; >> + >> + ret =3D intel_logical_ring_begin(req, 2 + GEN9_NUM_MOCS_ENTRIES); >> + if (ret) { >> + DRM_DEBUG("intel_logical_ring_begin failed %d\n", ret); >> + return ret; >> + } >> + >> + intel_logical_ring_emit(ringbuf, >> + MI_LOAD_REGISTER_IMM(GEN9_NUM_MOCS_ENTRIES / 2)); >> + >> + for (i =3D 0, count =3D 0; i < table->size / 2; i++, count +=3D 2) { >> + value =3D (table->table[count].l3cc_value & 0xffff) | >> + ((table->table[count + 1].l3cc_value & 0xffff) << 16); >> + >> + intel_logical_ring_emit(ringbuf, GEN9_LNCFCMOCS0 + i * 4); >> + intel_logical_ring_emit(ringbuf, value); >> + } >> + > same as above. > >> + if (table->size & 0x01) { >> + /* Odd table size - 1 left over */ >> + value =3D (table->table[count].l3cc_value & 0xffff) | >> + ((table->table[0].l3cc_value & 0xffff) << 16); >> + } else >> + value =3D filler; >> + >> + /* >> + * Now set the rest of the table to uncached - use entry 0 as >> + * this will be uncached. Leave the last pair uninitialised as >> + * they are reserved by the hardware. >> + */ >> + for (; i < GEN9_NUM_MOCS_ENTRIES / 2; i++) { >> + intel_logical_ring_emit(ringbuf, GEN9_LNCFCMOCS0 + i * 4); >> + intel_logical_ring_emit(ringbuf, value); >> + >> + value =3D filler; >> + } >> + >> + intel_logical_ring_emit(ringbuf, MI_NOOP); >> + intel_logical_ring_advance(ringbuf); >> + >> + return 0; >> +} >> + >> +/** >> + * intel_rcs_context_init_mocs() - program the MOCS register. >> + * >> + * @req: Request to set up the MOCS tables for. >> + * >> + * This function will emit a batch buffer with the values required for >> + * programming the MOCS register values for all the currently supported >> + * rings. >> + * >> + * These registers are partially stored in the RCS context, so they are >> + * emitted at the same time so that when a context is created these reg= isters >> + * are set up. These registers have to be emitted into the start of the >> + * context as setting the ELSP will re-init some of these registers back >> + * to the hw values. >> + * >> + * @return 0 on success, otherwise the error status. >> + */ >> +int intel_rcs_context_init_mocs(struct drm_i915_gem_request *req) >> +{ >> + struct drm_i915_mocs_table t; >> + int ret; >> + >> + if (get_mocs_settings(req->ring->dev, &t)) { > can be changed to "ret =3D get_mocs_settings();" to be consistent and=20 > handle error case first; otherwise in this case non-zero is treated as=20 > success case and zero as failure. > Maybe. The thing is there seems to be no error case here AFAICT. It's not an error for a pre-Gen9 platform not to have MOCS tables, and it's also not an error for other hardware to have MOCS tables to program. Apparently it was Peter's intention to make get_mocs_settings() return a boolean with no failure implications reporting whether the platform has applicable MOCS settings or not, which seems to make sense to me. > any sanity check on the received table, if at all required? > >> + /* Program the control registers */ >> + ret =3D emit_mocs_control_table(req, &t, GEN9_GFX_MOCS_0); >> + if (ret) >> + return ret; >> + >> + ret =3D emit_mocs_control_table(req, &t, GEN9_MFX0_MOCS_0); >> + if (ret) >> + return ret; >> + >> + ret =3D emit_mocs_control_table(req, &t, GEN9_MFX1_MOCS_0); >> + if (ret) >> + return ret; >> + >> + ret =3D emit_mocs_control_table(req, &t, GEN9_VEBOX_MOCS_0); >> + if (ret) >> + return ret; >> + >> + ret =3D emit_mocs_control_table(req, &t, GEN9_BLT_MOCS_0); >> + if (ret) >> + return ret; >> + >> + /* Now program the l3cc registers */ >> + ret =3D emit_mocs_l3cc_table(req, &t); >> + if (ret) >> + return ret; > > In case if we fail before setting tables for all rings, is that a valid=20 > setup? I mean do we need to revert the entries in those rings that are=20 > successfully setup? > Strictly speaking no, it wouldn't be a valid setup, but reverting back to the original entries wouldn't give a valid setup either, and most likely it would fail too because it would need to use basically the same path that just failed to re-program the original entries. >> + >> + DRM_DEBUG("MOCS: Table set in context.\n"); >> + } else { >> + DRM_DEBUG("MOCS: Table not supported on platform.\n"); > DRM_ERROR may be? > and it returns 0 in this case also as success; perhaps we can remove=20 > printing any message in this as we already WARN/DRM_ERROR in=20 > get_mocs_settings() or move it here and remove it that function. As explained earlier neither of these two conditions is an error, but the debug messages do seem a bit redundant, I've removed them locally. > > regards > Arun > Thanks. >> + } >> + >> + return 0; >> +} >> diff --git a/drivers/gpu/drm/i915/intel_mocs.h b/drivers/gpu/drm/i915/in= tel_mocs.h >> new file mode 100644 >> index 0000000..76e45b1 >> --- /dev/null >> +++ b/drivers/gpu/drm/i915/intel_mocs.h >> @@ -0,0 +1,57 @@ >> +/* >> + * Copyright (c) 2015 Intel Corporation >> + * >> + * Permission is hereby granted, free of charge, to any person obtainin= g a >> + * copy of this software and associated documentation files (the "Softw= are"), >> + * to deal in the Software without restriction, including without limit= ation >> + * the rights to use, copy, modify, merge, publish, distribute, sublice= nse, >> + * and/or sell copies of the Software, and to permit persons to whom the >> + * Software is furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice (including the= next >> + * paragraph) shall be included in all copies or substantial portions o= f the >> + * Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPR= ESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABIL= ITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT S= HALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR= OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARIS= ING FROM, >> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALIN= GS IN THE >> + * SOFTWARE. >> + */ >> + >> +#ifndef INTEL_MOCS_H >> +#define INTEL_MOCS_H >> + >> +/** >> + * DOC: Memory Objects Control State (MOCS) >> + * >> + * Motivation: >> + * In previous Gens the MOCS settings was a value that was set by user = land as >> + * part of the batch. In Gen9 this has changed to be a single table (pe= r ring) >> + * that all batches now reference by index instead of programming the M= OCS >> + * directly. >> + * >> + * The one wrinkle in this is that only PART of the MOCS tables are inc= luded >> + * in context (The GFX_MOCS_0 - GFX_MOCS_64 and the LNCFCMOCS0 - LNCFCM= OCS32 >> + * registers). The rest are not (the settings for the other rings). >> + * >> + * This table needs to be set at system start-up because the way the ta= ble >> + * interacts with the contexts and the GmmLib interface. >> + * >> + * >> + * Implementation: >> + * >> + * The tables (one per supported platform) are defined in intel_mocs.c >> + * and are programmed in the first batch after the context is loaded >> + * (with the hardware workarounds). This will then let the usual >> + * context handling keep the MOCS in step. >> + */ >> + >> +#include >> +#include "i915_drv.h" >> + >> +int intel_rcs_context_init_mocs(struct drm_i915_gem_request *req); >> + >> +#endif >> --=-=-=-- --==-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iF4EAREIAAYFAlWel58ACgkQg5k4nX1Sv1sddwEAmjGfaa1vKpvR9fmFUY/DXlRd IqaaId0XXo+mb7Ny2mMA/0q1xHLmGNMIaTVtu3wTodsE/UQXweYZ96dM21IkG8T7 =1/Ta -----END PGP SIGNATURE----- --==-=-=-- --===============0174554821== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============0174554821==--