Re: [PATCH] drm/i915: Split GPU commands definitions into separate header

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 13 Mar 2018, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Michal Wajdeczko (2018-03-13 11:21:21)
>> We should not mix MMIO with MI_INSTR definitions.
>> 
>> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>
>> +#define   MI_STORE_DWORD_INDEX_SHIFT 2
>> +/* Official intel docs are somewhat sloppy concerning MI_LOAD_REGISTER_IMM:
>
> Sanitize "/*\n" while we are here ?

How about adding spaces around operators and replacing (1 << N) with
BIT(N) while at it? Or in an immediate follow-up patch. If you're going
to cause conflicts, you could fix this stuff at minimum extra
disturbance.

BR,
Jani.

>
>> + * - Always issue a MI_NOOP _before_ the MI_LOAD_REGISTER_IMM - otherwise hw
>> + *   simply ignores the register load under certain conditions.
>> + * - One can actually load arbitrary many arbitrary registers: Simply issue x
>> + *   address/value pairs. Don't overdue it, though, x <= 2^4 must hold!
>> + */
>> +#define MI_LOAD_REGISTER_IMM(x)        MI_INSTR(0x22, 2*(x)-1)
>> +#define   MI_LRI_FORCE_POSTED          (1<<12)
>> +#define MI_STORE_REGISTER_MEM        MI_INSTR(0x24, 1)
>> +#define MI_STORE_REGISTER_MEM_GEN8   MI_INSTR(0x24, 2)
>> +#define   MI_SRM_LRM_GLOBAL_GTT                (1<<22)
>> +#define MI_FLUSH_DW            MI_INSTR(0x26, 1) /* for GEN6 */
>> +#define   MI_FLUSH_DW_STORE_INDEX      (1<<21)
>> +#define   MI_INVALIDATE_TLB            (1<<18)
>> +#define   MI_FLUSH_DW_OP_STOREDW       (1<<14)
>> +#define   MI_FLUSH_DW_OP_MASK          (3<<14)
>> +#define   MI_FLUSH_DW_NOTIFY           (1<<8)
>> +#define   MI_INVALIDATE_BSD            (1<<7)
>> +#define   MI_FLUSH_DW_USE_GTT          (1<<2)
>> +#define   MI_FLUSH_DW_USE_PPGTT                (0<<2)
>> +#define MI_LOAD_REGISTER_MEM      MI_INSTR(0x29, 1)
>> +#define MI_LOAD_REGISTER_MEM_GEN8  MI_INSTR(0x29, 2)
>> +#define MI_BATCH_BUFFER                MI_INSTR(0x30, 1)
>> +#define   MI_BATCH_NON_SECURE          (1)
>> +/* for snb/ivb/vlv this also means "batch in ppgtt" when ppgtt is enabled. */
>> +#define   MI_BATCH_NON_SECURE_I965     (1<<8)
>> +#define   MI_BATCH_PPGTT_HSW           (1<<8)
>> +#define   MI_BATCH_NON_SECURE_HSW      (1<<13)
>> +#define MI_BATCH_BUFFER_START  MI_INSTR(0x31, 0)
>> +#define   MI_BATCH_GTT             (2<<6) /* aliased with (1<<7) on gen4 */
>> +#define MI_BATCH_BUFFER_START_GEN8     MI_INSTR(0x31, 1)
>> +#define   MI_BATCH_RESOURCE_STREAMER (1<<10)
>
>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
>> index 81cdbbf..8f2c71a 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
>> @@ -7,7 +7,8 @@
>>  #include "i915_gem_batch_pool.h"
>>  #include "i915_gem_timeline.h"
>>  
>> -#include "i915_reg.h" /* FIXME split out i915_gpu_commands.h */
>> +#include "intel_gpu_commands.h"
>
> Alphabetical?
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx