Re: [Intel-gfx] [PATCH 3/3] drm/i915/display/mst: Enable virtual channel payload allocation earlier

2019-11-13 Thread Lucas De Marchi 

On Thu, Nov 07, 2019 at 11:18:37PM +, Jose Souza wrote:

On Thu, 2019-11-07 at 15:10 -0800, Lucas De Marchi wrote:

On Thu, Nov 07, 2019 at 10:56:09PM +, Jose Souza wrote:
> On Thu, 2019-11-07 at 14:44 -0800, Lucas De Marchi wrote:
> > On Thu, Nov 07, 2019 at 01:45:59PM -0800, Jose Souza wrote:
> > > This register was being enabled after enable TRANS_DDI_FUNC_CTL
> > > and
> > > PIPECONF/TRANS_CONF while BSpec states that it should be set
> > > when
> > > enabling TRANS_DDI_FUNC_CTL.
> > >
> > > BSpec: 49190
> >
> > not what I read here.
> >
> > 8j. Configure TRANS_DDI_FUNC_CTL2 if port sync mode needs to be
> > configured.
> > Then configure and enable TRANS_DDI_FUNC_CTL.
>
> We call icl_enable_trans_port_sync() in haswell_crtc_enable() and
> then
> a few lines later intel_ddi_enable_transcoder_func(), if not do
> that
> right away was a problem people working in port sync would get this
> issue.

not what I meant. I meant the spec says to enable TRANS_DDI_FUNC_CTL,
do step 8k, and then enable pipe VC payload allocation.

We aren't doing step 8k anywhere though, as I noted below.


8k is a VRR step that we don't support yet.


if we ever add it, then this would need to be split again. Anyway, I
think it's ok for now.

Reviewed-by: Lucas De Marchi 

Lucas De Marchi




Lucas De Marchi

> > 8l. If DisplayPort multistream - Enable pipe VC payload
> > allocation in
> > TRANS_DDI_FUNC_CTL
> >
> > But yes, this needs to be done before TRANS_CONF.
> >
> > > BSpec: 22243
> >
> > same here.  But as long as we don't do step 8k, I think they can
> > in
> > fact
> > be combined.
> >
> > These cover TGL and ICL only, while the code goes until haswell.
> > Are
> > you
> > sure it's safe for the others?
> >
>
> I had only checked if the register exist since HSW but now I
> checked
> HSW, BDW and SKL sequence all of then requires this.
>
> BSpec: 4223
> BSpec: 4163
> BSpec: 4231
>
>
> > thanks
> > Lucas De Marchi
> >
> > > Cc: Lucas De Marchi 
> > > Signed-off-by: José Roberto de Souza 
> > > ---
> > > drivers/gpu/drm/i915/display/intel_ddi.c | 18 ++---
> > > 
> > > -
> > > drivers/gpu/drm/i915/display/intel_display.c |  6 --
> > > 2 files changed, 2 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > index 398c6f054a6e..3d5fce878600 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > @@ -1803,22 +1803,6 @@ void intel_ddi_set_dp_msa(const struct
> > > intel_crtc_state *crtc_state,
> > >  I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
> > > }
> > >
> > > -void intel_ddi_set_vc_payload_alloc(const struct
> > > intel_crtc_state
> > > *crtc_state,
> > > -bool state)
> > > -{
> > > -struct intel_crtc *crtc = to_intel_crtc(crtc_state-
> > > >uapi.crtc);
> > > -struct drm_i915_private *dev_priv = to_i915(crtc-
> > > >base.dev);
> > > -enum transcoder cpu_transcoder = crtc_state-
> > > >cpu_transcoder;
> > > -u32 temp;
> > > -
> > > -temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> > > -if (state == true)
> > > -temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> > > -else
> > > -temp &= ~TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> > > -I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), temp);
> > > -}
> > > -
> > > /*
> > >  * Returns the TRANS_DDI_FUNC_CTL value based on CRTC state.
> > >  *
> > > @@ -1924,6 +1908,8 @@ void
> > > intel_ddi_enable_transcoder_func(const
> > > struct intel_crtc_state *crtc_state)
> > >  u32 temp;
> > >
> > >  temp =
> > > intel_ddi_transcoder_func_reg_val_get(crtc_state);
> > > +if (intel_crtc_has_type(crtc_state,
> > > INTEL_OUTPUT_DP_MST))
> > > +temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> > >  I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), temp);
> > > }
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 551de2baa569..3b4aea253f8c 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -6670,9 +6670,6 @@ static void haswell_crtc_enable(struct
> > > intel_crtc_state *pipe_config,
> > >  if (pipe_config->has_pch_encoder)
> > >  lpt_pch_enable(state, pipe_config);
> > >
> > > -if (intel_crtc_has_type(pipe_config,
> > > INTEL_OUTPUT_DP_MST))
> > > -intel_ddi_set_vc_payload_alloc(pipe_config,
> > > true);
> > > -
> > >  assert_vblank_disabled(crtc);
> > >  intel_crtc_vblank_on(pipe_config);
> > >
> > > @@ -6783,9 +6780,6 @@ static void haswell_crtc_disable(struct
> > > intel_crtc_state *old_crtc_state,
> > >  if (!transcoder_is_dsi(cpu_transcoder))
> > >  intel_disable_pipe(old_crtc_state);
> > >
> > > -if (intel_crtc_has_type(old_crtc_state,
> > >

Re: [Intel-gfx] [PATCH 3/3] drm/i915/display/mst: Enable virtual channel payload allocation earlier

2019-11-07 Thread Souza, Jose 
On Thu, 2019-11-07 at 15:10 -0800, Lucas De Marchi wrote:
> On Thu, Nov 07, 2019 at 10:56:09PM +, Jose Souza wrote:
> > On Thu, 2019-11-07 at 14:44 -0800, Lucas De Marchi wrote:
> > > On Thu, Nov 07, 2019 at 01:45:59PM -0800, Jose Souza wrote:
> > > > This register was being enabled after enable TRANS_DDI_FUNC_CTL
> > > > and
> > > > PIPECONF/TRANS_CONF while BSpec states that it should be set
> > > > when
> > > > enabling TRANS_DDI_FUNC_CTL.
> > > > 
> > > > BSpec: 49190
> > > 
> > > not what I read here.
> > > 
> > > 8j. Configure TRANS_DDI_FUNC_CTL2 if port sync mode needs to be
> > > configured.
> > > Then configure and enable TRANS_DDI_FUNC_CTL.
> > 
> > We call icl_enable_trans_port_sync() in haswell_crtc_enable() and
> > then
> > a few lines later intel_ddi_enable_transcoder_func(), if not do
> > that
> > right away was a problem people working in port sync would get this
> > issue.
> 
> not what I meant. I meant the spec says to enable TRANS_DDI_FUNC_CTL,
> do step 8k, and then enable pipe VC payload allocation.
> 
> We aren't doing step 8k anywhere though, as I noted below.

8k is a VRR step that we don't support yet.

> 
> Lucas De Marchi
> 
> > > 8l. If DisplayPort multistream - Enable pipe VC payload
> > > allocation in
> > > TRANS_DDI_FUNC_CTL
> > > 
> > > But yes, this needs to be done before TRANS_CONF.
> > > 
> > > > BSpec: 22243
> > > 
> > > same here.  But as long as we don't do step 8k, I think they can
> > > in
> > > fact
> > > be combined.
> > > 
> > > These cover TGL and ICL only, while the code goes until haswell.
> > > Are
> > > you
> > > sure it's safe for the others?
> > > 
> > 
> > I had only checked if the register exist since HSW but now I
> > checked
> > HSW, BDW and SKL sequence all of then requires this.
> > 
> > BSpec: 4223
> > BSpec: 4163
> > BSpec: 4231
> > 
> > 
> > > thanks
> > > Lucas De Marchi
> > > 
> > > > Cc: Lucas De Marchi 
> > > > Signed-off-by: José Roberto de Souza 
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_ddi.c | 18 ++---
> > > > 
> > > > -
> > > > drivers/gpu/drm/i915/display/intel_display.c |  6 --
> > > > 2 files changed, 2 insertions(+), 22 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > index 398c6f054a6e..3d5fce878600 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > > > @@ -1803,22 +1803,6 @@ void intel_ddi_set_dp_msa(const struct
> > > > intel_crtc_state *crtc_state,
> > > > I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
> > > > }
> > > > 
> > > > -void intel_ddi_set_vc_payload_alloc(const struct
> > > > intel_crtc_state
> > > > *crtc_state,
> > > > -   bool state)
> > > > -{
> > > > -   struct intel_crtc *crtc = to_intel_crtc(crtc_state-
> > > > >uapi.crtc);
> > > > -   struct drm_i915_private *dev_priv = to_i915(crtc-
> > > > >base.dev);
> > > > -   enum transcoder cpu_transcoder = crtc_state-
> > > > >cpu_transcoder;
> > > > -   u32 temp;
> > > > -
> > > > -   temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> > > > -   if (state == true)
> > > > -   temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> > > > -   else
> > > > -   temp &= ~TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> > > > -   I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), temp);
> > > > -}
> > > > -
> > > > /*
> > > >  * Returns the TRANS_DDI_FUNC_CTL value based on CRTC state.
> > > >  *
> > > > @@ -1924,6 +1908,8 @@ void
> > > > intel_ddi_enable_transcoder_func(const
> > > > struct intel_crtc_state *crtc_state)
> > > > u32 temp;
> > > > 
> > > > temp =
> > > > intel_ddi_transcoder_func_reg_val_get(crtc_state);
> > > > +   if (intel_crtc_has_type(crtc_state,
> > > > INTEL_OUTPUT_DP_MST))
> > > > +   temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> > > > I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), temp);
> > > > }
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 551de2baa569..3b4aea253f8c 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -6670,9 +6670,6 @@ static void haswell_crtc_enable(struct
> > > > intel_crtc_state *pipe_config,
> > > > if (pipe_config->has_pch_encoder)
> > > > lpt_pch_enable(state, pipe_config);
> > > > 
> > > > -   if (intel_crtc_has_type(pipe_config,
> > > > INTEL_OUTPUT_DP_MST))
> > > > -   intel_ddi_set_vc_payload_alloc(pipe_config,
> > > > true);
> > > > -
> > > > assert_vblank_disabled(crtc);
> > > > intel_crtc_vblank_on(pipe_config);
> > > > 
> > > > @@ -6783,9 +6780,6 @@ static void haswell_crtc_disable(struct
> > > > intel_crtc_state *old_crtc_state,
> > > > if (!transcoder_is_dsi(cpu_transcoder))

Re: [Intel-gfx] [PATCH 3/3] drm/i915/display/mst: Enable virtual channel payload allocation earlier

2019-11-07 Thread Lucas De Marchi 

On Thu, Nov 07, 2019 at 10:56:09PM +, Jose Souza wrote:

On Thu, 2019-11-07 at 14:44 -0800, Lucas De Marchi wrote:

On Thu, Nov 07, 2019 at 01:45:59PM -0800, Jose Souza wrote:
> This register was being enabled after enable TRANS_DDI_FUNC_CTL and
> PIPECONF/TRANS_CONF while BSpec states that it should be set when
> enabling TRANS_DDI_FUNC_CTL.
>
> BSpec: 49190

not what I read here.

8j. Configure TRANS_DDI_FUNC_CTL2 if port sync mode needs to be
configured.
Then configure and enable TRANS_DDI_FUNC_CTL.


We call icl_enable_trans_port_sync() in haswell_crtc_enable() and then
a few lines later intel_ddi_enable_transcoder_func(), if not do that
right away was a problem people working in port sync would get this
issue.


not what I meant. I meant the spec says to enable TRANS_DDI_FUNC_CTL,
do step 8k, and then enable pipe VC payload allocation.

We aren't doing step 8k anywhere though, as I noted below.

Lucas De Marchi





8l. If DisplayPort multistream - Enable pipe VC payload allocation in
TRANS_DDI_FUNC_CTL

But yes, this needs to be done before TRANS_CONF.

> BSpec: 22243

same here.  But as long as we don't do step 8k, I think they can in
fact
be combined.

These cover TGL and ICL only, while the code goes until haswell. Are
you
sure it's safe for the others?



I had only checked if the register exist since HSW but now I checked
HSW, BDW and SKL sequence all of then requires this.

BSpec: 4223
BSpec: 4163
BSpec: 4231



thanks
Lucas De Marchi

> Cc: Lucas De Marchi 
> Signed-off-by: José Roberto de Souza 
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 18 ++---
> -
> drivers/gpu/drm/i915/display/intel_display.c |  6 --
> 2 files changed, 2 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 398c6f054a6e..3d5fce878600 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1803,22 +1803,6 @@ void intel_ddi_set_dp_msa(const struct
> intel_crtc_state *crtc_state,
>I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
> }
>
> -void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state
> *crtc_state,
> -  bool state)
> -{
> -  struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -  struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -  enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> -  u32 temp;
> -
> -  temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> -  if (state == true)
> -  temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> -  else
> -  temp &= ~TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> -  I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), temp);
> -}
> -
> /*
>  * Returns the TRANS_DDI_FUNC_CTL value based on CRTC state.
>  *
> @@ -1924,6 +1908,8 @@ void intel_ddi_enable_transcoder_func(const
> struct intel_crtc_state *crtc_state)
>u32 temp;
>
>temp = intel_ddi_transcoder_func_reg_val_get(crtc_state);
> +  if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST))
> +  temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
>I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), temp);
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 551de2baa569..3b4aea253f8c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6670,9 +6670,6 @@ static void haswell_crtc_enable(struct
> intel_crtc_state *pipe_config,
>if (pipe_config->has_pch_encoder)
>lpt_pch_enable(state, pipe_config);
>
> -  if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
> -  intel_ddi_set_vc_payload_alloc(pipe_config, true);
> -
>assert_vblank_disabled(crtc);
>intel_crtc_vblank_on(pipe_config);
>
> @@ -6783,9 +6780,6 @@ static void haswell_crtc_disable(struct
> intel_crtc_state *old_crtc_state,
>if (!transcoder_is_dsi(cpu_transcoder))
>intel_disable_pipe(old_crtc_state);
>
> -  if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST))
> -  intel_ddi_set_vc_payload_alloc(old_crtc_state, false);
> -
>if (INTEL_GEN(dev_priv) >= 11)
>icl_disable_transcoder_port_sync(old_crtc_state);
>
> --
> 2.24.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] drm/i915/display/mst: Enable virtual channel payload allocation earlier

2019-11-07 Thread Souza, Jose 
On Thu, 2019-11-07 at 14:44 -0800, Lucas De Marchi wrote:
> On Thu, Nov 07, 2019 at 01:45:59PM -0800, Jose Souza wrote:
> > This register was being enabled after enable TRANS_DDI_FUNC_CTL and
> > PIPECONF/TRANS_CONF while BSpec states that it should be set when
> > enabling TRANS_DDI_FUNC_CTL.
> > 
> > BSpec: 49190
> 
> not what I read here.
> 
> 8j. Configure TRANS_DDI_FUNC_CTL2 if port sync mode needs to be
> configured.
> Then configure and enable TRANS_DDI_FUNC_CTL.

We call icl_enable_trans_port_sync() in haswell_crtc_enable() and then
a few lines later intel_ddi_enable_transcoder_func(), if not do that
right away was a problem people working in port sync would get this
issue.

> 
> 8l. If DisplayPort multistream - Enable pipe VC payload allocation in
> TRANS_DDI_FUNC_CTL
> 
> But yes, this needs to be done before TRANS_CONF.
> 
> > BSpec: 22243
> 
> same here.  But as long as we don't do step 8k, I think they can in
> fact
> be combined.
> 
> These cover TGL and ICL only, while the code goes until haswell. Are
> you
> sure it's safe for the others?
> 

I had only checked if the register exist since HSW but now I checked
HSW, BDW and SKL sequence all of then requires this.

BSpec: 4223
BSpec: 4163
BSpec: 4231


> thanks
> Lucas De Marchi
> 
> > Cc: Lucas De Marchi 
> > Signed-off-by: José Roberto de Souza 
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 18 ++---
> > -
> > drivers/gpu/drm/i915/display/intel_display.c |  6 --
> > 2 files changed, 2 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index 398c6f054a6e..3d5fce878600 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -1803,22 +1803,6 @@ void intel_ddi_set_dp_msa(const struct
> > intel_crtc_state *crtc_state,
> > I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
> > }
> > 
> > -void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state
> > *crtc_state,
> > -   bool state)
> > -{
> > -   struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > -   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > -   enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> > -   u32 temp;
> > -
> > -   temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> > -   if (state == true)
> > -   temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> > -   else
> > -   temp &= ~TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> > -   I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), temp);
> > -}
> > -
> > /*
> >  * Returns the TRANS_DDI_FUNC_CTL value based on CRTC state.
> >  *
> > @@ -1924,6 +1908,8 @@ void intel_ddi_enable_transcoder_func(const
> > struct intel_crtc_state *crtc_state)
> > u32 temp;
> > 
> > temp = intel_ddi_transcoder_func_reg_val_get(crtc_state);
> > +   if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST))
> > +   temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> > I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), temp);
> > }
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 551de2baa569..3b4aea253f8c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -6670,9 +6670,6 @@ static void haswell_crtc_enable(struct
> > intel_crtc_state *pipe_config,
> > if (pipe_config->has_pch_encoder)
> > lpt_pch_enable(state, pipe_config);
> > 
> > -   if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
> > -   intel_ddi_set_vc_payload_alloc(pipe_config, true);
> > -
> > assert_vblank_disabled(crtc);
> > intel_crtc_vblank_on(pipe_config);
> > 
> > @@ -6783,9 +6780,6 @@ static void haswell_crtc_disable(struct
> > intel_crtc_state *old_crtc_state,
> > if (!transcoder_is_dsi(cpu_transcoder))
> > intel_disable_pipe(old_crtc_state);
> > 
> > -   if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST))
> > -   intel_ddi_set_vc_payload_alloc(old_crtc_state, false);
> > -
> > if (INTEL_GEN(dev_priv) >= 11)
> > icl_disable_transcoder_port_sync(old_crtc_state);
> > 
> > -- 
> > 2.24.0
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/3] drm/i915/display/mst: Enable virtual channel payload allocation earlier

2019-11-07 Thread Lucas De Marchi 

On Thu, Nov 07, 2019 at 01:45:59PM -0800, Jose Souza wrote:

This register was being enabled after enable TRANS_DDI_FUNC_CTL and
PIPECONF/TRANS_CONF while BSpec states that it should be set when
enabling TRANS_DDI_FUNC_CTL.

BSpec: 49190


not what I read here.

8j. Configure TRANS_DDI_FUNC_CTL2 if port sync mode needs to be configured.
Then configure and enable TRANS_DDI_FUNC_CTL.

8l. If DisplayPort multistream - Enable pipe VC payload allocation in 
TRANS_DDI_FUNC_CTL

But yes, this needs to be done before TRANS_CONF.


BSpec: 22243


same here.  But as long as we don't do step 8k, I think they can in fact
be combined.

These cover TGL and ICL only, while the code goes until haswell. Are you
sure it's safe for the others?

thanks
Lucas De Marchi


Cc: Lucas De Marchi 
Signed-off-by: José Roberto de Souza 
---
drivers/gpu/drm/i915/display/intel_ddi.c | 18 ++
drivers/gpu/drm/i915/display/intel_display.c |  6 --
2 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 398c6f054a6e..3d5fce878600 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1803,22 +1803,6 @@ void intel_ddi_set_dp_msa(const struct intel_crtc_state 
*crtc_state,
I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
}

-void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
-   bool state)
-{
-   struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-   enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
-   u32 temp;
-
-   temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
-   if (state == true)
-   temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
-   else
-   temp &= ~TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
-   I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), temp);
-}
-
/*
 * Returns the TRANS_DDI_FUNC_CTL value based on CRTC state.
 *
@@ -1924,6 +1908,8 @@ void intel_ddi_enable_transcoder_func(const struct 
intel_crtc_state *crtc_state)
u32 temp;

temp = intel_ddi_transcoder_func_reg_val_get(crtc_state);
+   if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST))
+   temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), temp);
}

diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 551de2baa569..3b4aea253f8c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6670,9 +6670,6 @@ static void haswell_crtc_enable(struct intel_crtc_state 
*pipe_config,
if (pipe_config->has_pch_encoder)
lpt_pch_enable(state, pipe_config);

-   if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
-   intel_ddi_set_vc_payload_alloc(pipe_config, true);
-
assert_vblank_disabled(crtc);
intel_crtc_vblank_on(pipe_config);

@@ -6783,9 +6780,6 @@ static void haswell_crtc_disable(struct intel_crtc_state 
*old_crtc_state,
if (!transcoder_is_dsi(cpu_transcoder))
intel_disable_pipe(old_crtc_state);

-   if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST))
-   intel_ddi_set_vc_payload_alloc(old_crtc_state, false);
-
if (INTEL_GEN(dev_priv) >= 11)
icl_disable_transcoder_port_sync(old_crtc_state);

--
2.24.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 3/3] drm/i915/display/mst: Enable virtual channel payload allocation earlier

2019-11-07 Thread José Roberto de Souza 
This register was being enabled after enable TRANS_DDI_FUNC_CTL and
PIPECONF/TRANS_CONF while BSpec states that it should be set when
enabling TRANS_DDI_FUNC_CTL.

BSpec: 49190
BSpec: 22243
Cc: Lucas De Marchi 
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 18 ++
 drivers/gpu/drm/i915/display/intel_display.c |  6 --
 2 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
b/drivers/gpu/drm/i915/display/intel_ddi.c
index 398c6f054a6e..3d5fce878600 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -1803,22 +1803,6 @@ void intel_ddi_set_dp_msa(const struct intel_crtc_state 
*crtc_state,
I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp);
 }
 
-void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
-   bool state)
-{
-   struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-   enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
-   u32 temp;
-
-   temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
-   if (state == true)
-   temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
-   else
-   temp &= ~TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
-   I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), temp);
-}
-
 /*
  * Returns the TRANS_DDI_FUNC_CTL value based on CRTC state.
  *
@@ -1924,6 +1908,8 @@ void intel_ddi_enable_transcoder_func(const struct 
intel_crtc_state *crtc_state)
u32 temp;
 
temp = intel_ddi_transcoder_func_reg_val_get(crtc_state);
+   if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST))
+   temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
I915_WRITE(TRANS_DDI_FUNC_CTL(cpu_transcoder), temp);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_display.c 
b/drivers/gpu/drm/i915/display/intel_display.c
index 551de2baa569..3b4aea253f8c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6670,9 +6670,6 @@ static void haswell_crtc_enable(struct intel_crtc_state 
*pipe_config,
if (pipe_config->has_pch_encoder)
lpt_pch_enable(state, pipe_config);
 
-   if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_DP_MST))
-   intel_ddi_set_vc_payload_alloc(pipe_config, true);
-
assert_vblank_disabled(crtc);
intel_crtc_vblank_on(pipe_config);
 
@@ -6783,9 +6780,6 @@ static void haswell_crtc_disable(struct intel_crtc_state 
*old_crtc_state,
if (!transcoder_is_dsi(cpu_transcoder))
intel_disable_pipe(old_crtc_state);
 
-   if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST))
-   intel_ddi_set_vc_payload_alloc(old_crtc_state, false);
-
if (INTEL_GEN(dev_priv) >= 11)
icl_disable_transcoder_port_sync(old_crtc_state);
 
-- 
2.24.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx