On Mon, 2018-04-02 at 12:00 -0700, Pandiyan, Dhinakaran wrote: > > > On Mon, 2018-04-02 at 11:23 -0700, Rodrigo Vivi wrote: > > On Fri, Mar 30, 2018 at 03:23:32PM -0700, José Roberto de Souza > > wrote: > > > When PSR/PSR2 is enabled hardware can do aux ch transactions by > > > it > > > self. > > > Spec requires that PSR is inactive before do any aux ch > > > transaction > > > in HSW and BDW, for skl+ there is a aux ch mutex but the use is > > > not > > > recommended. > > > So exiting PSR/PSR2 and waiting the transition to inactive to > > > prevent > > > any concurrent access between hardware and software in aux ch > > > registers. > > Wouldn't the debugfs interface that you added to read sink PSR status > always show PSR as disabled? Or do you intend to set > intel_dp->exit_psr_on_aux_ch_xfer = true?
Yeah it will always be disabled, unless when the hack in 'drm/i915: Keep IGT PSR and frontbuffer tests functional' is set, Rodrigo asked to remove the debugfs file that was controlling the state of the hack and enable/disable it around CRC read so I could do that for the whole i915_edp_psr_status too. > > > > > > > > VLV and CHV hardware don't do any aux as software is responsible > > > to > > > do all the buffer tracking and it sends the wake up aux ch > > > message > > > to sink. > > > > > > > ahh cool... I get back what I said on some previous patch. > > I like where it is going, but... > > > > > BSpec: 7530, 799 and 7532 > > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com> > > > Cc: Rodrigo Vivi <rodrigo.v...@intel.com> > > > Signed-off-by: José Roberto de Souza <jose.so...@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_dp.c | 38 > > > ++++++++++++++++++++++++++++++++++++++ > > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > > drivers/gpu/drm/i915/intel_psr.c | 8 ++++++++ > > > 3 files changed, 48 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > > b/drivers/gpu/drm/i915/intel_dp.c > > > index 62f82c4298ac..fedee4e7ed24 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > @@ -1062,6 +1062,41 @@ static uint32_t > > > skl_get_aux_send_ctl(struct intel_dp *intel_dp, > > > DP_AUX_CH_CTL_SYNC_PULSE_SKL(32); > > > } > > > > > > +/** > > > + * intel_dp_aux_ch_get - Get total control of aux ch registers > > > + * > > > + * By exiting or disabling any hardware feature that can also > > > use the aux ch > > > + * registers at the same time as driver, this function will give > > > total control > > > + * of aux ch registers to driver. > > > + */ > > > +static void intel_dp_aux_ch_get(struct intel_dp *intel_dp) > > > +{ > > > + if (!intel_dp->exit_psr_on_aux_ch_xfer) > > > + return; > > > + > > > + intel_psr_activate_block_get(intel_dp); > > > + intel_psr_exit(intel_dp, true); > > > > decision on exit and activate should be inside the block_get and > > block_put, > > based on current state of the counters. > > > > > +} > > > + > > > +/** > > > + * intel_dp_aux_ch_put - Release aux ch registers control > > > + * > > > + * It is the intel_dp_aux_ch_get() counterpart. > > > + */ > > > +static void intel_dp_aux_ch_put(struct intel_dp *intel_dp) > > > +{ > > > + if (!intel_dp->exit_psr_on_aux_ch_xfer) > > > + return; > > > + > > > + intel_psr_activate_block_put(intel_dp); > > > + /* Usually more than just one aux ch transaction is > > > executed when > > > + * handling some event, activating PSR right way would > > > cause several > > > + * msecs of delay waiting PSR to exit for each aux ch > > > transaction, so > > > + * here asking it to be scheduled. > > > + */ > > > + intel_psr_activate(intel_dp, true); > > > +} > > > + > > > static int > > > intel_dp_aux_xfer(struct intel_dp *intel_dp, > > > const uint8_t *send, int send_bytes, > > > @@ -1101,6 +1136,8 @@ intel_dp_aux_xfer(struct intel_dp > > > *intel_dp, > > > > > > intel_dp_check_edp(intel_dp); > > > > > > + intel_dp_aux_ch_get(intel_dp); > > > + > > > /* Try to wait for any previous AUX channel activity */ > > > for (try = 0; try < 3; try++) { > > > status = I915_READ_NOTRACE(ch_ctl); > > > @@ -1223,6 +1260,7 @@ intel_dp_aux_xfer(struct intel_dp > > > *intel_dp, > > > > > > ret = recv_bytes; > > > out: > > > + intel_dp_aux_ch_put(intel_dp); > > > pm_qos_update_request(&dev_priv->pm_qos, > > > PM_QOS_DEFAULT_VALUE); > > > > > > if (vdd) > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > > b/drivers/gpu/drm/i915/intel_drv.h > > > index 020b96324135..177478f0b032 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -1139,6 +1139,8 @@ struct intel_dp { > > > > > > /* Displayport compliance testing */ > > > struct intel_dp_compliance compliance; > > > + > > > + bool exit_psr_on_aux_ch_xfer; > > > }; > > > > > > struct intel_lspcon { > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > > b/drivers/gpu/drm/i915/intel_psr.c > > > index 8702dbafb42d..f88f12246a23 100644 > > > --- a/drivers/gpu/drm/i915/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > > @@ -665,6 +665,13 @@ void intel_psr_enable(struct intel_dp > > > *intel_dp, > > > msecs_to_jiffies(intel_dp- > > > >panel_power_cycle_delay * 5)); > > > } > > > > > > + /* From all platforms that supports PSR/PSR2 this 2 is > > > the ones that > > > + * don't have restrictions about use of the aux ch while > > > PSR/PSR2 is > > > + * enabled. > > > + */ > > > + if (!(IS_VALLEYVIEW(dev_priv) || > > > IS_CHERRYVIEW(dev_priv))) > > > + intel_dp->exit_psr_on_aux_ch_xfer = true; > > > + > > > unlock: > > > mutex_unlock(&dev_priv->psr.lock); > > > } > > > @@ -732,6 +739,7 @@ void intel_psr_disable(struct intel_dp > > > *intel_dp, > > > > > > dev_priv->psr.disable_source(intel_dp); > > > > > > + intel_dp->exit_psr_on_aux_ch_xfer = false; > > > /* Disable PSR on Sink */ > > > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, 0); > > > > > > -- > > > 2.16.3 > > > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx