[Intel-gfx] [PATCH] drm/i915/guc: Sanitory checks for platform that dont have GuC

[Srivatsa, Anusha]

>-----Original Message-----
>From: Vivi, Rodrigo
>Sent: Friday, October 7, 2016 7:06 AM
>To: Zanoni, Paulo R <paulo.r.zanoni at intel.com>
>Cc: intel-gfx at lists.freedesktop.org; Srivatsa, Anusha
><anusha.srivatsa at intel.com>
>Subject: Re: [Intel-gfx] [PATCH] drm/i915/guc: Sanitory checks for platform that
>dont have GuC
>
>On Fri, 2016-10-07 at 10:41 -0300, Paulo Zanoni wrote:
>> Em Qua, 2016-10-05 às 16:50 -0700, Anusha Srivatsa escreveu:
>> > i915.enable_guc_loading/submission=2 forces the usage of GuC.
>> > For platforms that do not have a GuC, asking the kernel to use a GuC
>> > should not result in an error state. Do extra checks to see if the
>> > platform even has a GuC or not, regardless of the kernel parameter.
>>
>> I'm not exactly sure who did what here, but I think Rodrigo deserves
>> some credit for his initial work (and possibly debug). Maybe add:
>> Based-on-patch-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
>I just drop a quickly patch that day, but she did the work, specially debugging it
>after.

I agree with you Paulo. In fact I was thinking of an exact way of giving credit to both you and Rodrigo since the patch is inspired by his patch and I have taken your suggestion as well. Will update the patch with "Based-on". 
>>
>>
>> > Cc: Zanoni Paulo <paulo.r.zanoni at intel.com>
>>
>> I see the Cc tag here but no Cc mail header with my email on it, so
>> this email wasn't colored blue on my intel-gfx folder. Why didn't I
>> actually get Cc'd here? Did you use git-send-email? This may be worth
>> investigating since it may happen again in the future, and you want to
>> make sure people you Cc actually get Cc'd.

I used git send-email. Will investigate it. 
>>
>> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_guc_loader.c | 15 +++++++++------
>> >  1 file changed, 9 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
>> > b/drivers/gpu/drm/i915/intel_guc_loader.c
>> > index 7ace96b..15d2d53 100644
>> > --- a/drivers/gpu/drm/i915/intel_guc_loader.c
>> > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
>> > @@ -717,13 +717,16 @@ void intel_guc_init(struct drm_device *dev)
>> >  	struct drm_i915_private *dev_priv = to_i915(dev);
>> >  	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
>> >  	const char *fw_path;
>> > -
>>
>> Please do not remove the blank line above.
>>
>> > +	if (!HAS_GUC(dev)) {
>> > +		i915.enable_guc_loading = 0;
>> > +		i915.enable_guc_submission = 0;
>> > +	} else {
>> >  	/* A negative value means "use platform default" */
>>
>> Bad indentation here.
>>
>> > -	if (i915.enable_guc_loading < 0)
>> > -		i915.enable_guc_loading = HAS_GUC_UCODE(dev);
>> > -	if (i915.enable_guc_submission < 0)
>> > -		i915.enable_guc_submission = HAS_GUC_SCHED(dev);
>> > -
>>
>> Please also do not remove the blank line above.
>>
>> Otherwise, looks good.

Will fix indentation issue. Thank you.
>> > +		if (i915.enable_guc_loading < 0)
>> > +			i915.enable_guc_loading =
>> > HAS_GUC_UCODE(dev);
>> > +		if (i915.enable_guc_submission < 0)
>> > +			i915.enable_guc_submission =
>> > HAS_GUC_SCHED(dev);
>> > +	}
>> >  	if (!HAS_GUC_UCODE(dev)) {
>> >  		fw_path = NULL;
>> >  	} else if (IS_SKYLAKE(dev)) {