On Tue, 10 Nov 2015, Wayne Boyer <[email protected]> wrote: > Beginning with SKL the DP Aux channel communication can be protected > using a built in H/W mutex. > > This patch provides an initial implementation for using that mutex. > The use is currently limited to protecting the sink crc request based > on feedback from the H/W designers indicating that using the mutex > for all aux channel communication is not recommended.
Please explain in the commit message what benefits this brings, and why it's a good idea. Just that something *can* be done in hardware does not mean that we should do it. BR, Jani. > > Cc: Rodrigo Vivi <[email protected]> > Signed-off-by: Wayne Boyer <[email protected]> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_reg.h | 5 ++++ > drivers/gpu/drm/i915/intel_dp.c | 52 > ++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 57 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index b12594b..98e991d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2604,6 +2604,7 @@ struct drm_i915_cmd_table { > > #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi) > #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg) > +#define HAS_AUX_MUTEX(dev) (INTEL_INFO(dev)->gen >= 9) > #define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev) || \ > IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \ > IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 8942532..a033e70 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4288,6 +4288,11 @@ enum skl_disp_power_wells { > #define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5) > #define DP_AUX_CH_CTL_SYNC_PULSE_SKL(c) ((c) - 1) > > +#define DP_AUX_MUTEX_A 0x6402C > +#define DP_AUX_MUTEX_B 0x6412C > +#define DP_AUX_MUTEX_ENABLE (1 << 31) > +#define DP_AUX_MUTEX_STATUS (1 << 30) > + > /* > * Computing GMCH M and N values for the Display Port link > * > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 2fad873..e9e1239 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -781,6 +781,47 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp > *intel_dp, > DP_AUX_CH_CTL_SYNC_PULSE_SKL(32); > } > > +static bool skl_aux_mutex(struct intel_dp *intel_dp, bool get) > +{ > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); > + struct drm_device *dev = intel_dig_port->base.base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + uint32_t aux_ch_mutex, status; > + int count = 0; > + > + if (!HAS_AUX_MUTEX(dev)) > + return false; > + > + /* > + * FIXME: determine actual aux channel > + * Hard coded to channel A for now to protect sink crc requests on eDP. > + */ > + aux_ch_mutex = DP_AUX_MUTEX_A; > + > + if (!get) { > + I915_WRITE(aux_ch_mutex, DP_AUX_MUTEX_ENABLE | > DP_AUX_MUTEX_STATUS); > + return false; > + } > + > + /* > + * The Bspec specifies waiting 500us between attempts to acquire the > + * mutex. Ten retries should be adequate to balance successfully > + * acquirng the mutex and spending too much time trying. > + */ > + while (count++ < 10) { > + I915_WRITE(aux_ch_mutex, DP_AUX_MUTEX_ENABLE); > + status = I915_READ(aux_ch_mutex); > + if (!(status & DP_AUX_MUTEX_STATUS)) > + return true; > + udelay(500); > + } > + > + return false; > +} > + > +#define skl_aux_mutex_get(dev) skl_aux_mutex(dev, true) > +#define skl_aux_mutex_put(dev) skl_aux_mutex(dev, false) > + > static int > intel_dp_aux_ch(struct intel_dp *intel_dp, > const uint8_t *send, int send_bytes, > @@ -4188,10 +4229,14 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 > *crc) > u8 buf; > int count, ret; > int attempts = 6; > + bool aux_mutex_acquired = false; > + > + aux_mutex_acquired = skl_aux_mutex_get(intel_dp); > > ret = intel_dp_sink_crc_start(intel_dp); > + > if (ret) > - return ret; > + goto release; > > do { > intel_wait_for_vblank(dev, intel_crtc->pipe); > @@ -4218,6 +4263,11 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 > *crc) > > stop: > intel_dp_sink_crc_stop(intel_dp); > + > +release: > + if (aux_mutex_acquired) > + aux_mutex_acquired = skl_aux_mutex_put(intel_dp); > + > return ret; > } -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
