| Message ID | 1486050166-32672-1-git-send-email-oscar.mateo@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 02, 2017 at 07:42:46AM -0800, Oscar Mateo wrote: > From: Michal Wajdeczko <michal.wajdeczko@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@intel.com> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com> Looks reasonable, Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Thu, Feb 02, 2017 at 07:42:46AM -0800, Oscar Mateo wrote: > From: Michal Wajdeczko <michal.wajdeczko@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@intel.com> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Signed-off-by: Oscar Mateo <oscar.mateo@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? > { > 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 > + 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 >
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@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@intel.com> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Signed-off-by: Oscar Mateo <oscar.mateo@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 >>
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) { 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); + 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);