Re: [Intel-gfx] [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
On Mon, Oct 22, 2018 at 09:26:06PM +0300, Ville Syrjälä wrote:
> On Mon, Oct 22, 2018 at 06:04:28PM +, Srivatsa, Anusha wrote:
> >
> >
> > >-Original Message-
> > >From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> > >Sent: Friday, October 19, 2018 4:12 PM
> > >To: Navare, Manasi D
> > >Cc: Srivatsa, Anusha ; intel-
> > >g...@lists.freedesktop.org; Singh, Gaurav K ;
> > >Jani
> > >Nikula
> > >Subject: Re: [v2 5/6] i915/dp/fec: Configure the Forward Error Correction
> > >bits.
> > >
> > >On Fri, Oct 19, 2018 at 01:29:05PM -0700, Manasi Navare wrote:
> > >> On Fri, Oct 19, 2018 at 10:58:29PM +0300, Ville Syrjälä wrote:
> > >> > On Fri, Oct 19, 2018 at 12:24:43PM -0700, Manasi Navare wrote:
> > >> > > On Fri, Oct 19, 2018 at 09:39:11PM +0300, Ville Syrjälä wrote:
> > >> > > > On Mon, Oct 15, 2018 at 02:50:36PM -0700, Anusha Srivatsa wrote:
> > >> > > > > If FEC is supported, the corresponding DP_TP_CTL register bits
> > >> > > > > have to be configured.
> > >> > > > >
> > >> > > > > The driver has to program the FEC_ENABLE in DP_TP_CTL[30]
> > >> > > > > register and wait till FEC_STATUS in DP_TP_CTL[28] is 1.
> > >> > > > > Also add the warn message to make sure that the control
> > >> > > > > register is already active while enabling FEC.
> > >> > > > >
> > >> > > > > v2:
> > >> > > > > - Change commit message. Configure fec state after
> > >> > > > > link training (Manasi, Gaurav)
> > >> > > > > - Remove redundent checks (Manasi)
> > >> > > > > - Remove the registers that get added automagically (Anusha)
> > >> > > > >
> > >> > > > > v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state()
> > >> > > > > (Gaurav)
> > >> > > > >
> > >> > > > > v4: rebased.
> > >> > > > >
> > >> > > > > Cc: Gaurav K Singh
> > >> > > > > Cc: Jani Nikula
> > >> > > > > Cc: Ville Syrjala
> > >> > > > > Cc: Manasi Navare
> > >> > > > > Signed-off-by: Anusha Srivatsa
> > >> > > > > ---
> > >> > > > > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> > >> > > > > drivers/gpu/drm/i915/intel_ddi.c | 2 ++
> > >> > > > > drivers/gpu/drm/i915/intel_dp.c | 27
> > >> > > > > +++ drivers/gpu/drm/i915/intel_drv.h
> > >> > > > > | 3 ++-
> > >> > > > > 4 files changed, 33 insertions(+), 1 deletion(-)
> > >> > > > >
> > >> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > >> > > > > b/drivers/gpu/drm/i915/i915_reg.h index
> > >> > > > > bcde78bc0027..c8d7fdcd7823 100644
> > >> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > >> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > >> > > > > @@ -9093,6 +9093,7 @@ enum skl_power_gate {
> > >> > > > > #define _DP_TP_CTL_B0x64140
> > >> > > > > #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A,
> > >_DP_TP_CTL_B)
> > >> > > > > #define DP_TP_CTL_ENABLE (1 << 31)
> > >> > > > > +#define DP_TP_CTL_FEC_ENABLE (1 << 30)
> > >> > > > > #define DP_TP_CTL_MODE_SST (0 << 27)
> > >> > > > > #define DP_TP_CTL_MODE_MST (1 << 27)
> > >> > > > > #define DP_TP_CTL_FORCE_ACT(1 << 25)
> > >> > > > > @@ -9111,6 +9112,7 @@ enum skl_power_gate {
> > >> > > > > #define _DP_TP_STATUS_A 0x64044
> > >> > > > > #define _DP_TP_STATUS_B 0x64144
> > >> > > > > #define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A,
> > >> > > > > _DP_TP_STATUS_B)
> > >> > > > > +#define DP_TP_STATUS_FEC_ENABLE_LIVE (1 << 28)
> > >> > > > > #define DP_TP_STATUS_IDLE_DONE (1 << 25)
> > >> > > > > #define DP_TP_STATUS_ACT_SENT (1 << 24)
> > >> > > > > #define DP_TP_STATUS_MODE_STATUS_MST (1 << 23)
> > >> > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > >> > > > > b/drivers/gpu/drm/i915/intel_ddi.c
> > >> > > > > index f531900165bf..67c013ea4d39 100644
> > >> > > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > >> > > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > >> > > > > @@ -2911,6 +2911,8 @@ static void intel_ddi_pre_enable_dp(struct
> > >intel_encoder *encoder,
> > >> > > > >
> > >> > > > > DP_DECOMPRESSION_EN);
> > >> > > > > intel_dp_sink_set_fec_ready(intel_dp, crtc_state,
> > >DP_FEC_READY);
> > >> > > > > intel_dp_start_link_train(intel_dp);
> > >> > > > > +intel_dp_enable_fec_state(intel_dp, crtc_state);
> > >> > > >
> > >> > > > This doesn't look like the correct spot. According to the spec
> > >> > > > it should be after stop_link_train() (where we set DP_TP_CTL to
> > >> > > > to normal).
> > >> > > >
> > >> > >
> > >> > > Yes I see in the display port enabling sequence page of the spec,
> > >> > > that it says enable FEC after DP_TP_CTL is set to Normal. Also the
> > >> > > FEC page says that this should be enabled only after ensuring that
> > >> > > link training completed. So according to that this call should
> > >> > > happen after
> >
Re: [Intel-gfx] [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
>-Original Message-
>From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
>Sent: Monday, October 22, 2018 11:26 AM
>To: Srivatsa, Anusha
>Cc: Navare, Manasi D ; intel-
>g...@lists.freedesktop.org; Singh, Gaurav K ; Jani
>Nikula
>Subject: Re: [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
>
>On Mon, Oct 22, 2018 at 06:04:28PM +, Srivatsa, Anusha wrote:
>>
>>
>> >-Original Message-
>> >From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
>> >Sent: Friday, October 19, 2018 4:12 PM
>> >To: Navare, Manasi D
>> >Cc: Srivatsa, Anusha ; intel-
>> >g...@lists.freedesktop.org; Singh, Gaurav K
>> >; Jani Nikula
>> >Subject: Re: [v2 5/6] i915/dp/fec: Configure the Forward Error Correction
>> >bits.
>> >
>> >On Fri, Oct 19, 2018 at 01:29:05PM -0700, Manasi Navare wrote:
>> >> On Fri, Oct 19, 2018 at 10:58:29PM +0300, Ville Syrjälä wrote:
>> >> > On Fri, Oct 19, 2018 at 12:24:43PM -0700, Manasi Navare wrote:
>> >> > > On Fri, Oct 19, 2018 at 09:39:11PM +0300, Ville Syrjälä wrote:
>> >> > > > On Mon, Oct 15, 2018 at 02:50:36PM -0700, Anusha Srivatsa wrote:
>> >> > > > > If FEC is supported, the corresponding DP_TP_CTL register
>> >> > > > > bits have to be configured.
>> >> > > > >
>> >> > > > > The driver has to program the FEC_ENABLE in DP_TP_CTL[30]
>> >> > > > > register and wait till FEC_STATUS in DP_TP_CTL[28] is 1.
>> >> > > > > Also add the warn message to make sure that the control
>> >> > > > > register is already active while enabling FEC.
>> >> > > > >
>> >> > > > > v2:
>> >> > > > > - Change commit message. Configure fec state after
>> >> > > > > link training (Manasi, Gaurav)
>> >> > > > > - Remove redundent checks (Manasi)
>> >> > > > > - Remove the registers that get added automagically
>> >> > > > > (Anusha)
>> >> > > > >
>> >> > > > > v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state()
>> >> > > > > (Gaurav)
>> >> > > > >
>> >> > > > > v4: rebased.
>> >> > > > >
>> >> > > > > Cc: Gaurav K Singh
>> >> > > > > Cc: Jani Nikula
>> >> > > > > Cc: Ville Syrjala
>> >> > > > > Cc: Manasi Navare
>> >> > > > > Signed-off-by: Anusha Srivatsa
>> >> > > > > ---
>> >> > > > > drivers/gpu/drm/i915/i915_reg.h | 2 ++
>> >> > > > > drivers/gpu/drm/i915/intel_ddi.c | 2 ++
>> >> > > > > drivers/gpu/drm/i915/intel_dp.c | 27
>> >> > > > > +++
>> >> > > > > +++ drivers/gpu/drm/i915/intel_drv.
>> >> > > > > +++ h
>> >> > > > > | 3 ++-
>> >> > > > > 4 files changed, 33 insertions(+), 1 deletion(-)
>> >> > > > >
>> >> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> >> > > > > b/drivers/gpu/drm/i915/i915_reg.h index
>> >> > > > > bcde78bc0027..c8d7fdcd7823 100644
>> >> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
>> >> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> >> > > > > @@ -9093,6 +9093,7 @@ enum skl_power_gate {
>> >> > > > > #define _DP_TP_CTL_B 0x64140
>> >> > > > > #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A,
>> >_DP_TP_CTL_B)
>> >> > > > > #define DP_TP_CTL_ENABLE(1 << 31)
>> >> > > > > +#define DP_TP_CTL_FEC_ENABLE(1 << 30)
>> >> > > > > #define DP_TP_CTL_MODE_SST (0 << 27)
>> >> > > > > #define DP_TP_CTL_MODE_MST (1 << 27)
>> >> > > > > #define DP_TP_CTL_FORCE_ACT (1 << 25)
>> >> > > > > @@ -9111,6 +9112,7 @@ enum skl_power_gate {
>> >> > > > > #define _DP_TP_STATUS_A 0x64044
>> >> > > > > #define _DP_TP_STATUS_B 0x64144
>> >> > > > > #define DP_TP_STATUS(port) _MMIO_PORT(port,
>> >> > > > > _DP_TP_STATUS_A,
>> >> > > > > _DP_TP_STATUS_B)
>> >> > > > > +#define DP_TP_STATUS_FEC_ENABLE_LIVE(1 << 28)
>> >> > > > > #define DP_TP_STATUS_IDLE_DONE (1 <<
>25)
>> >> > > > > #define DP_TP_STATUS_ACT_SENT (1 << 24)
>> >> > > > > #define DP_TP_STATUS_MODE_STATUS_MST(1 <<
>23)
>> >> > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>> >> > > > > b/drivers/gpu/drm/i915/intel_ddi.c
>> >> > > > > index f531900165bf..67c013ea4d39 100644
>> >> > > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
>> >> > > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> >> > > > > @@ -2911,6 +2911,8 @@ static void
>> >> > > > > intel_ddi_pre_enable_dp(struct
>> >intel_encoder *encoder,
>> >> > > > > DP_DECOMPRESSION_EN);
>> >> > > > > intel_dp_sink_set_fec_ready(intel_dp, crtc_state,
>> >DP_FEC_READY);
>> >> > > > > intel_dp_start_link_train(intel_dp);
>> >> > > > > + intel_dp_enable_fec_state(intel_dp, crtc_state);
>> >> > > >
>> >> > > > This doesn't look like the correct spot. According to the
>> >> > > > spec it should be after stop_link_train() (where we set
>> >> > > > DP_TP_CTL to to normal).
>> >> > > >
>> >> > >
>> >> > > Yes I see in the display port enabling sequence page of the
>> >> > > spec,
Re: [Intel-gfx] [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
On Mon, Oct 22, 2018 at 06:04:28PM +, Srivatsa, Anusha wrote:
>
>
> >-Original Message-
> >From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> >Sent: Friday, October 19, 2018 4:12 PM
> >To: Navare, Manasi D
> >Cc: Srivatsa, Anusha ; intel-
> >g...@lists.freedesktop.org; Singh, Gaurav K ; Jani
> >Nikula
> >Subject: Re: [v2 5/6] i915/dp/fec: Configure the Forward Error Correction
> >bits.
> >
> >On Fri, Oct 19, 2018 at 01:29:05PM -0700, Manasi Navare wrote:
> >> On Fri, Oct 19, 2018 at 10:58:29PM +0300, Ville Syrjälä wrote:
> >> > On Fri, Oct 19, 2018 at 12:24:43PM -0700, Manasi Navare wrote:
> >> > > On Fri, Oct 19, 2018 at 09:39:11PM +0300, Ville Syrjälä wrote:
> >> > > > On Mon, Oct 15, 2018 at 02:50:36PM -0700, Anusha Srivatsa wrote:
> >> > > > > If FEC is supported, the corresponding DP_TP_CTL register bits
> >> > > > > have to be configured.
> >> > > > >
> >> > > > > The driver has to program the FEC_ENABLE in DP_TP_CTL[30]
> >> > > > > register and wait till FEC_STATUS in DP_TP_CTL[28] is 1.
> >> > > > > Also add the warn message to make sure that the control
> >> > > > > register is already active while enabling FEC.
> >> > > > >
> >> > > > > v2:
> >> > > > > - Change commit message. Configure fec state after
> >> > > > > link training (Manasi, Gaurav)
> >> > > > > - Remove redundent checks (Manasi)
> >> > > > > - Remove the registers that get added automagically (Anusha)
> >> > > > >
> >> > > > > v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state()
> >> > > > > (Gaurav)
> >> > > > >
> >> > > > > v4: rebased.
> >> > > > >
> >> > > > > Cc: Gaurav K Singh
> >> > > > > Cc: Jani Nikula
> >> > > > > Cc: Ville Syrjala
> >> > > > > Cc: Manasi Navare
> >> > > > > Signed-off-by: Anusha Srivatsa
> >> > > > > ---
> >> > > > > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> >> > > > > drivers/gpu/drm/i915/intel_ddi.c | 2 ++
> >> > > > > drivers/gpu/drm/i915/intel_dp.c | 27
> >> > > > > +++ drivers/gpu/drm/i915/intel_drv.h
> >> > > > > | 3 ++-
> >> > > > > 4 files changed, 33 insertions(+), 1 deletion(-)
> >> > > > >
> >> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> > > > > b/drivers/gpu/drm/i915/i915_reg.h index
> >> > > > > bcde78bc0027..c8d7fdcd7823 100644
> >> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> > > > > @@ -9093,6 +9093,7 @@ enum skl_power_gate {
> >> > > > > #define _DP_TP_CTL_B 0x64140
> >> > > > > #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A,
> >_DP_TP_CTL_B)
> >> > > > > #define DP_TP_CTL_ENABLE (1 << 31)
> >> > > > > +#define DP_TP_CTL_FEC_ENABLE (1 << 30)
> >> > > > > #define DP_TP_CTL_MODE_SST (0 << 27)
> >> > > > > #define DP_TP_CTL_MODE_MST (1 << 27)
> >> > > > > #define DP_TP_CTL_FORCE_ACT (1 << 25)
> >> > > > > @@ -9111,6 +9112,7 @@ enum skl_power_gate {
> >> > > > > #define _DP_TP_STATUS_A 0x64044
> >> > > > > #define _DP_TP_STATUS_B 0x64144
> >> > > > > #define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A,
> >> > > > > _DP_TP_STATUS_B)
> >> > > > > +#define DP_TP_STATUS_FEC_ENABLE_LIVE (1 << 28)
> >> > > > > #define DP_TP_STATUS_IDLE_DONE (1 << 25)
> >> > > > > #define DP_TP_STATUS_ACT_SENT(1 << 24)
> >> > > > > #define DP_TP_STATUS_MODE_STATUS_MST (1 << 23)
> >> > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> >> > > > > b/drivers/gpu/drm/i915/intel_ddi.c
> >> > > > > index f531900165bf..67c013ea4d39 100644
> >> > > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> >> > > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >> > > > > @@ -2911,6 +2911,8 @@ static void intel_ddi_pre_enable_dp(struct
> >intel_encoder *encoder,
> >> > > > > DP_DECOMPRESSION_EN);
> >> > > > >intel_dp_sink_set_fec_ready(intel_dp, crtc_state,
> >DP_FEC_READY);
> >> > > > >intel_dp_start_link_train(intel_dp);
> >> > > > > + intel_dp_enable_fec_state(intel_dp, crtc_state);
> >> > > >
> >> > > > This doesn't look like the correct spot. According to the spec
> >> > > > it should be after stop_link_train() (where we set DP_TP_CTL to
> >> > > > to normal).
> >> > > >
> >> > >
> >> > > Yes I see in the display port enabling sequence page of the spec,
> >> > > that it says enable FEC after DP_TP_CTL is set to Normal. Also the
> >> > > FEC page says that this should be enabled only after ensuring that
> >> > > link training completed. So according to that this call should happen
> >> > > after
> >intel_dp_stop_link_train().
> >> > > So yes move this to after intel_dp_stop_link_training().
> >> > >
> >> > > > > +
> >> > > > >if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
> >> > > > >intel_dp_stop_link_train(intel_dp);
> >> > > > >
> >> > > > > diff --git
Re: [Intel-gfx] [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
On Mon, Oct 22, 2018 at 11:04:28AM -0700, Srivatsa, Anusha wrote:
>
>
> >-Original Message-
> >From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
> >Sent: Friday, October 19, 2018 4:12 PM
> >To: Navare, Manasi D
> >Cc: Srivatsa, Anusha ; intel-
> >g...@lists.freedesktop.org; Singh, Gaurav K ; Jani
> >Nikula
> >Subject: Re: [v2 5/6] i915/dp/fec: Configure the Forward Error Correction
> >bits.
> >
> >On Fri, Oct 19, 2018 at 01:29:05PM -0700, Manasi Navare wrote:
> >> On Fri, Oct 19, 2018 at 10:58:29PM +0300, Ville Syrjälä wrote:
> >> > On Fri, Oct 19, 2018 at 12:24:43PM -0700, Manasi Navare wrote:
> >> > > On Fri, Oct 19, 2018 at 09:39:11PM +0300, Ville Syrjälä wrote:
> >> > > > On Mon, Oct 15, 2018 at 02:50:36PM -0700, Anusha Srivatsa wrote:
> >> > > > > If FEC is supported, the corresponding DP_TP_CTL register bits
> >> > > > > have to be configured.
> >> > > > >
> >> > > > > The driver has to program the FEC_ENABLE in DP_TP_CTL[30]
> >> > > > > register and wait till FEC_STATUS in DP_TP_CTL[28] is 1.
> >> > > > > Also add the warn message to make sure that the control
> >> > > > > register is already active while enabling FEC.
> >> > > > >
> >> > > > > v2:
> >> > > > > - Change commit message. Configure fec state after
> >> > > > > link training (Manasi, Gaurav)
> >> > > > > - Remove redundent checks (Manasi)
> >> > > > > - Remove the registers that get added automagically (Anusha)
> >> > > > >
> >> > > > > v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state()
> >> > > > > (Gaurav)
> >> > > > >
> >> > > > > v4: rebased.
> >> > > > >
> >> > > > > Cc: Gaurav K Singh
> >> > > > > Cc: Jani Nikula
> >> > > > > Cc: Ville Syrjala
> >> > > > > Cc: Manasi Navare
> >> > > > > Signed-off-by: Anusha Srivatsa
> >> > > > > ---
> >> > > > > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> >> > > > > drivers/gpu/drm/i915/intel_ddi.c | 2 ++
> >> > > > > drivers/gpu/drm/i915/intel_dp.c | 27
> >> > > > > +++ drivers/gpu/drm/i915/intel_drv.h
> >> > > > > | 3 ++-
> >> > > > > 4 files changed, 33 insertions(+), 1 deletion(-)
> >> > > > >
> >> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> > > > > b/drivers/gpu/drm/i915/i915_reg.h index
> >> > > > > bcde78bc0027..c8d7fdcd7823 100644
> >> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> > > > > @@ -9093,6 +9093,7 @@ enum skl_power_gate {
> >> > > > > #define _DP_TP_CTL_B 0x64140
> >> > > > > #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A,
> >_DP_TP_CTL_B)
> >> > > > > #define DP_TP_CTL_ENABLE (1 << 31)
> >> > > > > +#define DP_TP_CTL_FEC_ENABLE (1 << 30)
> >> > > > > #define DP_TP_CTL_MODE_SST (0 << 27)
> >> > > > > #define DP_TP_CTL_MODE_MST (1 << 27)
> >> > > > > #define DP_TP_CTL_FORCE_ACT (1 << 25)
> >> > > > > @@ -9111,6 +9112,7 @@ enum skl_power_gate {
> >> > > > > #define _DP_TP_STATUS_A 0x64044
> >> > > > > #define _DP_TP_STATUS_B 0x64144
> >> > > > > #define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A,
> >> > > > > _DP_TP_STATUS_B)
> >> > > > > +#define DP_TP_STATUS_FEC_ENABLE_LIVE (1 << 28)
> >> > > > > #define DP_TP_STATUS_IDLE_DONE (1 << 25)
> >> > > > > #define DP_TP_STATUS_ACT_SENT(1 << 24)
> >> > > > > #define DP_TP_STATUS_MODE_STATUS_MST (1 << 23)
> >> > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> >> > > > > b/drivers/gpu/drm/i915/intel_ddi.c
> >> > > > > index f531900165bf..67c013ea4d39 100644
> >> > > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> >> > > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >> > > > > @@ -2911,6 +2911,8 @@ static void intel_ddi_pre_enable_dp(struct
> >intel_encoder *encoder,
> >> > > > > DP_DECOMPRESSION_EN);
> >> > > > >intel_dp_sink_set_fec_ready(intel_dp, crtc_state,
> >DP_FEC_READY);
> >> > > > >intel_dp_start_link_train(intel_dp);
> >> > > > > + intel_dp_enable_fec_state(intel_dp, crtc_state);
> >> > > >
> >> > > > This doesn't look like the correct spot. According to the spec
> >> > > > it should be after stop_link_train() (where we set DP_TP_CTL to
> >> > > > to normal).
> >> > > >
> >> > >
> >> > > Yes I see in the display port enabling sequence page of the spec,
> >> > > that it says enable FEC after DP_TP_CTL is set to Normal. Also the
> >> > > FEC page says that this should be enabled only after ensuring that
> >> > > link training completed. So according to that this call should happen
> >> > > after
> >intel_dp_stop_link_train().
> >> > > So yes move this to after intel_dp_stop_link_training().
> >> > >
> >> > > > > +
> >> > > > >if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
> >> > > > >intel_dp_stop_link_train(intel_dp);
> >> > > > >
> >> > > > > diff --git
Re: [Intel-gfx] [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
>-Original Message-
>From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
>Sent: Friday, October 19, 2018 4:12 PM
>To: Navare, Manasi D
>Cc: Srivatsa, Anusha ; intel-
>g...@lists.freedesktop.org; Singh, Gaurav K ; Jani
>Nikula
>Subject: Re: [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
>
>On Fri, Oct 19, 2018 at 01:29:05PM -0700, Manasi Navare wrote:
>> On Fri, Oct 19, 2018 at 10:58:29PM +0300, Ville Syrjälä wrote:
>> > On Fri, Oct 19, 2018 at 12:24:43PM -0700, Manasi Navare wrote:
>> > > On Fri, Oct 19, 2018 at 09:39:11PM +0300, Ville Syrjälä wrote:
>> > > > On Mon, Oct 15, 2018 at 02:50:36PM -0700, Anusha Srivatsa wrote:
>> > > > > If FEC is supported, the corresponding DP_TP_CTL register bits
>> > > > > have to be configured.
>> > > > >
>> > > > > The driver has to program the FEC_ENABLE in DP_TP_CTL[30]
>> > > > > register and wait till FEC_STATUS in DP_TP_CTL[28] is 1.
>> > > > > Also add the warn message to make sure that the control
>> > > > > register is already active while enabling FEC.
>> > > > >
>> > > > > v2:
>> > > > > - Change commit message. Configure fec state after
>> > > > > link training (Manasi, Gaurav)
>> > > > > - Remove redundent checks (Manasi)
>> > > > > - Remove the registers that get added automagically (Anusha)
>> > > > >
>> > > > > v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state()
>> > > > > (Gaurav)
>> > > > >
>> > > > > v4: rebased.
>> > > > >
>> > > > > Cc: Gaurav K Singh
>> > > > > Cc: Jani Nikula
>> > > > > Cc: Ville Syrjala
>> > > > > Cc: Manasi Navare
>> > > > > Signed-off-by: Anusha Srivatsa
>> > > > > ---
>> > > > > drivers/gpu/drm/i915/i915_reg.h | 2 ++
>> > > > > drivers/gpu/drm/i915/intel_ddi.c | 2 ++
>> > > > > drivers/gpu/drm/i915/intel_dp.c | 27
>> > > > > +++ drivers/gpu/drm/i915/intel_drv.h
>> > > > > | 3 ++-
>> > > > > 4 files changed, 33 insertions(+), 1 deletion(-)
>> > > > >
>> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> > > > > b/drivers/gpu/drm/i915/i915_reg.h index
>> > > > > bcde78bc0027..c8d7fdcd7823 100644
>> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > > > > @@ -9093,6 +9093,7 @@ enum skl_power_gate {
>> > > > > #define _DP_TP_CTL_B0x64140
>> > > > > #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A,
>_DP_TP_CTL_B)
>> > > > > #define DP_TP_CTL_ENABLE (1 << 31)
>> > > > > +#define DP_TP_CTL_FEC_ENABLE (1 << 30)
>> > > > > #define DP_TP_CTL_MODE_SST (0 << 27)
>> > > > > #define DP_TP_CTL_MODE_MST (1 << 27)
>> > > > > #define DP_TP_CTL_FORCE_ACT(1 << 25)
>> > > > > @@ -9111,6 +9112,7 @@ enum skl_power_gate {
>> > > > > #define _DP_TP_STATUS_A 0x64044
>> > > > > #define _DP_TP_STATUS_B 0x64144
>> > > > > #define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A,
>> > > > > _DP_TP_STATUS_B)
>> > > > > +#define DP_TP_STATUS_FEC_ENABLE_LIVE (1 << 28)
>> > > > > #define DP_TP_STATUS_IDLE_DONE (1 << 25)
>> > > > > #define DP_TP_STATUS_ACT_SENT (1 << 24)
>> > > > > #define DP_TP_STATUS_MODE_STATUS_MST (1 << 23)
>> > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>> > > > > b/drivers/gpu/drm/i915/intel_ddi.c
>> > > > > index f531900165bf..67c013ea4d39 100644
>> > > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
>> > > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> > > > > @@ -2911,6 +2911,8 @@ static void intel_ddi_pre_enable_dp(struct
>intel_encoder *encoder,
>> > > > >DP_DECOMPRESSION_EN);
>> > > > > intel_dp_sink_set_fec_ready(intel_dp, crtc_state,
>DP_FEC_READY);
>> > > > > intel_dp_start_link_train(intel_dp);
>> > > > > +intel_dp_enable_fec_state(intel_dp, crtc_state);
>> > > >
>> > > > This doesn't look like the correct spot. According to the spec
>> > > > it should be after stop_link_train() (where we set DP_TP_CTL to
>> > > > to normal).
>> > > >
>> > >
>> > > Yes I see in the display port enabling sequence page of the spec,
>> > > that it says enable FEC after DP_TP_CTL is set to Normal. Also the
>> > > FEC page says that this should be enabled only after ensuring that
>> > > link training completed. So according to that this call should happen
>> > > after
>intel_dp_stop_link_train().
>> > > So yes move this to after intel_dp_stop_link_training().
>> > >
>> > > > > +
>> > > > > if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
>> > > > > intel_dp_stop_link_train(intel_dp);
>> > > > >
>> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> > > > > b/drivers/gpu/drm/i915/intel_dp.c index
>> > > > > b4e8af3142a2..b9f85502d9ff 100644
>> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > > > > @@ -3017,6 +3017,33 @@ void
Re: [Intel-gfx] [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
On Fri, Oct 19, 2018 at 01:29:05PM -0700, Manasi Navare wrote:
> On Fri, Oct 19, 2018 at 10:58:29PM +0300, Ville Syrjälä wrote:
> > On Fri, Oct 19, 2018 at 12:24:43PM -0700, Manasi Navare wrote:
> > > On Fri, Oct 19, 2018 at 09:39:11PM +0300, Ville Syrjälä wrote:
> > > > On Mon, Oct 15, 2018 at 02:50:36PM -0700, Anusha Srivatsa wrote:
> > > > > If FEC is supported, the corresponding
> > > > > DP_TP_CTL register bits have to be configured.
> > > > >
> > > > > The driver has to program the FEC_ENABLE in DP_TP_CTL[30] register
> > > > > and wait till FEC_STATUS in DP_TP_CTL[28] is 1.
> > > > > Also add the warn message to make sure that the control
> > > > > register is already active while enabling FEC.
> > > > >
> > > > > v2:
> > > > > - Change commit message. Configure fec state after
> > > > > link training (Manasi, Gaurav)
> > > > > - Remove redundent checks (Manasi)
> > > > > - Remove the registers that get added automagically (Anusha)
> > > > >
> > > > > v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state() (Gaurav)
> > > > >
> > > > > v4: rebased.
> > > > >
> > > > > Cc: Gaurav K Singh
> > > > > Cc: Jani Nikula
> > > > > Cc: Ville Syrjala
> > > > > Cc: Manasi Navare
> > > > > Signed-off-by: Anusha Srivatsa
> > > > > ---
> > > > > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> > > > > drivers/gpu/drm/i915/intel_ddi.c | 2 ++
> > > > > drivers/gpu/drm/i915/intel_dp.c | 27 +++
> > > > > drivers/gpu/drm/i915/intel_drv.h | 3 ++-
> > > > > 4 files changed, 33 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > > index bcde78bc0027..c8d7fdcd7823 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > > @@ -9093,6 +9093,7 @@ enum skl_power_gate {
> > > > > #define _DP_TP_CTL_B 0x64140
> > > > > #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A, _DP_TP_CTL_B)
> > > > > #define DP_TP_CTL_ENABLE(1 << 31)
> > > > > +#define DP_TP_CTL_FEC_ENABLE(1 << 30)
> > > > > #define DP_TP_CTL_MODE_SST (0 << 27)
> > > > > #define DP_TP_CTL_MODE_MST (1 << 27)
> > > > > #define DP_TP_CTL_FORCE_ACT (1 << 25)
> > > > > @@ -9111,6 +9112,7 @@ enum skl_power_gate {
> > > > > #define _DP_TP_STATUS_A 0x64044
> > > > > #define _DP_TP_STATUS_B 0x64144
> > > > > #define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A,
> > > > > _DP_TP_STATUS_B)
> > > > > +#define DP_TP_STATUS_FEC_ENABLE_LIVE(1 << 28)
> > > > > #define DP_TP_STATUS_IDLE_DONE (1 << 25)
> > > > > #define DP_TP_STATUS_ACT_SENT (1 << 24)
> > > > > #define DP_TP_STATUS_MODE_STATUS_MST(1 << 23)
> > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > > > index f531900165bf..67c013ea4d39 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > > > @@ -2911,6 +2911,8 @@ static void intel_ddi_pre_enable_dp(struct
> > > > > intel_encoder *encoder,
> > > > > DP_DECOMPRESSION_EN);
> > > > > intel_dp_sink_set_fec_ready(intel_dp, crtc_state, DP_FEC_READY);
> > > > > intel_dp_start_link_train(intel_dp);
> > > > > + intel_dp_enable_fec_state(intel_dp, crtc_state);
> > > >
> > > > This doesn't look like the correct spot. According to the spec it
> > > > should be after stop_link_train() (where we set DP_TP_CTL to
> > > > to normal).
> > > >
> > >
> > > Yes I see in the display port enabling sequence page of the spec, that it
> > > says enable FEC
> > > after DP_TP_CTL is set to Normal. Also the FEC page says that this should
> > > be enabled only after ensuring that link training
> > > completed. So according to that this call should happen after
> > > intel_dp_stop_link_train().
> > > So yes move this to after intel_dp_stop_link_training().
> > >
> > > > > +
> > > > > if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
> > > > > intel_dp_stop_link_train(intel_dp);
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index b4e8af3142a2..b9f85502d9ff 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -3017,6 +3017,33 @@ void intel_dp_sink_set_fec_ready(struct
> > > > > intel_dp *intel_dp,
> > > > > DRM_DEBUG_KMS("Failed to get FEC enabled in sink\n");
> > > > > }
> > > > >
> > > > > +void intel_dp_enable_fec_state(struct intel_dp *intel_dp,
> > > > > +const struct intel_crtc_state
> > > > > *crtc_state)
> > > > > +{
> > > > > + struct drm_i915_private *dev_priv =
Re: [Intel-gfx] [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
>-Original Message-
>From: Navare, Manasi D
>Sent: Friday, October 19, 2018 1:29 PM
>To: Ville Syrjälä
>Cc: Srivatsa, Anusha ; intel-
>g...@lists.freedesktop.org; Singh, Gaurav K ; Jani
>Nikula
>Subject: Re: [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
>
>On Fri, Oct 19, 2018 at 10:58:29PM +0300, Ville Syrjälä wrote:
>> On Fri, Oct 19, 2018 at 12:24:43PM -0700, Manasi Navare wrote:
>> > On Fri, Oct 19, 2018 at 09:39:11PM +0300, Ville Syrjälä wrote:
>> > > On Mon, Oct 15, 2018 at 02:50:36PM -0700, Anusha Srivatsa wrote:
>> > > > If FEC is supported, the corresponding DP_TP_CTL register bits
>> > > > have to be configured.
>> > > >
>> > > > The driver has to program the FEC_ENABLE in DP_TP_CTL[30]
>> > > > register and wait till FEC_STATUS in DP_TP_CTL[28] is 1.
>> > > > Also add the warn message to make sure that the control register
>> > > > is already active while enabling FEC.
>> > > >
>> > > > v2:
>> > > > - Change commit message. Configure fec state after
>> > > > link training (Manasi, Gaurav)
>> > > > - Remove redundent checks (Manasi)
>> > > > - Remove the registers that get added automagically (Anusha)
>> > > >
>> > > > v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state()
>> > > > (Gaurav)
>> > > >
>> > > > v4: rebased.
>> > > >
>> > > > Cc: Gaurav K Singh
>> > > > Cc: Jani Nikula
>> > > > Cc: Ville Syrjala
>> > > > Cc: Manasi Navare
>> > > > Signed-off-by: Anusha Srivatsa
>> > > > ---
>> > > > drivers/gpu/drm/i915/i915_reg.h | 2 ++
>> > > > drivers/gpu/drm/i915/intel_ddi.c | 2 ++
>> > > > drivers/gpu/drm/i915/intel_dp.c | 27
>> > > > +++ drivers/gpu/drm/i915/intel_drv.h |
>> > > > 3 ++-
>> > > > 4 files changed, 33 insertions(+), 1 deletion(-)
>> > > >
>> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> > > > b/drivers/gpu/drm/i915/i915_reg.h index
>> > > > bcde78bc0027..c8d7fdcd7823 100644
>> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > > > @@ -9093,6 +9093,7 @@ enum skl_power_gate {
>> > > > #define _DP_TP_CTL_B 0x64140
>> > > > #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A,
>_DP_TP_CTL_B)
>> > > > #define DP_TP_CTL_ENABLE (1 << 31)
>> > > > +#define DP_TP_CTL_FEC_ENABLE (1 << 30)
>> > > > #define DP_TP_CTL_MODE_SST (0 << 27)
>> > > > #define DP_TP_CTL_MODE_MST (1 << 27)
>> > > > #define DP_TP_CTL_FORCE_ACT (1 << 25)
>> > > > @@ -9111,6 +9112,7 @@ enum skl_power_gate {
>> > > > #define _DP_TP_STATUS_A 0x64044
>> > > > #define _DP_TP_STATUS_B 0x64144
>> > > > #define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A,
>> > > > _DP_TP_STATUS_B)
>> > > > +#define DP_TP_STATUS_FEC_ENABLE_LIVE (1 << 28)
>> > > > #define DP_TP_STATUS_IDLE_DONE (1 << 25)
>> > > > #define DP_TP_STATUS_ACT_SENT(1 << 24)
>> > > > #define DP_TP_STATUS_MODE_STATUS_MST (1 << 23)
>> > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>> > > > b/drivers/gpu/drm/i915/intel_ddi.c
>> > > > index f531900165bf..67c013ea4d39 100644
>> > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
>> > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> > > > @@ -2911,6 +2911,8 @@ static void intel_ddi_pre_enable_dp(struct
>intel_encoder *encoder,
>> > > > DP_DECOMPRESSION_EN);
>> > > >intel_dp_sink_set_fec_ready(intel_dp, crtc_state, DP_FEC_READY);
>> > > >intel_dp_start_link_train(intel_dp);
>> > > > + intel_dp_enable_fec_state(intel_dp, crtc_state);
>> > >
>> > > This doesn't look like the correct spot. According to the spec it
>> > > should be after stop_link_train() (where we set DP_TP_CTL to to
>> > > normal).
>> > >
>> >
>> > Yes I see in the display port enabling sequence page of the spec,
>> > that it says enable FEC after DP_TP_CTL is set to Normal. Also the
>> > FEC page says that this should be enabled only after ensuring that
>> > link training completed. So according to that this call should happen after
>intel_dp_stop_link_train().
>> > So yes move this to after intel_dp_stop_link_training().
>> >
>> > > > +
>> > > >if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
>> > > >intel_dp_stop_link_train(intel_dp);
>> > > >
>> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> > > > b/drivers/gpu/drm/i915/intel_dp.c index
>> > > > b4e8af3142a2..b9f85502d9ff 100644
>> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > > > @@ -3017,6 +3017,33 @@ void intel_dp_sink_set_fec_ready(struct
>intel_dp *intel_dp,
>> > > >DRM_DEBUG_KMS("Failed to get FEC enabled in sink\n"); }
>> > > >
>> > > > +void intel_dp_enable_fec_state(struct intel_dp *intel_dp,
>> > > > + const struct intel_crtc_state
>> > > >
Re: [Intel-gfx] [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
On Fri, Oct 19, 2018 at 10:58:29PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 19, 2018 at 12:24:43PM -0700, Manasi Navare wrote:
> > On Fri, Oct 19, 2018 at 09:39:11PM +0300, Ville Syrjälä wrote:
> > > On Mon, Oct 15, 2018 at 02:50:36PM -0700, Anusha Srivatsa wrote:
> > > > If FEC is supported, the corresponding
> > > > DP_TP_CTL register bits have to be configured.
> > > >
> > > > The driver has to program the FEC_ENABLE in DP_TP_CTL[30] register
> > > > and wait till FEC_STATUS in DP_TP_CTL[28] is 1.
> > > > Also add the warn message to make sure that the control
> > > > register is already active while enabling FEC.
> > > >
> > > > v2:
> > > > - Change commit message. Configure fec state after
> > > > link training (Manasi, Gaurav)
> > > > - Remove redundent checks (Manasi)
> > > > - Remove the registers that get added automagically (Anusha)
> > > >
> > > > v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state() (Gaurav)
> > > >
> > > > v4: rebased.
> > > >
> > > > Cc: Gaurav K Singh
> > > > Cc: Jani Nikula
> > > > Cc: Ville Syrjala
> > > > Cc: Manasi Navare
> > > > Signed-off-by: Anusha Srivatsa
> > > > ---
> > > > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> > > > drivers/gpu/drm/i915/intel_ddi.c | 2 ++
> > > > drivers/gpu/drm/i915/intel_dp.c | 27 +++
> > > > drivers/gpu/drm/i915/intel_drv.h | 3 ++-
> > > > 4 files changed, 33 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > > b/drivers/gpu/drm/i915/i915_reg.h
> > > > index bcde78bc0027..c8d7fdcd7823 100644
> > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > @@ -9093,6 +9093,7 @@ enum skl_power_gate {
> > > > #define _DP_TP_CTL_B 0x64140
> > > > #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A, _DP_TP_CTL_B)
> > > > #define DP_TP_CTL_ENABLE (1 << 31)
> > > > +#define DP_TP_CTL_FEC_ENABLE (1 << 30)
> > > > #define DP_TP_CTL_MODE_SST(0 << 27)
> > > > #define DP_TP_CTL_MODE_MST(1 << 27)
> > > > #define DP_TP_CTL_FORCE_ACT (1 << 25)
> > > > @@ -9111,6 +9112,7 @@ enum skl_power_gate {
> > > > #define _DP_TP_STATUS_A0x64044
> > > > #define _DP_TP_STATUS_B0x64144
> > > > #define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A,
> > > > _DP_TP_STATUS_B)
> > > > +#define DP_TP_STATUS_FEC_ENABLE_LIVE (1 << 28)
> > > > #define DP_TP_STATUS_IDLE_DONE(1 << 25)
> > > > #define DP_TP_STATUS_ACT_SENT (1 << 24)
> > > > #define DP_TP_STATUS_MODE_STATUS_MST (1 << 23)
> > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > > index f531900165bf..67c013ea4d39 100644
> > > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > > @@ -2911,6 +2911,8 @@ static void intel_ddi_pre_enable_dp(struct
> > > > intel_encoder *encoder,
> > > > DP_DECOMPRESSION_EN);
> > > > intel_dp_sink_set_fec_ready(intel_dp, crtc_state, DP_FEC_READY);
> > > > intel_dp_start_link_train(intel_dp);
> > > > + intel_dp_enable_fec_state(intel_dp, crtc_state);
> > >
> > > This doesn't look like the correct spot. According to the spec it
> > > should be after stop_link_train() (where we set DP_TP_CTL to
> > > to normal).
> > >
> >
> > Yes I see in the display port enabling sequence page of the spec, that it
> > says enable FEC
> > after DP_TP_CTL is set to Normal. Also the FEC page says that this should
> > be enabled only after ensuring that link training
> > completed. So according to that this call should happen after
> > intel_dp_stop_link_train().
> > So yes move this to after intel_dp_stop_link_training().
> >
> > > > +
> > > > if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
> > > > intel_dp_stop_link_train(intel_dp);
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index b4e8af3142a2..b9f85502d9ff 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -3017,6 +3017,33 @@ void intel_dp_sink_set_fec_ready(struct intel_dp
> > > > *intel_dp,
> > > > DRM_DEBUG_KMS("Failed to get FEC enabled in sink\n");
> > > > }
> > > >
> > > > +void intel_dp_enable_fec_state(struct intel_dp *intel_dp,
> > > > + const struct intel_crtc_state
> > > > *crtc_state)
> > > > +{
> > > > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > > + struct intel_digital_port *intel_dig_port =
> > > > dp_to_dig_port(intel_dp);
> > > > + enum port port = intel_dig_port->base.port;
> > > > + u32 val;
> > > > +
> > > > + /* FEC support exists for DP 1.4 only */
Re: [Intel-gfx] [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
On Fri, Oct 19, 2018 at 12:24:43PM -0700, Manasi Navare wrote:
> On Fri, Oct 19, 2018 at 09:39:11PM +0300, Ville Syrjälä wrote:
> > On Mon, Oct 15, 2018 at 02:50:36PM -0700, Anusha Srivatsa wrote:
> > > If FEC is supported, the corresponding
> > > DP_TP_CTL register bits have to be configured.
> > >
> > > The driver has to program the FEC_ENABLE in DP_TP_CTL[30] register
> > > and wait till FEC_STATUS in DP_TP_CTL[28] is 1.
> > > Also add the warn message to make sure that the control
> > > register is already active while enabling FEC.
> > >
> > > v2:
> > > - Change commit message. Configure fec state after
> > > link training (Manasi, Gaurav)
> > > - Remove redundent checks (Manasi)
> > > - Remove the registers that get added automagically (Anusha)
> > >
> > > v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state() (Gaurav)
> > >
> > > v4: rebased.
> > >
> > > Cc: Gaurav K Singh
> > > Cc: Jani Nikula
> > > Cc: Ville Syrjala
> > > Cc: Manasi Navare
> > > Signed-off-by: Anusha Srivatsa
> > > ---
> > > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> > > drivers/gpu/drm/i915/intel_ddi.c | 2 ++
> > > drivers/gpu/drm/i915/intel_dp.c | 27 +++
> > > drivers/gpu/drm/i915/intel_drv.h | 3 ++-
> > > 4 files changed, 33 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index bcde78bc0027..c8d7fdcd7823 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -9093,6 +9093,7 @@ enum skl_power_gate {
> > > #define _DP_TP_CTL_B 0x64140
> > > #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A, _DP_TP_CTL_B)
> > > #define DP_TP_CTL_ENABLE(1 << 31)
> > > +#define DP_TP_CTL_FEC_ENABLE(1 << 30)
> > > #define DP_TP_CTL_MODE_SST (0 << 27)
> > > #define DP_TP_CTL_MODE_MST (1 << 27)
> > > #define DP_TP_CTL_FORCE_ACT (1 << 25)
> > > @@ -9111,6 +9112,7 @@ enum skl_power_gate {
> > > #define _DP_TP_STATUS_A 0x64044
> > > #define _DP_TP_STATUS_B 0x64144
> > > #define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A,
> > > _DP_TP_STATUS_B)
> > > +#define DP_TP_STATUS_FEC_ENABLE_LIVE(1 << 28)
> > > #define DP_TP_STATUS_IDLE_DONE (1 << 25)
> > > #define DP_TP_STATUS_ACT_SENT (1 << 24)
> > > #define DP_TP_STATUS_MODE_STATUS_MST(1 << 23)
> > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > > b/drivers/gpu/drm/i915/intel_ddi.c
> > > index f531900165bf..67c013ea4d39 100644
> > > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > > @@ -2911,6 +2911,8 @@ static void intel_ddi_pre_enable_dp(struct
> > > intel_encoder *encoder,
> > > DP_DECOMPRESSION_EN);
> > > intel_dp_sink_set_fec_ready(intel_dp, crtc_state, DP_FEC_READY);
> > > intel_dp_start_link_train(intel_dp);
> > > + intel_dp_enable_fec_state(intel_dp, crtc_state);
> >
> > This doesn't look like the correct spot. According to the spec it
> > should be after stop_link_train() (where we set DP_TP_CTL to
> > to normal).
> >
>
> Yes I see in the display port enabling sequence page of the spec, that it
> says enable FEC
> after DP_TP_CTL is set to Normal. Also the FEC page says that this should be
> enabled only after ensuring that link training
> completed. So according to that this call should happen after
> intel_dp_stop_link_train().
> So yes move this to after intel_dp_stop_link_training().
>
> > > +
> > > if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
> > > intel_dp_stop_link_train(intel_dp);
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index b4e8af3142a2..b9f85502d9ff 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -3017,6 +3017,33 @@ void intel_dp_sink_set_fec_ready(struct intel_dp
> > > *intel_dp,
> > > DRM_DEBUG_KMS("Failed to get FEC enabled in sink\n");
> > > }
> > >
> > > +void intel_dp_enable_fec_state(struct intel_dp *intel_dp,
> > > +const struct intel_crtc_state *crtc_state)
> > > +{
> > > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > + enum port port = intel_dig_port->base.port;
> > > + u32 val;
> > > +
> > > + /* FEC support exists for DP 1.4 only */
> > > + if (intel_dp_is_edp(intel_dp))
> > > + return;
> > > +
> > > + /* If Display Compression is not enabled, FEC need not be configured */
> > > + if (!crtc_state->dsc_params.compression_enable)
> > > + return;
> > > +
> > > + val = I915_READ(DP_TP_CTL(port));
> > > + val |= DP_TP_CTL_FEC_ENABLE;
> > > + I915_WRITE(DP_TP_CTL(port), val);
> > >
Re: [Intel-gfx] [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
On Fri, Oct 19, 2018 at 09:39:11PM +0300, Ville Syrjälä wrote:
> On Mon, Oct 15, 2018 at 02:50:36PM -0700, Anusha Srivatsa wrote:
> > If FEC is supported, the corresponding
> > DP_TP_CTL register bits have to be configured.
> >
> > The driver has to program the FEC_ENABLE in DP_TP_CTL[30] register
> > and wait till FEC_STATUS in DP_TP_CTL[28] is 1.
> > Also add the warn message to make sure that the control
> > register is already active while enabling FEC.
> >
> > v2:
> > - Change commit message. Configure fec state after
> > link training (Manasi, Gaurav)
> > - Remove redundent checks (Manasi)
> > - Remove the registers that get added automagically (Anusha)
> >
> > v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state() (Gaurav)
> >
> > v4: rebased.
> >
> > Cc: Gaurav K Singh
> > Cc: Jani Nikula
> > Cc: Ville Syrjala
> > Cc: Manasi Navare
> > Signed-off-by: Anusha Srivatsa
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 2 ++
> > drivers/gpu/drm/i915/intel_ddi.c | 2 ++
> > drivers/gpu/drm/i915/intel_dp.c | 27 +++
> > drivers/gpu/drm/i915/intel_drv.h | 3 ++-
> > 4 files changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index bcde78bc0027..c8d7fdcd7823 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -9093,6 +9093,7 @@ enum skl_power_gate {
> > #define _DP_TP_CTL_B 0x64140
> > #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A, _DP_TP_CTL_B)
> > #define DP_TP_CTL_ENABLE (1 << 31)
> > +#define DP_TP_CTL_FEC_ENABLE (1 << 30)
> > #define DP_TP_CTL_MODE_SST(0 << 27)
> > #define DP_TP_CTL_MODE_MST(1 << 27)
> > #define DP_TP_CTL_FORCE_ACT (1 << 25)
> > @@ -9111,6 +9112,7 @@ enum skl_power_gate {
> > #define _DP_TP_STATUS_A0x64044
> > #define _DP_TP_STATUS_B0x64144
> > #define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A,
> > _DP_TP_STATUS_B)
> > +#define DP_TP_STATUS_FEC_ENABLE_LIVE (1 << 28)
> > #define DP_TP_STATUS_IDLE_DONE(1 << 25)
> > #define DP_TP_STATUS_ACT_SENT (1 << 24)
> > #define DP_TP_STATUS_MODE_STATUS_MST (1 << 23)
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index f531900165bf..67c013ea4d39 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2911,6 +2911,8 @@ static void intel_ddi_pre_enable_dp(struct
> > intel_encoder *encoder,
> > DP_DECOMPRESSION_EN);
> > intel_dp_sink_set_fec_ready(intel_dp, crtc_state, DP_FEC_READY);
> > intel_dp_start_link_train(intel_dp);
> > + intel_dp_enable_fec_state(intel_dp, crtc_state);
>
> This doesn't look like the correct spot. According to the spec it
> should be after stop_link_train() (where we set DP_TP_CTL to
> to normal).
>
Yes I see in the display port enabling sequence page of the spec, that it says
enable FEC
after DP_TP_CTL is set to Normal. Also the FEC page says that this should be
enabled only after ensuring that link training
completed. So according to that this call should happen after
intel_dp_stop_link_train().
So yes move this to after intel_dp_stop_link_training().
> > +
> > if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
> > intel_dp_stop_link_train(intel_dp);
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index b4e8af3142a2..b9f85502d9ff 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3017,6 +3017,33 @@ void intel_dp_sink_set_fec_ready(struct intel_dp
> > *intel_dp,
> > DRM_DEBUG_KMS("Failed to get FEC enabled in sink\n");
> > }
> >
> > +void intel_dp_enable_fec_state(struct intel_dp *intel_dp,
> > + const struct intel_crtc_state *crtc_state)
> > +{
> > + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > + enum port port = intel_dig_port->base.port;
> > + u32 val;
> > +
> > + /* FEC support exists for DP 1.4 only */
> > + if (intel_dp_is_edp(intel_dp))
> > + return;
> > +
> > + /* If Display Compression is not enabled, FEC need not be configured */
> > + if (!crtc_state->dsc_params.compression_enable)
> > + return;
> > +
> > + val = I915_READ(DP_TP_CTL(port));
> > + val |= DP_TP_CTL_FEC_ENABLE;
> > + I915_WRITE(DP_TP_CTL(port), val);
> > +
> > + if (intel_wait_for_register(dev_priv, DP_TP_STATUS(port),
> > + DP_TP_STATUS_FEC_ENABLE_LIVE,
> > + DP_TP_STATUS_FEC_ENABLE_LIVE,
> > +
Re: [Intel-gfx] [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
On Mon, Oct 15, 2018 at 02:50:36PM -0700, Anusha Srivatsa wrote:
> If FEC is supported, the corresponding
> DP_TP_CTL register bits have to be configured.
>
> The driver has to program the FEC_ENABLE in DP_TP_CTL[30] register
> and wait till FEC_STATUS in DP_TP_CTL[28] is 1.
> Also add the warn message to make sure that the control
> register is already active while enabling FEC.
>
> v2:
> - Change commit message. Configure fec state after
> link training (Manasi, Gaurav)
> - Remove redundent checks (Manasi)
> - Remove the registers that get added automagically (Anusha)
>
> v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state() (Gaurav)
>
> v4: rebased.
>
> Cc: Gaurav K Singh
> Cc: Jani Nikula
> Cc: Ville Syrjala
> Cc: Manasi Navare
> Signed-off-by: Anusha Srivatsa
> ---
> drivers/gpu/drm/i915/i915_reg.h | 2 ++
> drivers/gpu/drm/i915/intel_ddi.c | 2 ++
> drivers/gpu/drm/i915/intel_dp.c | 27 +++
> drivers/gpu/drm/i915/intel_drv.h | 3 ++-
> 4 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index bcde78bc0027..c8d7fdcd7823 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9093,6 +9093,7 @@ enum skl_power_gate {
> #define _DP_TP_CTL_B 0x64140
> #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A, _DP_TP_CTL_B)
> #define DP_TP_CTL_ENABLE(1 << 31)
> +#define DP_TP_CTL_FEC_ENABLE(1 << 30)
> #define DP_TP_CTL_MODE_SST (0 << 27)
> #define DP_TP_CTL_MODE_MST (1 << 27)
> #define DP_TP_CTL_FORCE_ACT (1 << 25)
> @@ -9111,6 +9112,7 @@ enum skl_power_gate {
> #define _DP_TP_STATUS_A 0x64044
> #define _DP_TP_STATUS_B 0x64144
> #define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A, _DP_TP_STATUS_B)
> +#define DP_TP_STATUS_FEC_ENABLE_LIVE(1 << 28)
> #define DP_TP_STATUS_IDLE_DONE (1 << 25)
> #define DP_TP_STATUS_ACT_SENT (1 << 24)
> #define DP_TP_STATUS_MODE_STATUS_MST(1 << 23)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index f531900165bf..67c013ea4d39 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2911,6 +2911,8 @@ static void intel_ddi_pre_enable_dp(struct
> intel_encoder *encoder,
> DP_DECOMPRESSION_EN);
> intel_dp_sink_set_fec_ready(intel_dp, crtc_state, DP_FEC_READY);
> intel_dp_start_link_train(intel_dp);
> + intel_dp_enable_fec_state(intel_dp, crtc_state);
This doesn't look like the correct spot. According to the spec it
should be after stop_link_train() (where we set DP_TP_CTL to
to normal).
> +
> if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
> intel_dp_stop_link_train(intel_dp);
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b4e8af3142a2..b9f85502d9ff 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3017,6 +3017,33 @@ void intel_dp_sink_set_fec_ready(struct intel_dp
> *intel_dp,
> DRM_DEBUG_KMS("Failed to get FEC enabled in sink\n");
> }
>
> +void intel_dp_enable_fec_state(struct intel_dp *intel_dp,
> +const struct intel_crtc_state *crtc_state)
> +{
> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + enum port port = intel_dig_port->base.port;
> + u32 val;
> +
> + /* FEC support exists for DP 1.4 only */
> + if (intel_dp_is_edp(intel_dp))
> + return;
> +
> + /* If Display Compression is not enabled, FEC need not be configured */
> + if (!crtc_state->dsc_params.compression_enable)
> + return;
> +
> + val = I915_READ(DP_TP_CTL(port));
> + val |= DP_TP_CTL_FEC_ENABLE;
> + I915_WRITE(DP_TP_CTL(port), val);
> +
> + if (intel_wait_for_register(dev_priv, DP_TP_STATUS(port),
> + DP_TP_STATUS_FEC_ENABLE_LIVE,
> + DP_TP_STATUS_FEC_ENABLE_LIVE,
> + 1))
> + DRM_ERROR("Timed out waiting for FEC Enable Status\n");
> +}
There is no reason to stick these into intel_dp.c when the
only caller is in intel_ddi.c.
We also seem to be missing platform checks. FEC doesn't appear to
be a thing prior to ICL. I guess the edp+compression_enable takes care
of this, but I think I'd rather stick a new fec_enable bool into
the crtc state. That way we get can add it to the state checker as well.
> +
> /* If the sink supports it, try to set the power state appropriately */
> void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
> {
> diff
Re: [Intel-gfx] [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
On Mon, Oct 15, 2018 at 02:50:36PM -0700, Anusha Srivatsa wrote:
> If FEC is supported, the corresponding
> DP_TP_CTL register bits have to be configured.
>
> The driver has to program the FEC_ENABLE in DP_TP_CTL[30] register
> and wait till FEC_STATUS in DP_TP_CTL[28] is 1.
> Also add the warn message to make sure that the control
> register is already active while enabling FEC.
>
> v2:
> - Change commit message. Configure fec state after
> link training (Manasi, Gaurav)
> - Remove redundent checks (Manasi)
> - Remove the registers that get added automagically (Anusha)
>
> v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state() (Gaurav)
>
> v4: rebased.
>
> Cc: Gaurav K Singh
> Cc: Jani Nikula
> Cc: Ville Syrjala
> Cc: Manasi Navare
> Signed-off-by: Anusha Srivatsa
> ---
> drivers/gpu/drm/i915/i915_reg.h | 2 ++
> drivers/gpu/drm/i915/intel_ddi.c | 2 ++
> drivers/gpu/drm/i915/intel_dp.c | 27 +++
> drivers/gpu/drm/i915/intel_drv.h | 3 ++-
> 4 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index bcde78bc0027..c8d7fdcd7823 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9093,6 +9093,7 @@ enum skl_power_gate {
> #define _DP_TP_CTL_B 0x64140
> #define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A, _DP_TP_CTL_B)
> #define DP_TP_CTL_ENABLE(1 << 31)
> +#define DP_TP_CTL_FEC_ENABLE(1 << 30)
> #define DP_TP_CTL_MODE_SST (0 << 27)
> #define DP_TP_CTL_MODE_MST (1 << 27)
> #define DP_TP_CTL_FORCE_ACT (1 << 25)
> @@ -9111,6 +9112,7 @@ enum skl_power_gate {
> #define _DP_TP_STATUS_A 0x64044
> #define _DP_TP_STATUS_B 0x64144
> #define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A, _DP_TP_STATUS_B)
> +#define DP_TP_STATUS_FEC_ENABLE_LIVE(1 << 28)
> #define DP_TP_STATUS_IDLE_DONE (1 << 25)
> #define DP_TP_STATUS_ACT_SENT (1 << 24)
> #define DP_TP_STATUS_MODE_STATUS_MST(1 << 23)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index f531900165bf..67c013ea4d39 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2911,6 +2911,8 @@ static void intel_ddi_pre_enable_dp(struct
> intel_encoder *encoder,
> DP_DECOMPRESSION_EN);
> intel_dp_sink_set_fec_ready(intel_dp, crtc_state, DP_FEC_READY);
> intel_dp_start_link_train(intel_dp);
> + intel_dp_enable_fec_state(intel_dp, crtc_state);
> +
> if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
> intel_dp_stop_link_train(intel_dp);
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b4e8af3142a2..b9f85502d9ff 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3017,6 +3017,33 @@ void intel_dp_sink_set_fec_ready(struct intel_dp
> *intel_dp,
> DRM_DEBUG_KMS("Failed to get FEC enabled in sink\n");
> }
>
> +void intel_dp_enable_fec_state(struct intel_dp *intel_dp,
> +const struct intel_crtc_state *crtc_state)
> +{
> + struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + enum port port = intel_dig_port->base.port;
> + u32 val;
> +
> + /* FEC support exists for DP 1.4 only */
> + if (intel_dp_is_edp(intel_dp))
> + return;
> +
> + /* If Display Compression is not enabled, FEC need not be configured */
> + if (!crtc_state->dsc_params.compression_enable)
> + return;
> +
> + val = I915_READ(DP_TP_CTL(port));
> + val |= DP_TP_CTL_FEC_ENABLE;
> + I915_WRITE(DP_TP_CTL(port), val);
> +
> + if (intel_wait_for_register(dev_priv, DP_TP_STATUS(port),
> + DP_TP_STATUS_FEC_ENABLE_LIVE,
> + DP_TP_STATUS_FEC_ENABLE_LIVE,
> + 1))
> + DRM_ERROR("Timed out waiting for FEC Enable Status\n");
I think in case of the error where we could not enable FEC,
we should also set the crtc_state->dsc_params.compression_enable = 0 since we
should not be enabling DSC without FEC enabled.
Jani/Anusha, thought?
Manasi
> +}
> +
> /* If the sink supports it, try to set the power state appropriately */
> void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
> {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index fbc9fa06e8be..e51d612a9f42 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1532,7 +1532,6 @@ intel_encoder_current_mode(struct intel_encoder
> *encoder);
[Intel-gfx] [v2 5/6] i915/dp/fec: Configure the Forward Error Correction bits.
If FEC is supported, the corresponding
DP_TP_CTL register bits have to be configured.
The driver has to program the FEC_ENABLE in DP_TP_CTL[30] register
and wait till FEC_STATUS in DP_TP_CTL[28] is 1.
Also add the warn message to make sure that the control
register is already active while enabling FEC.
v2:
- Change commit message. Configure fec state after
link training (Manasi, Gaurav)
- Remove redundent checks (Manasi)
- Remove the registers that get added automagically (Anusha)
v3: s/intel_dp_set_fec_state()/intel_dp_enable_fec_state() (Gaurav)
v4: rebased.
Cc: Gaurav K Singh
Cc: Jani Nikula
Cc: Ville Syrjala
Cc: Manasi Navare
Signed-off-by: Anusha Srivatsa
---
drivers/gpu/drm/i915/i915_reg.h | 2 ++
drivers/gpu/drm/i915/intel_ddi.c | 2 ++
drivers/gpu/drm/i915/intel_dp.c | 27 +++
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
4 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index bcde78bc0027..c8d7fdcd7823 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9093,6 +9093,7 @@ enum skl_power_gate {
#define _DP_TP_CTL_B 0x64140
#define DP_TP_CTL(port) _MMIO_PORT(port, _DP_TP_CTL_A, _DP_TP_CTL_B)
#define DP_TP_CTL_ENABLE (1 << 31)
+#define DP_TP_CTL_FEC_ENABLE (1 << 30)
#define DP_TP_CTL_MODE_SST(0 << 27)
#define DP_TP_CTL_MODE_MST(1 << 27)
#define DP_TP_CTL_FORCE_ACT (1 << 25)
@@ -9111,6 +9112,7 @@ enum skl_power_gate {
#define _DP_TP_STATUS_A0x64044
#define _DP_TP_STATUS_B0x64144
#define DP_TP_STATUS(port) _MMIO_PORT(port, _DP_TP_STATUS_A, _DP_TP_STATUS_B)
+#define DP_TP_STATUS_FEC_ENABLE_LIVE (1 << 28)
#define DP_TP_STATUS_IDLE_DONE(1 << 25)
#define DP_TP_STATUS_ACT_SENT (1 << 24)
#define DP_TP_STATUS_MODE_STATUS_MST (1 << 23)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f531900165bf..67c013ea4d39 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2911,6 +2911,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder
*encoder,
DP_DECOMPRESSION_EN);
intel_dp_sink_set_fec_ready(intel_dp, crtc_state, DP_FEC_READY);
intel_dp_start_link_train(intel_dp);
+ intel_dp_enable_fec_state(intel_dp, crtc_state);
+
if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
intel_dp_stop_link_train(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b4e8af3142a2..b9f85502d9ff 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3017,6 +3017,33 @@ void intel_dp_sink_set_fec_ready(struct intel_dp
*intel_dp,
DRM_DEBUG_KMS("Failed to get FEC enabled in sink\n");
}
+void intel_dp_enable_fec_state(struct intel_dp *intel_dp,
+ const struct intel_crtc_state *crtc_state)
+{
+ struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+ struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+ enum port port = intel_dig_port->base.port;
+ u32 val;
+
+ /* FEC support exists for DP 1.4 only */
+ if (intel_dp_is_edp(intel_dp))
+ return;
+
+ /* If Display Compression is not enabled, FEC need not be configured */
+ if (!crtc_state->dsc_params.compression_enable)
+ return;
+
+ val = I915_READ(DP_TP_CTL(port));
+ val |= DP_TP_CTL_FEC_ENABLE;
+ I915_WRITE(DP_TP_CTL(port), val);
+
+ if (intel_wait_for_register(dev_priv, DP_TP_STATUS(port),
+ DP_TP_STATUS_FEC_ENABLE_LIVE,
+ DP_TP_STATUS_FEC_ENABLE_LIVE,
+ 1))
+ DRM_ERROR("Timed out waiting for FEC Enable Status\n");
+}
+
/* If the sink supports it, try to set the power state appropriately */
void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
{
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fbc9fa06e8be..e51d612a9f42 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1532,7 +1532,6 @@ intel_encoder_current_mode(struct intel_encoder *encoder);
bool intel_port_is_tc(struct drm_i915_private *dev_priv, enum port port);
enum tc_port intel_port_to_tc(struct drm_i915_private *dev_priv,
enum port port);
-
enum pipe intel_get_pipe_from_connector(struct intel_connector *connector);
int intel_get_pipe_from_crtc_id_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
@@ -1712,6 +1711,8 @@ void
