Re: [Intel-gfx] [PATCH v2 1/1] drm/i915: Add and enable DP AUX CH mutex
On Wed, 2018-02-21 at 14:28 -0800, Rodrigo Vivi wrote:
> On Wed, Feb 21, 2018 at 01:12:08PM -0800, Souza, Jose wrote:
> > On Wed, 2018-02-21 at 12:45 -0800, Rodrigo Vivi wrote:
> > > On Tue, Feb 20, 2018 at 06:23:47PM -0800, José Roberto de Souza
> > > wrote:
> > > > When PSR/PSR2/GTC is enabled hardware can do AUX transactions
> > > > by it
> > > > self, so lets use the mutex register that is available in gen9+
> > > > to
> > > > avoid concurrent access by hardware and driver.
> > > >
> > > > Reference: https://01.org/sites/default/files/documentation/int
> > > > el-g
> > > > fx-prm-osrc-skl-vol12-display.pdf
> > > > Page 198 - AUX programming sequence
> > > >
> > >
> > > Cc: Ville
> > > Cc: DK
> > > > Cc: Rodrigo Vivi
> > > > Cc: Jani Nikula
> > > > Signed-off-by: José Roberto de Souza
> > > > ---
> > > > drivers/gpu/drm/i915/i915_reg.h | 9
> > > > drivers/gpu/drm/i915/intel_dp.c | 50
> > > >
> > > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > > 3 files changed, 60 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > index 1412abcb27d4..a62e3c1badab 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -5318,6 +5318,7 @@ enum {
> > > > #define _DPA_AUX_CH_DATA3 (dev_priv-
> > > > > info.display_mmio_offset + 0x6401c)
> > > >
> > > > #define _DPA_AUX_CH_DATA4 (dev_priv-
> > > > > info.display_mmio_offset + 0x64020)
> > > >
> > > > #define _DPA_AUX_CH_DATA5 (dev_priv-
> > > > > info.display_mmio_offset + 0x64024)
> > > >
> > > > +#define _DPA_AUX_CH_MUTEX (dev_priv-
> > > > > info.display_mmio_offset + 0x6402C)
> > > >
> > > >
> > > > #define _DPB_AUX_CH_CTL(dev_priv-
> > > > > info.display_mmio_offset + 0x64110)
> > > >
> > > > #define _DPB_AUX_CH_DATA1 (dev_priv-
> > > > > info.display_mmio_offset + 0x64114)
> > > >
> > > > @@ -5325,6 +5326,7 @@ enum {
> > > > #define _DPB_AUX_CH_DATA3 (dev_priv-
> > > > > info.display_mmio_offset + 0x6411c)
> > > >
> > > > #define _DPB_AUX_CH_DATA4 (dev_priv-
> > > > > info.display_mmio_offset + 0x64120)
> > > >
> > > > #define _DPB_AUX_CH_DATA5 (dev_priv-
> > > > > info.display_mmio_offset + 0x64124)
> > > >
> > > > +#define _DPB_AUX_CH_MUTEX (dev_priv-
> > > > > info.display_mmio_offset + 0x6412C)
> > > >
> > > >
> > > > #define _DPC_AUX_CH_CTL(dev_priv-
> > > > > info.display_mmio_offset + 0x64210)
> > > >
> > > > #define _DPC_AUX_CH_DATA1 (dev_priv-
> > > > > info.display_mmio_offset + 0x64214)
> > > >
> > > > @@ -5332,6 +5334,7 @@ enum {
> > > > #define _DPC_AUX_CH_DATA3 (dev_priv-
> > > > > info.display_mmio_offset + 0x6421c)
> > > >
> > > > #define _DPC_AUX_CH_DATA4 (dev_priv-
> > > > > info.display_mmio_offset + 0x64220)
> > > >
> > > > #define _DPC_AUX_CH_DATA5 (dev_priv-
> > > > > info.display_mmio_offset + 0x64224)
> > > >
> > > > +#define _DPC_AUX_CH_MUTEX (dev_priv-
> > > > > info.display_mmio_offset + 0x6422C)
> > > >
> > > >
> > > > #define _DPD_AUX_CH_CTL(dev_priv-
> > > > > info.display_mmio_offset + 0x64310)
> > > >
> > > > #define _DPD_AUX_CH_DATA1 (dev_priv-
> > > > > info.display_mmio_offset + 0x64314)
> > > >
> > > > @@ -5339,6 +5342,7 @@ enum {
> > > > #define _DPD_AUX_CH_DATA3 (dev_priv-
> > > > > info.display_mmio_offset + 0x6431c)
> > > >
> > > > #define _DPD_AUX_CH_DATA4 (dev_priv-
> > > > > info.display_mmio_offset + 0x64320)
> > > >
> > > > #define _DPD_AUX_CH_DATA5 (dev_priv-
> > > > > info.display_mmio_offset + 0x64324)
> > > >
> > > > +#define _DPD_AUX_CH_MUTEX (dev_priv-
> > > > > info.display_mmio_offset + 0x6432C)
> > > >
> > > >
> > > > #define _DPF_AUX_CH_CTL(dev_priv-
> > > > > info.display_mmio_offset + 0x64510)
> > > >
> > > > #define _DPF_AUX_CH_DATA1 (dev_priv-
> > > > > info.display_mmio_offset + 0x64514)
> > > >
> > > > @@ -5346,6 +5350,7 @@ enum {
> > > > #define _DPF_AUX_CH_DATA3 (dev_priv-
> > > > > info.display_mmio_offset + 0x6451c)
> > > >
> > > > #define _DPF_AUX_CH_DATA4 (dev_priv-
> > > > > info.display_mmio_offset + 0x64520)
> > > >
> > > > #define _DPF_AUX_CH_DATA5 (dev_priv-
> > > > > info.display_mmio_offset + 0x64524)
> > > >
> > > > +#define _DPF_AUX_CH_MUTEX (dev_priv-
> > > > > info.display_mmio_offset + 0x6452C)
> > > >
> > > >
> > > > #define DP_AUX_CH_CTL(port)_MMIO_PORT(port,
> > > > _DPA_AUX_CH_CTL, _DPB_AUX_CH_CTL)
> > > > #define DP_AUX_CH_DATA(port, i)_MMIO(_PORT(port,
> > > > _DPA_AUX_CH_DATA1, _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers
> > > > */
> > > > @@ -5378,6 +5383,10 @@ enum {
> > > > #define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
> > > > #define DP_AUX_CH_CTL_SYNC_PULSE_SKL(c) ((c) - 1)
> > > >
> > >
> > >
> > > I believe
Re: [Intel-gfx] [PATCH v2 1/1] drm/i915: Add and enable DP AUX CH mutex
On Thu, Feb 22, 2018 at 12:25:46PM -0800, Pandiyan, Dhinakaran wrote:
> On Thu, 2018-02-22 at 12:13 -0800, Rodrigo Vivi wrote:
> > On Thu, Feb 22, 2018 at 11:57:03AM -0800, Pandiyan, Dhinakaran wrote:
> > > On Thu, 2018-02-22 at 11:34 -0800, Rodrigo Vivi wrote:
> > > > On Thu, Feb 22, 2018 at 08:59:49PM +0200, Ville Syrjälä wrote:
> > > > > On Tue, Feb 20, 2018 at 06:23:47PM -0800, José Roberto de Souza wrote:
> > > > > > When PSR/PSR2/GTC is enabled hardware can do AUX transactions by it
> > > > > > self, so lets use the mutex register that is available in gen9+ to
> > > > > > avoid concurrent access by hardware and driver.
> > > > > >
> > > > > > Reference:
> > > > > > https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-skl-vol12-display.pdf
> > > > > > Page 198 - AUX programming sequence
> > > > >
> > > > > BTW since HSW/BDW don't have the mutex we should really be disabling
> > > > > PSR
> > > > > around aux ch usage on those platforms. Not sure how big of a locking
> > > > > inversion exercise it would be to make that happen though.
> > > >
> > > > True. But I believe that can be handled differently.
> > > > Probably with Aux power domains blocking PSR off "domain".
> > > > Something similar to DC off.
> > > >
> > > > I actually had patches for that lost somewhere... :(
> > > >
> > >
> > > Just a note, the spec explicitly says we should disable PSR around AUX
> > > transactions for BDW (and HSW iirc).
> > >
> > > About using the power domain infra, I am not quite sure. The _get() and
> > > _put() idiom makes sense when we want reference counting to handle
> > > multiple consumers that want PSR to be disabled. Does anything else need
> > > to enable/disable PSR?
> >
> > VLV/CHV vblanks?! :/
> >
>
> I keep forgetting VLV/CHV, sorry. This brings us back to the whole
> "calling blocking functions from atomic contexts" problem. Not sure how
> to handle that.
>
> Assuming we solve that problem, we should also probably avoid polluting
> the power domain code and add reference counting for PSR. However, since
> all this is not required for BDW/HSW, I am inclined towards a simpler
> solution for BDW/HSW AUX usage.
I agree. Then probably the simpler solution could than be applied on
VLV/CHV... who knows.
anyways separated from this patch here... let's keep this one clean.
>
>
> > >
> > >
> > > > >
> > > > > >
> > > > > > Cc: Rodrigo Vivi
> > > > > > Cc: Jani Nikula
> > > > > > Signed-off-by: José Roberto de Souza
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/i915_reg.h | 9
> > > > > > drivers/gpu/drm/i915/intel_dp.c | 50
> > > > > >
> > > > > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > > > > 3 files changed, 60 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > > > index 1412abcb27d4..a62e3c1badab 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > > > @@ -5318,6 +5318,7 @@ enum {
> > > > > > #define _DPA_AUX_CH_DATA3 (dev_priv->info.display_mmio_offset +
> > > > > > 0x6401c)
> > > > > > #define _DPA_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset +
> > > > > > 0x64020)
> > > > > > #define _DPA_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset +
> > > > > > 0x64024)
> > > > > > +#define _DPA_AUX_CH_MUTEX (dev_priv->info.display_mmio_offset +
> > > > > > 0x6402C)
> > > > > >
> > > > > > #define _DPB_AUX_CH_CTL
> > > > > > (dev_priv->info.display_mmio_offset + 0x64110)
> > > > > > #define _DPB_AUX_CH_DATA1 (dev_priv->info.display_mmio_offset +
> > > > > > 0x64114)
> > > > > > @@ -5325,6 +5326,7 @@ enum {
> > > > > > #define _DPB_AUX_CH_DATA3 (dev_priv->info.display_mmio_offset +
> > > > > > 0x6411c)
> > > > > > #define _DPB_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset +
> > > > > > 0x64120)
> > > > > > #define _DPB_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset +
> > > > > > 0x64124)
> > > > > > +#define _DPB_AUX_CH_MUTEX (dev_priv->info.display_mmio_offset +
> > > > > > 0x6412C)
> > > > > >
> > > > > > #define _DPC_AUX_CH_CTL
> > > > > > (dev_priv->info.display_mmio_offset + 0x64210)
> > > > > > #define _DPC_AUX_CH_DATA1 (dev_priv->info.display_mmio_offset +
> > > > > > 0x64214)
> > > > > > @@ -5332,6 +5334,7 @@ enum {
> > > > > > #define _DPC_AUX_CH_DATA3 (dev_priv->info.display_mmio_offset +
> > > > > > 0x6421c)
> > > > > > #define _DPC_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset +
> > > > > > 0x64220)
> > > > > > #define _DPC_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset +
> > > > > > 0x64224)
> > > > > > +#define _DPC_AUX_CH_MUTEX (dev_priv->info.display_mmio_offset +
> > > > > > 0x6422C)
> > > > > >
> > > > > > #define _DPD_AUX_CH_CTL
> > > > > > (dev_priv->info.display_mmio_offset + 0x64310)
> > > > > > #define _DPD_AUX_CH_DATA1 (dev_priv->info.display_mm
Re: [Intel-gfx] [PATCH v2 1/1] drm/i915: Add and enable DP AUX CH mutex
On Thu, 2018-02-22 at 12:13 -0800, Rodrigo Vivi wrote:
> On Thu, Feb 22, 2018 at 11:57:03AM -0800, Pandiyan, Dhinakaran wrote:
> > On Thu, 2018-02-22 at 11:34 -0800, Rodrigo Vivi wrote:
> > > On Thu, Feb 22, 2018 at 08:59:49PM +0200, Ville Syrjälä wrote:
> > > > On Tue, Feb 20, 2018 at 06:23:47PM -0800, José Roberto de Souza wrote:
> > > > > When PSR/PSR2/GTC is enabled hardware can do AUX transactions by it
> > > > > self, so lets use the mutex register that is available in gen9+ to
> > > > > avoid concurrent access by hardware and driver.
> > > > >
> > > > > Reference:
> > > > > https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-skl-vol12-display.pdf
> > > > > Page 198 - AUX programming sequence
> > > >
> > > > BTW since HSW/BDW don't have the mutex we should really be disabling PSR
> > > > around aux ch usage on those platforms. Not sure how big of a locking
> > > > inversion exercise it would be to make that happen though.
> > >
> > > True. But I believe that can be handled differently.
> > > Probably with Aux power domains blocking PSR off "domain".
> > > Something similar to DC off.
> > >
> > > I actually had patches for that lost somewhere... :(
> > >
> >
> > Just a note, the spec explicitly says we should disable PSR around AUX
> > transactions for BDW (and HSW iirc).
> >
> > About using the power domain infra, I am not quite sure. The _get() and
> > _put() idiom makes sense when we want reference counting to handle
> > multiple consumers that want PSR to be disabled. Does anything else need
> > to enable/disable PSR?
>
> VLV/CHV vblanks?! :/
>
I keep forgetting VLV/CHV, sorry. This brings us back to the whole
"calling blocking functions from atomic contexts" problem. Not sure how
to handle that.
Assuming we solve that problem, we should also probably avoid polluting
the power domain code and add reference counting for PSR. However, since
all this is not required for BDW/HSW, I am inclined towards a simpler
solution for BDW/HSW AUX usage.
> >
> >
> > > >
> > > > >
> > > > > Cc: Rodrigo Vivi
> > > > > Cc: Jani Nikula
> > > > > Signed-off-by: José Roberto de Souza
> > > > > ---
> > > > > drivers/gpu/drm/i915/i915_reg.h | 9
> > > > > drivers/gpu/drm/i915/intel_dp.c | 50
> > > > >
> > > > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > > > 3 files changed, 60 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > > index 1412abcb27d4..a62e3c1badab 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > > @@ -5318,6 +5318,7 @@ enum {
> > > > > #define _DPA_AUX_CH_DATA3(dev_priv->info.display_mmio_offset +
> > > > > 0x6401c)
> > > > > #define _DPA_AUX_CH_DATA4(dev_priv->info.display_mmio_offset +
> > > > > 0x64020)
> > > > > #define _DPA_AUX_CH_DATA5(dev_priv->info.display_mmio_offset +
> > > > > 0x64024)
> > > > > +#define _DPA_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset +
> > > > > 0x6402C)
> > > > >
> > > > > #define _DPB_AUX_CH_CTL
> > > > > (dev_priv->info.display_mmio_offset + 0x64110)
> > > > > #define _DPB_AUX_CH_DATA1(dev_priv->info.display_mmio_offset +
> > > > > 0x64114)
> > > > > @@ -5325,6 +5326,7 @@ enum {
> > > > > #define _DPB_AUX_CH_DATA3(dev_priv->info.display_mmio_offset +
> > > > > 0x6411c)
> > > > > #define _DPB_AUX_CH_DATA4(dev_priv->info.display_mmio_offset +
> > > > > 0x64120)
> > > > > #define _DPB_AUX_CH_DATA5(dev_priv->info.display_mmio_offset +
> > > > > 0x64124)
> > > > > +#define _DPB_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset +
> > > > > 0x6412C)
> > > > >
> > > > > #define _DPC_AUX_CH_CTL
> > > > > (dev_priv->info.display_mmio_offset + 0x64210)
> > > > > #define _DPC_AUX_CH_DATA1(dev_priv->info.display_mmio_offset +
> > > > > 0x64214)
> > > > > @@ -5332,6 +5334,7 @@ enum {
> > > > > #define _DPC_AUX_CH_DATA3(dev_priv->info.display_mmio_offset +
> > > > > 0x6421c)
> > > > > #define _DPC_AUX_CH_DATA4(dev_priv->info.display_mmio_offset +
> > > > > 0x64220)
> > > > > #define _DPC_AUX_CH_DATA5(dev_priv->info.display_mmio_offset +
> > > > > 0x64224)
> > > > > +#define _DPC_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset +
> > > > > 0x6422C)
> > > > >
> > > > > #define _DPD_AUX_CH_CTL
> > > > > (dev_priv->info.display_mmio_offset + 0x64310)
> > > > > #define _DPD_AUX_CH_DATA1(dev_priv->info.display_mmio_offset +
> > > > > 0x64314)
> > > > > @@ -5339,6 +5342,7 @@ enum {
> > > > > #define _DPD_AUX_CH_DATA3(dev_priv->info.display_mmio_offset +
> > > > > 0x6431c)
> > > > > #define _DPD_AUX_CH_DATA4(dev_priv->info.display_mmio_offset +
> > > > > 0x64320)
> > > > > #define _DPD_AUX_CH_DATA5(dev_priv->info.display_mmio_offset +
> > > > > 0x64324)
> > > > > +#define _DPD_AUX_CH_MUTEX(dev_priv->in
Re: [Intel-gfx] [PATCH v2 1/1] drm/i915: Add and enable DP AUX CH mutex
On Thu, Feb 22, 2018 at 11:57:03AM -0800, Pandiyan, Dhinakaran wrote:
> On Thu, 2018-02-22 at 11:34 -0800, Rodrigo Vivi wrote:
> > On Thu, Feb 22, 2018 at 08:59:49PM +0200, Ville Syrjälä wrote:
> > > On Tue, Feb 20, 2018 at 06:23:47PM -0800, José Roberto de Souza wrote:
> > > > When PSR/PSR2/GTC is enabled hardware can do AUX transactions by it
> > > > self, so lets use the mutex register that is available in gen9+ to
> > > > avoid concurrent access by hardware and driver.
> > > >
> > > > Reference:
> > > > https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-skl-vol12-display.pdf
> > > > Page 198 - AUX programming sequence
> > >
> > > BTW since HSW/BDW don't have the mutex we should really be disabling PSR
> > > around aux ch usage on those platforms. Not sure how big of a locking
> > > inversion exercise it would be to make that happen though.
> >
> > True. But I believe that can be handled differently.
> > Probably with Aux power domains blocking PSR off "domain".
> > Something similar to DC off.
> >
> > I actually had patches for that lost somewhere... :(
> >
>
> Just a note, the spec explicitly says we should disable PSR around AUX
> transactions for BDW (and HSW iirc).
>
> About using the power domain infra, I am not quite sure. The _get() and
> _put() idiom makes sense when we want reference counting to handle
> multiple consumers that want PSR to be disabled. Does anything else need
> to enable/disable PSR?
VLV/CHV vblanks?! :/
>
>
> > >
> > > >
> > > > Cc: Rodrigo Vivi
> > > > Cc: Jani Nikula
> > > > Signed-off-by: José Roberto de Souza
> > > > ---
> > > > drivers/gpu/drm/i915/i915_reg.h | 9
> > > > drivers/gpu/drm/i915/intel_dp.c | 50
> > > >
> > > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > > 3 files changed, 60 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > index 1412abcb27d4..a62e3c1badab 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -5318,6 +5318,7 @@ enum {
> > > > #define _DPA_AUX_CH_DATA3 (dev_priv->info.display_mmio_offset +
> > > > 0x6401c)
> > > > #define _DPA_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset +
> > > > 0x64020)
> > > > #define _DPA_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset +
> > > > 0x64024)
> > > > +#define _DPA_AUX_CH_MUTEX (dev_priv->info.display_mmio_offset +
> > > > 0x6402C)
> > > >
> > > > #define _DPB_AUX_CH_CTL
> > > > (dev_priv->info.display_mmio_offset + 0x64110)
> > > > #define _DPB_AUX_CH_DATA1 (dev_priv->info.display_mmio_offset +
> > > > 0x64114)
> > > > @@ -5325,6 +5326,7 @@ enum {
> > > > #define _DPB_AUX_CH_DATA3 (dev_priv->info.display_mmio_offset +
> > > > 0x6411c)
> > > > #define _DPB_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset +
> > > > 0x64120)
> > > > #define _DPB_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset +
> > > > 0x64124)
> > > > +#define _DPB_AUX_CH_MUTEX (dev_priv->info.display_mmio_offset +
> > > > 0x6412C)
> > > >
> > > > #define _DPC_AUX_CH_CTL
> > > > (dev_priv->info.display_mmio_offset + 0x64210)
> > > > #define _DPC_AUX_CH_DATA1 (dev_priv->info.display_mmio_offset +
> > > > 0x64214)
> > > > @@ -5332,6 +5334,7 @@ enum {
> > > > #define _DPC_AUX_CH_DATA3 (dev_priv->info.display_mmio_offset +
> > > > 0x6421c)
> > > > #define _DPC_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset +
> > > > 0x64220)
> > > > #define _DPC_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset +
> > > > 0x64224)
> > > > +#define _DPC_AUX_CH_MUTEX (dev_priv->info.display_mmio_offset +
> > > > 0x6422C)
> > > >
> > > > #define _DPD_AUX_CH_CTL
> > > > (dev_priv->info.display_mmio_offset + 0x64310)
> > > > #define _DPD_AUX_CH_DATA1 (dev_priv->info.display_mmio_offset +
> > > > 0x64314)
> > > > @@ -5339,6 +5342,7 @@ enum {
> > > > #define _DPD_AUX_CH_DATA3 (dev_priv->info.display_mmio_offset +
> > > > 0x6431c)
> > > > #define _DPD_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset +
> > > > 0x64320)
> > > > #define _DPD_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset +
> > > > 0x64324)
> > > > +#define _DPD_AUX_CH_MUTEX (dev_priv->info.display_mmio_offset +
> > > > 0x6432C)
> > > >
> > > > #define _DPF_AUX_CH_CTL
> > > > (dev_priv->info.display_mmio_offset + 0x64510)
> > > > #define _DPF_AUX_CH_DATA1 (dev_priv->info.display_mmio_offset +
> > > > 0x64514)
> > > > @@ -5346,6 +5350,7 @@ enum {
> > > > #define _DPF_AUX_CH_DATA3 (dev_priv->info.display_mmio_offset +
> > > > 0x6451c)
> > > > #define _DPF_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset +
> > > > 0x64520)
> > > > #define _DPF_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset +
> > > > 0x64524)
> > > > +#define _DPF_AUX_CH_MUTEX (dev
Re: [Intel-gfx] [PATCH v2 1/1] drm/i915: Add and enable DP AUX CH mutex
On Thu, 2018-02-22 at 11:34 -0800, Rodrigo Vivi wrote:
> On Thu, Feb 22, 2018 at 08:59:49PM +0200, Ville Syrjälä wrote:
> > On Tue, Feb 20, 2018 at 06:23:47PM -0800, José Roberto de Souza wrote:
> > > When PSR/PSR2/GTC is enabled hardware can do AUX transactions by it
> > > self, so lets use the mutex register that is available in gen9+ to
> > > avoid concurrent access by hardware and driver.
> > >
> > > Reference:
> > > https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-skl-vol12-display.pdf
> > > Page 198 - AUX programming sequence
> >
> > BTW since HSW/BDW don't have the mutex we should really be disabling PSR
> > around aux ch usage on those platforms. Not sure how big of a locking
> > inversion exercise it would be to make that happen though.
>
> True. But I believe that can be handled differently.
> Probably with Aux power domains blocking PSR off "domain".
> Something similar to DC off.
>
> I actually had patches for that lost somewhere... :(
>
Just a note, the spec explicitly says we should disable PSR around AUX
transactions for BDW (and HSW iirc).
About using the power domain infra, I am not quite sure. The _get() and
_put() idiom makes sense when we want reference counting to handle
multiple consumers that want PSR to be disabled. Does anything else need
to enable/disable PSR?
> >
> > >
> > > Cc: Rodrigo Vivi
> > > Cc: Jani Nikula
> > > Signed-off-by: José Roberto de Souza
> > > ---
> > > drivers/gpu/drm/i915/i915_reg.h | 9
> > > drivers/gpu/drm/i915/intel_dp.c | 50
> > >
> > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > 3 files changed, 60 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 1412abcb27d4..a62e3c1badab 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -5318,6 +5318,7 @@ enum {
> > > #define _DPA_AUX_CH_DATA3(dev_priv->info.display_mmio_offset +
> > > 0x6401c)
> > > #define _DPA_AUX_CH_DATA4(dev_priv->info.display_mmio_offset +
> > > 0x64020)
> > > #define _DPA_AUX_CH_DATA5(dev_priv->info.display_mmio_offset +
> > > 0x64024)
> > > +#define _DPA_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset +
> > > 0x6402C)
> > >
> > > #define _DPB_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> > > 0x64110)
> > > #define _DPB_AUX_CH_DATA1(dev_priv->info.display_mmio_offset +
> > > 0x64114)
> > > @@ -5325,6 +5326,7 @@ enum {
> > > #define _DPB_AUX_CH_DATA3(dev_priv->info.display_mmio_offset +
> > > 0x6411c)
> > > #define _DPB_AUX_CH_DATA4(dev_priv->info.display_mmio_offset +
> > > 0x64120)
> > > #define _DPB_AUX_CH_DATA5(dev_priv->info.display_mmio_offset +
> > > 0x64124)
> > > +#define _DPB_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset +
> > > 0x6412C)
> > >
> > > #define _DPC_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> > > 0x64210)
> > > #define _DPC_AUX_CH_DATA1(dev_priv->info.display_mmio_offset +
> > > 0x64214)
> > > @@ -5332,6 +5334,7 @@ enum {
> > > #define _DPC_AUX_CH_DATA3(dev_priv->info.display_mmio_offset +
> > > 0x6421c)
> > > #define _DPC_AUX_CH_DATA4(dev_priv->info.display_mmio_offset +
> > > 0x64220)
> > > #define _DPC_AUX_CH_DATA5(dev_priv->info.display_mmio_offset +
> > > 0x64224)
> > > +#define _DPC_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset +
> > > 0x6422C)
> > >
> > > #define _DPD_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> > > 0x64310)
> > > #define _DPD_AUX_CH_DATA1(dev_priv->info.display_mmio_offset +
> > > 0x64314)
> > > @@ -5339,6 +5342,7 @@ enum {
> > > #define _DPD_AUX_CH_DATA3(dev_priv->info.display_mmio_offset +
> > > 0x6431c)
> > > #define _DPD_AUX_CH_DATA4(dev_priv->info.display_mmio_offset +
> > > 0x64320)
> > > #define _DPD_AUX_CH_DATA5(dev_priv->info.display_mmio_offset +
> > > 0x64324)
> > > +#define _DPD_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset +
> > > 0x6432C)
> > >
> > > #define _DPF_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> > > 0x64510)
> > > #define _DPF_AUX_CH_DATA1(dev_priv->info.display_mmio_offset +
> > > 0x64514)
> > > @@ -5346,6 +5350,7 @@ enum {
> > > #define _DPF_AUX_CH_DATA3(dev_priv->info.display_mmio_offset +
> > > 0x6451c)
> > > #define _DPF_AUX_CH_DATA4(dev_priv->info.display_mmio_offset +
> > > 0x64520)
> > > #define _DPF_AUX_CH_DATA5(dev_priv->info.display_mmio_offset +
> > > 0x64524)
> > > +#define _DPF_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset +
> > > 0x6452C)
> > >
> > > #define DP_AUX_CH_CTL(port) _MMIO_PORT(port, _DPA_AUX_CH_CTL,
> > > _DPB_AUX_CH_CTL)
> > > #define DP_AUX_CH_DATA(port, i) _MMIO(_PORT(port, _DPA_AUX_CH_DATA1,
> > > _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
> > >
Re: [Intel-gfx] [PATCH v2 1/1] drm/i915: Add and enable DP AUX CH mutex
On Thu, Feb 22, 2018 at 08:59:49PM +0200, Ville Syrjälä wrote:
> On Tue, Feb 20, 2018 at 06:23:47PM -0800, José Roberto de Souza wrote:
> > When PSR/PSR2/GTC is enabled hardware can do AUX transactions by it
> > self, so lets use the mutex register that is available in gen9+ to
> > avoid concurrent access by hardware and driver.
> >
> > Reference:
> > https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-skl-vol12-display.pdf
> > Page 198 - AUX programming sequence
>
> BTW since HSW/BDW don't have the mutex we should really be disabling PSR
> around aux ch usage on those platforms. Not sure how big of a locking
> inversion exercise it would be to make that happen though.
True. But I believe that can be handled differently.
Probably with Aux power domains blocking PSR off "domain".
Something similar to DC off.
I actually had patches for that lost somewhere... :(
>
> >
> > Cc: Rodrigo Vivi
> > Cc: Jani Nikula
> > Signed-off-by: José Roberto de Souza
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 9
> > drivers/gpu/drm/i915/intel_dp.c | 50
> >
> > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > 3 files changed, 60 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 1412abcb27d4..a62e3c1badab 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5318,6 +5318,7 @@ enum {
> > #define _DPA_AUX_CH_DATA3 (dev_priv->info.display_mmio_offset + 0x6401c)
> > #define _DPA_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset + 0x64020)
> > #define _DPA_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset + 0x64024)
> > +#define _DPA_AUX_CH_MUTEX (dev_priv->info.display_mmio_offset + 0x6402C)
> >
> > #define _DPB_AUX_CH_CTL(dev_priv->info.display_mmio_offset +
> > 0x64110)
> > #define _DPB_AUX_CH_DATA1 (dev_priv->info.display_mmio_offset + 0x64114)
> > @@ -5325,6 +5326,7 @@ enum {
> > #define _DPB_AUX_CH_DATA3 (dev_priv->info.display_mmio_offset + 0x6411c)
> > #define _DPB_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset + 0x64120)
> > #define _DPB_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset + 0x64124)
> > +#define _DPB_AUX_CH_MUTEX (dev_priv->info.display_mmio_offset + 0x6412C)
> >
> > #define _DPC_AUX_CH_CTL(dev_priv->info.display_mmio_offset +
> > 0x64210)
> > #define _DPC_AUX_CH_DATA1 (dev_priv->info.display_mmio_offset + 0x64214)
> > @@ -5332,6 +5334,7 @@ enum {
> > #define _DPC_AUX_CH_DATA3 (dev_priv->info.display_mmio_offset + 0x6421c)
> > #define _DPC_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset + 0x64220)
> > #define _DPC_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset + 0x64224)
> > +#define _DPC_AUX_CH_MUTEX (dev_priv->info.display_mmio_offset + 0x6422C)
> >
> > #define _DPD_AUX_CH_CTL(dev_priv->info.display_mmio_offset +
> > 0x64310)
> > #define _DPD_AUX_CH_DATA1 (dev_priv->info.display_mmio_offset + 0x64314)
> > @@ -5339,6 +5342,7 @@ enum {
> > #define _DPD_AUX_CH_DATA3 (dev_priv->info.display_mmio_offset + 0x6431c)
> > #define _DPD_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset + 0x64320)
> > #define _DPD_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset + 0x64324)
> > +#define _DPD_AUX_CH_MUTEX (dev_priv->info.display_mmio_offset + 0x6432C)
> >
> > #define _DPF_AUX_CH_CTL(dev_priv->info.display_mmio_offset +
> > 0x64510)
> > #define _DPF_AUX_CH_DATA1 (dev_priv->info.display_mmio_offset + 0x64514)
> > @@ -5346,6 +5350,7 @@ enum {
> > #define _DPF_AUX_CH_DATA3 (dev_priv->info.display_mmio_offset + 0x6451c)
> > #define _DPF_AUX_CH_DATA4 (dev_priv->info.display_mmio_offset + 0x64520)
> > #define _DPF_AUX_CH_DATA5 (dev_priv->info.display_mmio_offset + 0x64524)
> > +#define _DPF_AUX_CH_MUTEX (dev_priv->info.display_mmio_offset + 0x6452C)
> >
> > #define DP_AUX_CH_CTL(port)_MMIO_PORT(port, _DPA_AUX_CH_CTL,
> > _DPB_AUX_CH_CTL)
> > #define DP_AUX_CH_DATA(port, i)_MMIO(_PORT(port, _DPA_AUX_CH_DATA1,
> > _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
> > @@ -5378,6 +5383,10 @@ enum {
> > #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_CH_MUTEX(port) _MMIO_PORT(port, _DPA_AUX_CH_MUTEX,
> > _DPB_AUX_CH_MUTEX)
> > +#define DP_AUX_CH_MUTEX_ENABLE (1 << 31)
> > +#define DP_AUX_CH_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 f20b25f98e5a..af07563bafba 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1081,6 +1081,45 @@ static uint32_t intel_dp_get_aux_send_ctl(struct
> > intel_dp *intel_dp,
> > aux_clock_divider);
> > }
> >
> > +st
Re: [Intel-gfx] [PATCH v2 1/1] drm/i915: Add and enable DP AUX CH mutex
On Tue, Feb 20, 2018 at 06:23:47PM -0800, José Roberto de Souza wrote:
> When PSR/PSR2/GTC is enabled hardware can do AUX transactions by it
> self, so lets use the mutex register that is available in gen9+ to
> avoid concurrent access by hardware and driver.
>
> Reference:
> https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-skl-vol12-display.pdf
> Page 198 - AUX programming sequence
BTW since HSW/BDW don't have the mutex we should really be disabling PSR
around aux ch usage on those platforms. Not sure how big of a locking
inversion exercise it would be to make that happen though.
>
> Cc: Rodrigo Vivi
> Cc: Jani Nikula
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/i915_reg.h | 9
> drivers/gpu/drm/i915/intel_dp.c | 50
>
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 3 files changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1412abcb27d4..a62e3c1badab 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5318,6 +5318,7 @@ enum {
> #define _DPA_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6401c)
> #define _DPA_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64020)
> #define _DPA_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64024)
> +#define _DPA_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6402C)
>
> #define _DPB_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> 0x64110)
> #define _DPB_AUX_CH_DATA1(dev_priv->info.display_mmio_offset + 0x64114)
> @@ -5325,6 +5326,7 @@ enum {
> #define _DPB_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6411c)
> #define _DPB_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64120)
> #define _DPB_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64124)
> +#define _DPB_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6412C)
>
> #define _DPC_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> 0x64210)
> #define _DPC_AUX_CH_DATA1(dev_priv->info.display_mmio_offset + 0x64214)
> @@ -5332,6 +5334,7 @@ enum {
> #define _DPC_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6421c)
> #define _DPC_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64220)
> #define _DPC_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64224)
> +#define _DPC_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6422C)
>
> #define _DPD_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> 0x64310)
> #define _DPD_AUX_CH_DATA1(dev_priv->info.display_mmio_offset + 0x64314)
> @@ -5339,6 +5342,7 @@ enum {
> #define _DPD_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6431c)
> #define _DPD_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64320)
> #define _DPD_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64324)
> +#define _DPD_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6432C)
>
> #define _DPF_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> 0x64510)
> #define _DPF_AUX_CH_DATA1(dev_priv->info.display_mmio_offset + 0x64514)
> @@ -5346,6 +5350,7 @@ enum {
> #define _DPF_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6451c)
> #define _DPF_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64520)
> #define _DPF_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64524)
> +#define _DPF_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6452C)
>
> #define DP_AUX_CH_CTL(port) _MMIO_PORT(port, _DPA_AUX_CH_CTL,
> _DPB_AUX_CH_CTL)
> #define DP_AUX_CH_DATA(port, i) _MMIO(_PORT(port, _DPA_AUX_CH_DATA1,
> _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
> @@ -5378,6 +5383,10 @@ enum {
> #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_CH_MUTEX(port)_MMIO_PORT(port, _DPA_AUX_CH_MUTEX,
> _DPB_AUX_CH_MUTEX)
> +#define DP_AUX_CH_MUTEX_ENABLE (1 << 31)
> +#define DP_AUX_CH_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 f20b25f98e5a..af07563bafba 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1081,6 +1081,45 @@ static uint32_t intel_dp_get_aux_send_ctl(struct
> intel_dp *intel_dp,
> aux_clock_divider);
> }
>
> +static bool intel_dp_aux_ch_trylock(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_i915_private *dev_priv =
> + to_i915(intel_dig_port->base.base.dev);
> +
> + if (INTEL_GEN(dev_priv) < 9)
> + return true;
> +
> + I915_WRITE(intel_dp->aux_ch_mutex_reg, DP_AUX_CH_MUTEX_ENABLE);
> +
Re: [Intel-gfx] [PATCH v2 1/1] drm/i915: Add and enable DP AUX CH mutex
On Wed, Feb 21, 2018 at 01:12:08PM -0800, Souza, Jose wrote:
> On Wed, 2018-02-21 at 12:45 -0800, Rodrigo Vivi wrote:
> > On Tue, Feb 20, 2018 at 06:23:47PM -0800, José Roberto de Souza
> > wrote:
> > > When PSR/PSR2/GTC is enabled hardware can do AUX transactions by it
> > > self, so lets use the mutex register that is available in gen9+ to
> > > avoid concurrent access by hardware and driver.
> > >
> > > Reference: https://01.org/sites/default/files/documentation/intel-g
> > > fx-prm-osrc-skl-vol12-display.pdf
> > > Page 198 - AUX programming sequence
> > >
> >
> > Cc: Ville
> > Cc: DK
> > > Cc: Rodrigo Vivi
> > > Cc: Jani Nikula
> > > Signed-off-by: José Roberto de Souza
> > > ---
> > > drivers/gpu/drm/i915/i915_reg.h | 9
> > > drivers/gpu/drm/i915/intel_dp.c | 50
> > >
> > > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > > 3 files changed, 60 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 1412abcb27d4..a62e3c1badab 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -5318,6 +5318,7 @@ enum {
> > > #define _DPA_AUX_CH_DATA3(dev_priv-
> > > >info.display_mmio_offset + 0x6401c)
> > > #define _DPA_AUX_CH_DATA4(dev_priv-
> > > >info.display_mmio_offset + 0x64020)
> > > #define _DPA_AUX_CH_DATA5(dev_priv-
> > > >info.display_mmio_offset + 0x64024)
> > > +#define _DPA_AUX_CH_MUTEX(dev_priv-
> > > >info.display_mmio_offset + 0x6402C)
> > >
> > > #define _DPB_AUX_CH_CTL (dev_priv-
> > > >info.display_mmio_offset + 0x64110)
> > > #define _DPB_AUX_CH_DATA1(dev_priv-
> > > >info.display_mmio_offset + 0x64114)
> > > @@ -5325,6 +5326,7 @@ enum {
> > > #define _DPB_AUX_CH_DATA3(dev_priv-
> > > >info.display_mmio_offset + 0x6411c)
> > > #define _DPB_AUX_CH_DATA4(dev_priv-
> > > >info.display_mmio_offset + 0x64120)
> > > #define _DPB_AUX_CH_DATA5(dev_priv-
> > > >info.display_mmio_offset + 0x64124)
> > > +#define _DPB_AUX_CH_MUTEX(dev_priv-
> > > >info.display_mmio_offset + 0x6412C)
> > >
> > > #define _DPC_AUX_CH_CTL (dev_priv-
> > > >info.display_mmio_offset + 0x64210)
> > > #define _DPC_AUX_CH_DATA1(dev_priv-
> > > >info.display_mmio_offset + 0x64214)
> > > @@ -5332,6 +5334,7 @@ enum {
> > > #define _DPC_AUX_CH_DATA3(dev_priv-
> > > >info.display_mmio_offset + 0x6421c)
> > > #define _DPC_AUX_CH_DATA4(dev_priv-
> > > >info.display_mmio_offset + 0x64220)
> > > #define _DPC_AUX_CH_DATA5(dev_priv-
> > > >info.display_mmio_offset + 0x64224)
> > > +#define _DPC_AUX_CH_MUTEX(dev_priv-
> > > >info.display_mmio_offset + 0x6422C)
> > >
> > > #define _DPD_AUX_CH_CTL (dev_priv-
> > > >info.display_mmio_offset + 0x64310)
> > > #define _DPD_AUX_CH_DATA1(dev_priv-
> > > >info.display_mmio_offset + 0x64314)
> > > @@ -5339,6 +5342,7 @@ enum {
> > > #define _DPD_AUX_CH_DATA3(dev_priv-
> > > >info.display_mmio_offset + 0x6431c)
> > > #define _DPD_AUX_CH_DATA4(dev_priv-
> > > >info.display_mmio_offset + 0x64320)
> > > #define _DPD_AUX_CH_DATA5(dev_priv-
> > > >info.display_mmio_offset + 0x64324)
> > > +#define _DPD_AUX_CH_MUTEX(dev_priv-
> > > >info.display_mmio_offset + 0x6432C)
> > >
> > > #define _DPF_AUX_CH_CTL (dev_priv-
> > > >info.display_mmio_offset + 0x64510)
> > > #define _DPF_AUX_CH_DATA1(dev_priv-
> > > >info.display_mmio_offset + 0x64514)
> > > @@ -5346,6 +5350,7 @@ enum {
> > > #define _DPF_AUX_CH_DATA3(dev_priv-
> > > >info.display_mmio_offset + 0x6451c)
> > > #define _DPF_AUX_CH_DATA4(dev_priv-
> > > >info.display_mmio_offset + 0x64520)
> > > #define _DPF_AUX_CH_DATA5(dev_priv-
> > > >info.display_mmio_offset + 0x64524)
> > > +#define _DPF_AUX_CH_MUTEX(dev_priv-
> > > >info.display_mmio_offset + 0x6452C)
> > >
> > > #define DP_AUX_CH_CTL(port) _MMIO_PORT(port,
> > > _DPA_AUX_CH_CTL, _DPB_AUX_CH_CTL)
> > > #define DP_AUX_CH_DATA(port, i) _MMIO(_PORT(port,
> > > _DPA_AUX_CH_DATA1, _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
> > > @@ -5378,6 +5383,10 @@ enum {
> > > #define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
> > > #define DP_AUX_CH_CTL_SYNC_PULSE_SKL(c) ((c) - 1)
> > >
> >
> >
> > I believe _DPA_AUX_CH_MUTEX and _DPB_AUX_CH_MUTEX should be defined
> > here along
> > with the bits as Jani suggested.
> > I don't see any reason to tag that far from the bits and close to CTL
> > and DATA.
> >
> > > +#define DP_AUX_CH_MUTEX(port)_MMIO_PORT(port,
> > > _DPA_AUX_CH_MUTEX, _DPB_AUX_CH_MUTEX)
> > > +#define DP_AUX_CH_MUTEX_ENABLE (1 << 31)
> > > +#define DP_AUX_CH_MUTEX_STATUS (1 << 30)
> > > +
> > > /*
> > > * Computing GMCH M and N values for the Display Port link
> > > *
> > > diff --git a/drivers/gpu/drm/i915/intel_dp
Re: [Intel-gfx] [PATCH v2 1/1] drm/i915: Add and enable DP AUX CH mutex
On Wed, 2018-02-21 at 12:45 -0800, Rodrigo Vivi wrote:
> On Tue, Feb 20, 2018 at 06:23:47PM -0800, José Roberto de Souza
> wrote:
> > When PSR/PSR2/GTC is enabled hardware can do AUX transactions by it
> > self, so lets use the mutex register that is available in gen9+ to
> > avoid concurrent access by hardware and driver.
> >
> > Reference: https://01.org/sites/default/files/documentation/intel-g
> > fx-prm-osrc-skl-vol12-display.pdf
> > Page 198 - AUX programming sequence
> >
>
> Cc: Ville
> Cc: DK
> > Cc: Rodrigo Vivi
> > Cc: Jani Nikula
> > Signed-off-by: José Roberto de Souza
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 9
> > drivers/gpu/drm/i915/intel_dp.c | 50
> >
> > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > 3 files changed, 60 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 1412abcb27d4..a62e3c1badab 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5318,6 +5318,7 @@ enum {
> > #define _DPA_AUX_CH_DATA3 (dev_priv-
> > >info.display_mmio_offset + 0x6401c)
> > #define _DPA_AUX_CH_DATA4 (dev_priv-
> > >info.display_mmio_offset + 0x64020)
> > #define _DPA_AUX_CH_DATA5 (dev_priv-
> > >info.display_mmio_offset + 0x64024)
> > +#define _DPA_AUX_CH_MUTEX (dev_priv-
> > >info.display_mmio_offset + 0x6402C)
> >
> > #define _DPB_AUX_CH_CTL(dev_priv-
> > >info.display_mmio_offset + 0x64110)
> > #define _DPB_AUX_CH_DATA1 (dev_priv-
> > >info.display_mmio_offset + 0x64114)
> > @@ -5325,6 +5326,7 @@ enum {
> > #define _DPB_AUX_CH_DATA3 (dev_priv-
> > >info.display_mmio_offset + 0x6411c)
> > #define _DPB_AUX_CH_DATA4 (dev_priv-
> > >info.display_mmio_offset + 0x64120)
> > #define _DPB_AUX_CH_DATA5 (dev_priv-
> > >info.display_mmio_offset + 0x64124)
> > +#define _DPB_AUX_CH_MUTEX (dev_priv-
> > >info.display_mmio_offset + 0x6412C)
> >
> > #define _DPC_AUX_CH_CTL(dev_priv-
> > >info.display_mmio_offset + 0x64210)
> > #define _DPC_AUX_CH_DATA1 (dev_priv-
> > >info.display_mmio_offset + 0x64214)
> > @@ -5332,6 +5334,7 @@ enum {
> > #define _DPC_AUX_CH_DATA3 (dev_priv-
> > >info.display_mmio_offset + 0x6421c)
> > #define _DPC_AUX_CH_DATA4 (dev_priv-
> > >info.display_mmio_offset + 0x64220)
> > #define _DPC_AUX_CH_DATA5 (dev_priv-
> > >info.display_mmio_offset + 0x64224)
> > +#define _DPC_AUX_CH_MUTEX (dev_priv-
> > >info.display_mmio_offset + 0x6422C)
> >
> > #define _DPD_AUX_CH_CTL(dev_priv-
> > >info.display_mmio_offset + 0x64310)
> > #define _DPD_AUX_CH_DATA1 (dev_priv-
> > >info.display_mmio_offset + 0x64314)
> > @@ -5339,6 +5342,7 @@ enum {
> > #define _DPD_AUX_CH_DATA3 (dev_priv-
> > >info.display_mmio_offset + 0x6431c)
> > #define _DPD_AUX_CH_DATA4 (dev_priv-
> > >info.display_mmio_offset + 0x64320)
> > #define _DPD_AUX_CH_DATA5 (dev_priv-
> > >info.display_mmio_offset + 0x64324)
> > +#define _DPD_AUX_CH_MUTEX (dev_priv-
> > >info.display_mmio_offset + 0x6432C)
> >
> > #define _DPF_AUX_CH_CTL(dev_priv-
> > >info.display_mmio_offset + 0x64510)
> > #define _DPF_AUX_CH_DATA1 (dev_priv-
> > >info.display_mmio_offset + 0x64514)
> > @@ -5346,6 +5350,7 @@ enum {
> > #define _DPF_AUX_CH_DATA3 (dev_priv-
> > >info.display_mmio_offset + 0x6451c)
> > #define _DPF_AUX_CH_DATA4 (dev_priv-
> > >info.display_mmio_offset + 0x64520)
> > #define _DPF_AUX_CH_DATA5 (dev_priv-
> > >info.display_mmio_offset + 0x64524)
> > +#define _DPF_AUX_CH_MUTEX (dev_priv-
> > >info.display_mmio_offset + 0x6452C)
> >
> > #define DP_AUX_CH_CTL(port)_MMIO_PORT(port,
> > _DPA_AUX_CH_CTL, _DPB_AUX_CH_CTL)
> > #define DP_AUX_CH_DATA(port, i)_MMIO(_PORT(port,
> > _DPA_AUX_CH_DATA1, _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
> > @@ -5378,6 +5383,10 @@ enum {
> > #define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
> > #define DP_AUX_CH_CTL_SYNC_PULSE_SKL(c) ((c) - 1)
> >
>
>
> I believe _DPA_AUX_CH_MUTEX and _DPB_AUX_CH_MUTEX should be defined
> here along
> with the bits as Jani suggested.
> I don't see any reason to tag that far from the bits and close to CTL
> and DATA.
>
> > +#define DP_AUX_CH_MUTEX(port) _MMIO_PORT(port,
> > _DPA_AUX_CH_MUTEX, _DPB_AUX_CH_MUTEX)
> > +#define DP_AUX_CH_MUTEX_ENABLE (1 << 31)
> > +#define DP_AUX_CH_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 f20b25f98e5a..af07563bafba 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1081,6 +1081,45 @@ static uint32_t
> > intel_dp_get_aux_send_ctl(struct intel_dp *intel_dp,
> > aux_clock_divider)
> > ;
> > }
> >
> > +static bool intel_dp_aux_ch_trylock(struct intel_dp *intel_dp)
> >
Re: [Intel-gfx] [PATCH v2 1/1] drm/i915: Add and enable DP AUX CH mutex
On Tue, Feb 20, 2018 at 06:23:47PM -0800, José Roberto de Souza wrote:
> When PSR/PSR2/GTC is enabled hardware can do AUX transactions by it
> self, so lets use the mutex register that is available in gen9+ to
> avoid concurrent access by hardware and driver.
>
> Reference:
> https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-skl-vol12-display.pdf
> Page 198 - AUX programming sequence
>
Cc: Ville
Cc: DK
> Cc: Rodrigo Vivi
> Cc: Jani Nikula
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/i915_reg.h | 9
> drivers/gpu/drm/i915/intel_dp.c | 50
>
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> 3 files changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1412abcb27d4..a62e3c1badab 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5318,6 +5318,7 @@ enum {
> #define _DPA_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6401c)
> #define _DPA_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64020)
> #define _DPA_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64024)
> +#define _DPA_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6402C)
>
> #define _DPB_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> 0x64110)
> #define _DPB_AUX_CH_DATA1(dev_priv->info.display_mmio_offset + 0x64114)
> @@ -5325,6 +5326,7 @@ enum {
> #define _DPB_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6411c)
> #define _DPB_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64120)
> #define _DPB_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64124)
> +#define _DPB_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6412C)
>
> #define _DPC_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> 0x64210)
> #define _DPC_AUX_CH_DATA1(dev_priv->info.display_mmio_offset + 0x64214)
> @@ -5332,6 +5334,7 @@ enum {
> #define _DPC_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6421c)
> #define _DPC_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64220)
> #define _DPC_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64224)
> +#define _DPC_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6422C)
>
> #define _DPD_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> 0x64310)
> #define _DPD_AUX_CH_DATA1(dev_priv->info.display_mmio_offset + 0x64314)
> @@ -5339,6 +5342,7 @@ enum {
> #define _DPD_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6431c)
> #define _DPD_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64320)
> #define _DPD_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64324)
> +#define _DPD_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6432C)
>
> #define _DPF_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> 0x64510)
> #define _DPF_AUX_CH_DATA1(dev_priv->info.display_mmio_offset + 0x64514)
> @@ -5346,6 +5350,7 @@ enum {
> #define _DPF_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6451c)
> #define _DPF_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64520)
> #define _DPF_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64524)
> +#define _DPF_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6452C)
>
> #define DP_AUX_CH_CTL(port) _MMIO_PORT(port, _DPA_AUX_CH_CTL,
> _DPB_AUX_CH_CTL)
> #define DP_AUX_CH_DATA(port, i) _MMIO(_PORT(port, _DPA_AUX_CH_DATA1,
> _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
> @@ -5378,6 +5383,10 @@ enum {
> #define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
> #define DP_AUX_CH_CTL_SYNC_PULSE_SKL(c) ((c) - 1)
>
I believe _DPA_AUX_CH_MUTEX and _DPB_AUX_CH_MUTEX should be defined here along
with the bits as Jani suggested.
I don't see any reason to tag that far from the bits and close to CTL and DATA.
> +#define DP_AUX_CH_MUTEX(port)_MMIO_PORT(port, _DPA_AUX_CH_MUTEX,
> _DPB_AUX_CH_MUTEX)
> +#define DP_AUX_CH_MUTEX_ENABLE (1 << 31)
> +#define DP_AUX_CH_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 f20b25f98e5a..af07563bafba 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1081,6 +1081,45 @@ static uint32_t intel_dp_get_aux_send_ctl(struct
> intel_dp *intel_dp,
> aux_clock_divider);
> }
>
> +static bool intel_dp_aux_ch_trylock(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_i915_private *dev_priv =
> + to_i915(intel_dig_port->base.base.dev);
> +
> + if (INTEL_GEN(dev_priv) < 9)
> + return true;
> +
> + I915_WRITE(intel_dp->aux_ch_mutex_reg, DP_AUX_CH_MUTEX_ENABLE)
Re: [Intel-gfx] [PATCH v2 1/1] drm/i915: Add and enable DP AUX CH mutex
On Wed, 2018-02-21 at 00:59 -0800, Dhinakaran Pandiyan wrote:
> On Tuesday, February 20, 2018 6:23:47 PM PST José Roberto de Souza
> wrote:
> > When PSR/PSR2/GTC is enabled hardware can do AUX transactions by it
> > self, so lets use the mutex register that is available in gen9+ to
> > avoid concurrent access by hardware and driver.
> >
> > Reference:
> > https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc
> > -skl-vol
> > 12-display.pdf Page 198 - AUX programming sequence
> >
> > Cc: Rodrigo Vivi
> > Cc: Jani Nikula
> > Signed-off-by: José Roberto de Souza
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 9
> > drivers/gpu/drm/i915/intel_dp.c | 50
> >
> > drivers/gpu/drm/i915/intel_drv.h |
> > 1 +
> > 3 files changed, 60 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 1412abcb27d4..a62e3c1badab
> > 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -5318,6 +5318,7 @@ enum {
> > #define _DPA_AUX_CH_DATA3 (dev_priv-
> > >info.display_mmio_offset + 0x6401c)
> > #define _DPA_AUX_CH_DATA4 (dev_priv-
> > >info.display_mmio_offset + 0x64020)
> > #define _DPA_AUX_CH_DATA5 (dev_priv-
> > >info.display_mmio_offset + 0x64024)
> > +#define _DPA_AUX_CH_MUTEX (dev_priv-
> > >info.display_mmio_offset + 0x6402C)
> >
> > #define _DPB_AUX_CH_CTL(dev_priv-
> > >info.display_mmio_offset + 0x64110)
> > #define _DPB_AUX_CH_DATA1 (dev_priv-
> > >info.display_mmio_offset + 0x64114)
> > @@ -5325,6 +5326,7 @@ enum {
> > #define _DPB_AUX_CH_DATA3 (dev_priv-
> > >info.display_mmio_offset + 0x6411c)
> > #define _DPB_AUX_CH_DATA4 (dev_priv-
> > >info.display_mmio_offset + 0x64120)
> > #define _DPB_AUX_CH_DATA5 (dev_priv-
> > >info.display_mmio_offset + 0x64124)
> > +#define _DPB_AUX_CH_MUTEX (dev_priv-
> > >info.display_mmio_offset + 0x6412C)
> >
> > #define _DPC_AUX_CH_CTL(dev_priv-
> > >info.display_mmio_offset + 0x64210)
> > #define _DPC_AUX_CH_DATA1 (dev_priv-
> > >info.display_mmio_offset + 0x64214)
> > @@ -5332,6 +5334,7 @@ enum {
> > #define _DPC_AUX_CH_DATA3 (dev_priv-
> > >info.display_mmio_offset + 0x6421c)
> > #define _DPC_AUX_CH_DATA4 (dev_priv-
> > >info.display_mmio_offset + 0x64220)
> > #define _DPC_AUX_CH_DATA5 (dev_priv-
> > >info.display_mmio_offset + 0x64224)
> > +#define _DPC_AUX_CH_MUTEX (dev_priv-
> > >info.display_mmio_offset + 0x6422C)
> >
> > #define _DPD_AUX_CH_CTL(dev_priv-
> > >info.display_mmio_offset + 0x64310)
> > #define _DPD_AUX_CH_DATA1 (dev_priv-
> > >info.display_mmio_offset + 0x64314)
> > @@ -5339,6 +5342,7 @@ enum {
> > #define _DPD_AUX_CH_DATA3 (dev_priv-
> > >info.display_mmio_offset + 0x6431c)
> > #define _DPD_AUX_CH_DATA4 (dev_priv-
> > >info.display_mmio_offset + 0x64320)
> > #define _DPD_AUX_CH_DATA5 (dev_priv-
> > >info.display_mmio_offset + 0x64324)
> > +#define _DPD_AUX_CH_MUTEX (dev_priv-
> > >info.display_mmio_offset + 0x6432C)
> >
> > #define _DPF_AUX_CH_CTL(dev_priv-
> > >info.display_mmio_offset + 0x64510)
> > #define _DPF_AUX_CH_DATA1 (dev_priv-
> > >info.display_mmio_offset + 0x64514)
> > @@ -5346,6 +5350,7 @@ enum {
> > #define _DPF_AUX_CH_DATA3 (dev_priv-
> > >info.display_mmio_offset + 0x6451c)
> > #define _DPF_AUX_CH_DATA4 (dev_priv-
> > >info.display_mmio_offset + 0x64520)
> > #define _DPF_AUX_CH_DATA5 (dev_priv-
> > >info.display_mmio_offset + 0x64524)
> > +#define _DPF_AUX_CH_MUTEX (dev_priv-
> > >info.display_mmio_offset + 0x6452C)
> >
> > #define DP_AUX_CH_CTL(port)_MMIO_PORT(port,
> > _DPA_AUX_CH_CTL,
> > _DPB_AUX_CH_CTL) #define DP_AUX_CH_DATA(port, i)_MMIO(_PORT
> > (port,
> > _DPA_AUX_CH_DATA1, _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */
> > @@
> > -5378,6 +5383,10 @@ enum {
> > #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_CH_MUTEX(port) _MMIO_PORT(port,
> > _DPA_AUX_CH_MUTEX,
> > _DPB_AUX_CH_MUTEX) +#define DP_AUX_CH_MUTEX_ENABLE
> > (1 << 31)
> > +#define DP_AUX_CH_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 f20b25f98e5a..af07563bafba
> > 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1081,6 +1081,45 @@ static uint32_t
> > intel_dp_get_aux_send_ctl(struct
> > intel_dp *intel_dp, aux_clock_divider);
> > }
> >
> > +static bool intel_dp_aux_ch_trylock(struct intel_dp *intel_dp)
> > +{
> > + struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > + struct drm_i915_private *dev_priv =
> > + to_i915(intel_dig_port->base.base.dev);
> > +
> > + if (INTEL_GEN(dev_priv) < 9)
> > + re
Re: [Intel-gfx] [PATCH v2 1/1] drm/i915: Add and enable DP AUX CH mutex
On Tuesday, February 20, 2018 6:23:47 PM PST José Roberto de Souza wrote:
> When PSR/PSR2/GTC is enabled hardware can do AUX transactions by it
> self, so lets use the mutex register that is available in gen9+ to
> avoid concurrent access by hardware and driver.
>
> Reference:
> https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-skl-vol
> 12-display.pdf Page 198 - AUX programming sequence
>
> Cc: Rodrigo Vivi
> Cc: Jani Nikula
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/i915_reg.h | 9
> drivers/gpu/drm/i915/intel_dp.c | 50
> drivers/gpu/drm/i915/intel_drv.h |
> 1 +
> 3 files changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index 1412abcb27d4..a62e3c1badab 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5318,6 +5318,7 @@ enum {
> #define _DPA_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6401c)
> #define _DPA_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64020)
> #define _DPA_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64024)
> +#define _DPA_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6402C)
>
> #define _DPB_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> 0x64110)
> #define _DPB_AUX_CH_DATA1(dev_priv->info.display_mmio_offset + 0x64114)
> @@ -5325,6 +5326,7 @@ enum {
> #define _DPB_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6411c)
> #define _DPB_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64120)
> #define _DPB_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64124)
> +#define _DPB_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6412C)
>
> #define _DPC_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> 0x64210)
> #define _DPC_AUX_CH_DATA1(dev_priv->info.display_mmio_offset + 0x64214)
> @@ -5332,6 +5334,7 @@ enum {
> #define _DPC_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6421c)
> #define _DPC_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64220)
> #define _DPC_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64224)
> +#define _DPC_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6422C)
>
> #define _DPD_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> 0x64310)
> #define _DPD_AUX_CH_DATA1(dev_priv->info.display_mmio_offset + 0x64314)
> @@ -5339,6 +5342,7 @@ enum {
> #define _DPD_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6431c)
> #define _DPD_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64320)
> #define _DPD_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64324)
> +#define _DPD_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6432C)
>
> #define _DPF_AUX_CH_CTL (dev_priv->info.display_mmio_offset +
> 0x64510)
> #define _DPF_AUX_CH_DATA1(dev_priv->info.display_mmio_offset + 0x64514)
> @@ -5346,6 +5350,7 @@ enum {
> #define _DPF_AUX_CH_DATA3(dev_priv->info.display_mmio_offset + 0x6451c)
> #define _DPF_AUX_CH_DATA4(dev_priv->info.display_mmio_offset + 0x64520)
> #define _DPF_AUX_CH_DATA5(dev_priv->info.display_mmio_offset + 0x64524)
> +#define _DPF_AUX_CH_MUTEX(dev_priv->info.display_mmio_offset + 0x6452C)
>
> #define DP_AUX_CH_CTL(port) _MMIO_PORT(port, _DPA_AUX_CH_CTL,
> _DPB_AUX_CH_CTL) #define DP_AUX_CH_DATA(port, i) _MMIO(_PORT(port,
> _DPA_AUX_CH_DATA1, _DPB_AUX_CH_DATA1) + (i) * 4) /* 5 registers */ @@
> -5378,6 +5383,10 @@ enum {
> #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_CH_MUTEX(port)_MMIO_PORT(port, _DPA_AUX_CH_MUTEX,
> _DPB_AUX_CH_MUTEX) +#define DP_AUX_CH_MUTEX_ENABLE (1 << 31)
> +#define DP_AUX_CH_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 f20b25f98e5a..af07563bafba 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1081,6 +1081,45 @@ static uint32_t intel_dp_get_aux_send_ctl(struct
> intel_dp *intel_dp, aux_clock_divider);
> }
>
> +static bool intel_dp_aux_ch_trylock(struct intel_dp *intel_dp)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct drm_i915_private *dev_priv =
> + to_i915(intel_dig_port->base.base.dev);
> +
> + if (INTEL_GEN(dev_priv) < 9)
> + return true;
> +
> + I915_WRITE(intel_dp->aux_ch_mutex_reg, DP_AUX_CH_MUTEX_ENABLE);
> +
> + /* Spec says that mutex is acquired when status bit is read as unset,
> + * if set it should be read again after 500usec. Here waiting for
> + * 2msec or 4 tries before give up.
> + */
> + if (__intel_wait_for_register(dev_priv, intel_dp->aux_
