[Intel-gfx] [PATCH] drm/i915/guc: Make intel_guc_send a function pointer

[Oscar Mateo]

On 02/03/2017 03:48 AM, Michal Wajdeczko wrote:
> On Thu, Feb 02, 2017 at 07:42:46AM -0800, Oscar Mateo wrote:
>> From: Michal Wajdeczko <michal.wajdeczko at intel.com>
>>
>> Prepare for an alternate GuC communication interface.
>>
>> v2: Make a few functions static and name them correctly while we are at it (Oscar)
>>
>> Signed-off-by: Michel Thierry <michel.thierry at intel.com>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_uc.c | 25 +++++++++++++++----------
>>   drivers/gpu/drm/i915/intel_uc.h |  9 ++++++++-
>>   2 files changed, 23 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
>> index c46bc85..0e45ef0 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -25,16 +25,11 @@
>>   #include "i915_drv.h"
>>   #include "intel_uc.h"
>>   
>> -void intel_uc_init_early(struct drm_i915_private *dev_priv)
>> -{
>> -	mutex_init(&dev_priv->guc.send_mutex);
>> -}
>> -
>>   /*
>>    * Read GuC command/status register (SOFT_SCRATCH_0)
>>    * Return true if it contains a response rather than a command
>>    */
>> -static bool intel_guc_recv(struct intel_guc *guc, u32 *status)
>> +static bool guc_recv(struct intel_guc *guc, u32 *status)
>>   {
>>   	struct drm_i915_private *dev_priv = guc_to_i915(guc);
>>   
>> @@ -43,7 +38,10 @@ static bool intel_guc_recv(struct intel_guc *guc, u32 *status)
>>   	return INTEL_GUC_RECV_IS_RESPONSE(val);
>>   }
>>   
>> -int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
>> +/*
>> + * This function implements the MMIO based host to GuC interface.
>> + */
>> +static int guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
> Btw, while this change is fine for now, what if in the future we will need
> MMIO based interface for some kind of OOB communication to GuC that bypasses
> alternate interface?
I see. So you'd prefer to have an intel_guc_send for normal 
communication with the GuC and an intel_guc_send_mmio for the few cases 
where you want to fallback to a MMIO style? I'll change this and resend.
>>   {
>>   	struct drm_i915_private *dev_priv = guc_to_i915(guc);
>>   	u32 status;
>> @@ -71,9 +69,9 @@ int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
>>   	 * up to that length of time, then switch to a slower sleep-wait loop.
>>   	 * No inte_guc_send command should ever take longer than 10ms.
>>   	 */
>> -	ret = wait_for_us(intel_guc_recv(guc, &status), 10);
>> +	ret = wait_for_us(guc_recv(guc, &status), 10);
>>   	if (ret)
>> -		ret = wait_for(intel_guc_recv(guc, &status), 10);
>> +		ret = wait_for(guc_recv(guc, &status), 10);
>>   	if (status != INTEL_GUC_STATUS_SUCCESS) {
>>   		/*
>>   		 * Either the GuC explicitly returned an error (which
>> @@ -98,6 +96,14 @@ int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
>>   	return ret;
>>   }
>>   
>> +void intel_uc_init_early(struct drm_i915_private *dev_priv)
>> +{
>> +	struct intel_guc *guc = &dev_priv->guc;
>> +
>> +	mutex_init(&dev_priv->guc.send_mutex);
> No need to use dev_priv here, you can refer to the guc directly.
>
> Regards,
> Michal
ACK. I'll fix it in the next version.

>> +	guc->send = guc_send_mmio;
>> +}
>> +
>>   int intel_guc_sample_forcewake(struct intel_guc *guc)
>>   {
>>   	struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> @@ -113,4 +119,3 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
>>   
>>   	return intel_guc_send(guc, action, ARRAY_SIZE(action));
>>   }
>> -
>> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
>> index d74f4d3..aed8653 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.h
>> +++ b/drivers/gpu/drm/i915/intel_uc.h
>> @@ -174,6 +174,9 @@ struct intel_guc {
>>   
>>   	/* To serialize the intel_guc_send actions */
>>   	struct mutex send_mutex;
>> +
>> +	/* GuC's FW specific send function */
>> +	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
>>   };
>>   
>>   struct intel_huc {
>> @@ -185,9 +188,13 @@ struct intel_huc {
>>   
>>   /* intel_uc.c */
>>   void intel_uc_init_early(struct drm_i915_private *dev_priv);
>> -int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len);
>>   int intel_guc_sample_forcewake(struct intel_guc *guc);
>>   
>> +static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
>> +{
>> +	return guc->send(guc, action, len);
>> +}
>> +
>>   /* intel_guc_loader.c */
>>   extern void intel_guc_init(struct drm_i915_private *dev_priv);
>>   extern int intel_guc_setup(struct drm_i915_private *dev_priv);
>> -- 
>> 1.9.1
>>