On Fri, Aug 7, 2015 at 12:21 PM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
> On Fri, Aug 07, 2015 at 10:21:34AM +0100, Michel Thierry wrote:
>> On 8/6/2015 11:00 PM, Daniel Vetter wrote:
>> >On Thu, Aug 6, 2015 at 10:17 PM, kbuild test robot
>> ><fengguang.wu@xxxxxxxxx> wrote:
>> >> 1070 if (IS_ENABLED(CONFIG_X86_32))
>> >> 1071 /* While we have a proliferation of size_t variables
>> >> 1072 * we cannot represent the full ppgtt size on 32bit,
>> >> 1073 * so limit it to the same size as the GGTT (currently
>> >> 1074 * 2GiB).
>> >> 1075 */
>> >> 1076 ppgtt->base.total = to_i915(ppgtt->base.dev)->gtt.base.total;
>> >> 1077 ppgtt->base.cleanup = gen8_ppgtt_cleanup;
>> >> 1078 ppgtt->base.allocate_va_range = gen8_alloc_va_range;
>> >> 1079 ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
>> >> 1080 ppgtt->base.clear_range = gen8_ppgtt_clear_range;
>> >> 1081 ppgtt->base.unbind_vma = ppgtt_unbind_vma;
>> >> 1082 ppgtt->base.bind_vma = ppgtt_bind_vma;
>> >> 1083
>> >> 1084 ppgtt->switch_mm = gen8_mm_switch;
>> >> 1085
>> >>>1086 ret = __pdp_init(false, &ppgtt->pdp);
>> >
>> >So the first argument of pdp_init ist struct drm_device *dev and yes
>> >the first thing it does is deref it.
>> >
>>
>> *dev is used only for I915_PDPES_PER_PDP/USES_FULL_48BIT_PPGTT,
>> which in this path is always false. I didn't expect kbuild to
>> complain. I'll change it with the other modifications I'm about to
>> send.
>
> Wrong. dev is never deferenced even though it is passed around.
Yeah I spotted that too right after sending out my mail.
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 41c5e7c9c8ab..37283d5a8a4a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2549,12 +2549,12 @@ struct drm_i915_cmd_table {
>
> #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6)
> #define HAS_LOGICAL_RING_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 8)
> -#define USES_PPGTT(dev) (i915.enable_ppgtt)
> -#define USES_FULL_PPGTT(dev) (i915.enable_ppgtt >= 2)
> +#define USES_PPGTT() (i915.enable_ppgtt)
> +#define USES_FULL_PPGTT() (i915.enable_ppgtt >= 2)
> #ifdef CONFIG_X86_64
> -# define USES_FULL_48BIT_PPGTT(dev) (i915.enable_ppgtt == 3)
> +# define USES_FULL_48BIT_PPGTT() (i915.enable_ppgtt == 3)
> #else
> -# define USES_FULL_48BIT_PPGTT(dev) false
> +# define USES_FULL_48BIT_PPGTT() false
> #endif
I'd vote to abolish all these macros and just add an enum for ppgtt modes:
enum i915_ppgtt_mode {
PPGTT_DISABLED = 0,
ALIASING_PPGTT = 1,
FULL_PPGTT = 2,
FULL_48B_PPGTT = 3 };
Then just open-code the checks. Of course separate patch from anything
else really.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
