Re: [RFC 02/11] drm/i915: Introduce uevent for full GPU reset.

From: Tomas Elf <tomas.elf@intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Intel-GFX@Lists.FreeDesktop.Org
Subject: Re: [RFC 02/11] drm/i915: Introduce uevent for full GPU reset.
Date: Tue, 16 Jun 2015 16:43:55 +0100	[thread overview]
Message-ID: <5580443B.8000206@intel.com> (raw)
In-Reply-To: <20150616134357.GV23637@phenom.ffwll.local>

On 16/06/2015 14:43, Daniel Vetter wrote:
> On Mon, Jun 08, 2015 at 06:03:20PM +0100, Tomas Elf wrote:
>> The TDR ULT used to validate this patch series requires a special uevent for
>> full GPU resets in order to distinguish between different kinds of resets.
>>
>> Signed-off-by: Tomas Elf <tomas.elf@intel.com>
>
> Why duplicate the uevent we send out from i915_reset_and_wakeup? At least
> I can't spot what this gets us in addition to the existing one.
> -Daniel

Look at this line:
 >> +		reset_event[0] = kasprintf(GFP_KERNEL, "%s", "GPU RESET=0");

It doesn't exist in reset_and_wakeup (specifically, the "GPU RESET=0" 
part). It's a uevent that happens at the time of the actual GPU reset 
(GDRST register write). In the subsequent TDR commit we add another one 
to the point of the actual engine reset, which also includes information 
about what exact engine was reset.

The uevents in reset_and_wakeup only tell the user that an error has 
been detected and that some kind of reset has happened, these new 
uevents specify exactly what kind of reset has happened. This particular 
one on its own it's not very meaningful since there is only one 
supported form of reset at this point but once we add engine reset 
support it's useful to be able to discern the types of resets from each 
other (GPU reset, RCS engine reset, VCS engine reset, VCS2 engine reset, 
BCS engine reset, VECS engine reset).

Does that make sense?

Thanks,
Tomas

>
>> ---
>>   drivers/gpu/drm/i915/intel_uncore.c |   29 ++++++++++++++++++++++-------
>>   1 file changed, 22 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index d96d15f..770f526 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -1449,18 +1449,33 @@ static int gen6_do_reset(struct drm_device *dev)
>>
>>   int intel_gpu_reset(struct drm_device *dev)
>>   {
>> -	if (INTEL_INFO(dev)->gen >= 6)
>> -		return gen6_do_reset(dev);
>> +	int ret = -ENODEV;
>> +	int gen = INTEL_INFO(dev)->gen;
>> +
>> +	if (gen >= 6)
>> +		ret = gen6_do_reset(dev);
>>   	else if (IS_GEN5(dev))
>> -		return ironlake_do_reset(dev);
>> +		ret = ironlake_do_reset(dev);
>>   	else if (IS_G4X(dev))
>> -		return g4x_do_reset(dev);
>> +		ret = g4x_do_reset(dev);
>>   	else if (IS_G33(dev))
>> -		return g33_do_reset(dev);
>> +		ret = g33_do_reset(dev);
>>   	else if (INTEL_INFO(dev)->gen >= 3)
>> -		return i915_do_reset(dev);
>> +		ret = i915_do_reset(dev);
>>   	else
>> -		return -ENODEV;
>> +		WARN(1, "Full GPU reset not supported on gen %d\n", gen);
>> +
>> +	if (!ret) {
>> +		char *reset_event[2];
>> +
>> +		reset_event[1] = NULL;
>> +		reset_event[0] = kasprintf(GFP_KERNEL, "%s", "GPU RESET=0");
>> +		kobject_uevent_env(&dev->primary->kdev->kobj,
>> +				KOBJ_CHANGE, reset_event);
>> +		kfree(reset_event[0]);
>> +	}
>> +
>> +	return ret;
>>   }
>>
>>   void intel_uncore_check_errors(struct drm_device *dev)
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx