Re: drm/i915: Check GuC load status for Host to GuC action and SLPC status

["Kamble, Sagar A" <sagar.a.kamble@intel.com>]

Thanks for the review Deepak.

Have incorporated the changes and will send in next series.


On 8/20/2016 10:40 AM, Deepak S wrote:
>
>
> On 20/08/16 10:39 AM, Sagar Arun Kamble wrote:
>> Host to GuC actions should not be invoked when GuC isn't loaded hence
>> add early return in i915_guc_action if GuC load status is not SUCCESS.
>> Also, SLPC status has to be linked with GuC load status to make sure
>> SLPC actions get invoked when GuC is loaded.
>>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_guc_submission.c | 5 +++++
>>   drivers/gpu/drm/i915/intel_drv.h           | 4 ++++
>>   2 files changed, 9 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
>> b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index 680d9b4..27c937b 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -78,6 +78,8 @@ static inline bool host2guc_action_response(struct 
>> drm_i915_private *dev_priv,
>>   int i915_guc_action(struct intel_guc *guc, u32 *data, u32 len)
>>   {
>>       struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> +    struct intel_guc_fw *guc_fw = &guc->guc_fw;
>> +
> remove the blank line
>>       u32 status;
>>       int i;
>>       int ret;
>> @@ -85,6 +87,9 @@ int i915_guc_action(struct intel_guc *guc, u32 
>> *data, u32 len)
>>       if (WARN_ON(len < 1 || len > 15))
>>           return -EINVAL;
>>   +    if (WARN_ON(guc_fw->guc_fw_load_status != GUC_FIRMWARE_SUCCESS))
>> +        return -ENODEV;
>> +
>>       intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>>         dev_priv->guc.action_count += 1;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index c46d619..71936dc 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1694,8 +1694,12 @@ bool chv_phy_powergate_ch(struct 
>> drm_i915_private *dev_priv, enum dpio_phy phy,
>>     static inline int intel_slpc_active(struct drm_i915_private 
>> *dev_priv)
>>   {
>> +    struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
>>       int ret = 0;
>>   +    if (guc_fw->guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
>> +        return 0;
>> +
> Since we are initializing ret=0, I think can do "return ret" right?
>>       if (dev_priv->guc.slpc.vma && dev_priv->guc.slpc.enabled)
>>           ret = 1;
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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