Re: [Intel-gfx] [PATCH i-g-t v3] lib/igt_kms: Force outputs to use full range RGB

[Conselvan De Oliveira, Ander]

On Tue, 2017-05-09 at 13:22 +0300, Mika Kahola wrote:
> On Tue, 2017-04-18 at 16:04 +0300, Ander Conselvan de Oliveira wrote:
> > In at least SKL and GLK (possibly other devices too), using a cursor
> > plane to scan out an fb might result in a different pipe crc than
> > when
> > using a regular plane at the same position with the same fb while
> > using
> > the CSC logic to limit the color range. The differences could be
> > caused
> > by the cursor plane being limited to 8 bpc while the regular planes
> > support higher bit depths, leading to slightly different values to be
> > used internally. This is evidenced by the failures happening with
> > specific color values, 0.5 for example, but that's mostly
> > speculation.
> > 
> > To avoid misterious failures caused by limited range rgb, force all
> > tests to use full range. It is still possible for tests to override
> > this
> > if necessary.
> 
> By this way, we know for sure what is the color range in use.
> 
> Reviewed-by: Mika Kahola 

Thanks, patch pushed.

Ander

> 
> >  
> > v2: Add more details to the commit message.
> > v3: Force all tests to use full range.
> > Cc: Ville Syrjälä 
> > Signed-off-by: Ander Conselvan de Oliveira
> > 
> > ---
> >  lib/igt_kms.c | 7 ++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index 5811414..9f72913 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -1455,10 +1455,15 @@ static void igt_output_refresh(igt_output_t
> > *output)
> >        -1);
> >     }
> >  
> > -   if (output->config.connector)
> > +   if (output->config.connector) {
> >     igt_atomic_fill_connector_props(display, output,
> >     IGT_NUM_CONNECTOR_PROPS,
> > igt_connector_prop_names);
> >  
> > +   kmstest_set_connector_broadcast_rgb(display->drm_fd,
> > +   output-
> > > config.connector,
> > 
> > +   BROADCAST_RGB_FU
> > LL);
> > +   }
> > +
> >     if (output->use_override_mode)
> >     output->config.default_mode = output->override_mode;
> >  
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 4/5] drm/i915/glk: Program pipe gamma and degamma tables

[Conselvan De Oliveira, Ander]

On Thu, 2017-01-26 at 16:21 +0200, Ville Syrjälä wrote:
> On Thu, Jan 26, 2017 at 01:24:24PM +0200, Ander Conselvan de Oliveira wrote:
> > The gamma tables in Geminilake were changed. There is no split-gamma
> > mode. Instead, there is a dedicated degamma table that is enabled
> > whenever pipe CSC is enabled.
> > 
> > The dedicated gamma table has 16 bit precision but doesn't support
> > separate channels. Since that doesn't match the per-channel format of
> > the degamma LUT property, for now only a linear table is loaded and the
> > property ignored.
> > 
> > v2: Remove empty line. (Ville)
> > Reuse broadwell code. (Ville)
> > 
> > Cc: Ville Syrjälä 
> > Signed-off-by: Ander Conselvan de Oliveira  > l.com>
> > ---
> >  drivers/gpu/drm/i915/i915_pci.c|  1 +
> >  drivers/gpu/drm/i915/i915_reg.h| 14 +
> >  drivers/gpu/drm/i915/intel_color.c | 60
> > +-
> >  3 files changed, 74 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_pci.c
> > b/drivers/gpu/drm/i915/i915_pci.c
> > index ecb487b..df2051b 100644
> > --- a/drivers/gpu/drm/i915/i915_pci.c
> > +++ b/drivers/gpu/drm/i915/i915_pci.c
> > @@ -403,6 +403,7 @@ static const struct intel_device_info
> > intel_geminilake_info = {
> >     .platform = INTEL_GEMINILAKE,
> >     .is_alpha_support = 1,
> >     .ddb_size = 1024,
> > +   .color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 }
> >  };
> >  
> >  static const struct intel_device_info intel_kabylake_info = {
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 06bbe55..e029691 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -8181,12 +8181,26 @@ enum {
> >  #define _PAL_PREC_EXT_GC_MAX_A 0x4A420
> >  #define _PAL_PREC_EXT_GC_MAX_B 0x4AC20
> >  #define _PAL_PREC_EXT_GC_MAX_C 0x4B420
> > +#define _PAL_PREC_EXT2_GC_MAX_A0x4A430
> > +#define _PAL_PREC_EXT2_GC_MAX_B0x4AC30
> > +#define _PAL_PREC_EXT2_GC_MAX_C0x4B430
> >  
> >  #define PREC_PAL_INDEX(pipe)   _MMIO_PIPE(pipe,
> > _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B)
> >  #define PREC_PAL_DATA(pipe)_MMIO_PIPE(pipe,
> > _PAL_PREC_DATA_A, _PAL_PREC_DATA_B)
> >  #define PREC_PAL_GC_MAX(pipe, i)   _MMIO(_PIPE(pipe,
> > _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B) + (i) * 4)
> >  #define PREC_PAL_EXT_GC_MAX(pipe, i)   _MMIO(_PIPE(pipe,
> > _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B) + (i) * 4)
> >  
> > +#define _PRE_CSC_GAMC_INDEX_A  0x4A484
> > +#define _PRE_CSC_GAMC_INDEX_B  0x4AC84
> > +#define _PRE_CSC_GAMC_INDEX_C  0x4B484
> > +#define   PRE_CSC_GAMC_AUTO_INCREMENT  (1 << 10)
> > +#define _PRE_CSC_GAMC_DATA_A   0x4A488
> > +#define _PRE_CSC_GAMC_DATA_B   0x4AC88
> > +#define _PRE_CSC_GAMC_DATA_C   0x4B488
> > +
> > +#define PRE_CSC_GAMC_INDEX(pipe)   _MMIO_PIPE(pipe,
> > _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
> > +#define PRE_CSC_GAMC_DATA(pipe)_MMIO_PIPE(pipe,
> > _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
> > +
> >  /* pipe CSC & degamma/gamma LUTs on CHV */
> >  #define _CGM_PIPE_A_CSC_COEFF01(VLV_DISPLAY_BASE + 0x67900)
> >  #define _CGM_PIPE_A_CSC_COEFF23(VLV_DISPLAY_BASE + 0x67904)
> > diff --git a/drivers/gpu/drm/i915/intel_color.c
> > b/drivers/gpu/drm/i915/intel_color.c
> > index 82a3bc9..2125aa3 100644
> > --- a/drivers/gpu/drm/i915/intel_color.c
> > +++ b/drivers/gpu/drm/i915/intel_color.c
> > @@ -380,7 +380,9 @@ static void bdw_load_gamma_lut(struct drm_crtc_state
> > *state, u32 offset)
> >     WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
> >  
> >     I915_WRITE(PREC_PAL_INDEX(pipe),
> > -      PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT | offset);
> > +      (offset ? PAL_PREC_SPLIT_MODE : 0) |
> > +      PAL_PREC_AUTO_INCREMENT |
> > +      offset);
> 
> This confused me for a bit. I was thinking we're using this to write the
> deamma part for the split gamma case as well, which would end up
> disabling the split gamma mode when doing that. But that's not actually
> what's happening since you had another function to write the degamma
> half.
> 
> >  
> >     if (state->gamma_lut) {
> >     struct drm_color_lut *lut =
> > @@ -443,6 +445,59 @@ static void broadwell_load_luts(struct drm_crtc_state
> > *state)
> >     I915_WRITE(PREC_PAL_INDEX(pipe), 0);
> >  }
> >  
> > +static void glk_load_degamma_lut(struct drm_crtc_state *state)
> > +{
> > +   struct drm_i915_private *dev_priv = to_i915(state->crtc->dev);
> > +   enum pipe pipe = to_intel_crtc(state->crtc)->pipe;
> > +   const uint32_t lut_size = 33;
> > +   uint32_t i;
> > +
> > +   /*
> > +    * When setting the auto-increment bit, the hardware seems to
> > +    * ignore the index bits, so we need to reset it to index 0
> > +    * separately.
> > +    */
> 
> Interesting. Do we know if the same 

Re: [Intel-gfx] [PATCH] drm/i915: Use IS_GEN9 and IS_LP to put Skylake and Kabylake in the same bucket.

[Conselvan De Oliveira, Ander]

On Fri, 2017-01-20 at 08:22 -0800, Rodrigo Vivi wrote:
> On Thu, Jan 19, 2017 at 10:56 PM, Jani Nikula  wrote:
> > 
> > On Fri, 20 Jan 2017, Anusha Srivatsa  wrote:
> > > 
> > > With GLK was introduced IS_LP.
> > > With this, we can use IS_GEN9 and IS_LP to put Skylake
> > > and Kabylake in the same bucket. The main intention is
> > > to simplify the if and else statements that gets added
> > > for every new platform.
> > > 
> > > This approach proposes the idea to create a common bucket
> > > to put multiple platforms (SKL and KBL in this case)
> > > together. Another approach would be to introduce IS_GEN9_BC
> > > as suggested by Rodrigo, where _BC stands for Big Core.
> > I think we need to come up with an alias like IS_GEN9_BC(), although I'm
> > not convinced about that paraticular name. The current (FOO || BAR) for
> > specific FOO and BAR is *much* faster to read than the slightly
> > confusing (GROUP && !SUBGROUP).
> I don't have a strong opinion on this one except that I believe we
> need to group somehow.
> 
> But I didn't try to merge the reviewed is_gen9_bc because someone
> raised concerns on irc like naking it...
> maybe Mika or Marteen, I can't remember...
> Ander, do you remember who complained that day on irc?

I think it was Joonas. He didn't like my suggestion of _HP for high power. To be
honest, I don't like it either. :)
> 
> I remember Tvrtko was the one in favor of using !is_lp...

IS_GEN9_NP for not-low power. Or even normal power.

> 
> I agree _BC (Big Core) is not a good name... but I couldn't come with
> something better. :(
> 
> So either way works for me as long as we bucket is somehow...
> So, comments?

I don't know either. I think there's value in the change and I wouldn't mind if
it is called _BC, and that's why that patch has my R-b. Wouldn't it be easier to
do sed 's/IS_GEN9_BC/something else/' if some one has a great moment of
inspiration?

Ander

> 
> > 
> > 
> > One other comment inline.
> > 
> > > 
> > > The i915_drv.c was let out of this patch on purpose
> > > because that is really a decision per platform, just like
> > > other cases where IS_KABYLAKE is different from IS_SKYLAKE.
> > > 
> > > Cc: Rodrigo Vivi 
> > > Cc: Ander Conselvan de Oliveira 
> > > Cc: Jani Nikula 
> > > Signed-off-by: Anusha Srivatsa 
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c  | 12 ++--
> > >  drivers/gpu/drm/i915/intel_audio.c   |  2 +-
> > >  drivers/gpu/drm/i915/intel_color.c   |  4 ++--
> > >  drivers/gpu/drm/i915/intel_ddi.c | 20 ++--
> > >  drivers/gpu/drm/i915/intel_device_info.c |  2 +-
> > >  drivers/gpu/drm/i915/intel_display.c | 12 ++--
> > >  drivers/gpu/drm/i915/intel_dp.c  |  4 ++--
> > >  drivers/gpu/drm/i915/intel_dpll_mgr.c|  2 +-
> > >  drivers/gpu/drm/i915/intel_fbc.c |  2 +-
> > >  drivers/gpu/drm/i915/intel_i2c.c |  4 ++--
> > >  drivers/gpu/drm/i915/intel_mocs.c|  2 +-
> > >  drivers/gpu/drm/i915/intel_pm.c  | 11 +--
> > >  12 files changed, 38 insertions(+), 39 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index fa69d72..a4fdb9f 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -1224,20 +1224,20 @@ static int i915_frequency_info(struct seq_file *m,
> > > void *unused)
> > > 
> > >   max_freq = (IS_GEN9_LP(dev_priv) ? rp_state_cap >> 0 :
> > >   rp_state_cap >> 16) & 0xff;
> > > - max_freq *= (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv) ?
> > > + max_freq *= ((IS_GEN9(dev_priv) && !IS_LP(dev_priv)) ?
> > >    GEN9_FREQ_SCALER : 1);
> > There's already superfluous braces to begin with, please don't add more!
> > Same all around.
> indeed
> 
> > 
> > 
> > > 
> > >   seq_printf(m, "Lowest (RPN) frequency: %dMHz\n",
> > >  intel_gpu_freq(dev_priv, max_freq));
> > > 
> > >   max_freq = (rp_state_cap & 0xff00) >> 8;
> > > - max_freq *= (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv) ?
> > > + max_freq *= ((IS_GEN9(dev_priv) && !IS_LP(dev_priv)) ?
> > >    GEN9_FREQ_SCALER : 1);
> > >   seq_printf(m, "Nominal (RP1) frequency: %dMHz\n",
> > >  intel_gpu_freq(dev_priv, max_freq));
> > > 
> > >   max_freq = (IS_GEN9_LP(dev_priv) ? rp_state_cap >> 16 :
> > >   rp_state_cap >> 0) & 0xff;
> > > - max_freq *= (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv) ?
> > > + max_freq *= ((IS_GEN9(dev_priv) && !IS_LP(dev_priv)) ?
> > >    GEN9_FREQ_SCALER : 1);
> > >   

Re: [Intel-gfx] [PATCH] drm/i915: Initialize num_scalers for skl and glk too

[Conselvan De Oliveira, Ander]

On Tue, 2017-01-03 at 15:35 +0200, Ville Syrjälä wrote:
> On Mon, Jan 02, 2017 at 03:54:41PM +0200, Ander Conselvan de Oliveira wrote:
> > 
> > After commit 1c74eeaf16b8 ("drm/i915: Move number of scalers initialization
> > to
> > runtime init"), scalers are not initialized properly for skl and glk
> > since num_scalers is left as 0 for those platforms.
> > 
> > Fixes: 1c74eeaf16b8 ("drm/i915: Move number of scalers initialization to
> > runtime init")
> > Cc: Nabendu Maiti 
> > Cc: Chris Wilson  (v2)
> > Cc: Ander Conselvan de Oliveira 
> > Cc: Ander Conselvan de Oliveira 
> > Cc: Daniel Vetter 
> > Cc: Jani Nikula 
> > Cc: intel-gfx@lists.freedesktop.org
> > Signed-off-by: Ander Conselvan de Oliveira  > l.com>
> Fixes my CCS test. What happened was apparently the BIOS leaving the
> scaler enabled and then when i915 took over all but the preferred mode
> of the display came out garbled on account of the scaler output not
> fitting within the h/vactive of the mode.
> 
> Tested-by: Ville Syrjälä 
> Reviewed-by: Ville Syrjälä 

Pushed. Thanks for reviewing.

Ander

> 
> > 
> > ---
> >  drivers/gpu/drm/i915/intel_device_info.c | 9 ++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> > b/drivers/gpu/drm/i915/intel_device_info.c
> > index 1b5ffc4..f642f6d 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > @@ -310,6 +310,12 @@ void intel_device_info_runtime_init(struct
> > drm_i915_private *dev_priv)
> >     struct intel_device_info *info = mkwrite_device_info(dev_priv);
> >     enum pipe pipe;
> >  
> > +   if (INTEL_GEN(dev_priv) >= 9) {
> > +   info->num_scalers[PIPE_A] = 2;
> > +   info->num_scalers[PIPE_B] = 2;
> > +   info->num_scalers[PIPE_C] = 1;
> > +   }
> > +
> >     /*
> >      * Skylake and Broxton currently don't expose the topmost plane as
> > its
> >      * use is exclusive with the legacy cursor and we only want to
> > expose
> > @@ -325,9 +331,6 @@ void intel_device_info_runtime_init(struct
> > drm_i915_private *dev_priv)
> >     info->num_sprites[PIPE_A] = 2;
> >     info->num_sprites[PIPE_B] = 2;
> >     info->num_sprites[PIPE_C] = 1;
> > -   info->num_scalers[PIPE_A] = 2;
> > -   info->num_scalers[PIPE_B] = 2;
> > -   info->num_scalers[PIPE_C] = 1;
> >     } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> >     for_each_pipe(dev_priv, pipe)
> >     info->num_sprites[pipe] = 2;
> > -- 
> > 2.5.5
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 7/7] drm/i915: Move intel_atomic_get_shared_dpll_state() to intel_dpll_mgr.c

[Conselvan De Oliveira, Ander]

On Fri, 2016-12-30 at 20:04 +0100, Daniel Vetter wrote:
> On Thu, Dec 29, 2016 at 05:22:13PM +0200, Ander Conselvan de Oliveira wrote:
> > 
> > The function intel_atomic_get_shared_dpll_state() is only called from
> > intel_dpll_mgr.c and it concerns the same data structures as the other
> > functions in that file, so move it there and make it static.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira  > l.com>
> > Suggested-by: Daniel Vetter 
> Always love some doc work, thanks for respinning this series.
> 
> Reviewed-by: Daniel Vetter 

Pushed. Thanks for the reviews.

Ander

> 
> > 
> > ---
> >  drivers/gpu/drm/i915/intel_atomic.c   | 31 ---
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 31 +++
> >  drivers/gpu/drm/i915/intel_drv.h  |  2 --
> >  3 files changed, 31 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> > b/drivers/gpu/drm/i915/intel_atomic.c
> > index fa6dc43..aa9160e 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic.c
> > @@ -265,37 +265,6 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
> >     return 0;
> >  }
> >  
> > -static void
> > -intel_atomic_duplicate_dpll_state(struct drm_i915_private *dev_priv,
> > -     struct intel_shared_dpll_state
> > *shared_dpll)
> > -{
> > -   enum intel_dpll_id i;
> > -
> > -   /* Copy shared dpll state */
> > -   for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> > -   struct intel_shared_dpll *pll = _priv->shared_dplls[i];
> > -
> > -   shared_dpll[i] = pll->state;
> > -   }
> > -}
> > -
> > -struct intel_shared_dpll_state *
> > -intel_atomic_get_shared_dpll_state(struct drm_atomic_state *s)
> > -{
> > -   struct intel_atomic_state *state = to_intel_atomic_state(s);
> > -
> > -   WARN_ON(!drm_modeset_is_locked(>dev-
> > >mode_config.connection_mutex));
> > -
> > -   if (!state->dpll_set) {
> > -   state->dpll_set = true;
> > -
> > -   intel_atomic_duplicate_dpll_state(to_i915(s->dev),
> > -     state->shared_dpll);
> > -   }
> > -
> > -   return state->shared_dpll;
> > -}
> > -
> >  struct drm_atomic_state *
> >  intel_atomic_state_alloc(struct drm_device *dev)
> >  {
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > index 57d4271..c92a255 100644
> > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > @@ -80,6 +80,37 @@ skl_find_link_pll(struct drm_i915_private *dev_priv, int
> > clock)
> >     return pll;
> >  }
> >  
> > +static void
> > +intel_atomic_duplicate_dpll_state(struct drm_i915_private *dev_priv,
> > +     struct intel_shared_dpll_state
> > *shared_dpll)
> > +{
> > +   enum intel_dpll_id i;
> > +
> > +   /* Copy shared dpll state */
> > +   for (i = 0; i < dev_priv->num_shared_dpll; i++) {
> > +   struct intel_shared_dpll *pll = _priv->shared_dplls[i];
> > +
> > +   shared_dpll[i] = pll->state;
> > +   }
> > +}
> > +
> > +static struct intel_shared_dpll_state *
> > +intel_atomic_get_shared_dpll_state(struct drm_atomic_state *s)
> > +{
> > +   struct intel_atomic_state *state = to_intel_atomic_state(s);
> > +
> > +   WARN_ON(!drm_modeset_is_locked(>dev-
> > >mode_config.connection_mutex));
> > +
> > +   if (!state->dpll_set) {
> > +   state->dpll_set = true;
> > +
> > +   intel_atomic_duplicate_dpll_state(to_i915(s->dev),
> > +     state->shared_dpll);
> > +   }
> > +
> > +   return state->shared_dpll;
> > +}
> > +
> >  /**
> >   * intel_get_shared_dpll_by_id - get a DPLL given its id
> >   * @dev_priv: i915 device instance
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 5ee1719..6b02dac 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1815,8 +1815,6 @@ void intel_crtc_destroy_state(struct drm_crtc *crtc,
> >        struct drm_crtc_state *state);
> >  struct drm_atomic_state *intel_atomic_state_alloc(struct drm_device *dev);
> >  void intel_atomic_state_clear(struct drm_atomic_state *);
> > -struct intel_shared_dpll_state *
> > -intel_atomic_get_shared_dpll_state(struct drm_atomic_state *s);
> >  
> >  static inline struct intel_crtc_state *
> >  intel_atomic_get_crtc_state(struct drm_atomic_state *state,
> > -- 
> > 2.5.5
> > 
-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are 

Re: [Intel-gfx] [PATCH] drm/i915: Fix intel_psr_init() kerneldoc

[Conselvan De Oliveira, Ander]

On Tue, 2016-11-29 at 13:48 +0200, Ander Conselvan de Oliveira wrote:
> In commit c39055b072f8 ("drm/i915: Pass dev_priv to
> intel_setup_outputs()"), I forgot to update the kerneldoc for
> intel_psr_init() init, leading to warnings when building the
> documentation:
> 
> drivers/gpu/drm/i915/intel_psr.c:822: warning: No description found for 
> parameter 'dev_priv'
> drivers/gpu/drm/i915/intel_psr.c:822: warning: Excess function parameter 
> 'dev' description in 'intel_psr_init'
> 
> Fixes: c39055b072f8 ("drm/i915: Pass dev_priv to intel_setup_outputs()")
> Cc: Ander Conselvan de Oliveira 
> Cc: Ville Syrjälä 
> Cc: Daniel Vetter 
> Cc: Jani Nikula 
> Cc: intel-gfx@lists.freedesktop.org
> Signed-off-by: Ander Conselvan de Oliveira 
> 

Pushed with Daniel's irc r-b.

Ander

> ---
>  drivers/gpu/drm/i915/intel_psr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 5c3616e..d5f8d03 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -813,7 +813,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
>  
>  /**
>   * intel_psr_init - Init basic PSR work and mutex.
> - * @dev: DRM device
> + * @dev_priv: i915 device private
>   *
>   * This function is  called only once at driver load to initialize basic
>   * PSR stuff.
-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [Regression report] Weekly regression report WW40

[Conselvan De Oliveira, Ander]

On Thu, 2016-10-06 at 13:11 +0300, Jani Nikula wrote:
> On Wed, 05 Oct 2016, "Argotti, Yann"  wrote:
> > 
> > > 
> > > > 
> > > > On Mon, 03 Oct 2016, Jairo Miramontes
> > > >  wrote:
> > > > > 
> > > > > This week regressions
> > > > In the past we used "regression", "bisect_pending", and "bisected" in
> > > > the bugzilla "Keywords" field. Can we start using those again, please?
> > > I think this is a very good idea Jani. So we can start to scrub current
> > > regression (and then igt linked one) and update accordingly.
> > > Yann
> > Two additional thoughts are:
> > - add "regression_pending" (vs "regression") as well as keyword to
> > indicate where bug reporter has doubt on the fact it is or not a
> > regression.
> I think it should be enough, at least for a start (or should I say
> re-start), to freely use "regression" when things worked in the past but
> do not work anymore. We can then drop the keyword if it's not proven to
> be a regression.
> 
> No strong feelings on "regression_pending", though. But I don't think
> you can add keywords at will, I think the field only accepts a
> predefined set of keywords. So you'd need to talk to Ander or Martin
> (Cc'd) to get the new one to bugzilla.

I don't have admin access in bugzilla, so I would just forward such requests to
Martin.

Ander
-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Ro.CI.BAT: failure for series starting with [1/3] drm/i915: Remove intel_clock_t typedef

[Conselvan De Oliveira, Ander]

On Thu, 2016-05-12 at 15:12 +, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/3] drm/i915: Remove intel_clock_t typedef
> URL   : https://patchwork.freedesktop.org/series/6718/
> State : failure
> 
> == Summary ==
> 
> Series 6718v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/6718/revisions/1/mbox
> 
> Test core_auth:
> Subgroup basic-auth:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test core_prop_blob:
> Subgroup basic:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test drv_getparams_basic:
> Subgroup basic-eu-total:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-subslice-total:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test drv_hangman:
> Subgroup error-state-basic:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test drv_module_reload_basic:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test gem_basic:
> Subgroup bad-close:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup create-close:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup create-fd-close:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test gem_busy:
> Subgroup basic-blt:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-bsd:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-bsd1:
> skip   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-bsd2:
> skip   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-parallel-blt:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-parallel-bsd:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-parallel-bsd1:
> skip   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-parallel-bsd2:
> skip   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-parallel-render:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-parallel-vebox:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-render:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-vebox:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test gem_close_race:
> Subgroup basic-process:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-threads:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test gem_cpu_reloc:
> Subgroup basic:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test gem_cs_prefetch:
> Subgroup basic-default:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test gem_cs_tlb:
> Subgroup basic-default:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test gem_ctx_basic:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test gem_ctx_create:
> Subgroup basic:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-files:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test gem_ctx_exec:
> Subgroup basic:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test gem_ctx_param:
> Subgroup basic:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-default:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test gem_ctx_switch:
> Subgroup basic-default:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Test gem_exec_basic:
> Subgroup basic-blt:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-bsd:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-bsd1:
> skip   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-bsd2:
> skip   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-default:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-render:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup basic-vebox:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)
> Subgroup gtt-blt:
> pass   -> INCOMPLETE (ro-hsw-i3-4010u)

The dmesg log during the test run is empty.

That shouldn't be caused by renaming a couple of structs, so I pushed the
patches to dinq.

Ander

> WARNING: Long output truncated
> fi-snb-i7-2600 failed to connect after reboot
> 
> Results at /archive/results/CI_IGT_test/RO_Patchwork_863/
> 
> 86821b8 drm-intel-nightly: 2016y-05m-12d-14h-21m-45s UTC integration manifest
> 16afcce drm/i915: Remove intel_limit_t typedef
> 3c45000 drm/i915: Remove intel_range_t 

Re: [Intel-gfx] [PATCH v2 07/10] drm/i915: Undiplicate VLV signal level code

[Conselvan De Oliveira, Ander]

On Tue, 2016-04-19 at 13:37 -0700, Jim Bride wrote:
> On Wed, Apr 13, 2016 at 08:47:50PM +0300, Ander Conselvan de Oliveira wrote:
> > The logic for setting signal levels is used for both HDMI and DP with
> > small variations. But it is similar enough to put behind a function
> > called from the encoders.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira <
> > ander.conselvan.de.olive...@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h   |  4 
> >  drivers/gpu/drm/i915/intel_dp.c   | 43 
> > ---
> >  drivers/gpu/drm/i915/intel_dpio_phy.c | 26 +
> >  drivers/gpu/drm/i915/intel_hdmi.c | 14 
> >  4 files changed, 49 insertions(+), 38 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 85c0610..f2481a2 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -3565,6 +3565,10 @@ void chv_phy_pre_encoder_enable(struct intel_encoder
> > *encoder);
> >  void chv_phy_release_cl2_override(struct intel_encoder *encoder);
> >  void chv_phy_post_disable(struct intel_encoder *encoder);
> >  
> > +void vlv_set_phy_signal_level(struct intel_encoder *encoder,
> > + u32 demph_reg_value, u32 preemph_reg_value,
> > + u32 uniqtranscale_reg_value, u32 tx3_demph);
> > +
> >  int intel_gpu_freq(struct drm_i915_private *dev_priv, int val);
> >  int intel_freq_opcode(struct drm_i915_private *dev_priv, int val);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index cdacd8b..3e42355 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3009,16 +3009,10 @@ intel_dp_pre_emphasis_max(struct intel_dp *intel_dp,
> > uint8_t voltage_swing)
> >  
> >  static uint32_t vlv_signal_levels(struct intel_dp *intel_dp)
> >  {
> > -   struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > -   struct drm_i915_private *dev_priv = dev->dev_private;
> > -   struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
> > -   struct intel_crtc *intel_crtc =
> > -   to_intel_crtc(dport->base.base.crtc);
> > +   struct intel_encoder *encoder = _to_dig_port(intel_dp)->base;
> > unsigned long demph_reg_value, preemph_reg_value,
> > uniqtranscale_reg_value;
> > uint8_t train_set = intel_dp->train_set[0];
> > -   enum dpio_channel port = vlv_dport_to_channel(dport);
> > -   int pipe = intel_crtc->pipe;
> >  
> > switch (train_set & DP_TRAIN_PRE_EMPHASIS_MASK) {
> > case DP_TRAIN_PRE_EMPH_LEVEL_0:
> > @@ -3093,16 +3087,8 @@ static uint32_t vlv_signal_levels(struct intel_dp
> > *intel_dp)
> > return 0;
> > }
> >  
> > -   mutex_lock(_priv->sb_lock);
> > -   vlv_dpio_write(dev_priv, pipe, VLV_TX_DW5(port), 0x);
> > -   vlv_dpio_write(dev_priv, pipe, VLV_TX_DW4(port), demph_reg_value);
> > -   vlv_dpio_write(dev_priv, pipe, VLV_TX_DW2(port),
> > -uniqtranscale_reg_value);
> > -   vlv_dpio_write(dev_priv, pipe, VLV_TX_DW3(port), 0x0C782040);
> > -   vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW11(port), 0x0003);
> > -   vlv_dpio_write(dev_priv, pipe, VLV_PCS_DW9(port),
> > preemph_reg_value);
> > -   vlv_dpio_write(dev_priv, pipe, VLV_TX_DW5(port), 0x8000);
> > -   mutex_unlock(_priv->sb_lock);
> > +   vlv_set_phy_signal_level(encoder, demph_reg_value,
> > preemph_reg_value,
> > +uniqtranscale_reg_value, 0);
> >  
> > return 0;
> >  }
> > @@ -4285,17 +4271,6 @@ intel_dp_long_pulse(struct intel_connector
> > *intel_connector)
> > intel_dp->compliance_test_type = 0;
> > intel_dp->compliance_test_data = 0;
> >  
> > -   /*
> > -* If we were in MST mode, and device is not there,
> > -* get out of MST mode
> > -*/
> > -   if (intel_dp->is_mst) {
> > -   DRM_DEBUG_KMS("MST device may have disappeared %d
> > vs %d\n",
> > - intel_dp->is_mst, intel_dp
> > ->mst_mgr.mst_state);
> > -   intel_dp->is_mst = false;
> > -   drm_dp_mst_topology_mgr_set_mst(_dp->mst_mgr,
> > -   intel_dp->is_mst);
> > -   }
> > goto out;
> > }
> >  
> > @@ -4355,6 +4330,18 @@ intel_dp_long_pulse(struct intel_connector
> > *intel_connector)
> >  out:
> > if (status != connector_status_connected) {
> > intel_dp_unset_edid(intel_dp);
> > +
> > +   /*
> > +* If we were in MST mode, and device is not there,
> > +* get out of MST mode
> > +*/
> > +   if (intel_dp->is_mst) {
> > +   DRM_DEBUG_KMS("MST device may have disappeared %d
> > vs %d\n",
> > + intel_dp->is_mst, intel_dp
> > ->mst_mgr.mst_state);
> > +   

Re: [Intel-gfx] [PATCHv3 2/4] drm/i915: Store the dpll config in crtc_state->shared_dpll

[Conselvan De Oliveira, Ander]

On Wed, 2016-04-06 at 17:23 +0530, Durgadoss R wrote:
> Currently, the required shared dpll is saved in the crtc_state.
> Similarly, this patch saves the dpll config values also, so that
> these values (through crtc_state->shared_dpll->config.hw_state)
> can be used for upfront link training.
> 
> Signed-off-by: Durgadoss R 
> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 1175eeb..cad10f2 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -248,6 +248,7 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll,
>pipe_name(crtc->pipe));
>  
>   intel_shared_dpll_config_get(shared_dpll, pll, crtc);
> + crtc_state->shared_dpll->config = shared_dpll[i];

This overwrites the state stored in dev_priv->shared_dpll[i].config, so it means
we loose the current state set in the hardware. If the atomic check fails after
this, the software tracking of the hw state gets messed up.

Ander

>  }
>  
>  void intel_shared_dpll_commit(struct drm_atomic_state *state)
-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/8] drm/i915: Merge ironlake_get_refclk() into its only caller

[Conselvan De Oliveira, Ander]

On Mon, 2016-03-14 at 15:55 +0200, Ville Syrjälä wrote:
> On Mon, Mar 14, 2016 at 10:55:41AM +0200, Ander Conselvan de Oliveira wrote:
> > A previous patche made ironlake_get_refclk() very simple, so merge
> > it into its only caller.
> 
> Again I'd like to keep the pch and gmch code as similar as possible.
> So could do the same for the gmch code.
> 
> I already had a patch in my lvds_downclock branch that moved some of
> the gmch platform differences out from i9xx_get_refclk() into the
> caller, so I guess might as well move the whole thing I suppose.

I think we should just split i9xx_crtc_compute_clock() into more platform
specific functions and kill i9xx_get_reclk(), intel_limit() and ->find_dpll().
We are jumping through hoops to make the code look like it's generic, but there
is a lot of platform specific details. IMO it would be a lot easier to read that
way.

I wrote some patches going into that direction today. The end result looks like
the following:

static int chv_crtc_compute_clock(struct intel_crtc *crtc,
  struct intel_crtc_state *crtc_state)
{
int refclk;
bool ok;
const intel_limit_t *limit;

memset(_state->dpll_hw_state, 0,
   sizeof(crtc_state->dpll_hw_state));

if (crtc_state->has_dsi_encoder)
return 0;

limit = _limits_chv;

if (!crtc_state->clock_set) {
refclk = 10;

ok = chv_find_best_dpll(limit, crtc_state,
crtc_state->port_clock, refclk,
NULL, _state->dpll);
if (!ok) {
DRM_ERROR("Couldn't find PLL settings for mode!\n");
return -EINVAL;
}
}

chv_compute_dpll(crtc, crtc_state);

return 0;
}

static int vlv_crtc_compute_clock(struct intel_crtc *crtc,
  struct intel_crtc_state *crtc_state)
{
int refclk;
bool ok;
const intel_limit_t *limit;

memset(_state->dpll_hw_state, 0,
   sizeof(crtc_state->dpll_hw_state));

if (crtc_state->has_dsi_encoder)
return 0;

limit = _limits_vlv;

if (!crtc_state->clock_set) {
refclk = 10;

ok = vlv_find_best_dpll(limit, crtc_state,
crtc_state->port_clock, refclk,
NULL, _state->dpll);
if (!ok) {
DRM_ERROR("Couldn't find PLL settings for mode!\n");
return -EINVAL;
}
}

vlv_compute_dpll(crtc, crtc_state);

return 0;
}

static int gen2_crtc_compute_clock(struct intel_crtc *crtc,
   struct intel_crtc_state *crtc_state)
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
int refclk;
const intel_limit_t *limit;

memset(_state->dpll_hw_state, 0,
   sizeof(crtc_state->dpll_hw_state));

if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
intel_panel_use_ssc(dev_priv)) {
refclk = dev_priv->vbt.lvds_ssc_freq;
DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
} else {
refclk = 48000;
}

if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
limit = _limits_i8xx_lvds;
else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO))
limit = _limits_i8xx_dvo;
else
limit = _limits_i8xx_dac;

if (!crtc_state->clock_set &&
!i9xx_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
 refclk, NULL, _state->dpll)) {
DRM_ERROR("Couldn't find PLL settings for mode!\n");
return -EINVAL;
}

i8xx_compute_dpll(crtc, crtc_state, NULL);

return 0;
}

static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
   struct intel_crtc_state *crtc_state)
{
struct drm_device *dev = crtc->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
int refclk;
bool ok;
const intel_limit_t *limit;

memset(_state->dpll_hw_state, 0,
   sizeof(crtc_state->dpll_hw_state));

if (crtc_state->has_dsi_encoder)
return 0;

if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
intel_panel_use_ssc(dev_priv)) {
refclk = dev_priv->vbt.lvds_ssc_freq;
DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
} else {
refclk = 96000;
}

if (!crtc_state->clock_set) {
/*
 * 

Re: [Intel-gfx] [PATCH 1/8] drm/i915: Remove checks for clone config with LVDS in ILK+ dpll code

[Conselvan De Oliveira, Ander]

On Mon, 2016-03-14 at 15:51 +0200, Ville Syrjälä wrote:
> On Mon, Mar 14, 2016 at 10:55:40AM +0200, Ander Conselvan de Oliveira wrote:
> > LVDS is not cloneable, so the check is unnecessary. Removing it makes
> > the surrouding code a bit simpler.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira <
> > ander.conselvan.de.olive...@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 31 ---
> >  1 file changed, 4 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 2d151ad..e7d6584 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8593,30 +8593,9 @@ static int ironlake_get_refclk(struct
> > intel_crtc_state *crtc_state)
> >  {
> > struct drm_device *dev = crtc_state->base.crtc->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > -   struct drm_atomic_state *state = crtc_state->base.state;
> > -   struct drm_connector *connector;
> > -   struct drm_connector_state *connector_state;
> > -   struct intel_encoder *encoder;
> > -   int num_connectors = 0, i;
> > -   bool is_lvds = false;
> > -
> > -   for_each_connector_in_state(state, connector, connector_state, i) {
> > -   if (connector_state->crtc != crtc_state->base.crtc)
> > -   continue;
> > -
> > -   encoder = to_intel_encoder(connector_state->best_encoder);
> > -
> > -   switch (encoder->type) {
> > -   case INTEL_OUTPUT_LVDS:
> > -   is_lvds = true;
> > -   break;
> > -   default:
> > -   break;
> > -   }
> > -   num_connectors++;
> > -   }
> >  
> > -   if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2)
> > {
> 
> We have the exact same checks in the gmch code as well. For consistency
> you should change those as well.

True. Would it be ok in a follow-up patch? I did that today now that I started
doing some clean ups for i9xx_crtc_compute_clock(). If not, I can resend.

Thanks,
Ander

> 
> > +   if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> > +   intel_panel_use_ssc(dev_priv)) {
> > DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
> >   dev_priv->vbt.lvds_ssc_freq);
> > return dev_priv->vbt.lvds_ssc_freq;
> > @@ -8842,7 +8821,7 @@ static uint32_t ironlake_compute_dpll(struct
> > intel_crtc *intel_crtc,
> > struct drm_connector_state *connector_state;
> > struct intel_encoder *encoder;
> > uint32_t dpll;
> > -   int factor, num_connectors = 0, i;
> > +   int factor, i;
> > bool is_lvds = false, is_sdvo = false;
> >  
> > for_each_connector_in_state(state, connector, connector_state, i) {
> > @@ -8862,8 +8841,6 @@ static uint32_t ironlake_compute_dpll(struct
> > intel_crtc *intel_crtc,
> > default:
> > break;
> > }
> > -
> > -   num_connectors++;
> > }
> >  
> > /* Enable autotuning of the PLL clock (if permissible) */
> > @@ -8917,7 +8894,7 @@ static uint32_t ironlake_compute_dpll(struct
> > intel_crtc *intel_crtc,
> > break;
> > }
> >  
> > -   if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2)
> > +   if (is_lvds && intel_panel_use_ssc(dev_priv))
> > dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
> > else
> > dpll |= PLL_REF_INPUT_DREFCLK;
> > -- 
> > 2.4.3
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for Shared pll improvements (rev4)

[Conselvan De Oliveira, Ander]

On Tue, 2016-03-08 at 16:01 +, Patchwork wrote:
> == Series Details ==
> 
> Series: Shared pll improvements (rev4)
> URL   : https://patchwork.freedesktop.org/series/3850/
> State : warning
> 
> == Summary ==
> 
> Series 3850v4 Shared pll improvements
> http://patchwork.freedesktop.org/api/1.0/series/3850/revisions/4/mbox/
> 
> Test kms_flip:
> Subgroup basic-flip-vs-dpms:
> dmesg-warn -> PASS   (bdw-ultra)
> Test kms_frontbuffer_tracking:
> Subgroup basic:
> pass   -> DMESG-WARN (snb-x220t)

https://bugs.freedesktop.org/show_bug.cgi?id=94349

> Test kms_pipe_crc_basic:
> Subgroup nonblocking-crc-pipe-b:
> dmesg-warn -> PASS   (hsw-gt2)
> Test pm_rpm:
> Subgroup basic-pci-d3-state:
> dmesg-fail -> FAIL   (snb-x220t)

https://bugs.freedesktop.org/show_bug.cgi?id=93123

> pass   -> DMESG-WARN (snb-dellxps)

https://bugs.freedesktop.org/show_bug.cgi?id=94349

> Subgroup basic-rte:
> dmesg-warn -> PASS   (snb-x220t)
> dmesg-warn -> PASS   (snb-dellxps)
> dmesg-warn -> PASS   (bsw-nuc-2)
> 
> bdw-nuci7total:186  pass:174  dwarn:0   dfail:0   fail:0   skip:12 
> bdw-ultratotal:186  pass:167  dwarn:0   dfail:0   fail:0   skip:19 
> bsw-nuc-2total:186  pass:151  dwarn:0   dfail:0   fail:0   skip:35 
> byt-nuc  total:186  pass:154  dwarn:0   dfail:0   fail:0   skip:32 
> hsw-brixbox  total:186  pass:166  dwarn:0   dfail:0   fail:0   skip:20 
> hsw-gt2  total:186  pass:170  dwarn:1   dfail:0   fail:0   skip:15 
> ilk-hp8440p  total:186  pass:127  dwarn:0   dfail:0   fail:0   skip:59 
> ivb-t430stotal:186  pass:164  dwarn:0   dfail:0   fail:0   skip:22 
> skl-i5k-2total:186  pass:165  dwarn:0   dfail:0   fail:0   skip:21 
> skl-i7k-2total:186  pass:165  dwarn:0   dfail:0   fail:0   skip:21 
> snb-dellxps  total:186  pass:155  dwarn:1   dfail:0   fail:0   skip:30 
> snb-x220ttotal:186  pass:155  dwarn:1   dfail:0   fail:1   skip:29 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1542/
> 
> b519bbd9633eca6bc8e8e80588b48bcee447c330 drm-intel-nightly: 2016y-03m-08d-13h
> -00m-35s UTC integration manifest
> f8848e64f47e4200ca993ab5bb868bd6c68767a3 drm/i915: Make SKL/KBL DPLL0 managed
> by the shared dpll code
> 4ebc277121e7ac78ec5b276285036adc7fd0378c drm/i915: Manage HSW/BDW LCPLLs with
> the shared dpll interface
> 3a4e1a242b5bcfd9f13dd396148496cd5023e148 drm/i915: Move BXT pll configuration
> logic to intel_dpll_mgr.c
> 7c5af214173d1b70d5700182ac13c1426b420101 drm/i915: Move SKL/KLB pll selection
> logic to intel_dpll_mgr.c
> bbd624df74bdedbbdd6aeed20dff43e8cc2c1bf2 drm/i915: Move HSW/BDW pll selection
> logic to intel_dpll_mgr.c
> 7dfc6d5ac9dbca040722b3f8160d701303224e23 drm/i915: Refactor platform specifics
> out of intel_get_shared_dpll()
> 0e6c3ed940919ec78fcd17fe817854463239c949 drm/i915: Use a table to initilize
> shared dplls
> 2a434bf66a3a5db28cee6fdcb302007554cbac5e drm/i915: Move shared dpll function
> prototypes to intel_dpll_mgr.h
> 4654f45012aa9cce7a906a11f3ef6833826f0c2a drm/i915: Move shared dpll struct
> definitions to separate header file
> 29d479929a350c2623dae07f400623f33ece8f1a drm/i915: Store a direct pointer to
> shared dpll in intel_crtc_state
> 12b8330b15a5feb3e6a3cf41216a2f2f803ee755 drm/i915: Split
> intel_get_shared_dpll() into smaller functions
> dfa9c8044c69fb361e7482c5c58b997618eb163d drm/i915: Move ddi shared dpll code
> to intel_dpll_mgr.c
> 8584a2d3faa95cfe6094391d7a20d1a1f3a790c7 drm/i915: Move shared dpll code to a
> new file
> 
-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 12/13] drm/i915: Manage HSW/BDW LCPLLs with the shared dpll interface

[Conselvan De Oliveira, Ander]

On Tue, 2016-03-08 at 12:05 +0100, Maarten Lankhorst wrote:
> Op 26-02-16 om 14:54 schreef Ander Conselvan de Oliveira:
> > Manage the LCPLLs used with DisplayPort, so that all the HSW/BDW DPLLs
> > are managed by the shared dpll code.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira <
> > ander.conselvan.de.olive...@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c  | 18 
> >  drivers/gpu/drm/i915/intel_display.c  | 35 
> >  drivers/gpu/drm/i915/intel_dp.c   | 23 +-
> >  drivers/gpu/drm/i915/intel_dp_mst.c   |  4 --
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 79 +++-
> > ---
> >  drivers/gpu/drm/i915/intel_dpll_mgr.h |  5 ++-
> >  drivers/gpu/drm/i915/intel_drv.h  |  1 -
> >  7 files changed, 110 insertions(+), 55 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index ad7888c..3cb9f36 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -992,17 +992,13 @@ hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
> >  {
> > struct intel_shared_dpll *pll;
> >  
> > -   if (intel_encoder->type == INTEL_OUTPUT_HDMI ||
> > -   intel_encoder->type == INTEL_OUTPUT_ANALOG) {
> > -   pll = intel_get_shared_dpll(intel_crtc, crtc_state,
> > -   intel_encoder);
> > -   if (!pll)
> > -   DRM_DEBUG_DRIVER("failed to find PLL for pipe
> > %c\n",
> > -pipe_name(intel_crtc->pipe));
> > -   return pll;
> > -   } else {
> > -   return true;
> > -   }
> > +   pll = intel_get_shared_dpll(intel_crtc, crtc_state,
> > +   intel_encoder);
> > +   if (!pll)
> > +   DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
> > +pipe_name(intel_crtc->pipe));
> > +
> > +   return pll;
> >  }
> Eventually the reliance on intel_encoder->type should end here, and based on
> the connector.

Do you mean that the parameter to get_shared_dpll() should be connector instead
of encoder?

> It would fix some kms tests, but that would be better to do in a future patch.
> > ...
> > 
> > +static bool hsw_ddi_lcpll_get_hw_state(struct drm_i915_private *dev_priv,
> > +  struct intel_shared_dpll *pll,
> > +  struct intel_dpll_hw_state
> > *hw_state)
> > +{
> > +   /*
> > +* LC PLL is kept enabled all the time since it drives CDCLK. The
> > +* state checker still expects it to be disabled when it is not
> > used
> > +* by any crtc. To avoid adding a case to LC PLL, just tell the
> > +* state checker what it expects.
> > +*/
> > +   if (pll->config.crtc_mask)
> > +   return true;
> > +   else
> > +   return false;
> > +}
> Wouldn't it be better to return true or the real hardware state then, and set
> the ALWAYS_ON flag from the next patch?

That's exactly what v2 of this patch does. :)

Ander

> > +static const struct intel_shared_dpll_funcs hsw_ddi_lcpll_funcs = {
> > +   .enable = hsw_ddi_lcpll_enable,
> > +   .disable = hsw_ddi_lcpll_disable,
> > +   .get_hw_state = hsw_ddi_lcpll_get_hw_state,
> > +};
> > +
> >  struct skl_dpll_regs {
> > i915_reg_t ctl, cfgcr1, cfgcr2;
> >  };
> > @@ -1559,9 +1617,12 @@ static const struct intel_dpll_mgr pch_pll_mgr = {
> >  };
> >  
> >  static const struct dpll_info hsw_plls[] = {
> > -   { "WRPLL 1", DPLL_ID_WRPLL1, _ddi_wrpll_funcs },
> > -   { "WRPLL 2", DPLL_ID_WRPLL2, _ddi_wrpll_funcs },
> > -   { "SPLL",DPLL_ID_SPLL,   _ddi_spll_funcs },
> > +   { "WRPLL 1",DPLL_ID_WRPLL1, _ddi_wrpll_funcs },
> > +   { "WRPLL 2",DPLL_ID_WRPLL2, _ddi_wrpll_funcs },
> > +   { "SPLL",   DPLL_ID_SPLL,   _ddi_spll_funcs },
> > +   { "LCPLL 810",  DPLL_ID_LCPLL_810,  _ddi_lcpll_funcs },
> > +   { "LCPLL 1350", DPLL_ID_LCPLL_1350, _ddi_lcpll_funcs },
> > +   { "LCPLL 2700", DPLL_ID_LCPLL_2700, _ddi_lcpll_funcs },
> > { NULL, -1, NULL, },
> >  };
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> > index 82e53f5..872878e 100644
> > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> > @@ -49,13 +49,16 @@ enum intel_dpll_id {
> > DPLL_ID_WRPLL1 = 0,
> > DPLL_ID_WRPLL2 = 1,
> > DPLL_ID_SPLL = 2,
> > +   DPLL_ID_LCPLL_810 = 3,
> > +   DPLL_ID_LCPLL_1350 = 4,
> > +   DPLL_ID_LCPLL_2700 = 5,
> >  
> > /* skl */
> > DPLL_ID_SKL_DPLL1 = 0,
> > DPLL_ID_SKL_DPLL2 = 1,
> > DPLL_ID_SKL_DPLL3 = 2,
> >  };
> > -#define I915_NUM_PLLS 3
> > +#define I915_NUM_PLLS 6
> >  
> >  struct intel_dpll_hw_state {
> > /* i9xx, pch plls */
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index c9e5030..18aa287 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ 

Re: [Intel-gfx] [PATCH] drm/i915: Fall back to zero vswing/preemph if the sink doesn't like the last good values

[Conselvan De Oliveira, Ander]

On Fri, 2015-10-30 at 18:47 +0200, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> My Lenovo STM STDP3100 miniDP->VGA dongle doesn't seem to like it when
> we try to start link training with non-zero vswing/preemphasis. So when
> the initial link training DPCD write fails, retry it with zero values.

Does the device NACKs the request?

Ander

> 
> Fixes a bunch of errors like so:
> [drm:intel_dp_start_link_train [i915]] *ERROR* failed to enable link training
> 
> Cc: Mika Kahola 
> Cc: Sivakumar Thulasimani 
> Cc: Ander Conselvan de Oliveira 
> Fixes: 5fa836a9d859 ("drm/i915: DP link training optimization")
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index ba4cbf5..9529a6e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3750,10 +3750,17 @@ intel_dp_link_training_clock_recovery(struct intel_dp
> *intel_dp)
>  
>   DP |= DP_PORT_EN;
>  
> +again:
>   /* clock recovery */
>   if (!intel_dp_reset_link_train(intel_dp, ,
>  DP_TRAINING_PATTERN_1 |
>  DP_LINK_SCRAMBLING_DISABLE)) {
> + if (intel_dp->train_set_valid) {
> + DRM_DEBUG_KMS("Sink rejected link training request,
> trying again with zero values\n");
> + intel_dp->train_set_valid = false;
> + goto again;
> + }
> +
>   DRM_ERROR("failed to enable link training\n");
>   return;
>   }
-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/15] drm/i915: Don't pass *DP around to link training functions

[Conselvan De Oliveira, Ander]

On Mon, 2015-10-19 at 10:15 +0530, Thulasimani, Sivakumar wrote:
> 
> On 10/5/2015 12:31 PM, Ander Conselvan de Oliveira wrote:
> > It just makes the code more confusing, so just reference intel_dp_>DP
> > directly. The old behavior of not updating the value in intel_dp if link
> > training fail is preserved by saving the previous value of DP in the
> > stack and restoring the old value in case of failure.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira <
> > ander.conselvan.de.olive...@intel.com>
> > --
> > 
> > I'm not sure the old behavior is correct, but to err in the side of
> > caution I tried not to change it.
> > 
> > ---
> >   drivers/gpu/drm/i915/intel_dp.c | 66 -
> > 
> >   1 file changed, 33 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index c420edf..391a367 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3601,7 +3601,6 @@ intel_dp_set_signal_levels(struct intel_dp *intel_dp,
> > uint32_t *DP)
> >   
> >   static bool
> >   intel_dp_set_link_train(struct intel_dp *intel_dp,
> > -   uint32_t *DP,
> > uint8_t dp_train_pat)
> >   {
> > struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > @@ -3610,9 +3609,9 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
> > uint8_t buf[sizeof(intel_dp->train_set) + 1];
> > int ret, len;
> >   
> > -   _intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
> > +   _intel_dp_set_link_train(intel_dp, _dp->DP, dp_train_pat);
> >   
> > -   I915_WRITE(intel_dp->output_reg, *DP);
> > +   I915_WRITE(intel_dp->output_reg, intel_dp->DP);
> > POSTING_READ(intel_dp->output_reg);
> >   
> > buf[0] = dp_train_pat;
> > @@ -3633,17 +3632,17 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
> >   }
> >   
> >   static bool
> > -intel_dp_reset_link_train(struct intel_dp *intel_dp, uint32_t *DP,
> > +intel_dp_reset_link_train(struct intel_dp *intel_dp,
> > uint8_t dp_train_pat)
> >   {
> > if (!intel_dp->train_set_valid)
> > memset(intel_dp->train_set, 0, sizeof(intel_dp
> > ->train_set));
> > -   intel_dp_set_signal_levels(intel_dp, DP);
> > -   return intel_dp_set_link_train(intel_dp, DP, dp_train_pat);
> > +   intel_dp_set_signal_levels(intel_dp, _dp->DP);
> > +   return intel_dp_set_link_train(intel_dp, dp_train_pat);
> >   }
> >   
> >   static bool
> > -intel_dp_update_link_train(struct intel_dp *intel_dp, uint32_t *DP,
> > +intel_dp_update_link_train(struct intel_dp *intel_dp,
> >const uint8_t link_status[DP_LINK_STATUS_SIZE])
> >   {
> > struct intel_digital_port *intel_dig_port =
> > dp_to_dig_port(intel_dp);
> > @@ -3652,9 +3651,9 @@ intel_dp_update_link_train(struct intel_dp *intel_dp,
> > uint32_t *DP,
> > int ret;
> >   
> > intel_get_adjust_train(intel_dp, link_status);
> > -   intel_dp_set_signal_levels(intel_dp, DP);
> > +   intel_dp_set_signal_levels(intel_dp, _dp->DP);
> >   
> > -   I915_WRITE(intel_dp->output_reg, *DP);
> > +   I915_WRITE(intel_dp->output_reg, intel_dp->DP);
> > POSTING_READ(intel_dp->output_reg);
> >   
> > ret = drm_dp_dpcd_write(_dp->aux, DP_TRAINING_LANE0_SET,
> > @@ -3695,7 +3694,7 @@ static void intel_dp_set_idle_link_train(struct
> > intel_dp *intel_dp)
> >   }
> >   
> >   /* Enable corresponding port and start training pattern 1 */
> > -static void
> > +static bool
> >   intel_dp_link_training_clock_recovery(struct intel_dp *intel_dp)
> >   {
> > struct drm_encoder *encoder = _to_dig_port(intel_dp)
> > ->base.base;
> > @@ -3703,9 +3702,9 @@ intel_dp_link_training_clock_recovery(struct intel_dp
> > *intel_dp)
> > int i;
> > uint8_t voltage;
> > int voltage_tries, loop_tries;
> > -   uint32_t DP = intel_dp->DP;
> > uint8_t link_config[2];
> > uint8_t link_bw, rate_select;
> > +   uint8_t link_status[DP_LINK_STATUS_SIZE];
> >   
> > if (HAS_DDI(dev))
> > intel_ddi_prepare_link_retrain(encoder);
> > @@ -3727,22 +3726,20 @@ intel_dp_link_training_clock_recovery(struct
> > intel_dp *intel_dp)
> > link_config[1] = DP_SET_ANSI_8B10B;
> > drm_dp_dpcd_write(_dp->aux, DP_DOWNSPREAD_CTRL, link_config,
> > 2);
> >   
> > -   DP |= DP_PORT_EN;
> > +   intel_dp->DP |= DP_PORT_EN;
> >   
> > /* clock recovery */
> > -   if (!intel_dp_reset_link_train(intel_dp, ,
> > +   if (!intel_dp_reset_link_train(intel_dp,
> >DP_TRAINING_PATTERN_1 |
> >DP_LINK_SCRAMBLING_DISABLE)) {
> > DRM_ERROR("failed to enable link training\n");
> > -   return;
> > +   return false;
> > }
> >   
> > voltage = 0xff;
> > voltage_tries = 0;
> > loop_tries = 0;
> > for (;;) {
> > -   uint8_t link_status[DP_LINK_STATUS_SIZE];
> > -
> > 

Re: [Intel-gfx] [PATCH 01/13] drm/i915: Eliminate usage of plane_wm_parameters from ILK-style WM code

[Conselvan De Oliveira, Ander]

On Thu, 2015-08-20 at 18:11 -0700, Matt Roper wrote:
 Just pull the info out of the plane state structure rather than staging
 it in an additional structure.
 
 Signed-off-by: Matt Roper matthew.d.ro...@intel.com

Reviewed-by: Ander Conselvan de Oliveira conselv...@gmail.com

 ---
  drivers/gpu/drm/i915/intel_pm.c | 133 
 +---
  1 file changed, 70 insertions(+), 63 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index fff0c22..c9cf7cf 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -1791,9 +1791,6 @@ struct ilk_pipe_wm_parameters {
   bool active;
   uint32_t pipe_htotal;
   uint32_t pixel_rate;
 - struct intel_plane_wm_parameters pri;
 - struct intel_plane_wm_parameters spr;
 - struct intel_plane_wm_parameters cur;
  };
  
  struct ilk_wm_maximums {
 @@ -1815,25 +1812,25 @@ struct intel_wm_config {
   * mem_value must be in 0.1us units.
   */
  static uint32_t ilk_compute_pri_wm(const struct ilk_pipe_wm_parameters 
 *params,
 +const struct intel_plane_state *pstate,
  uint32_t mem_value,
  bool is_lp)
  {
 + int bpp = pstate-base.fb ? pstate-base.fb-bits_per_pixel / 8 : 0;
   uint32_t method1, method2;
  
 - if (!params-active || !params-pri.enabled)
 + if (!params-active || !pstate-visible)
   return 0;
  
 - method1 = ilk_wm_method1(params-pixel_rate,
 -  params-pri.bytes_per_pixel,
 -  mem_value);
 + method1 = ilk_wm_method1(params-pixel_rate, bpp, mem_value);
  
   if (!is_lp)
   return method1;
  
   method2 = ilk_wm_method2(params-pixel_rate,
params-pipe_htotal,
 -  params-pri.horiz_pixels,
 -  params-pri.bytes_per_pixel,
 +  drm_rect_width(pstate-dst),
 +  bpp,
mem_value);
  
   return min(method1, method2);
 @@ -1844,20 +1841,20 @@ static uint32_t ilk_compute_pri_wm(const struct 
 ilk_pipe_wm_parameters 
 *params,
   * mem_value must be in 0.1us units.
   */
  static uint32_t ilk_compute_spr_wm(const struct ilk_pipe_wm_parameters 
 *params,
 +const struct intel_plane_state *pstate,
  uint32_t mem_value)
  {
 + int bpp = pstate-base.fb ? pstate-base.fb-bits_per_pixel / 8 : 0;
   uint32_t method1, method2;
  
 - if (!params-active || !params-spr.enabled)
 + if (!params-active || !pstate-visible)
   return 0;
  
 - method1 = ilk_wm_method1(params-pixel_rate,
 -  params-spr.bytes_per_pixel,
 -  mem_value);
 + method1 = ilk_wm_method1(params-pixel_rate, bpp, mem_value);
   method2 = ilk_wm_method2(params-pixel_rate,
params-pipe_htotal,
 -  params-spr.horiz_pixels,
 -  params-spr.bytes_per_pixel,
 +  drm_rect_width(pstate-dst),
 +  bpp,
mem_value);
   return min(method1, method2);
  }
 @@ -1867,28 +1864,32 @@ static uint32_t ilk_compute_spr_wm(const struct 
 ilk_pipe_wm_parameters 
 *params,
   * mem_value must be in 0.1us units.
   */
  static uint32_t ilk_compute_cur_wm(const struct ilk_pipe_wm_parameters 
 *params,
 +const struct intel_plane_state *pstate,
  uint32_t mem_value)
  {
 - if (!params-active || !params-cur.enabled)
 + int bpp = pstate-base.fb ? pstate-base.fb-bits_per_pixel / 8 : 0;
 +
 + if (!params-active || !pstate-visible)
   return 0;
  
   return ilk_wm_method2(params-pixel_rate,
 params-pipe_htotal,
 -   params-cur.horiz_pixels,
 -   params-cur.bytes_per_pixel,
 +   drm_rect_width(pstate-dst),
 +   bpp,
 mem_value);
  }
  
  /* Only for WM_LP. */
  static uint32_t ilk_compute_fbc_wm(const struct ilk_pipe_wm_parameters 
 *params,
 +const struct intel_plane_state *pstate,
  uint32_t pri_val)
  {
 - if (!params-active || !params-pri.enabled)
 + int bpp = pstate-base.fb ? pstate-base.fb-bits_per_pixel / 8 : 0;
 +
 + if (!params-active || !pstate-visible)
   return 0;
  
 - return ilk_wm_fbc(pri_val,
 -   params-pri.horiz_pixels,
 -   params-pri.bytes_per_pixel);
 + return ilk_wm_fbc(pri_val, drm_rect_width(pstate-dst), bpp);
  }
  
  static unsigned 

Re: [Intel-gfx] [PATCH] drm/i915: Don't check modeset state in the hw state force restore path

[Conselvan De Oliveira, Ander]

On Mon, 2015-06-01 at 15:41 +0300, Ander Conselvan de Oliveira wrote:
 Since the force restore logic will restore the CRTCs state one at a
 time, it is possible that the state will be inconsistent until the whole
 operation finishes. A call to intel_modeset_check_state() is done once
 it's over, so don't check the state multiple times in between. This
 regression was introduced in:
 
 commit 7f27126ea3db6ade886f18fd39caf0ff0cd1d37f
 Author: Jesse Barnes jbar...@virtuousgeek.org
 Date:   Wed Nov 5 14:26:06 2014 -0800
 
 drm/i915: factor out compute_config from __intel_set_mode v3
 
 Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=94431
 Cc: Jesse Barnes jbar...@virtuousgeek.org
 Signed-off-by: Ander Conselvan de Oliveira 
 ander.conselvan.de.olive...@intel.com
 ---
 
 Hi,
 
 This patch applies on top of nightly, but it is only relevant without
 Maarten's drm/i915: Convert to atomic, part 2 series, because of the
 changes to the hw state read out and force restore logic.
 
 The regression exists since 3.19.

Same patch adapated for 4.1-rc6 here:
https://bugzilla.kernel.org/attachment.cgi?id=178461

Ander

 ---
  drivers/gpu/drm/i915/intel_display.c | 21 -
  1 file changed, 12 insertions(+), 9 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 16e159d..24fb7ce 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -87,7 +87,8 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
  struct intel_crtc_state *pipe_config);
  
  static int intel_set_mode(struct drm_crtc *crtc,
 -   struct drm_atomic_state *state);
 +   struct drm_atomic_state *state,
 +   bool check);
  static int intel_framebuffer_init(struct drm_device *dev,
 struct intel_framebuffer *ifb,
 struct drm_mode_fb_cmd2 *mode_cmd,
 @@ -10282,7 +10283,7 @@ retry:
  
   drm_mode_copy(crtc_state-base.mode, mode);
  
 - if (intel_set_mode(crtc, state)) {
 + if (intel_set_mode(crtc, state, true)) {
   DRM_DEBUG_KMS(failed to set mode on load-detect pipe\n);
   if (old-release_fb)
   old-release_fb-funcs-destroy(old-release_fb);
 @@ -10356,7 +10357,7 @@ void intel_release_load_detect_pipe(struct 
 drm_connector *connector,
   if (ret)
   goto fail;
  
 - ret = intel_set_mode(crtc, state);
 + ret = intel_set_mode(crtc, state, true);
   if (ret)
   goto fail;
  
 @@ -12832,20 +12833,22 @@ static int __intel_set_mode(struct drm_crtc 
 *modeset_crtc,
  }
  
  static int intel_set_mode_with_config(struct drm_crtc *crtc,
 -   struct intel_crtc_state *pipe_config)
 +   struct intel_crtc_state *pipe_config,
 +   bool check)
  {
   int ret;
  
   ret = __intel_set_mode(crtc, pipe_config);
  
 - if (ret == 0)
 + if (ret == 0  check)
   intel_modeset_check_state(crtc-dev);
  
   return ret;
  }
  
  static int intel_set_mode(struct drm_crtc *crtc,
 -   struct drm_atomic_state *state)
 +   struct drm_atomic_state *state,
 +   bool check)
  {
   struct intel_crtc_state *pipe_config;
   int ret = 0;
 @@ -12856,7 +12859,7 @@ static int intel_set_mode(struct drm_crtc *crtc,
   goto out;
   }
  
 - ret = intel_set_mode_with_config(crtc, pipe_config);
 + ret = intel_set_mode_with_config(crtc, pipe_config, check);
   if (ret)
   goto out;
  
 @@ -12933,7 +12936,7 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
   intel_modeset_setup_plane_state(state, crtc, crtc-mode,
   crtc-primary-fb, crtc-x, crtc-y);
  
 - ret = intel_set_mode(crtc, state);
 + ret = intel_set_mode(crtc, state, false);
   if (ret)
   drm_atomic_state_free(state);
  }
 @@ -13133,7 +13136,7 @@ static int intel_crtc_set_config(struct drm_mode_set 
 *set)
  
   primary_plane_was_visible = primary_plane_visible(set-crtc);
  
 - ret = intel_set_mode_with_config(set-crtc, pipe_config);
 + ret = intel_set_mode_with_config(set-crtc, pipe_config, true);
  
   if (ret == 0 
   pipe_config-base.enable 

-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [Intel-gfx] [PATCH 16/19] drm/i915: Check lane sharing between pipes B C using atomic state

[Conselvan De Oliveira, Ander]

On Thu, 2015-03-19 at 20:58 +, Konduru, Chandra wrote:
 
  -Original Message-
  From: Conselvan De Oliveira, Ander
  Sent: Friday, March 13, 2015 2:49 AM
  To: intel-gfx@lists.freedesktop.org
  Cc: Konduru, Chandra; Conselvan De Oliveira, Ander
  Subject: [PATCH 16/19] drm/i915: Check lane sharing between pipes B  C 
  using
  atomic state
  
  Makes that code atomic ready.
  
  Signed-off-by: Ander Conselvan de Oliveira
  ander.conselvan.de.olive...@intel.com
  ---
   drivers/gpu/drm/i915/intel_display.c | 49 ++-
  -
   1 file changed, 42 insertions(+), 7 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_display.c
  b/drivers/gpu/drm/i915/intel_display.c
  index e720a48..8c97186 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -5537,13 +5537,20 @@ bool intel_connector_get_hw_state(struct
  intel_connector *connector)
  return encoder-get_hw_state(encoder, pipe);  }
  
  -static int pipe_required_fdi_lanes(struct drm_device *dev, enum pipe pipe)
  +static int pipe_required_fdi_lanes(struct drm_atomic_state *state,
  +  enum pipe pipe)
   {
  struct intel_crtc *crtc =
  -   to_intel_crtc(intel_get_crtc_for_pipe(dev, pipe));
  +   to_intel_crtc(intel_get_crtc_for_pipe(state-dev, pipe));
  +   struct intel_crtc_state *crtc_state;
  +
  +   crtc_state = intel_atomic_get_crtc_state(state, crtc);
  +   if (WARN_ON(IS_ERR(crtc_state))) {
  +   /* Cause modeset to fail due to excess lanes. */
  +   return 5;
  +   }
  
  -   if (crtc-base.state-enable 
  -   crtc-config-has_pch_encoder)
  +   if (crtc_state-base.enable  crtc_state-has_pch_encoder)
  return crtc-config-fdi_lanes;
  
  return 0;
  @@ -5552,6 +5559,8 @@ static int pipe_required_fdi_lanes(struct drm_device
  *dev, enum pipe pipe)  static bool ironlake_check_fdi_lanes(struct 
  drm_device
  *dev, enum pipe pipe,
   struct intel_crtc_state *pipe_config)  {
  +   struct drm_atomic_state *state = pipe_config-base.state;
  +
  DRM_DEBUG_KMS(checking fdi config on pipe %c, lanes %i\n,
pipe_name(pipe), pipe_config-fdi_lanes);
  if (pipe_config-fdi_lanes  4) {
  @@ -5579,7 +5588,7 @@ static bool ironlake_check_fdi_lanes(struct
  drm_device *dev, enum pipe pipe,
  return true;
  case PIPE_B:
  if (pipe_config-fdi_lanes  2 
  -   pipe_required_fdi_lanes(dev, PIPE_C)  0) {
  +   pipe_required_fdi_lanes(state, PIPE_C)  0) {
  DRM_DEBUG_KMS(invalid shared fdi lane config on
  pipe %c: %i lanes\n,
pipe_name(pipe), pipe_config-fdi_lanes);
  return false;
  @@ -5591,7 +5600,7 @@ static bool ironlake_check_fdi_lanes(struct
  drm_device *dev, enum pipe pipe,
pipe_name(pipe), pipe_config-fdi_lanes);
  return false;
  }
  -   if (pipe_required_fdi_lanes(dev, PIPE_B)  2) {
  +   if (pipe_required_fdi_lanes(state, PIPE_B)  2) {
  DRM_DEBUG_KMS(fdi link B uses too many lanes to
  enable link C\n);
  return false;
  }
  @@ -5601,15 +5610,41 @@ static bool ironlake_check_fdi_lanes(struct
  drm_device *dev, enum pipe pipe,
  }
   }
  
  +static int add_pipe_b_c_to_state(struct drm_atomic_state *state) {
  +   struct intel_crtc *pipe_B =
  +   to_intel_crtc(intel_get_crtc_for_pipe(state-dev, PIPE_B));
  +   struct intel_crtc *pipe_C =
  +   to_intel_crtc(intel_get_crtc_for_pipe(state-dev, PIPE_C));
  +   struct intel_crtc_state *crtc_state;
  +
  +   crtc_state = intel_atomic_get_crtc_state(state, pipe_B);
  +   if (IS_ERR(crtc_state))
  +   return PTR_ERR(crtc_state);
  +
  +   crtc_state = intel_atomic_get_crtc_state(state, pipe_C);
  +   if (IS_ERR(crtc_state))
  +   return PTR_ERR(crtc_state);
  +
  +   return 0;
  +}
  +
   #define RETRY 1
   static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
 struct intel_crtc_state *pipe_config)  {
  struct drm_device *dev = intel_crtc-base.dev;
  struct drm_display_mode *adjusted_mode = pipe_config-
  base.adjusted_mode;
  -   int lane, link_bw, fdi_dotclock;
  +   int lane, link_bw, fdi_dotclock, ret;
  bool setup_ok, needs_recompute = false;
  
  +   if (IS_IVYBRIDGE(dev) 
  +   (intel_crtc-pipe == PIPE_B || intel_crtc-pipe == PIPE_C)) {
  +   ret = add_pipe_b_c_to_state(pipe_config-base.state);
 
 In this scenario, crtc_states are created for both pipe B  C as an operation
 on one can affect the other. I may be missing something here, 
 but where is the other crtc_state being used: compute/check flow and/or 
 commit flow?

The function pipe_required_fdi_lanes() above is changed in this patch to
use the crtc_state

Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept

[Conselvan De Oliveira, Ander]

On Wed, 2015-03-11 at 13:35 +0200, Ander Conselvan de Oliveira wrote:
 Remove the global modeset resource function that would disable the
 bifurcation bit, and instead enable/disable it when enabling the pch
 transcoder. The mode set consistency check should prevent us from
 disabling the bit if pipe C is enabled so the change should be safe.
 
 Note that this doens't affect the logic that prevents the bit being
 set while a pipe is active, since the patch retains the behavior of
 only chaging the bit if necessary. Because of the checks during mode
 set, the first change would necessarily happen with both pipes B and
 C disabled, and any subsequent write would be skipped.
 
 v2: Only change the bit during pch trancoder enable. (Ville)

Oops, I forgot the sob line.

Signed-off-by: Ander Conselvan de Oliveira
ander.conselvan.de.olive...@intel.com

 ---
  drivers/gpu/drm/i915/intel_display.c | 46 
 
  1 file changed, 10 insertions(+), 36 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 4008bf4..bfbd829 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -3153,32 +3153,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc 
 *crtc)
   return crtc-base.state-enable  crtc-config-has_pch_encoder;
  }
  
 -static void ivb_modeset_global_resources(struct drm_device *dev)
 -{
 - struct drm_i915_private *dev_priv = dev-dev_private;
 - struct intel_crtc *pipe_B_crtc =
 - to_intel_crtc(dev_priv-pipe_to_crtc_mapping[PIPE_B]);
 - struct intel_crtc *pipe_C_crtc =
 - to_intel_crtc(dev_priv-pipe_to_crtc_mapping[PIPE_C]);
 - uint32_t temp;
 -
 - /*
 -  * When everything is off disable fdi C so that we could enable fdi B
 -  * with all lanes. Note that we don't care about enabled pipes without
 -  * an enabled pch encoder.
 -  */
 - if (!pipe_has_enabled_pch(pipe_B_crtc) 
 - !pipe_has_enabled_pch(pipe_C_crtc)) {
 - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B))  FDI_RX_ENABLE);
 - WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C))  FDI_RX_ENABLE);
 -
 - temp = I915_READ(SOUTH_CHICKEN1);
 - temp = ~FDI_BC_BIFURCATION_SELECT;
 - DRM_DEBUG_KMS(disabling fdi C rx\n);
 - I915_WRITE(SOUTH_CHICKEN1, temp);
 - }
 -}
 -
  /* The FDI link training functions for ILK/Ibexpeak. */
  static void ironlake_fdi_link_train(struct drm_crtc *crtc)
  {
 @@ -3834,20 +3808,23 @@ static void 
 ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
  I915_READ(VSYNCSHIFT(cpu_transcoder)));
  }
  
 -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
 +static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
   uint32_t temp;
  
   temp = I915_READ(SOUTH_CHICKEN1);
 - if (temp  FDI_BC_BIFURCATION_SELECT)
 + if (!!(temp  FDI_BC_BIFURCATION_SELECT) == enable)
   return;
  
   WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B))  FDI_RX_ENABLE);
   WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C))  FDI_RX_ENABLE);
  
 - temp |= FDI_BC_BIFURCATION_SELECT;
 - DRM_DEBUG_KMS(enabling fdi C rx\n);
 + temp = ~FDI_BC_BIFURCATION_SELECT;
 + if (enable)
 + temp |= FDI_BC_BIFURCATION_SELECT;
 +
 + DRM_DEBUG_KMS(%sabling fdi C rx\n, enable ? en : dis);
   I915_WRITE(SOUTH_CHICKEN1, temp);
   POSTING_READ(SOUTH_CHICKEN1);
  }
 @@ -3855,20 +3832,19 @@ static void cpt_enable_fdi_bc_bifurcation(struct 
 drm_device *dev)
  static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc 
 *intel_crtc)
  {
   struct drm_device *dev = intel_crtc-base.dev;
 - struct drm_i915_private *dev_priv = dev-dev_private;
  
   switch (intel_crtc-pipe) {
   case PIPE_A:
   break;
   case PIPE_B:
   if (intel_crtc-config-fdi_lanes  2)
 - WARN_ON(I915_READ(SOUTH_CHICKEN1)  
 FDI_BC_BIFURCATION_SELECT);
 + cpt_set_fdi_bc_bifurcation(dev, false);
   else
 - cpt_enable_fdi_bc_bifurcation(dev);
 + cpt_set_fdi_bc_bifurcation(dev, true);
  
   break;
   case PIPE_C:
 - cpt_enable_fdi_bc_bifurcation(dev);
 + cpt_set_fdi_bc_bifurcation(dev, true);
  
   break;
   default:
 @@ -13056,8 +13032,6 @@ static void intel_init_display(struct drm_device *dev)
   } else if (IS_IVYBRIDGE(dev)) {
   /* FIXME: detect B0+ stepping and use auto training */
   dev_priv-display.fdi_link_train = ivb_manual_fdi_link_train;
 - dev_priv-display.modeset_global_resources =
 - ivb_modeset_global_resources;
   } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
   dev_priv-display.fdi_link_train = 

Re: [Intel-gfx] [PATCH] drm/i915: Simplify the way BC bifurcation state consistency is kept

[Conselvan De Oliveira, Ander]

On Wed, 2015-03-11 at 15:10 +0200, Ville Syrjälä wrote:
 On Wed, Mar 11, 2015 at 01:35:43PM +0200, Ander Conselvan de Oliveira wrote:
  Remove the global modeset resource function that would disable the
  bifurcation bit, and instead enable/disable it when enabling the pch
  transcoder. The mode set consistency check should prevent us from
  disabling the bit if pipe C is enabled so the change should be safe.
  
  Note that this doens't affect the logic that prevents the bit being
  set while a pipe is active, since the patch retains the behavior of
  only chaging the bit if necessary. Because of the checks during mode
  set, the first change would necessarily happen with both pipes B and
  C disabled, and any subsequent write would be skipped.
  
  v2: Only change the bit during pch trancoder enable. (Ville)
  ---
   drivers/gpu/drm/i915/intel_display.c | 46 
  
   1 file changed, 10 insertions(+), 36 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index 4008bf4..bfbd829 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -3153,32 +3153,6 @@ static bool pipe_has_enabled_pch(struct intel_crtc 
  *crtc)
  return crtc-base.state-enable  crtc-config-has_pch_encoder;
   }
   
  -static void ivb_modeset_global_resources(struct drm_device *dev)
  -{
  -   struct drm_i915_private *dev_priv = dev-dev_private;
  -   struct intel_crtc *pipe_B_crtc =
  -   to_intel_crtc(dev_priv-pipe_to_crtc_mapping[PIPE_B]);
  -   struct intel_crtc *pipe_C_crtc =
  -   to_intel_crtc(dev_priv-pipe_to_crtc_mapping[PIPE_C]);
  -   uint32_t temp;
  -
  -   /*
  -* When everything is off disable fdi C so that we could enable fdi B
  -* with all lanes. Note that we don't care about enabled pipes without
  -* an enabled pch encoder.
  -*/
  -   if (!pipe_has_enabled_pch(pipe_B_crtc) 
  -   !pipe_has_enabled_pch(pipe_C_crtc)) {
  -   WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B))  FDI_RX_ENABLE);
  -   WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C))  FDI_RX_ENABLE);
  -
  -   temp = I915_READ(SOUTH_CHICKEN1);
  -   temp = ~FDI_BC_BIFURCATION_SELECT;
  -   DRM_DEBUG_KMS(disabling fdi C rx\n);
  -   I915_WRITE(SOUTH_CHICKEN1, temp);
  -   }
  -}
  -
   /* The FDI link training functions for ILK/Ibexpeak. */
   static void ironlake_fdi_link_train(struct drm_crtc *crtc)
   {
  @@ -3834,20 +3808,23 @@ static void 
  ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc,
 I915_READ(VSYNCSHIFT(cpu_transcoder)));
   }
   
  -static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev)
  +static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable)
   {
  struct drm_i915_private *dev_priv = dev-dev_private;
  uint32_t temp;
   
  temp = I915_READ(SOUTH_CHICKEN1);
  -   if (temp  FDI_BC_BIFURCATION_SELECT)
  +   if (!!(temp  FDI_BC_BIFURCATION_SELECT) == enable)
  return;
   
  WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B))  FDI_RX_ENABLE);
  WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C))  FDI_RX_ENABLE);
   
  -   temp |= FDI_BC_BIFURCATION_SELECT;
  -   DRM_DEBUG_KMS(enabling fdi C rx\n);
  +   temp = ~FDI_BC_BIFURCATION_SELECT;
  +   if (enable)
  +   temp |= FDI_BC_BIFURCATION_SELECT;
  +
  +   DRM_DEBUG_KMS(%sabling fdi C rx\n, enable ? en : dis);
  I915_WRITE(SOUTH_CHICKEN1, temp);
  POSTING_READ(SOUTH_CHICKEN1);
   }
  @@ -3855,20 +3832,19 @@ static void cpt_enable_fdi_bc_bifurcation(struct 
  drm_device *dev)
   static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc 
  *intel_crtc)
   {
  struct drm_device *dev = intel_crtc-base.dev;
  -   struct drm_i915_private *dev_priv = dev-dev_private;
   
  switch (intel_crtc-pipe) {
  case PIPE_A:
  break;
  case PIPE_B:
  if (intel_crtc-config-fdi_lanes  2)
  -   WARN_ON(I915_READ(SOUTH_CHICKEN1)  
  FDI_BC_BIFURCATION_SELECT);
  +   cpt_set_fdi_bc_bifurcation(dev, false);
 
 So we just replace the globla_resources enforced assumed state with an
 explicit state change here. Should be perfectly fine since pipe is not
 active at this point.
 
 
 What really confuses me about the whole FDI bifurcation code is
 ironlake_check_fdi_lanes(). First of all I would rewrite it a bit like
 so:
 
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -5615,14 +5615,13 @@ static bool ironlake_check_fdi_lanes(struct 
 drm_device *dev, enum pipe pipe,
 }
 return true;
 case PIPE_C:
 -   if (!pipe_has_enabled_pch(pipe_B_crtc) ||
 -   pipe_B_crtc-config-fdi_lanes = 2) {
 -   if (pipe_config-fdi_lanes  2) {
 -   DRM_DEBUG_KMS(invalid shared fdi lane config 
 on pipe %c: %i lanes\n,
 -  

Re: [Intel-gfx] [PATCH 21/23] drm/i915: Convert intel_pipe_will_have_type() to using atomic state

[Conselvan De Oliveira, Ander]

On Wed, 2015-03-04 at 17:03 +0100, Daniel Vetter wrote:
 On Tue, Mar 03, 2015 at 03:22:15PM +0200, Ander Conselvan de Oliveira wrote:
  Pass a crtc_state to it and find whether the pipe has an encoder of a
  given type by looking at the drm_atomic_state the crtc_state points to.
  
  Note that is possible to reach i9xx_get_refclk() without a proper atomic
  state, since in the function vlv_force_pll_on() a minimally initialized
  crtc_state is allocated in the stack. With the current code, it is not
  possible to end up in a call to intel_pipe_will_have_type() with that
  bogus atomic state. To avoid future problems, a comment is added to warn
  people changing that code.
  
  Signed-off-by: Ander Conselvan de Oliveira 
  ander.conselvan.de.olive...@intel.com
  ---
   drivers/gpu/drm/i915/i915_drv.h  |   2 +-
   drivers/gpu/drm/i915/intel_display.c | 134 
  +--
   2 files changed, 83 insertions(+), 53 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_drv.h 
  b/drivers/gpu/drm/i915/i915_drv.h
  index 0c6ba2d..aab4421 100644
  --- a/drivers/gpu/drm/i915/i915_drv.h
  +++ b/drivers/gpu/drm/i915/i915_drv.h
  @@ -541,7 +541,7 @@ struct drm_i915_display_funcs {
   * Returns true on success, false on failure.
   */
  bool (*find_dpll)(const struct intel_limit *limit,
  - struct intel_crtc *crtc,
  + struct intel_crtc_state *crtc_state,
int target, int refclk,
struct dpll *match_clock,
struct dpll *best_clock);
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index 518903e..f3652f9 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -431,25 +431,37 @@ bool intel_pipe_has_type(struct intel_crtc *crtc, 
  enum intel_output_type type)
* intel_pipe_has_type() but looking at encoder-new_crtc instead of
* encoder-crtc.
*/
  -static bool intel_pipe_will_have_type(struct intel_crtc *crtc, int type)
  +static bool intel_pipe_will_have_type(const struct intel_crtc_state 
  *crtc_state,
  + int type)
   {
  -   struct drm_device *dev = crtc-base.dev;
  +   struct drm_atomic_state *state = crtc_state-base.state;
  +   struct drm_connector_state *connector_state;
  struct intel_encoder *encoder;
  +   int i;
  +
  +   for (i = 0; i  state-num_connector; i++) {
  +   if (!state-connectors[i])
  +   continue;
  +
  +   connector_state = state-connector_states[i];
  +   if (connector_state-crtc != crtc_state-base.crtc)
  +   continue;
   
  -   for_each_intel_encoder(dev, encoder)
  -   if (encoder-new_crtc == crtc  encoder-type == type)
  +   encoder = to_intel_encoder(connector_state-best_encoder);
  +   if (encoder-type == type)
  return true;
  +   }
   
  return false;
   }
 
 The tricky bit here is that we must have all the connectors added to the
 drm_atomic_sate for the given crtc. Otherwise there might be no connector
 at all and we'd return a bogus answer. drm_atomic_add_affected_connectors
 is the function which does this for you, and I think we need to call it
 somewhere in the top-level compute_config code. And I haven't spotted that
 call anywhere in your series.

I did add it in patch 12, but now I realize it won't be called for the
disable case, since the call is in intel_modeset_pipe_config(). I'll
move that to intel_modeset_compute_config().

Ander

 Otherwise the implementation of the new will_have_type looks correct.
 -Daniel
 
   
  -static const intel_limit_t *intel_ironlake_limit(struct intel_crtc *crtc,
  -   int refclk)
  +static const intel_limit_t *
  +intel_ironlake_limit(struct intel_crtc_state *crtc_state, int refclk)
   {
  -   struct drm_device *dev = crtc-base.dev;
  +   struct drm_device *dev = crtc_state-base.crtc-dev;
  const intel_limit_t *limit;
   
  -   if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
  +   if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
  if (intel_is_dual_link_lvds(dev)) {
  if (refclk == 10)
  limit = intel_limits_ironlake_dual_lvds_100m;
  @@ -467,20 +479,21 @@ static const intel_limit_t 
  *intel_ironlake_limit(struct intel_crtc *crtc,
  return limit;
   }
   
  -static const intel_limit_t *intel_g4x_limit(struct intel_crtc *crtc)
  +static const intel_limit_t *
  +intel_g4x_limit(struct intel_crtc_state *crtc_state)
   {
  -   struct drm_device *dev = crtc-base.dev;
  +   struct drm_device *dev = crtc_state-base.crtc-dev;
  const intel_limit_t *limit;
   
  -   if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
  +   if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
  if 

Re: [Intel-gfx] [PATCH 11/23] drm/i915: Copy the staged connector config to the legacy atomic state

[Conselvan De Oliveira, Ander]

On Wed, 2015-03-04 at 16:46 +0100, Daniel Vetter wrote:
 On Tue, Mar 03, 2015 at 03:22:05PM +0200, Ander Conselvan de Oliveira wrote:
  With this in place, we can start converting pieces of the modeset code
  to look at the connector atomic state instead of the staged config.
  
  Signed-off-by: Ander Conselvan de Oliveira 
  ander.conselvan.de.olive...@intel.com
  ---
   drivers/gpu/drm/i915/intel_display.c | 23 ---
   1 file changed, 20 insertions(+), 3 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index 108d3d2..4e90cb4 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -11607,9 +11607,11 @@ intel_set_config_compute_mode_changes(struct 
  drm_mode_set *set,
   static int
   intel_modeset_stage_output_state(struct drm_device *dev,
   struct drm_mode_set *set,
  -struct intel_set_config *config)
  +struct intel_set_config *config,
  +struct drm_atomic_state *state)
   {
  struct intel_connector *connector;
  +   struct drm_connector_state *connector_state;
  struct intel_encoder *encoder;
  struct intel_crtc *crtc;
  int ro;
  @@ -11673,6 +11675,14 @@ intel_modeset_stage_output_state(struct drm_device 
  *dev,
  }
  connector-new_encoder-new_crtc = to_intel_crtc(new_crtc);
   
  +   connector_state =
  +   drm_atomic_get_connector_state(state, connector-base);
  +   if (IS_ERR(connector_state))
  +   return PTR_ERR(connector_state);
  +
  +   connector_state-crtc = new_crtc;
  +   connector_state-best_encoder = connector-new_encoder-base;
  +
  DRM_DEBUG_KMS([CONNECTOR:%d:%s] to [CRTC:%d]\n,
  connector-base.base.id,
  connector-base.name,
  @@ -11705,9 +11715,16 @@ intel_modeset_stage_output_state(struct drm_device 
  *dev,
  }
  /* Now we've also updated encoder-new_crtc for all encoders. */
  for_each_intel_connector(dev, connector) {
  -   if (connector-new_encoder)
  +   connector_state =
  +   drm_atomic_get_connector_state(state, connector-base);
  +
  +   if (connector-new_encoder) {
  if (connector-new_encoder != connector-encoder)
  connector-encoder = connector-new_encoder;
  +   } else {
  +   connector_state-crtc = NULL;
  +   }
  +
 
 Unecessary line. Of course you've put that in there to check that I
 actually read your patches ;-)

It actually is needed, since the other hunk we only update the
connector's crtc when it has an enabled connector. It makes sense for
the staged config, since the new_crtc field is in the encoder, which
would be NULL in that case. The loop just above this one sets new_crtc
to NULL for these encoders, but for the connectors it was just more
convenient to set it here.

Ander

 
 Cheers, Daniel
 
  }
  for_each_intel_crtc(dev, crtc) {
  crtc-new_enabled = false;
  @@ -11816,7 +11833,7 @@ static int intel_crtc_set_config(struct 
  drm_mode_set *set)
   
  state-acquire_ctx = dev-mode_config.acquire_ctx;
   
  -   ret = intel_modeset_stage_output_state(dev, set, config);
  +   ret = intel_modeset_stage_output_state(dev, set, config, state);
  if (ret)
  goto fail;
   
  -- 
  2.1.0
  
  ___
  Intel-gfx mailing list
  Intel-gfx@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 

-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx