Re: [Intel-gfx] [PATCH 06/12] drm/i915: Pass plane to vlv_compute_drain_latency()

[Jesse Barnes]

On 02/27/2015 10:09 AM, Ville Syrjälä wrote:
> On Fri, Feb 27, 2015 at 09:57:20AM -0800, Jesse Barnes wrote:
>> On 02/10/2015 05:28 AM, ville.syrj...@linux.intel.com wrote:
>>> From: Ville Syrjälä 
>>>
>>> Now that we have drm_planes for the cursor and primary we can move the
>>> pixel_size handling into vlv_compute_drain_latency() and just pass the
>>> appropriate plane to it.
>>>
>>> Signed-off-by: Ville Syrjälä 
>>> ---
>>>  drivers/gpu/drm/i915/intel_pm.c | 42 
>>> -
>>>  1 file changed, 16 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c 
>>> b/drivers/gpu/drm/i915/intel_pm.c
>>> index 5515d10..fffcf64 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -727,16 +727,26 @@ static void vlv_write_wm_values(struct intel_crtc 
>>> *crtc,
>>>  }
>>>  
>>>  static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc,
>>> -int pixel_size)
>>> +struct drm_plane *plane)
>>>  {
>>> struct drm_device *dev = crtc->dev;
>>> -   int entries, prec_mult, drain_latency;
>>> -   int clock = to_intel_crtc(crtc)->config->base.adjusted_mode.crtc_clock;
>>> +   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>> +   int entries, prec_mult, drain_latency, pixel_size;
>>> +   int clock = intel_crtc->config->base.adjusted_mode.crtc_clock;
>>> const int high_precision = IS_CHERRYVIEW(dev) ? 16 : 64;
>>>  
>>> +   /*
>>> +* FIXME the plane might have an fb
>>> +* but be invisible (eg. due to clipping)
>>> +*/
>>> +   if (!intel_crtc->active || !plane->fb)
>>> +   return 0;
>>> +
>>> if (WARN(clock == 0, "Pixel clock is zero!\n"))
>>> return 0;
>>>  
>>> +   pixel_size = drm_format_plane_cpp(plane->fb->pixel_format, 0);
>>> +
>>> if (WARN(pixel_size == 0, "Pixel size is zero!\n"))
>>> return 0;
>>>  
>>> @@ -770,31 +780,11 @@ static void vlv_update_drain_latency(struct drm_crtc 
>>> *crtc)
>>> struct drm_device *dev = crtc->dev;
>>> struct drm_i915_private *dev_priv = dev->dev_private;
>>> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>> -   int pixel_size;
>>> enum pipe pipe = intel_crtc->pipe;
>>> struct vlv_wm_values wm = dev_priv->wm.vlv;
>>>  
>>> -   wm.ddl[pipe].primary = 0;
>>> -   wm.ddl[pipe].cursor = 0;
>>> -
>>> -   if (!intel_crtc_active(crtc)) {
>>> -   vlv_write_wm_values(intel_crtc, &wm);
>>> -   return;
>>> -   }
>>> -
>>> -   /* Primary plane Drain Latency */
>>> -   pixel_size = crtc->primary->fb->bits_per_pixel / 8; /* BPP */
>>> -   wm.ddl[pipe].primary = vlv_compute_drain_latency(crtc, pixel_size);
>>> -
>>> -   /* Cursor Drain Latency
>>> -* BPP is always 4 for cursor
>>> -*/
>>> -   pixel_size = 4;
>>> -
>>> -   /* Program cursor DL only if it is enabled */
>>> -   if (intel_crtc->cursor_base)
>>> -   wm.ddl[pipe].cursor =
>>> -   vlv_compute_drain_latency(crtc, pixel_size);
>>> +   wm.ddl[pipe].primary = vlv_compute_drain_latency(crtc, crtc->primary);
>>> +   wm.ddl[pipe].cursor = vlv_compute_drain_latency(crtc, crtc->cursor);
>>>  
>>> vlv_write_wm_values(intel_crtc, &wm);
>>>  }
>>> @@ -961,7 +951,7 @@ static void valleyview_update_sprite_wm(struct 
>>> drm_plane *plane,
>>>  
>>> if (enabled)
>>> wm.ddl[pipe].sprite[sprite] =
>>> -   vlv_compute_drain_latency(crtc, pixel_size);
>>> +   vlv_compute_drain_latency(crtc, plane);
>>> else
>>> wm.ddl[pipe].sprite[sprite] = 0;
>>>  
>>>
>>
>> Nice.
> 
> Actually this is going to have issues now that atomic is partially in.
> We'd need to look at plane->state->fb now, but that steps on the same
> turf as Matt's latest patches.

Yeah there's some overlap, but mostly it looked like these would make
things a little easier by removing duplication and clearing things up
(not to mention the bug fixes).

Jesse

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

Re: [Intel-gfx] [PATCH 08/12] drm/i915: Make sure PND deadline mode is enabled on VLV/CHV

[Jesse Barnes]

On 02/10/2015 05:28 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Poke at the CBR1_VLV register during init_clock_gating to make sure the
> PND deadline scheme is used.
> 
> The hardware has two modes of operation wrt. watermarks:
> 
> 1) PND deadline mode:
>  - memory request deadline is calculated from actual FIFO level * DDL
>  - WM1 watermark values are unused (AFAIK)
>  - WM watermark level defines when to start fetching data from memory
>(assuming trickle feed is not used)
> 
> 2) backup mode
>  - deadline is based on FIFO status, DDL is unused
>  - FIFO split into three regions with WM and WM1 watermarks, each
>part specifying a different FIFO status
> 
> We want to use the PND deadline mode, so let's make sure the chicken
> bit is in the correct position on init.
> 
> Also take the opportunity to refactor the shared code between VLV and
> CHV to a shared function.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  3 +++
>  drivers/gpu/drm/i915/intel_pm.c | 19 +--
>  2 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 433d7e7..5d377df 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4177,6 +4177,9 @@ enum skl_disp_power_wells {
>  #define DDL_PRECISION_LOW(0<<7)
>  #define DRAIN_LATENCY_MASK   0x7f
>  
> +#define CBR1_VLV (VLV_DISPLAY_BASE + 0x70400)
> +#define  CBR_PND_DEADLINE_DISABLE(1<<31)
> +
>  /* FIFO watermark sizes etc */
>  #define G4X_FIFO_LINE_SIZE   64
>  #define I915_FIFO_LINE_SIZE  64
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e53038e..c021e92 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6197,11 +6197,22 @@ static void ivybridge_init_clock_gating(struct 
> drm_device *dev)
>   gen6_check_mch_setup(dev);
>  }
>  
> +static void vlv_init_display_clock_gating(struct drm_i915_private *dev_priv)
> +{
> + I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
> +
> + /*
> +  * Disable trickle feed and enable pnd deadline calculation
> +  */
> + I915_WRITE(MI_ARB_VLV, MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE);
> + I915_WRITE(CBR1_VLV, 0);
> +}
> +
>  static void valleyview_init_clock_gating(struct drm_device *dev)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
>  
> - I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
> + vlv_init_display_clock_gating(dev_priv);
>  
>   /* WaDisableEarlyCull:vlv */
>   I915_WRITE(_3D_CHICKEN3,
> @@ -6249,8 +6260,6 @@ static void valleyview_init_clock_gating(struct 
> drm_device *dev)
>   I915_WRITE(GEN7_UCGCTL4,
>  I915_READ(GEN7_UCGCTL4) | GEN7_L3BANK2X_CLOCK_GATE_DISABLE);
>  
> - I915_WRITE(MI_ARB_VLV, MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE);
> -
>   /*
>* BSpec says this must be set, even though
>* WaDisable4x2SubspanOptimization isn't listed for VLV.
> @@ -6287,9 +6296,7 @@ static void cherryview_init_clock_gating(struct 
> drm_device *dev)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
>  
> - I915_WRITE(DSPCLK_GATE_D, VRHUNIT_CLOCK_GATE_DISABLE);
> -
> - I915_WRITE(MI_ARB_VLV, MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE);
> + vlv_init_display_clock_gating(dev_priv);
>  
>   /* WaVSRefCountFullforceMissDisable:chv */
>   /* WaDSRefCountFullforceMissDisable:chv */
> 

Is the deadline mode preference documented somewhere?  Do we need
another bspec bug filed?

Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 07/12] drm/i915: Read out display FIFO size on VLV/CHV

[Jesse Barnes]

On 02/12/2015 10:59 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> VLV/CHV have similar DSPARB registers as older platforms, just more of
> them due to more planes. Add a bit of code to read out the current FIFO
> split from the registers. Will be useful later when we improve the WM
> calculations.
> 
> v2: Add display_mmio_offset to DSPARB
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  5 +++-
>  drivers/gpu/drm/i915/intel_pm.c | 55 
> +
>  2 files changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b35aaf3..3b48f4b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4026,7 +4026,7 @@ enum skl_disp_power_wells {
>  #define   DPINVGTT_STATUS_MASK   0xff
>  #define   DPINVGTT_STATUS_MASK_CHV   0xfff
>  
> -#define DSPARB   0x70030
> +#define DSPARB   (dev_priv->info.display_mmio_offset + 
> 0x70030)
>  #define   DSPARB_CSTART_MASK (0x7f << 7)
>  #define   DSPARB_CSTART_SHIFT7
>  #define   DSPARB_BSTART_MASK (0x7f)
> @@ -4034,6 +4034,9 @@ enum skl_disp_power_wells {
>  #define   DSPARB_BEND_SHIFT  9 /* on 855 */
>  #define   DSPARB_AEND_SHIFT  0
>  
> +#define DSPARB2  (VLV_DISPLAY_BASE + 0x70060) /* vlv/chv 
> */
> +#define DSPARB3  (VLV_DISPLAY_BASE + 0x7006c) /* chv */
> +
>  /* pnv/gen4/g4x/vlv/chv */
>  #define DSPFW1   (dev_priv->info.display_mmio_offset + 
> 0x70034)
>  #define   DSPFW_SR_SHIFT 23
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index fffcf64..e53038e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -280,6 +280,61 @@ void intel_set_memory_cxsr(struct drm_i915_private 
> *dev_priv, bool enable)
>   */
>  static const int pessimal_latency_ns = 5000;
>  
> +#define VLV_FIFO_START(dsparb, dsparb2, lo_shift, hi_shift) \
> + dsparb) >> (lo_shift)) & 0xff) | dsparb2) >> (hi_shift)) & 0x1) 
> << 8))
> +
> +static int vlv_get_fifo_size(struct drm_device *dev,
> +   enum pipe pipe, int plane)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + int sprite0_start, sprite1_start, size;
> +
> + switch (pipe) {
> + uint32_t dsparb, dsparb2, dsparb3;
> + case PIPE_A:
> + dsparb = I915_READ(DSPARB);
> + dsparb2 = I915_READ(DSPARB2);
> + sprite0_start = VLV_FIFO_START(dsparb, dsparb2, 0, 0);
> + sprite1_start = VLV_FIFO_START(dsparb, dsparb2, 8, 4);
> + break;
> + case PIPE_B:
> + dsparb = I915_READ(DSPARB);
> + dsparb2 = I915_READ(DSPARB2);
> + sprite0_start = VLV_FIFO_START(dsparb, dsparb2, 16, 8);
> + sprite1_start = VLV_FIFO_START(dsparb, dsparb2, 24, 12);
> + break;
> + case PIPE_C:
> + dsparb2 = I915_READ(DSPARB2);
> + dsparb3 = I915_READ(DSPARB3);
> + sprite0_start = VLV_FIFO_START(dsparb3, dsparb2, 0, 16);
> + sprite1_start = VLV_FIFO_START(dsparb3, dsparb2, 8, 20);
> + break;
> + default:
> + return 0;
> + }
> +
> + switch (plane) {
> + case 0:
> + size = sprite0_start;
> + break;
> + case 1:
> + size = sprite1_start - sprite0_start;
> + break;
> + case 2:
> + size = 512 - 1 - sprite1_start;
> + break;
> + default:
> + return 0;
> + }
> +
> + DRM_DEBUG_KMS("Pipe %c %s %c FIFO size: %d\n",
> +   pipe_name(pipe), plane == 0 ? "primary" : "sprite",
> +   plane == 0 ? plane_name(pipe) : sprite_name(pipe, plane - 
> 1),
> +   size);
> +
> + return size;
> +}
> +
>  static int i9xx_get_fifo_size(struct drm_device *dev, int plane)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
> 

Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 06/12] drm/i915: Pass plane to vlv_compute_drain_latency()

[Jesse Barnes]

On 02/10/2015 05:28 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Now that we have drm_planes for the cursor and primary we can move the
> pixel_size handling into vlv_compute_drain_latency() and just pass the
> appropriate plane to it.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 42 
> -
>  1 file changed, 16 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5515d10..fffcf64 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -727,16 +727,26 @@ static void vlv_write_wm_values(struct intel_crtc *crtc,
>  }
>  
>  static uint8_t vlv_compute_drain_latency(struct drm_crtc *crtc,
> -  int pixel_size)
> +  struct drm_plane *plane)
>  {
>   struct drm_device *dev = crtc->dev;
> - int entries, prec_mult, drain_latency;
> - int clock = to_intel_crtc(crtc)->config->base.adjusted_mode.crtc_clock;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + int entries, prec_mult, drain_latency, pixel_size;
> + int clock = intel_crtc->config->base.adjusted_mode.crtc_clock;
>   const int high_precision = IS_CHERRYVIEW(dev) ? 16 : 64;
>  
> + /*
> +  * FIXME the plane might have an fb
> +  * but be invisible (eg. due to clipping)
> +  */
> + if (!intel_crtc->active || !plane->fb)
> + return 0;
> +
>   if (WARN(clock == 0, "Pixel clock is zero!\n"))
>   return 0;
>  
> + pixel_size = drm_format_plane_cpp(plane->fb->pixel_format, 0);
> +
>   if (WARN(pixel_size == 0, "Pixel size is zero!\n"))
>   return 0;
>  
> @@ -770,31 +780,11 @@ static void vlv_update_drain_latency(struct drm_crtc 
> *crtc)
>   struct drm_device *dev = crtc->dev;
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - int pixel_size;
>   enum pipe pipe = intel_crtc->pipe;
>   struct vlv_wm_values wm = dev_priv->wm.vlv;
>  
> - wm.ddl[pipe].primary = 0;
> - wm.ddl[pipe].cursor = 0;
> -
> - if (!intel_crtc_active(crtc)) {
> - vlv_write_wm_values(intel_crtc, &wm);
> - return;
> - }
> -
> - /* Primary plane Drain Latency */
> - pixel_size = crtc->primary->fb->bits_per_pixel / 8; /* BPP */
> - wm.ddl[pipe].primary = vlv_compute_drain_latency(crtc, pixel_size);
> -
> - /* Cursor Drain Latency
> -  * BPP is always 4 for cursor
> -  */
> - pixel_size = 4;
> -
> - /* Program cursor DL only if it is enabled */
> - if (intel_crtc->cursor_base)
> - wm.ddl[pipe].cursor =
> - vlv_compute_drain_latency(crtc, pixel_size);
> + wm.ddl[pipe].primary = vlv_compute_drain_latency(crtc, crtc->primary);
> + wm.ddl[pipe].cursor = vlv_compute_drain_latency(crtc, crtc->cursor);
>  
>   vlv_write_wm_values(intel_crtc, &wm);
>  }
> @@ -961,7 +951,7 @@ static void valleyview_update_sprite_wm(struct drm_plane 
> *plane,
>  
>   if (enabled)
>   wm.ddl[pipe].sprite[sprite] =
> - vlv_compute_drain_latency(crtc, pixel_size);
> + vlv_compute_drain_latency(crtc, plane);
>   else
>   wm.ddl[pipe].sprite[sprite] = 0;
>  
> 

Nice.

Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/12] drm/i915: Reorganize VLV DDL setup

[Jesse Barnes]

- ~((DDL_PRECISION_HIGH | DRAIN_LATENCY_MASK) << 
> DDL_SPRITE_SHIFT(sprite));
> + struct vlv_wm_values wm = dev_priv->wm.vlv;
>  
>   if (enabled)
> - sprite_dl |= vlv_compute_drain_latency(crtc, pixel_size) << 
> DDL_SPRITE_SHIFT(sprite);
> + wm.ddl[pipe].sprite[sprite] =
> + vlv_compute_drain_latency(crtc, pixel_size);
> + else
> + wm.ddl[pipe].sprite[sprite] = 0;
>  
> - I915_WRITE(VLV_DDL(pipe), sprite_dl);
> + vlv_write_wm_values(intel_crtc, &wm);
>  }
>  
>  static void g4x_update_wm(struct drm_crtc *crtc)
> 

Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 04/12] drm/i915: Hide VLV DDL precision handling

[Jesse Barnes]

On 02/10/2015 05:28 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Move the DDL precision handling into vlv_compute_drain_latency() so the
> callers don't have to duplicate the same code to deal with it.

A little painful due to the addition of the #define changes, but I think
it's right, and will actually be easier to fix if wrong anyway due to
the removal of duplication.

Reviewed-by: Jesse Barnes 

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

Re: [Intel-gfx] [PATCH 03/12] drm/i915: Simplify VLV drain latency computation

[Jesse Barnes]

On 02/10/2015 05:28 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> The current drain lantency computation relies on hardcoded limits to
> determine when the to use the low vs. high precision multiplier.
> Rewrite the code to use a more straightforward approach.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0f0281a..d6c6c1b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -727,12 +727,15 @@ static bool vlv_compute_drain_latency(struct drm_crtc 
> *crtc,
>   return false;
>  
>   entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
> - if (IS_CHERRYVIEW(dev))
> - *prec_mult = (entries > 32) ? 16 : 8;
> - else
> - *prec_mult = (entries > 128) ? 64 : 32;
> +
> + *prec_mult = IS_CHERRYVIEW(dev) ? 16 : 64;
>   *drain_latency = (64 * (*prec_mult) * 4) / entries;
>  
> + if (*drain_latency > DRAIN_LATENCY_MASK) {
> + *prec_mult /= 2;
> + *drain_latency = (64 * (*prec_mult) * 4) / entries;
> + }
> +
>   if (*drain_latency > DRAIN_LATENCY_MASK)
>   *drain_latency = DRAIN_LATENCY_MASK;
>  
> 

Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/12] drm/i915: Kill DRAIN_LATENCY_PRECISION_* defines

[Jesse Barnes]

On 02/10/2015 05:28 AM, ville.syrj...@linux.intel.com wrote:
> @@ -957,8 +954,7 @@ static void valleyview_update_sprite_wm(struct
drm_plane *plane,
>   int plane_prec;
>   int sprite_dl;
>   int prec_mult;
> - const int high_precision = IS_CHERRYVIEW(dev) ?
> - DRAIN_LATENCY_PRECISION_32 : DRAIN_LATENCY_PRECISION_64;
> + const int high_precision = IS_CHERRYVIEW(dev) ? 16 : 64;
>  
>   sprite_dl = I915_READ(VLV_DDL(pipe)) & 
> ~(DDL_SPRITE_PRECISION_HIGH(sprite) |
>   (DRAIN_LATENCY_MASK << DDL_SPRITE_SHIFT(sprite)));
> 

This hunk changes things; did it belong in the earlier patch?

With that explained:
Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/12] drm/i915: Reduce CHV DDL multiplier to 16/8

[Jesse Barnes]

On 02/10/2015 05:28 AM, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Apparently we must yet halve the DDL drain latency from what we're
> using currently. This little nugget is not in any spec, but came
> down through the grapevine.
> 
> This makes the displays a bit more stable. Not quite fully stable but at
> least they don't fall over immediately on driver load.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 1 +
>  drivers/gpu/drm/i915/intel_pm.c | 6 +++---
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4ee1964..d8a0205 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4166,6 +4166,7 @@ enum skl_disp_power_wells {
>  #define   DSPFW_PLANEA_WM1_HI_MASK   (1<<0)
>  
>  /* drain latency register values*/
> +#define DRAIN_LATENCY_PRECISION_88
>  #define DRAIN_LATENCY_PRECISION_16   16
>  #define DRAIN_LATENCY_PRECISION_32   32
>  #define DRAIN_LATENCY_PRECISION_64   64
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3c64810..a70bce4 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -728,8 +728,8 @@ static bool vlv_compute_drain_latency(struct drm_crtc 
> *crtc,
>  
>   entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
>   if (IS_CHERRYVIEW(dev))
> - *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_32 :
> -DRAIN_LATENCY_PRECISION_16;
> + *prec_mult = (entries > 32) ? DRAIN_LATENCY_PRECISION_16 :
> +   DRAIN_LATENCY_PRECISION_8;
>   else
>   *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
>  DRAIN_LATENCY_PRECISION_32;
> @@ -759,7 +759,7 @@ static void vlv_update_drain_latency(struct drm_crtc 
> *crtc)
>   enum pipe pipe = intel_crtc->pipe;
>   int plane_prec, prec_mult, plane_dl;
>   const int high_precision = IS_CHERRYVIEW(dev) ?
> - DRAIN_LATENCY_PRECISION_32 : DRAIN_LATENCY_PRECISION_64;
> + DRAIN_LATENCY_PRECISION_16 : DRAIN_LATENCY_PRECISION_64;
>  
>   plane_dl = I915_READ(VLV_DDL(pipe)) & ~(DDL_PLANE_PRECISION_HIGH |
>  DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_HIGH |
> 

You're also changing the entries threshold; is that due to the reduced
drain latency precsion also?

Would be good to file a bspec bug on this so we don't lose it.

Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC v2] drm/i915: Android native sync support

[Jesse Barnes]

On 01/28/2015 02:07 AM, Chris Wilson wrote:
> On Wed, Jan 28, 2015 at 10:50:18AM +0100, Daniel Vetter wrote:
>> On Wed, Jan 28, 2015 at 09:23:46AM +, Chris Wilson wrote:
>>> On Wed, Jan 28, 2015 at 10:22:15AM +0100, Daniel Vetter wrote:
 On Mon, Jan 26, 2015 at 09:08:03AM +, Chris Wilson wrote:
> On Mon, Jan 26, 2015 at 08:52:39AM +0100, Daniel Vetter wrote:
>> I think the problem will be platforms that want full explicit fence (like
>> android) but allow delayed creation of the fence fd from a gl sync object
>> (like the android egl extension allows).
>>
>> I'm not sure yet how to best expose that really since just creating a
>> fence from the implicit request attached to the batch might upset the
>> interface purists with the mix in implicit and explicit fencing ;-) Hence
>> why I think for now we should just do the eager fd creation at execbuf
>> until ppl scream (well maybe not merge this patch until ppl scream ...).
>
> Everything we do is buffer centric. Even in the future with random bits
> of memory, we will still use buffers behind the scenes. From an
> interface perspective, it is clearer to me if we say "give me a fence for
> this buffer". Exactly the same way as we say "is this buffer busy" or
> "wait on this buffer". The change is that we now hand back an fd to slot
> into an event loop. That, to me, is a much smaller evolutionary step of
> the existing API, and yet more versatile than just attaching one to the
> execbuf.

 The problem is that big parts of the world do not subscribe to that buffer
 centric view of gfx. Imo since those parts will be the primary users of
 this interface we should try to fit their ideas as much as feasible. Later
 on (if we need it) we can add some glue to tie in the buffer-centric
 implicit model with the explicit model.
>>>
>>> They won't be using execbuffer either. So what's your point?
>>
>> Android very much will user execbuffer. And even the in-flight buffered
>> svm stuff will keep on using execbuf (just without any relocs).
> 
> So still buffer-centric and would benefit from the more general and more
> explict fencing rather than just execbuf. If you look at the throttling
> in mesa, you can already see a place that would rather create a fence on
> a buffer rather than its very approximate execbuf tracking.
>  
>> And once we indeed can make the case (plus have the hw) for direct
>> userspace submission I think the kernel shouldn't be in the business of
>> creating fence objects: The ring will be fully under control of
>> userspace, and that's the only place we could insert a seqno into. So
>> again the same trust issues.
> 
> No buffers, no requests, nothing for the kernel to do. No impact on
> designing an API that is useful today.

If Mesa really wants this, we should investigate intra-batch fences
again, both with and without buffer tracking (because afaik Mesa wants
bufferless SVM too).

But you said you think an fd is too heavyweight even?  What do you mean
by that?  Or were you just preferring re-use of an existing object (i.e.
the buffer) that we already track & pass around?

Jesse

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

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Disallow pin ioctl completely for kms drivers

[Jesse Barnes]

On 11/24/2014 06:13 AM, Chris Wilson wrote:
> On Mon, Nov 24, 2014 at 03:10:05PM +0100, Daniel Vetter wrote:
>> On Mon, Nov 24, 2014 at 10:35:29AM +, Chris Wilson wrote:
>>> Pinning is a useful tool and it would also be useful to have again on
>>> gen6+.
>>
>> I think softpin or similar is doable with ppgtt. But with shared ggtt I'm
>> not really enthusiastic about providing this kind of rope to userspace.
>> And softpin is a different type of pinning, so we don't really lose
>> anything by ripping out the userspace hard pinning ioctls.
> 
> I am not talking about softpin, but being able to pin memory and a GGTT
> binding itself is useful.

I see you merged this over Chris's objections and then shot down his
revert.  I'm not clear on why you're so opposed to the pin ioctl?  It's
a privileged op and definitely has its uses as Chris has repeatedly
pointed out.

Why so insistent on dropping this particular ioctl?  Do you see it
causing actual problems?  Or do you just like preventing userspace from
doing things you don't agree with?

(Sorry, catching up on ancient backlog from intel-gfx, so maybe there's
a thread I missed when re-looking at this.  If so, please point me at it.)

Thanks,
Jesse
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/5] agp/intel: Serialise after GTT updates

[Jesse Barnes]

On Wed, 14 Jan 2015 11:20:55 +
Chris Wilson  wrote:

> diff --git a/drivers/char/agp/intel-gtt.c
> b/drivers/char/agp/intel-gtt.c index 92aa43fa8d70..15685ca39193 100644
> --- a/drivers/char/agp/intel-gtt.c
> +++ b/drivers/char/agp/intel-gtt.c
> @@ -225,7 +225,7 @@ static int i810_insert_dcache_entries(struct
> agp_memory *mem, off_t pg_start,
> intel_private.driver->write_entry(addr, i, type);
>   }
> - readl(intel_private.gtt+i-1);
> + readl(intel_private.gtt+pg_start);

Any idea why?  This one scares me...  is it that the read is being
serviced from the WC buffer w/o being flushed?  Or is the compiler
optimizing the last read based on the previous write?

Writing a non-sequential address should also cause a flush, but I don't
remember the rules for reads.  We should get this figured out while we
have an easy way to reproduce and a willing tester.

Thanks,
Jesse
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/4] drm/cache: Try to be smarter about clflushing on x86

[Jesse Barnes]

On 12/14/2014 4:59 AM, Chris Wilson wrote:

One of the things wbinvd is considered evil for is that it blocks the
CPU for an indeterminate amount of time - upsetting latency critcial
aspects of the OS. For example, the x86/mm has similar code to use
wbinvd for large clflushes that caused a bit of controversy with RT:

http://linux-kernel.2935.n7.nabble.com/PATCH-x86-Use-clflush-instead-of-wbinvd-whenever-possible-when-changing-mapping-td493751.html

and also the use of wbinvd in the nvidia driver has also been noted as
evil by RT folks.

However as the wbinvd still exists, it can't be all that bad...


Yeah there are definitely tradeoffs here.  In this particular case, 
we're trying to flush out a ~140M object on every frame, which just 
seems silly.


There's definitely room for optimization in Mesa too; avoiding a mapping 
that marks a large bo as dirty would be good, but if we improve the 
kernel in this area, even sloppy apps and existing binaries will speed up.


Maybe we could apply this only on !llc systems or something?  I wonder 
how much wbinvd performance varies across microarchitectures; maybe 
Thomas's issue isn't really relevant anymore (at least one can hope).


When digging into this, I found that an optimization to remove the IPI 
for wbinvd was clobbered during a merge; maybe that should be 
resurrected too.  Surely a single, global wbinvd is sufficient; we don't 
need to do n_cpus^2 wbinvd + the associated invalidation bus signals here...


Alternately, we could insert some delays into this path just to make it 
extra clear to userspace that they really shouldn't be hitting this in 
the common case (and provide some additional interfaces to let them 
avoid it by allowing flushing and dirty management in userspace).


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

Re: [Intel-gfx] [PATCH] drm/i915/i9xx: check for panel on pipe before asserting panel unlock bits

[Jesse Barnes]

On 11 Dec 2014 00:40:28 -0800
shuang...@intel.com wrote:

> Tested-By: PRC QA PRTS (Patch Regression Test System Contact: 
> shuang...@intel.com)
> -Summary-
> Platform  Delta  drm-intel-nightly  Series Applied
> PNV  364/364  364/364
> ILK  +1-4  364/366  361/366
> SNB  448/450  448/450
> IVB  497/498  497/498
> BYT  289/289  289/289
> HSW  563/564  563/564
> BDW  417/417  417/417
> -Detailed-
> Platform  Testdrm-intel-nightly  
> Series Applied
> *ILK  igt_kms_pipe_crc_basic_bad-pipe  PASS(2, M26)  DMESG_WARN(1, 
> M26)
> *ILK  igt_kms_flip_busy-flip-interruptible  PASS(5, M26)  
> DMESG_WARN(1, M26)
> *ILK  igt_kms_flip_flip-vs-rmfb-interruptible  NSPT(1, M26)PASS(5, M26)   
>DMESG_WARN(1, M26)
>  ILK  igt_kms_flip_plain-flip-ts-check-interruptible  DMESG_WARN(1, 
> M26)PASS(4, M26)  DMESG_WARN(1, M26)
>  ILK  igt_kms_flip_wf_vblank-ts-check  DMESG_WARN(7, M26)PASS(21, M26M37) 
>  PASS(1, M26)
> Note: You need to pay more attention to line start with '*'

Based on this log and the one for saving and restoring the GMbus clock,
it looks like we have some inconsistent results on ILK.  Can you look
into the logs and file bugs against those tests if they're no already
filed?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915/i9xx: check for panel on pipe before asserting panel unlock bits

[Jesse Barnes]

Should address a warning reported in #79824.

References: https://bugs.freedesktop.org/show_bug.cgi?id=79824
Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index c424c36..a19544b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1607,7 +1607,9 @@ static void i9xx_enable_pll(struct intel_crtc *crtc)
BUG_ON(INTEL_INFO(dev)->gen >= 5);
 
/* PLL is protected by panel, make sure we can write it */
-   if (IS_MOBILE(dev) && !IS_I830(dev))
+   if (IS_MOBILE(dev) && !IS_I830(dev) &&
+   (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) ||
+intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP)))
assert_panel_unlocked(dev_priv, crtc->pipe);
 
/* Enable DVO 2x clock on both PLLs if necessary */
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH] drm/i915: save/restore GMBUS freq across suspend/resume on gen4

[Jesse Barnes]

On Wed, 10 Dec 2014 22:35:37 +0200
Ville Syrjälä  wrote:

> On Wed, Dec 10, 2014 at 12:16:05PM -0800, Jesse Barnes wrote:
> > Should probably just init this in the GMbus code all the time,
> > based on the cdclk and HPLL like we do on newer platforms.  Ville
> > has code for that in a rework branch, but until then we can fix
> > this bug fairly easily.
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=76301
> > Signed-off-by: Jesse Barnes 
> 
> My cdclk extraction code doesn't seem to agree with this register for
> this particular bug reporter at least. So I think I need to go double
> check my code. The other options are that GMBUS clock isn't derived
> from cdclk on that platform, or that the HPLL/cdclk bits in configdb
> are simply not valid for this particular chipset.
> 
> In the meantime however, we can at least get some machines working
> with this patch. I'm not entirely sure which platforms have this
> register, but IS_GEN4() looks safe enough since my 946GZ has it and
> the reporter has a G41.
> 
> Reviewed-by: Ville Syrjälä 

Great, thanks.  Jani, can you pick this up?  I'll bounce the original
over to stable@ too.

Thanks,
Jesse
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: save/restore GMBUS freq across suspend/resume on gen4

[Jesse Barnes]

Should probably just init this in the GMbus code all the time, based on
the cdclk and HPLL like we do on newer platforms.  Ville has code for
that in a rework branch, but until then we can fix this bug fairly
easily.

References: https://bugs.freedesktop.org/show_bug.cgi?id=76301
Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/i915_drv.h | 1 +
 drivers/gpu/drm/i915/i915_reg.h | 1 +
 drivers/gpu/drm/i915/i915_suspend.c | 8 
 3 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2725243..f33102b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -925,6 +925,7 @@ struct i915_suspend_saved_registers {
u32 savePIPEB_LINK_N1;
u32 saveMCHBAR_RENDER_STANDBY;
u32 savePCH_PORT_HOTPLUG;
+   u16 saveGCDGMBUS;
 };
 
 struct vlv_s0ix_state {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index dc03fac..f7c2856 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -76,6 +76,7 @@
 #define   I915_GC_RENDER_CLOCK_166_MHZ (0 << 0)
 #define   I915_GC_RENDER_CLOCK_200_MHZ (1 << 0)
 #define   I915_GC_RENDER_CLOCK_333_MHZ (4 << 0)
+#define GCDGMBUS 0xcc
 #define PCI_LBPC 0xf4 /* legacy/combination backlight modes, also called LBB */
 
 
diff --git a/drivers/gpu/drm/i915/i915_suspend.c 
b/drivers/gpu/drm/i915/i915_suspend.c
index dfe6617..2636882 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -303,6 +303,10 @@ int i915_save_state(struct drm_device *dev)
}
}
 
+   if (IS_GEN4(dev))
+   pci_read_config_word(dev->pdev, GCDGMBUS,
+&dev_priv->regfile.saveGCDGMBUS);
+
/* Cache mode state */
if (INTEL_INFO(dev)->gen < 7)
dev_priv->regfile.saveCACHE_MODE_0 = I915_READ(CACHE_MODE_0);
@@ -331,6 +335,10 @@ int i915_restore_state(struct drm_device *dev)
mutex_lock(&dev->struct_mutex);
 
i915_gem_restore_fences(dev);
+
+   if (IS_GEN4(dev))
+   pci_write_config_word(dev->pdev, GCDGMBUS,
+ dev_priv->regfile.saveGCDGMBUS);
i915_restore_display(dev);
 
if (!drm_core_check_feature(dev, DRIVER_MODESET)) {
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915: calculate pfit changes in set_config v3

[Jesse Barnes]

On Tue, 9 Dec 2014 22:16:33 +0100
Daniel Vetter  wrote:

> On Tue, Dec 9, 2014 at 6:51 PM, Jesse Barnes  wrote:
> > On Tue, 11 Nov 2014 12:30:47 -0800
> > Jesse Barnes  wrote:
> >
> >> This should allow us to avoid mode sets for some panel fitter config
> >> changes.
> >>
> >> v2:
> >>   - fixup pfit comment (Ander)
> >> v3:
> >>   - fixup pfit disable shortcut, only apply to gen4 for now (Jesse)
> >>
> >
> > Daniel pointed out offline that in the !fastboot case, this will fail
> > to do the pipe size update if we ever try to skip the mode set when
> > changing the pfit state.
> >
> > To address that we'll need to handle the pfit changes more generally
> > anyway, which gets tricky when we're trying to discard the BIOS state.
> >
> > Ander, if you want to tackle this as part of your multi-pipe stuff,
> > feel free, otherwise I'll get to it after the holidays.
> 
> Quick addition with two more smaller things we've discussed a bit too:
> - We need a dedicated testcase for this special pfit path. Currently
> all igts disable the pipe again before applying changes, so won't
> exercise this. We need new testcase which does direct mode changes
> (i.e. native -> scaled, scaled->native and scaled->scaled) to make
> sure this works well. With atomc we could even make sure it happens in
> just 1 vblank, but current interfaces don't allow a pageflip
> completion event for setCrtc, so that part has to wait.
> 
> - I think the overall decision logic for modeset-or-not should switch
> over to comparing the entire hw state (as stored in
> intel_crtc_config). That way we can drop the fastboot hack which
> copies the adjusted_mode from the pipe config to crtc->mode since we
> only compare the resulting adjusted mode. This should also be more
> robust in handling corner cases like the hdmi infoframes logic that we
> had to take out again.
> 
> Anyway just these two to summarize our offline discussion.

Yeah thanks... more tests as usual are needed.  We just need a way in
igt to see whether a full mode set happened, then we can check against
our assumptions that one should or shouldn't happen for a given config
change.

And yeah the logic needs to be shuffled around a lot more.  I meant to
grab more stuff from compute_mode_changes; right now it's just a set of
special cases, and not very complete.  If we add a new pipe config
compare function, we can just special case the fast paths like you
mentioned, and add a custom pfit path as well (somewhere in between a
simple flip and a full mode set, at least for some platforms).

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: calculate pfit changes in set_config v3

[Jesse Barnes]

On Tue, 11 Nov 2014 12:30:47 -0800
Jesse Barnes  wrote:

> This should allow us to avoid mode sets for some panel fitter config
> changes.
> 
> v2:
>   - fixup pfit comment (Ander)
> v3:
>   - fixup pfit disable shortcut, only apply to gen4 for now (Jesse)
> 

Daniel pointed out offline that in the !fastboot case, this will fail
to do the pipe size update if we ever try to skip the mode set when
changing the pfit state.

To address that we'll need to handle the pfit changes more generally
anyway, which gets tricky when we're trying to discard the BIOS state.

Ander, if you want to tackle this as part of your multi-pipe stuff,
feel free, otherwise I'll get to it after the holidays.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Don't complain about stolen conflicts on gen3

[Jesse Barnes]

On Fri, 11 Apr 2014 15:55:17 +0200
Daniel Vetter  wrote:

> Apparently stuff works that way on those machines.
> 
> I agree with Chris' concern that this is a bit risky but imo worth a
> shot in -next just for fun. Afaics all these machines have the pci
> resources allocated like that by the BIOS, so I suspect that it's all
> ok.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76983
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71031
> Tested-by: lu hua 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/i915/i915_gem_stolen.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
> b/drivers/gpu/drm/i915/i915_gem_stolen.c index
> 62ef55ba061c..99d147af173a 100644 ---
> a/drivers/gpu/drm/i915/i915_gem_stolen.c +++
> b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -93,7 +93,11 @@ static
> unsigned long i915_stolen_to_physical(struct drm_device *dev) r =
> devm_request_mem_region(dev->dev, base + 1, dev_priv->gtt.stolen_size
> - 1, "Graphics Stolen Memory");
> - if (r == NULL) {
> + /*
> +  * GEN3 firmware likes to smash pci bridges into the
> stolen
> +  * range. Apparently this works.
> +  */
> + if (r == NULL && !IS_GEN3(dev)) {
>   DRM_ERROR("conflict detected with stolen
> region: [0x%08x - 0x%08x]\n", base, base +
> (uint32_t)dev_priv->gtt.stolen_size); base = 0;


Yeah just to allay fears: the decode priority on the GMCH is fixed and
specific.  The stolen range is demarcated by some regs which the GMCH
decodes before it tries going out into PCI space.  So it's safe to see
the stolen range under the bus0 window (probably even under some device
window down the range) but does make things messier for us.

Reviewed-by: Jesse Barnes 

Looks like the reporter gave a t-b too.

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

Re: [Intel-gfx] [PATCH] drm/i915: don't always do full mode sets when infoframes are enabled

[Jesse Barnes]

On Mon,  1 Dec 2014 09:54:28 -0800
Jesse Barnes  wrote:

> Partial revert of
> 
> commit 206645910b9796bff13fcdb67bdca166b724ba62
> Author: Jesse Barnes 
> Date:   Wed Nov 5 14:26:09 2014 -0800
> 
> drm/i915: check for audio and infoframe changes across mode sets
> v2
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=86683
> Signed-off-by: Jesse Barnes 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index 42bcbea..fe27a2c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11601,10 +11601,12 @@ static int intel_crtc_set_config(struct
> drm_mode_set *set) to_intel_crtc(set->crtc)->config.has_audio)
>   config->mode_changed = true;
>  
> - /* Force mode sets for any infoframe stuff */
> - if (pipe_config->has_infoframe ||
> - to_intel_crtc(set->crtc)->config.has_infoframe)
> - config->mode_changed = true;
> + /*
> +  * Note we have an issue here with infoframes:
> current code
> +  * only updates them on the full mode set path per hw
> +  * requirements.  So here we should be checking for
> any
> +  * required changes and forcing a mode set.
> +  */
>   }
>  
>   /* set_mode will free it in the mode_changed case */


This got a tested-by from QA on the referenced bug, so I guess we
should pick it up as the revert.

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

Re: [Intel-gfx] [PATCH 2/3] drm/i915: Android sync points for i915 v3

[Jesse Barnes]

On Thu, 4 Dec 2014 12:21:01 +0100
Daniel Vetter  wrote:

> On Wed, Dec 03, 2014 at 11:49:06AM -0800, Jesse Barnes wrote:
> > Expose an ioctl to create Android fences based on the Android sync point
> > infrastructure (which in turn is based on DMA-buf fences).  Just a
> > sketch at this point, no testing has been done.
> > 
> > There are a couple of goals here:
> >   1) allow applications and libraries to create fences without an
> >  associated buffer
> >   2) re-use a common API so userspace doesn't have to impedance mismatch
> >  between different driver implementations too much
> >   3) allow applications and libraries to use explicit synchronization if
> >  they choose by exposing fences directly
> > 
> > v2: use struct fence directly using Maarten's new interface
> > v3: use new i915 request structure (Jesse)
> > make i915 fences a compile time option since Android support is still
> > in staging (Jesse)
> > check for request complete after arming IRQs (Chris)
> > add timeline id to ioctl (Tvrtko)
> > 
> > Signed-off-by: Jesse Barnes 
> 
> Same high-level comment as before:
> 
> I still don't see the point in a separate get_fence ioctl when we want
> both in&out fences for execbuf (and later on atomic flips) for android.
> This separate ioctl here looks incomplete and just means you have to do 2
> ioctls at least to actually get at the fence for an execbuf.

Yeah I'm working on converting over to putting the creation calls into
execbuf instead; that also makes per-ring/context handling easier, and
also the display stuff easier, since we won't have to worry about
enumerating crtcs and planes as separate timelines.

> And android sync stuff still needs to be de-staged. On that: The function
> names to wrap a struct fence into a syncpt file and then insert the file
> into an fd slot need a bit of name bikeshedding since it's not clear that
> it's just a userspace interface wrapper now.

I don't think we'll be able to de-stage without a user, but I can add a
patch to do that as part of the series.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/3] android: add sync_fence_create_dma

[Jesse Barnes]

From: Maarten Lankhorst 

This allows users of dma fences to create a android fence.

Cc: Daniel Vetter 
Cc: Jesse Barnes 
Signed-off-by: Maarten Lankhorst 
---
 drivers/staging/android/sync.c | 13 +
 drivers/staging/android/sync.h |  3 ++-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 7bdb62b..0ee333e 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -188,7 +188,7 @@ static void fence_check_cb_func(struct fence *f, struct 
fence_cb *cb)
 }
 
 /* TODO: implement a create which takes more that one sync_pt */
-struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt)
+struct sync_fence *sync_fence_create_dma(const char *name, struct fence *pt)
 {
struct sync_fence *fence;
 
@@ -199,16 +199,21 @@ struct sync_fence *sync_fence_create(const char *name, 
struct sync_pt *pt)
fence->num_fences = 1;
atomic_set(&fence->status, 1);
 
-   fence->cbs[0].sync_pt = &pt->base;
+   fence->cbs[0].sync_pt = pt;
fence->cbs[0].fence = fence;
-   if (fence_add_callback(&pt->base, &fence->cbs[0].cb,
-  fence_check_cb_func))
+   if (fence_add_callback(pt, &fence->cbs[0].cb, fence_check_cb_func))
atomic_dec(&fence->status);
 
sync_fence_debug_add(fence);
 
return fence;
 }
+EXPORT_SYMBOL(sync_fence_create_dma);
+
+struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt)
+{
+   return sync_fence_create_dma(name, &pt->base);
+}
 EXPORT_SYMBOL(sync_fence_create);
 
 struct sync_fence *sync_fence_fdget(int fd)
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
index a21b79f..50052e4 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -250,8 +250,9 @@ void sync_pt_free(struct sync_pt *pt);
  * @pt:sync_pt to add to the fence
  *
  * Creates a fence containg @pt.  Once this is called, the fence takes
- * ownership of @pt.
+ * a reference on @pt.
  */
+struct sync_fence *sync_fence_create_dma(const char *name, struct fence *pt);
 struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt);
 
 /*
-- 
1.9.1

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

[Intel-gfx] [RFC] Updated Android sync patches

[Jesse Barnes]

Still have a few remaining todo items, but I'd like some feedback on these
before adding a bunch of stuff on top.

Rough todo list:
  - support for other rings
  - support for display
  - tests
  - userspace usage (I have a Mesa patch that needs a refresh, but it's
trivial, the interesting bit will be using this in Wayland and
X/DDX)

Thanks,
Jesse

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

[Intel-gfx] [PATCH 2/3] drm/i915: Android sync points for i915 v3

[Jesse Barnes]

Expose an ioctl to create Android fences based on the Android sync point
infrastructure (which in turn is based on DMA-buf fences).  Just a
sketch at this point, no testing has been done.

There are a couple of goals here:
  1) allow applications and libraries to create fences without an
 associated buffer
  2) re-use a common API so userspace doesn't have to impedance mismatch
 between different driver implementations too much
  3) allow applications and libraries to use explicit synchronization if
 they choose by exposing fences directly

v2: use struct fence directly using Maarten's new interface
v3: use new i915 request structure (Jesse)
make i915 fences a compile time option since Android support is still
in staging (Jesse)
check for request complete after arming IRQs (Chris)
add timeline id to ioctl (Tvrtko)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/Kconfig |  14 ++
 drivers/gpu/drm/i915/Makefile|   1 +
 drivers/gpu/drm/i915/i915_dma.c  |   5 +
 drivers/gpu/drm/i915/i915_drv.h  |  20 +++
 drivers/gpu/drm/i915/i915_gem.c  |   5 +
 drivers/gpu/drm/i915/i915_irq.c  |   4 +-
 drivers/gpu/drm/i915/i915_sync.c | 325 +++
 include/uapi/drm/i915_drm.h  |  23 +++
 8 files changed, 396 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/i915_sync.c

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 4e39ab3..6b23d17 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -43,6 +43,20 @@ config DRM_I915_KMS
 
  If in doubt, say "Y".
 
+config DRM_I915_SYNC
+   bool "Enable explicit sync support"
+   depends on DRM_I915
+   default y
+   select STAGING
+   select ANDROID
+   select SYNC
+   help
+ Choose this option to enable Android native sync support and the
+ corresponding i915 driver code to expose it.  Slightly increases
+ driver size and pulls in sync support from staging.
+
+ If in doubt, say "Y".
+
 config DRM_I915_FBDEV
bool "Enable legacy fbdev support for the modesetting intel driver"
depends on DRM_I915
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index e4083e4..45e155f 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -56,6 +56,7 @@ i915-y += intel_audio.o \
  intel_sprite.o
 i915-$(CONFIG_ACPI)+= intel_acpi.o intel_opregion.o
 i915-$(CONFIG_DRM_I915_FBDEV)  += intel_fbdev.o
+i915-$(CONFIG_DRM_I915_SYNC)   += i915_sync.o
 
 # modesetting output/encoder code
 i915-y += dvo_ch7017.o \
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 887d88f..621bd7f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1062,6 +1062,11 @@ const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, 
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_get_reset_stats_ioctl, 
DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, 
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+#ifdef CONFIG_DRM_I915_SYNC
+   DRM_IOCTL_DEF_DRV(I915_GEM_FENCE, i915_sync_create_fence_ioctl, 
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+#else
+   DRM_IOCTL_DEF_DRV(I915_GEM_FENCE, drm_noop, 
DRM_UNLOCKED|DRM_RENDER_ALLOW),
+#endif
 };
 
 int i915_max_ioctl = ARRAY_SIZE(i915_ioctls);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 049482f..367d95a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1525,6 +1525,9 @@ struct i915_workarounds {
u32 count;
 };
 
+struct i915_sync_timeline;
+
+
 struct drm_i915_private {
struct drm_device *dev;
struct kmem_cache *slab;
@@ -1560,6 +1563,9 @@ struct drm_i915_private {
uint32_t last_seqno, next_seqno;
 
struct drm_dma_handle *status_page_dmah;
+
+   struct i915_sync_timeline *sync_tl[I915_NUM_RINGS];
+
struct resource mch_res;
 
/* protects the irq masks */
@@ -2509,6 +2515,20 @@ void i915_init_vm(struct drm_i915_private *dev_priv,
 void i915_gem_free_object(struct drm_gem_object *obj);
 void i915_gem_vma_destroy(struct i915_vma *vma);
 
+/* i915_sync.c */
+#ifdef CONFIG_DRM_I915_SYNC
+int i915_sync_init(struct drm_i915_private *dev_priv);
+void i915_sync_fini(struct drm_i915_private *dev_priv);
+int i915_sync_create_fence_ioctl(struct drm_device *dev, void *data,
+struct drm_file *file);
+#else
+static inline int i915_sync_init(struct drm_i915_private *dev_priv)
+{
+   return 0;
+}
+static inline void i915_sync_fini(struct drm_i915_private *dev_priv) { }
+#endif
+
 #define PIN_MAPPABLE 0x1
 #define PIN_NONBLOCK 0x2
 #define PIN_GLOBAL 0x4
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/driv

[Intel-gfx] [PATCH 3/3] drm/i915: add fences to the request struct

[Jesse Barnes]

This simplifies the sync code quite a bit.  I don't think we'll be able
to get away with using the core fence code's seqno support, since we'll
be moving away from simple seqno comparisions with the scheduler and
preemption, but the additional code is pretty minimal anyway, and lets
us add additional debugging as needed, so it's probably fine to keep
either way.

We still need to add support for other rings here; we ought to be able
to do that with the timeline field of the ioctl (which will include
other "rings" like the display flip queue for example).

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/i915_drv.h  |  6 +++
 drivers/gpu/drm/i915/i915_sync.c | 89 +++-
 2 files changed, 40 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 367d95a..2725243 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -49,6 +49,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* General customization:
  */
@@ -2025,6 +2026,11 @@ struct drm_i915_gem_request {
struct drm_i915_file_private *file_priv;
/** file_priv list entry for this request */
struct list_head client_list;
+
+   /* core fence obj for this request, may be exported */
+   struct fence fence;
+
+   wait_queue_t wait;
 };
 
 void i915_gem_request_free(struct kref *req_ref);
diff --git a/drivers/gpu/drm/i915/i915_sync.c b/drivers/gpu/drm/i915/i915_sync.c
index 4741b0c..ea54b34 100644
--- a/drivers/gpu/drm/i915/i915_sync.c
+++ b/drivers/gpu/drm/i915/i915_sync.c
@@ -50,25 +50,10 @@ static spinlock_t fence_lock;
  * with other Android timelines and aggregated into sync_fences, etc.
  *
  * TODO:
- *   rebase on top of Chris's seqno/request stuff and use requests
  *   allow non-RCS fences (need ring/context association)
  */
 
-struct i915_fence {
-   struct fence base;
-   struct intel_engine_cs *ring;
-   struct intel_context *ctx;
-   wait_queue_t wait;
-   struct drm_i915_gem_request *request;
-};
-
-#define to_intel_fence(x) container_of(x, struct i915_fence, base)
-
-int __wait_seqno(struct intel_engine_cs *ring, u32 seqno,
-unsigned reset_counter,
-bool interruptible,
-struct timespec *timeout,
-struct drm_i915_file_private *file_priv);
+#define to_i915_request(x) container_of(x, struct drm_i915_gem_request, fence)
 
 static const char *i915_fence_get_driver_name(struct fence *fence)
 {
@@ -77,24 +62,24 @@ static const char *i915_fence_get_driver_name(struct fence 
*fence)
 
 static const char *i915_fence_get_timeline_name(struct fence *fence)
 {
-   struct i915_fence *intel_fence = to_intel_fence(fence);
+   struct drm_i915_gem_request *req = to_i915_request(fence);
 
-   return intel_fence->ring->name;
+   return req->ring->name;
 }
 
 static int i915_fence_check(wait_queue_t *wait, unsigned mode, int flags,
void *key)
 {
-   struct i915_fence *intel_fence = wait->private;
-   struct intel_engine_cs *ring = intel_fence->ring;
+   struct drm_i915_gem_request *req = wait->private;
+   struct intel_engine_cs *ring = req->ring;
 
-   if (!i915_gem_request_completed(intel_fence->request, false))
+   if (!i915_gem_request_completed(req, false))
return 0;
 
-   fence_signal_locked(&intel_fence->base);
+   fence_signal_locked(&req->fence);
 
__remove_wait_queue(&ring->irq_queue, wait);
-   fence_put(&intel_fence->base);
+   fence_put(&req->fence);
ring->irq_put(ring);
 
return 0;
@@ -102,26 +87,26 @@ static int i915_fence_check(wait_queue_t *wait, unsigned 
mode, int flags,
 
 static bool i915_fence_enable_signaling(struct fence *fence)
 {
-   struct i915_fence *intel_fence = to_intel_fence(fence);
-   struct intel_engine_cs *ring = intel_fence->ring;
+   struct drm_i915_gem_request *req = to_i915_request(fence);
+   struct intel_engine_cs *ring = req->ring;
struct drm_i915_private *dev_priv = ring->dev->dev_private;
-   wait_queue_t *wait = &intel_fence->wait;
+   wait_queue_t *wait = &req->wait;
 
/* queue fence wait queue on irq queue and get fence */
-   if (i915_gem_request_completed(intel_fence->request, false) ||
+   if (i915_gem_request_completed(req, false) ||
i915_terminally_wedged(&dev_priv->gpu_error))
return false;
 
if (!ring->irq_get(ring))
return false;
 
-   if (i915_gem_request_completed(intel_fence->request, false)) {
+   if (i915_gem_request_completed(req, false)) {
ring->irq_put(ring);
return true;
}
 
wait->flags = 0;
-   wait->private = intel

[Intel-gfx] [PATCH] drm/i915: don't always do full mode sets when infoframes are enabled

[Jesse Barnes]

Partial revert of

commit 206645910b9796bff13fcdb67bdca166b724ba62
Author: Jesse Barnes 
Date:   Wed Nov 5 14:26:09 2014 -0800

drm/i915: check for audio and infoframe changes across mode sets v2

References: https://bugs.freedesktop.org/show_bug.cgi?id=86683
Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 42bcbea..fe27a2c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11601,10 +11601,12 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
to_intel_crtc(set->crtc)->config.has_audio)
config->mode_changed = true;
 
-   /* Force mode sets for any infoframe stuff */
-   if (pipe_config->has_infoframe ||
-   to_intel_crtc(set->crtc)->config.has_infoframe)
-   config->mode_changed = true;
+   /*
+* Note we have an issue here with infoframes: current code
+* only updates them on the full mode set path per hw
+* requirements.  So here we should be checking for any
+* required changes and forcing a mode set.
+*/
}
 
/* set_mode will free it in the mode_changed case */
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Jesse Barnes]

On Mon, 1 Dec 2014 17:16:25 +0100
Daniel Vetter  wrote:

> On Mon, Dec 01, 2014 at 12:25:45PM +0200, Jani Nikula wrote:
> > On Thu, 06 Nov 2014, Jesse Barnes  wrote:
> > > If these change (e.g. after a modeset following a fastboot), we need to
> > > do a full mode set.
> > >
> > > v2:
> > >   - put under pipe_config check so we don't deref a null state (Jesse)
> > >
> > > Signed-off-by: Jesse Barnes 
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 12 +++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index cb96f11..c86eee6 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct 
> > > drm_mode_set *set)
> > >  &modeset_pipes,
> > >  &prepare_pipes,
> > >  &disable_pipes);
> > > - if (IS_ERR(pipe_config))
> > > + if (IS_ERR(pipe_config)) {
> > >   goto fail;
> > > + } else if (pipe_config) {
> > > + if (to_intel_crtc(set->crtc)->new_config->has_audio !=
> > > + to_intel_crtc(set->crtc)->config.has_audio)
> > > + config->mode_changed = true;
> > > +
> > > + /* Force mode sets for any infoframe stuff */
> > > + if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
> > > + to_intel_crtc(set->crtc)->config.has_infoframe)
> > 
> > Jesse, is "||" correct here? This is bound to force a lot of mode sets.
> > 
> > See https://bugs.freedesktop.org/show_bug.cgi?id=86683
> 
> Indeed. And I think the approach is the wrong way round. Imo we should
> check whether the computed pipe_config matches and have a few specific
> cases that we can handle when it mismatches (e.g. pipe_src_w/h). Plus some
> special logic if quirk flags are set (so that we force a full modeset in
> cases like this to get rid of the boot-up takever state quirk flag).
> 
> I think best course of action here is to revert this patch, also because
> it's kinda at the 1w deadline with set for regression. Even though this
> here is imo QA's fault for taking way too long to deliver the bisect
> result and noticing the issue (the patch is almost 1 month old by now).
> 
> Jesse?

I'd rather just drop the infoframe check if that's causing trouble.
The audio one should probably still stay.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Jesse Barnes]

On Mon, 01 Dec 2014 12:25:45 +0200
Jani Nikula  wrote:

> On Thu, 06 Nov 2014, Jesse Barnes  wrote:
> > If these change (e.g. after a modeset following a fastboot), we need to
> > do a full mode set.
> >
> > v2:
> >   - put under pipe_config check so we don't deref a null state (Jesse)
> >
> > Signed-off-by: Jesse Barnes 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 12 +++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index cb96f11..c86eee6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct 
> > drm_mode_set *set)
> >&modeset_pipes,
> >&prepare_pipes,
> >&disable_pipes);
> > -   if (IS_ERR(pipe_config))
> > +   if (IS_ERR(pipe_config)) {
> > goto fail;
> > +   } else if (pipe_config) {
> > +   if (to_intel_crtc(set->crtc)->new_config->has_audio !=
> > +   to_intel_crtc(set->crtc)->config.has_audio)
> > +   config->mode_changed = true;
> > +
> > +   /* Force mode sets for any infoframe stuff */
> > +   if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
> > +   to_intel_crtc(set->crtc)->config.has_infoframe)
> 
> Jesse, is "||" correct here? This is bound to force a lot of mode sets.
> 
> See https://bugs.freedesktop.org/show_bug.cgi?id=86683

Yeah, we talked about it on IRC.  I was being extra conservative here,
since we don't yet have code to analyze whether we need a full mode set
due to something that will require an infoframe change.  Currently, the
only way for us to write a new infoframe is through a mode set, and the
old code would just happen to do a mode set most of the time we wanted
it to (i.e. when the mode timings changed), but I don't think we'd ever
thought of the implications of infoframe changes for stuff that doesn't
require a mode set.

We could drop this hunk and continue to play it fast & loose, but what
we really need here is a fuller check about whether we need a mode set
due to requiring an infoframe change, which means pulling them apart a
bit.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Don't clobber crtc->new_config when nothing changes

[Jesse Barnes]

On Fri, 21 Nov 2014 21:00:36 +0200
ville.syrj...@linux.intel.com wrote:

> From: Ville Syrjälä 
> 
> When doing a nop modeset we currently leave crtc->new_config point at
> the already freed temporary pipe_config. That will anger the sanity
> checks in intel_modeset_update_state() when the nop modeset gets
> followed by a GPU reset on gen3/4 where the display block gets fully
> reinitialized during the reset.
> 
> So leave crtc->new_config alone until we know a modeset is actually
> required.
> 
> Cc: Jesse Barnes 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 853697f..3218455 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10907,7 +10907,6 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
>   }
>   intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
>  "[modeset]");
> - to_intel_crtc(crtc)->new_config = pipe_config;
>  
>  out:
>   return pipe_config;
> @@ -10933,6 +10932,9 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>  
>   *saved_mode = crtc->mode;
>  
> + if (modeset_pipes)
> + to_intel_crtc(crtc)->new_config = pipe_config;
> +
>   /*
>* See if the config requires any additional preparation, e.g.
>* to adjust global state with pipes off.  We need to do this
> @@ -11466,12 +11468,12 @@ static int intel_crtc_set_config(struct 
> drm_mode_set *set)
>   ret = PTR_ERR(pipe_config);
>   goto fail;
>   } else if (pipe_config) {
> - if (to_intel_crtc(set->crtc)->new_config->has_audio !=
> + if (pipe_config->has_audio !=
>   to_intel_crtc(set->crtc)->config.has_audio)
>   config->mode_changed = true;
>  
>   /* Force mode sets for any infoframe stuff */
> - if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
> +     if (pipe_config->has_infoframe ||
>   to_intel_crtc(set->crtc)->config.has_infoframe)
>   config->mode_changed = true;
>   }

Yeah looks fine.

Reviewed-by: Jesse Barnes 

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915/ibx: check port in infoframe_enabled

[Jesse Barnes]

On Thu, 20 Nov 2014 22:57:32 +0100
Daniel Vetter  wrote:

> On Thu, Nov 20, 2014 at 01:24:14PM -0800, Jesse Barnes wrote:
> > Just like on g4x we need to check the port enable bit here.
> > 
> > Signed-off-by: Jesse Barnes 
> > ---
> >  drivers/gpu/drm/i915/intel_hdmi.c | 7 ++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > b/drivers/gpu/drm/i915/intel_hdmi.c index ec87333..cc48b51 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -222,10 +222,15 @@ static bool ibx_infoframe_enabled(struct
> > drm_encoder *encoder) struct drm_device *dev = encoder->dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct intel_crtc *intel_crtc =
> > to_intel_crtc(encoder->crtc);
> > +   struct intel_digital_port *intel_dig_port =
> > enc_to_dig_port(encoder); int reg =
> > TVIDEO_DIP_CTL(intel_crtc->pipe); u32 val = I915_READ(reg);
> > +   u32 port = VIDEO_DIP_PORT(intel_dig_port->port);
> >  
> > -   return val & VIDEO_DIP_ENABLE;
> > +   if (port == (val & VIDEO_DIP_PORT_MASK))
> > +   return val & VIDEO_DIP_ENABLE;
> 
> I think the only thing that can go wrong is us or the bios making a
> complete mess out of the pipe->port assignment for infoframes here. So
> I've thought more of
> 
> if (val & VIDEO_DIP_ENABLE) {
>   WARN_ON(port != (val & VIDEO_DIP_PORT_MASK));
>   return true;
> }
> 
> return false;
> 
> I.e. just double-checking that everything is as expected. Otherwise
> we'd need to collect infoframes from all pipes I think. But since we
> don't have any code to clean up such a mess I hope it doesn't exist
> in the real world out there.

So in that case, we'll set the has_infoframes to true even if
infoframes are enabled on the wrong port, which means we might skip a
mode set later which would clean them up and set the right port...

I like the idea of the WARN though.

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

[Intel-gfx] [PATCH] drm/i915/vlv: check port in infoframe_enabled v2

[Jesse Barnes]

Same as IBX and G4x, they all share the same genetic material.

v2: we all need a bit more port in our lives

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_hdmi.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index cc48b51..43f17ff 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -328,10 +328,15 @@ static bool vlv_infoframe_enabled(struct drm_encoder 
*encoder)
struct drm_device *dev = encoder->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
int reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
u32 val = I915_READ(reg);
+   u32 port = intel_dig_port->port;
 
-   return val & VIDEO_DIP_ENABLE;
+   if (port == (val & VIDEO_DIP_PORT_MASK))
+   return val & VIDEO_DIP_ENABLE;
+
+   return false;
 }
 
 static void hsw_write_infoframe(struct drm_encoder *encoder,
-- 
1.9.1

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

[Intel-gfx] [PATCH] drm/i915/vlv: check port in infoframe_enabled

[Jesse Barnes]

Same as IBX and G4x, they all share the same genetic material.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_hdmi.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index cc48b51..36e8c53 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -328,10 +328,14 @@ static bool vlv_infoframe_enabled(struct drm_encoder 
*encoder)
struct drm_device *dev = encoder->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
int reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
u32 val = I915_READ(reg);
 
-   return val & VIDEO_DIP_ENABLE;
+   if (port == (val & VIDEO_DIP_PORT_MASK))
+   return val & VIDEO_DIP_ENABLE;
+
+   return false;
 }
 
 static void hsw_write_infoframe(struct drm_encoder *encoder,
-- 
1.9.1

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

[Intel-gfx] [PATCH 2/2] drm/i915/ibx: check port in infoframe_enabled

[Jesse Barnes]

Just like on g4x we need to check the port enable bit here.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_hdmi.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index ec87333..cc48b51 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -222,10 +222,15 @@ static bool ibx_infoframe_enabled(struct drm_encoder 
*encoder)
struct drm_device *dev = encoder->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
u32 val = I915_READ(reg);
+   u32 port = VIDEO_DIP_PORT(intel_dig_port->port);
 
-   return val & VIDEO_DIP_ENABLE;
+   if (port == (val & VIDEO_DIP_PORT_MASK))
+   return val & VIDEO_DIP_ENABLE;
+
+   return false;
 }
 
 static void cpt_write_infoframe(struct drm_encoder *encoder,
-- 
1.9.1

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

[Intel-gfx] [PATCH 1/2] drm/i915/g4x: fix g4x infoframe readout

[Jesse Barnes]

Need to check the port too.

Reported-by: Daniel Vetter 
Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_hdmi.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index f58e883..ec87333 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -170,9 +170,13 @@ static bool g4x_infoframe_enabled(struct drm_encoder 
*encoder)
 {
struct drm_device *dev = encoder->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
u32 val = I915_READ(VIDEO_DIP_CTL);
 
-   return val & VIDEO_DIP_ENABLE;
+   if (VIDEO_DIP_PORT(intel_dig_port->port) == (val & VIDEO_DIP_PORT_MASK))
+   return val & VIDEO_DIP_ENABLE;
+
+   return false;
 }
 
 static void ibx_write_infoframe(struct drm_encoder *encoder,
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2

[Jesse Barnes]

On Thu, 20 Nov 2014 16:49:42 +0100
Daniel Vetter  wrote:

> On Tue, Nov 18, 2014 at 09:45:52AM -0800, Jesse Barnes wrote:
> > Just like we do in the HDMI code, set the infoframe flag if we detect
> > that infoframes are enabled.
> > 
> > v2: check for actual infoframe status as in hdmi code (Daniel)
> > 
> > Signed-off-by: Jesse Barnes 
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 8 
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 07c5625..24110c9 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2075,6 +2075,14 @@ void intel_ddi_get_config(struct intel_encoder 
> > *encoder,
> > break;
> > }
> >  
> > +   if (encoder->type == INTEL_OUTPUT_HDMI) {
> 
> Hm I dind't look too closely apparently at this. You again rely upon sw
> state here, just encoder->type this time around. Which means you can't
> upcast the intel_hdmi struct, and you also can't really rely upon the
> encoder->crtc link (that's all just about to get reconstructed). Imo the
> code should have stayed in the TRANS_DDI_MODE_SELECT_HDMI case.

Hm, we look at the encoder->type later and check for eDP, so I figured
it must be valid at this point.  It's easy enough to move though if
not...

> The later depency upon encoder->crtc is an issue for everything !g4x, but
> on hsw there's the additional issue that you have to look at the cpu
> transcoder and I guess that part blows up.

I'll check out Paulo's trace; haven't looked yet.

> g4x infoframe readout is probably broken too because it doesn't check that
> the port selected is the one actually queried for.

Yeah that looks like a separate bug from the infoframe_enabled
additions.

> 
> Overall I think we need to:
> - Inline the g4x readout into the hdmi get_config function and check the
>   port.
> - Inline the ibx/cpt readout code into the relevant get_pipe_config
>   functions (well pch config) since that state is per-pipe. We should
>   probably double-check the port, too.
> - Same inline for vlv and hsw, with the addition that we need to make sure
>   on hsw to not try to read this for the edp transcoder.

I'll check, we may not need to inline since we should be able to get
the port info easily enough.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/5] drm/i915: remove intel_pipe_set_base() (v4)

[Jesse Barnes]

   INTEL_FRONTBUFFER_PRIMARY(pipe));
> - mutex_unlock(&dev->struct_mutex);
> - crtc->primary->fb = NULL;
> - }
> + crtc->primary->funcs->disable_plane(crtc->primary);
>  
>   /* Update computed state. */
>   list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> @@ -9578,6 +9504,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>   struct drm_framebuffer *old_fb = crtc->primary->fb;
>   struct drm_i915_gem_object *obj = intel_fb_obj(fb);
>   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct drm_plane *primary = crtc->primary;
> + struct intel_plane *intel_plane = to_intel_plane(primary);
>   enum pipe pipe = intel_crtc->pipe;
>   struct intel_unpin_work *work;
>   struct intel_engine_cs *ring;
> @@ -9736,8 +9664,15 @@ free_work:
>  
>   if (ret == -EIO) {
>  out_hang:
> - intel_crtc_wait_for_pending_flips(crtc);
> - ret = intel_pipe_set_base(crtc, crtc->x, crtc->y, fb);
> + ret = primary->funcs->update_plane(primary, crtc, fb,
> +intel_plane->crtc_x,
> +intel_plane->crtc_y,
> +intel_plane->crtc_h,
> +intel_plane->crtc_w,
> +intel_plane->src_x,
> +intel_plane->src_y,
> +intel_plane->src_h,
> +intel_plane->src_w);
>   if (ret == 0 && event) {
>   spin_lock_irq(&dev->event_lock);
>   drm_send_vblank_event(dev, pipe, event);
> @@ -10913,26 +10848,15 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>* on the DPLL.
>*/
>   for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
> - struct drm_framebuffer *old_fb = crtc->primary->fb;
> - struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
> - struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> + struct drm_plane *primary = intel_crtc->base.primary;
> + int vdisplay, hdisplay;
>  
> - mutex_lock(&dev->struct_mutex);
> - ret = intel_pin_and_fence_fb_obj(crtc->primary, fb, NULL);
> - if (ret != 0) {
> - DRM_ERROR("pin & fence failed\n");
> - mutex_unlock(&dev->struct_mutex);
> - goto done;
> - }
> - if (old_fb)
> - intel_unpin_fb_obj(old_obj);
> - i915_gem_track_fb(old_obj, obj,
> -   INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe));
> - mutex_unlock(&dev->struct_mutex);
> -
> - crtc->primary->fb = fb;
> - crtc->x = x;
> - crtc->y = y;
> + drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
> + ret = primary->funcs->update_plane(primary, &intel_crtc->base,
> +fb, 0, 0,
> +hdisplay, vdisplay,
> +x << 16, y << 16,
> +hdisplay << 16, vdisplay << 
> 16);
>   }
>  
>   /* Now enable the clocks, plane, pipe, and connectors that we set up. */
> @@ -11398,11 +11322,14 @@ static int intel_crtc_set_config(struct 
> drm_mode_set *set)
>          disable_pipes);
>   } else if (config->fb_changed) {
>   struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
> -
> - intel_crtc_wait_for_pending_flips(set->crtc);
> -
> - ret = intel_pipe_set_base(set->crtc,
> -   set->x, set->y, set->fb);
> + struct drm_plane *primary = set->crtc->primary;
> + int vdisplay, hdisplay;
> +
> + drm_crtc_get_hv_timing(set->mode, &hdisplay, &vdisplay);
> + ret = primary->funcs->update_plane(primary, set->crtc, set->fb,
> +0, 0, hdisplay, vdisplay,
> +set->x << 16, set->y << 16,
> +hdisplay << 16, vdisplay << 
> 16);
>  
>   /*
>* We need to make sure the primary plane is re-enabled if it

Acked-and-mourned-by: Jesse Barnes 

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: add turbo boost trace point

[Jesse Barnes]

On Wed, 19 Nov 2014 17:19:23 +0100
Daniel Vetter  wrote:

> On Wed, Nov 19, 2014 at 07:39:17AM -0800, Jesse Barnes wrote:
> > On Wed, 19 Nov 2014 15:00:04 +0100
> > Daniel Vetter  wrote:
> > 
> > > On Tue, Nov 18, 2014 at 01:12:29PM -0800, Jesse Barnes wrote:
> > > > Might be helpful for debugging places where userspace ends up boosting
> > > > or waiting where it doesn't intend to.
> > > 
> > > Might be feels a bit weak justification for a new tracepoint imo. Please
> > > drum up more support.
> > 
> > I put it together for some media playback debugging we're doing on
> > Chrome, where I suspect we're boosting when we don't want to (possibly
> > due to userspace waits).
> 
> Hm, I've discussed this exact topic many moons ago with vpg folks and
> they've said that the boosting done for media workloads on the idle->busy
> transition annoys them. Iirc the plan we've hashed out was to add an
> execbuf flag to prevent the execbuf boosting.
> 
> We've also discussed the wait boosting but that's imo just a bug in libva
> ;-) And for debugging pointless waits we already have good tracepoints
> imo.

We have tracing for waits, but my expectation was that we may end up
growing or adding new boost points elsewhere, so having an explicit
trace would make sense anyway.

But we've already typed many more lines about this than the patch
itself contains, so feel free to drop/ignore.  It's easy enough to add
on an ad-hoc basis.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: add turbo boost trace point

[Jesse Barnes]

On Wed, 19 Nov 2014 15:00:04 +0100
Daniel Vetter  wrote:

> On Tue, Nov 18, 2014 at 01:12:29PM -0800, Jesse Barnes wrote:
> > Might be helpful for debugging places where userspace ends up boosting
> > or waiting where it doesn't intend to.
> 
> Might be feels a bit weak justification for a new tracepoint imo. Please
> drum up more support.

I put it together for some media playback debugging we're doing on
Chrome, where I suspect we're boosting when we don't want to (possibly
due to userspace waits).

> 
> > Signed-off-by: Jesse Barnes 
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c   |  6 --
> >  drivers/gpu/drm/i915/i915_trace.h | 15 +++
> >  drivers/gpu/drm/i915/intel_pm.c   |  9 +++--
> >  3 files changed, 26 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c 
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 86cf428..b03cb07 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4209,8 +4209,10 @@ i915_gem_object_ggtt_unpin(struct 
> > drm_i915_gem_object *obj)
> > struct i915_vma *vma = i915_gem_obj_to_ggtt(obj);
> >  
> > BUG_ON(!vma);
> > -   BUG_ON(vma->pin_count == 0);
> > -   BUG_ON(!i915_gem_obj_ggtt_bound(obj));
> > +   if (WARN(vma->pin_count == 0, "bad pin count\n"))
> > +   return;
> > +   if (WARN(!i915_gem_obj_ggtt_bound(obj), "obj not bound\n"))
> > +   return;
> 
> Separate patch. Can you please split it out with the usual "BUG_ON
> considered harmful" commit message?

Oops forgot about that hunk.  Yeah I was debugging a mode setting
failure and ran into this BUG, which made things a little tougher when
I only had one machine...  But this hunk can be ignored.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: add turbo boost trace point

[Jesse Barnes]

On 18 Nov 2014 19:15:18 -0800
shuang...@intel.com wrote:

> Tested-By: PRC QA PRTS (Patch Regression Test System Contact: 
> shuang...@intel.com)
> -Summary-
> Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
> BYT: pass/total=290/290->290/290
> PNV: pass/total=362/362->362/362
> ILK: pass/total=381/381->379/381
> IVB: pass/total=522/559->522/559
> SNB: pass/total=444/444->444/444
> HSW: pass/total=595/595->595/595
> BDW: pass/total=436/436->436/436
> -Detailed-
> test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, 
> machine_id...)...->result_with_patch_applied(count, machine_id)...
> ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-wf_vblank-interruptible, 
> DMESG_WARN(2, M26)PASS(2, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)
> ILK: Intel_gpu_tools, igt_kms_flip_single-buffer-flip-vs-dpms-off-vs-modeset, 
> DMESG_WARN(1, M26)PASS(3, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26)

Looks like these two tests are flakey; this patch shouldn't affect
those cases.  Do we have bugs open for these subtests already?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: expose a fixed brightness range to userspace

[Jesse Barnes]

We can still map 1000 entries back to 256, it will just mean some of
them won't result in any brightness change.

If that's not acceptable, I guess a dynamic 1:1 mapping is sufficient.

Jesse

On Tue, 18 Nov 2014 23:18:35 +
"Eoff, Ullysses A"  wrote:

> Thanks Jesse for the ack.
> 
> Unfortunately I just learned from Stéphane that there
> are certain devices which only support 256 levels, so this
> patch would do us no good at solving the real issue for
> such devices.
> 
> Why can't we just use a dynamic 1:1 mapping as was
> suggested before?  I would vote for that instead.
> 
> ----
> U. Artie 
> 
> > -Original Message-
> > From: Jesse Barnes [mailto:jbar...@virtuousgeek.org]
> > Sent: Tuesday, November 18, 2014 9:23 AM
> > To: Eoff, Ullysses A
> > Cc: intel-gfx@lists.freedesktop.org; Jani Nikula
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915: expose a fixed brightness range 
> > to userspace
> > 
> > On Tue, 11 Nov 2014 12:30:38 -0800
> > "U. Artie Eoff"  wrote:
> > 
> > > A userspace brightness range that is larger than the hardware
> > > brightness range does not have a 1:1 mapping and can result
> > > in brightness != actual_brightness for some values.
> > >
> > > Expose a fixed brightness range of [0..1000] to userspace so
> > > that all values can map 1:1 into the hardware brightness
> > > range.  This would assume that the hardware range is always
> > > greater than 1000, otherwise we're right back to the original
> > > issue.
> > >
> > > This patch is based on Jani Nikula's proposed change in the
> > > referenced ML thread, except use the range [0..1000] instead;
> > > which was recommended by Jesse Barnes for smoother backlight
> > > transitions.
> > >
> > > Reference: 
> > > http://lists.freedesktop.org/archives/intel-gfx/2014-November/055221.html
> > > Signed-off-by: U. Artie Eoff 
> > > ---
> > >  drivers/gpu/drm/i915/intel_panel.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_panel.c 
> > > b/drivers/gpu/drm/i915/intel_panel.c
> > > index 4d63839..f74f5f2 100644
> > > --- a/drivers/gpu/drm/i915/intel_panel.c
> > > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > > @@ -1057,7 +1057,7 @@ static int intel_backlight_device_register(struct 
> > > intel_connector *connector)
> > >* Note: Everything should work even if the backlight device max
> > >* presented to the userspace is arbitrarily chosen.
> > >*/
> > > - props.max_brightness = panel->backlight.max;
> > > + props.max_brightness = 1000;
> > >   props.brightness = scale_hw_to_user(connector,
> > >   panel->backlight.level,
> > >   props.max_brightness);
> > 
> > Yeah looks ok to me.  I guess Jani has to ack it too.
> > 
> > Reviewed-by: Jesse Barnes 
> > 
> > --
> > Jesse Barnes, Intel Open Source Technology Center
> 


-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/5] drm/i915: remove intel_pipe_set_base() (v3)

[Jesse Barnes]

On Mon, 17 Nov 2014 18:10:39 -0800
Matt Roper  wrote:

> -static int
> -intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
> - struct drm_framebuffer *fb)
> -{

I'm gonna miss this old function...  But on the plus side I won't
confuse pipe_set_base and set_pipe_base anymore, always got that wrong
when searching.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: add turbo boost trace point

[Jesse Barnes]

Might be helpful for debugging places where userspace ends up boosting
or waiting where it doesn't intend to.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/i915_gem.c   |  6 --
 drivers/gpu/drm/i915/i915_trace.h | 15 +++
 drivers/gpu/drm/i915/intel_pm.c   |  9 +++--
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 86cf428..b03cb07 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4209,8 +4209,10 @@ i915_gem_object_ggtt_unpin(struct drm_i915_gem_object 
*obj)
struct i915_vma *vma = i915_gem_obj_to_ggtt(obj);
 
BUG_ON(!vma);
-   BUG_ON(vma->pin_count == 0);
-   BUG_ON(!i915_gem_obj_ggtt_bound(obj));
+   if (WARN(vma->pin_count == 0, "bad pin count\n"))
+   return;
+   if (WARN(!i915_gem_obj_ggtt_bound(obj), "obj not bound\n"))
+   return;
 
if (--vma->pin_count == 0)
obj->pin_mappable = false;
diff --git a/drivers/gpu/drm/i915/i915_trace.h 
b/drivers/gpu/drm/i915/i915_trace.h
index 751d4ad..d710fe1 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -691,6 +691,21 @@ TRACE_EVENT(switch_mm,
  __entry->dev, __entry->ring, __entry->to, __entry->vm)
 );
 
+TRACE_EVENT(turbo_boost,
+   TP_PROTO(u32 freq),
+   TP_ARGS(freq),
+
+   TP_STRUCT__entry(
+   __field(u32, freq)
+   ),
+
+   TP_fast_assign(
+   __entry->freq = freq;
+   ),
+
+   TP_printk("turbo boost to %d MHz", __entry->freq)
+);
+
 #endif /* _I915_TRACE_H_ */
 
 /* This part must be outside protection */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7558ba2..2944593 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4483,10 +4483,15 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv)
 
mutex_lock(&dev_priv->rps.hw_lock);
if (dev_priv->rps.enabled) {
-   if (IS_VALLEYVIEW(dev))
+   if (IS_VALLEYVIEW(dev)) {
valleyview_set_rps(dev_priv->dev, 
dev_priv->rps.max_freq_softlimit);
-   else
+   trace_turbo_boost(vlv_gpu_freq(dev_priv,
+  
dev_priv->rps.max_freq_softlimit));
+   } else {
gen6_set_rps(dev_priv->dev, 
dev_priv->rps.max_freq_softlimit);
+   trace_turbo_boost(dev_priv->rps.max_freq_softlimit * 
50);
+   }
+
dev_priv->rps.last_adj = 0;
}
mutex_unlock(&dev_priv->rps.hw_lock);
-- 
1.9.1

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

[Intel-gfx] [PATCH] drm/i915/ddi: set has_infoframe flag on DDI too v2

[Jesse Barnes]

Just like we do in the HDMI code, set the infoframe flag if we detect
that infoframes are enabled.

v2: check for actual infoframe status as in hdmi code (Daniel)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_ddi.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 07c5625..24110c9 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2075,6 +2075,14 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
break;
}
 
+   if (encoder->type == INTEL_OUTPUT_HDMI) {
+   struct intel_hdmi *intel_hdmi =
+   enc_to_intel_hdmi(&encoder->base);
+
+   if (intel_hdmi->infoframe_enabled(&encoder->base))
+   pipe_config->has_infoframe = true;
+   }
+
if (intel_display_power_is_enabled(dev_priv, POWER_DOMAIN_AUDIO)) {
temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
if (temp & AUDIO_OUTPUT_ENABLE(intel_crtc->pipe))
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH] drm/i915: expose a fixed brightness range to userspace

[Jesse Barnes]

On Tue, 11 Nov 2014 12:30:38 -0800
"U. Artie Eoff"  wrote:

> A userspace brightness range that is larger than the hardware
> brightness range does not have a 1:1 mapping and can result
> in brightness != actual_brightness for some values.
> 
> Expose a fixed brightness range of [0..1000] to userspace so
> that all values can map 1:1 into the hardware brightness
> range.  This would assume that the hardware range is always
> greater than 1000, otherwise we're right back to the original
> issue.
> 
> This patch is based on Jani Nikula's proposed change in the
> referenced ML thread, except use the range [0..1000] instead;
> which was recommended by Jesse Barnes for smoother backlight
> transitions.
> 
> Reference: 
> http://lists.freedesktop.org/archives/intel-gfx/2014-November/055221.html
> Signed-off-by: U. Artie Eoff 
> ---
>  drivers/gpu/drm/i915/intel_panel.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_panel.c 
> b/drivers/gpu/drm/i915/intel_panel.c
> index 4d63839..f74f5f2 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1057,7 +1057,7 @@ static int intel_backlight_device_register(struct 
> intel_connector *connector)
>* Note: Everything should work even if the backlight device max
>* presented to the userspace is arbitrarily chosen.
>*/
> - props.max_brightness = panel->backlight.max;
> + props.max_brightness = 1000;
>   props.brightness = scale_hw_to_user(connector,
>   panel->backlight.level,
>               props.max_brightness);

Yeah looks ok to me.  I guess Jani has to ack it too.

Reviewed-by: Jesse Barnes 

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915/ddi: set has_infoframe flag on DDI too

[Jesse Barnes]

On Tue, 18 Nov 2014 09:14:05 +0100
Daniel Vetter  wrote:

> On Mon, Nov 17, 2014 at 01:08:46PM -0800, Jesse Barnes wrote:
> > Just like we do in the HDMI code, set the infoframe flag if we detect an
> > HDMI sink.
> > 
> > Reported-by: Paulo Zanoni 
> > Signed-off-by: Jesse Barnes 
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 86745da..78576e0 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2062,6 +2062,7 @@ void intel_ddi_get_config(struct intel_encoder 
> > *encoder,
> > switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
> > case TRANS_DDI_MODE_SELECT_HDMI:
> > pipe_config->has_hdmi_sink = true;
> > +   pipe_config->has_infoframe = true;
> 
> Infoframes aren't controlled by this bit here but set in
> HSW_TVIDEO_DIP_CTL. Since the point of this is to detect mismatches
> between the bios and what we'd like to have for fastboot I think we need
> to check that register. Instead of blindly deriving state to appease the
> cross checker.

Oh you're right; I was thinking of compute_config.  I'll send a follow
up.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/2] drm/i915/ddi: add break in DDI mode select switch

[Jesse Barnes]

The lack of a break here wasn't for falling through to some other
important code, so made me do a double take.  Add a break just to make
things a little less confusing.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_ddi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 78576e0..66c373b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2063,6 +2063,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
case TRANS_DDI_MODE_SELECT_HDMI:
pipe_config->has_hdmi_sink = true;
pipe_config->has_infoframe = true;
+   break;
case TRANS_DDI_MODE_SELECT_DVI:
case TRANS_DDI_MODE_SELECT_FDI:
break;
-- 
1.9.1

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

[Intel-gfx] [PATCH 1/2] drm/i915/ddi: set has_infoframe flag on DDI too

[Jesse Barnes]

Just like we do in the HDMI code, set the infoframe flag if we detect an
HDMI sink.

Reported-by: Paulo Zanoni 
Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_ddi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 86745da..78576e0 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2062,6 +2062,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
switch (temp & TRANS_DDI_MODE_SELECT_MASK) {
case TRANS_DDI_MODE_SELECT_HDMI:
pipe_config->has_hdmi_sink = true;
+   pipe_config->has_infoframe = true;
case TRANS_DDI_MODE_SELECT_DVI:
case TRANS_DDI_MODE_SELECT_FDI:
break;
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH] drm/i915: Propagate invalid setcrtc cloning errors back to userspace

[Jesse Barnes]

On Mon, 17 Nov 2014 11:17:22 -0800
Matt Roper  wrote:

> On Mon, Nov 17, 2014 at 08:06:47PM +0100, Daniel Vetter wrote:
> > On Mon, Nov 17, 2014 at 09:59:28AM -0800, Matt Roper wrote:
> > > When invalid cloning configurations were detected during modeset, we
> > > never copied the error code into the return value variable, leading us
> > > to return 0 (success) to userspace.
> > > 
> > > Testcase: igt/kms_setmode
> > > Signed-off-by: Matt Roper 
> > 
> > I guess this is a regression from 
> > 
> > commit 50f5275698df4490046cc5b4ed2018abb642a803
> > Author: Jesse Barnes 
> > Date:   Fri Nov 7 13:11:00 2014 -0800
> > 
> > drm/i915: use compute_config in set_config v4
> > 
> > Is this the one we have a bugzilla for already? Jesse?
> > -Daniel
> 
> Looks like it might be this one:
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86226

Ah you found it already?  Nice.

Reviewed-by: Jesse Barnes 

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 05/10] drm/i915: Add debugfs interface for Displayport debug and compliance testing

[Jesse Barnes]

On Fri, 14 Nov 2014 07:00:17 +1000
Dave Airlie  wrote:

> > Aside from all this (and now with my community hat on) just adding
> > code to get a sticker (labelled "passed DP validation") which is
> > separate code and not used by actual users is imo not useful for
> > merging upstream. But that's just my own opinion, not sure what's
> > Dave's stance here or whether there's much precendence either way.
> >
> > So short answer: I still think exposing this with properties is the
> > right approach, presuming we really need it (and it's not just to
> > paper over a deficient link training logic in the kernel). I also
> > think it'll be less code since we can simplify the debugfs option
> > parser.  
> 
> Don't expose DP stuff in properties, I don't want users controlling
> the parameters of the DP link in any way. I can't see any use
> in userspace for controlling this stuff. So I'm happier with debugfs,
> to avoid making an ABI we hate later.
> 
> Yes I do prefer we make DP validation go via the same paths,
> but some parts of DP validation require things userspace shouldn't
> be allowed setup, and for those we should have bypasses, everything
> else should be done via normal channels.

Yeah, agreed on re-using code paths.  I just don't think it means we
have to re-use ioctl or property entry points.  The internals of the
debugfs file can just as easily call internal functions as the
ioctl/property code, so I think we can be covered that way too.

I guess we'll have to check out Todd's latest patches when he posts
them.

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

Re: [Intel-gfx] [PATCH 05/10] drm/i915: Add debugfs interface for Displayport debug and compliance testing

[Jesse Barnes]

On Thu, 23 Oct 2014 17:43:01 +0200
Daniel Vetter  wrote:

> On Thu, Oct 23, 2014 at 10:58:59AM -0200, Paulo Zanoni wrote:
> > 2014-10-23 10:50 GMT-02:00 Daniel Vetter :
> > > I think it's better to expose this as drm properties on the DP
> > > connector. This has a pile of upsides:
> > > - We don't need to invent a new parser.
> > > - Users can play with them to test out different theories.
> > > - We could even use this to fix broken panels/boards without vbt
> > > or anything using our plan to set up the initial config with a dt
> > > firmware blob.
> > >
> > > So would be a lot more useful than this private interface.
> > >
> > > For the properties themselves I think we should go with explicit
> > > enumerations with default value "automatic" and then an
> > > enumeration of all values allowed by DP. For the naming of the
> > > properties I guess something like DP_link_lanes,
> > > DP_link_clock, ... would look pretty. The properties should be in
> > > dev->mode_config (since they're reallly generic) and I think we
> > > want one function to register them all in one go in the
> > > drm_dp_helper.c library. Maybe we could even put the values into
> > > the existing dp source struct so that we don't have to reinvent
> > > the decode logic every single time.
> > 
> > I disagree. I do prefer the debugfs thing. Think about all the
> > examples of people that break their systems by passing
> > i915.enable_foo=1 or i915.enable_rc6=7 and then open bug reports...
> > With debug stuff exposed as DP properties, we're going to increase
> > that problem, and in a way that will make it even harder to detect.
> > I really really think these things should be exposed only to the
> > debugfs users, because then if the debugfs file is closed, we can
> > just ignore all the arguments and keep doing "normal" unaffected
> > modesets.
> > 
> > If we don't want the parser, maybe we can make it a binary protocol:
> > we'lll still have to parse it, but it can get much easier.
> 
> We already expose piles of funky stuff to users in these properties
> (e.g. audio on hdmi). And contrary to the module options most of
> these will just result in black screens if you don't understand what
> you're doing, so I think the risk is low. There's also a bit of an
> issue with the current interface as it's not clear which line
> corresponds to which DP interface. Using properties would solve this.
> Also these options have a much more direct impact - if you set them
> and the screen goes dark then the user hopefully realizes that this
> is something they shouldn't touch.
> 
> For fbc and rc6 and all those the problem is that nothing really
> happens, the system just becomes a bit more unstable and randomly
> crashes. Which users then report.
> 
> But If you're really concerned about users doing crappy stuff we
> could add a module option to drm_dp_helper.c which hides these, and
> then taint the kernel if any user sets them. But I really think this
> is overkill. -Daniel

Just chatted with Todd about the state of this patchset.  He's going to
post an update today or tomorrow and summarize the feedback from July
ad what he's done to address it, add changelogs, and address outstanding
review feedback from this posting.

I like the idea of exposing some DP stuff like lane count, preemph,
etc, as new properties, but I don't think it's reasonable to block the
testing stuff on it, for a few reasons:
  1) we should think pretty hard about how we want new ABI like
 this exposed
  2) the compliance spec changes pretty regularly, so keeping an
 unstable interface for it in debugfs may make sense anyway
  3) even if we decide to move the userland test code over to
 properties in the future, the fact that debugfs is unstable means
 we can drop it at that time

So I asked Todd to file a JIRA for the properties feature request,
since it's definitely worth looking at.

Which isn't to say the current interface doesn't have issues, just that
Todd shouldn't rewrite things again to include a new, stable ABI,
probably keeping compliance testing and additional DP test coverage out
of the tree for even longer.

Thoughts, objections?

Thanks,
Jesse
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/2] drm/i915/hdmi: fetch infoframe status in get_config v3

[Jesse Barnes]

This is useful for checking things later.

v2:
  - fix hsw infoframe enabled check (Ander)
v3:
  - set infoframe status in compute_config too (Ville)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_hdmi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 0c9fe4b..f58e883 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -982,6 +982,9 @@ bool intel_hdmi_compute_config(struct intel_encoder 
*encoder,
 
pipe_config->has_hdmi_sink = intel_hdmi->has_hdmi_sink;
 
+   if (pipe_config->has_hdmi_sink)
+   pipe_config->has_infoframe = true;
+
if (intel_hdmi->color_range_auto) {
/* See CEA-861-E - 5.1 Default Encoding Parameters */
if (pipe_config->has_hdmi_sink &&
-- 
1.9.1

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

[Intel-gfx] [PATCH 2/2] drm/i915: calculate pfit changes in set_config v3

[Jesse Barnes]

This should allow us to avoid mode sets for some panel fitter config
changes.

v2:
  - fixup pfit comment (Ander)
v3:
  - fixup pfit disable shortcut, only apply to gen4 for now (Jesse)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 73 ++--
 1 file changed, 62 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 58ed4ef..e7175b1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2861,17 +2861,8 @@ static void intel_update_pipe_size(struct intel_crtc 
*crtc)
return;
 
/*
-* Update pipe size and adjust fitter if needed: the reason for this is
-* that in compute_mode_changes we check the native mode (not the pfit
-* mode) to see if we can flip rather than do a full mode set. In the
-* fastboot case, we'll flip, but if we don't update the pipesrc and
-* pfit state, we'll end up with a big fb scanned out into the wrong
-* sized surface.
-*
-* To fix this properly, we need to hoist the checks up into
-* compute_mode_changes (or above), check the actual pfit state and
-* whether the platform allows pfit disable with pipe active, and only
-* then update the pipesrc and pfit state, even on the flip path.
+* See intel_pfit_changed for info on when we're allowed to
+* do this w/o a pipe shutdown.
 */
 
adjusted_mode = &crtc->config.adjusted_mode;
@@ -11266,6 +11257,62 @@ static void disable_crtc_nofb(struct intel_crtc *crtc)
crtc->new_config = NULL;
 }
 
+/* Do we need a mode set due to pfit changes? */
+static bool intel_pfit_changed(struct drm_device *dev,
+  struct intel_crtc_config *new_config,
+  struct intel_crtc_config *cur_config)
+{
+   bool ret = false;
+
+   if (HAS_DDI(dev) || HAS_PCH_SPLIT(dev)) {
+   /*
+* On PCH platforms we can disable pfit w/o a pipe shutdown,
+* otherwise we'll need a mode set.
+*/
+   if (new_config->pch_pfit.enabled &&
+   cur_config->pch_pfit.enabled)
+   ret = false;
+   else if (new_config->pch_pfit.enabled &&
+!cur_config->pch_pfit.enabled)
+   ret = true;
+   else if (!new_config->pch_pfit.enabled &&
+cur_config->pch_pfit.enabled)
+   ret = false;
+   else if (!new_config->pch_pfit.enabled &&
+!cur_config->pch_pfit.enabled)
+   ret = false;
+   } else if (IS_GEN4(dev)) {
+   /*
+* We may be able to get away with this on more chips, but
+* we need more testing here.
+*/
+   bool new_enabled, old_enabled;
+
+   new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE);
+   old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE);
+
+   /* 9xx only needs a shutdown to disable pfit */
+   if (new_enabled && old_enabled)
+   ret = false;
+   else if (new_enabled && !old_enabled)
+   ret = false;
+   else if (!new_enabled && old_enabled)
+   ret = true;
+   else if (!new_enabled && !old_enabled)
+   ret = false;
+   } else {
+   bool new_enabled, old_enabled;
+
+   new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE);
+   old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE);
+
+   if (new_enabled != old_enabled)
+   ret = true;
+   }
+
+   return ret;
+}
+
 static int intel_crtc_set_config(struct drm_mode_set *set)
 {
struct drm_device *dev;
@@ -11334,6 +11381,10 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
to_intel_crtc(set->crtc)->config.has_infoframe)
config->mode_changed = true;
+
+   if (intel_pfit_changed(dev, 
to_intel_crtc(set->crtc)->new_config,
+  &to_intel_crtc(set->crtc)->config))
+   config->mode_changed = true;
}
 
/* set_mode will free it in the mode_changed case */
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH] drm/i915: Use correct pipe config to update pll dividers. V2

[Jesse Barnes]

On Tue, 11 Nov 2014 09:29:18 -0800
Bob Paauwe  wrote:

> Use the new pipe config values to calculate the updated pll dividers.
> 
> This regression was introduced in
> 
> commit 0dbdf89f27b17ae1eceed6782c2917f74cbb5d59
> Author: Ander Conselvan de Oliveira
>  Date:   Wed Oct 29 11:32:33
> 2014 +0200
> 
> drm/i915: Add infrastructure for choosing DPLLs before disabling
> crtcs
> 
>   and
> 
>   commit 00d958817dd3daaa452c221387ddaf23d1e4c06f
>   Author: Ander Conselvan de Oliveira
>  Date:   Wed Oct 29 11:32:36
> 2014 +0200
> 
>   drm/i915: Covert remaining platforms to choose DPLLS
> before disabling CRTCs
> 
> v2: Use intel_pipe_will_have_type() to look at new configuration -
> Ander
> 
> Signed-off-by: Bob Paauwe 
> CC: Ander Conselvan de Oliveira
>  ---
>  drivers/gpu/drm/i915/intel_display.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index ff071a7..667d72a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5730,24 +5730,24 @@ static void i9xx_update_pll_dividers(struct
> intel_crtc *crtc, u32 fp, fp2 = 0;
>  
>   if (IS_PINEVIEW(dev)) {
> - fp = pnv_dpll_compute_fp(&crtc->config.dpll);
> + fp = pnv_dpll_compute_fp(&crtc->new_config->dpll);
>   if (reduced_clock)
>   fp2 = pnv_dpll_compute_fp(reduced_clock);
>   } else {
> - fp = i9xx_dpll_compute_fp(&crtc->config.dpll);
> + fp = i9xx_dpll_compute_fp(&crtc->new_config->dpll);
>   if (reduced_clock)
>   fp2 = i9xx_dpll_compute_fp(reduced_clock);
>   }
>  
> - crtc->config.dpll_hw_state.fp0 = fp;
> + crtc->new_config->dpll_hw_state.fp0 = fp;
>  
>   crtc->lowfreq_avail = false;
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> + if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
>   reduced_clock && i915.powersave) {
> - crtc->config.dpll_hw_state.fp1 = fp2;
> + crtc->new_config->dpll_hw_state.fp1 = fp2;
>   crtc->lowfreq_avail = true;
>   } else {
> - crtc->config.dpll_hw_state.fp1 = fp;
> + crtc->new_config->dpll_hw_state.fp1 = fp;
>   }
>  }
>  

Fixes things on my 945 here.  Thanks.

Tested-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config v2

[Jesse Barnes]

On Tue, 11 Nov 2014 16:23:03 +0100
Daniel Vetter  wrote:

> On Tue, Nov 11, 2014 at 05:19:46PM +0200, Ville Syrjälä wrote:
> > On Tue, Nov 11, 2014 at 04:00:12PM +0100, Daniel Vetter wrote:
> > > On Wed, Nov 05, 2014 at 02:26:08PM -0800, Jesse Barnes wrote:
> > > > This is useful for checking things later.
> > > > 
> > > > v2:
> > > >   - fix hsw infoframe enabled check (Ander)
> > > > 
> > > > Signed-off-by: Jesse Barnes 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_drv.h  |  4 +++
> > > >  drivers/gpu/drm/i915/intel_hdmi.c | 62 
> > > > +++
> > > >  2 files changed, 66 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> > > > b/drivers/gpu/drm/i915/intel_drv.h
> > > > index d53ac23..8aa80e1 100644
> > > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > > @@ -292,6 +292,9 @@ struct intel_crtc_config {
> > > >  * between pch encoders and cpu encoders. */
> > > > bool has_pch_encoder;
> > > >  
> > > > +   /* Are we sending infoframes on the attached port */
> > > > +   bool has_infoframe;
> > > 
> > > The cross-checking of this new hw state is missing. I've added that while
> > > applying.
> > 
> > Where is the compute_config part of this? I can't see it in any patch,
> > but maybe I'm just blind.
> 
> Yeah, seems to be missing, thanks for catching this. Jesse/Ander, can you
> pls supply a fixup quickly - I'd need to drop the series until this one
> otherwise I think.

Ah right missed the other side.  Will post a patch today.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC] dpms handling on atomic drivers

[Jesse Barnes]

On Thu, 6 Nov 2014 19:35:51 -0500
Rob Clark  wrote:

> On Thu, Nov 6, 2014 at 6:29 PM, Daniel Vetter  wrote:
> >
> > That aside I guess I need to elaborate on what makes dpms special in
> > i915, and why there's a real difference between crtc->enable == true
> > && ->active == false and crtc->enable == false in i915. For complex
> > configs we do resource checking (shared dplls) and that's done in
> > the modeset. For a pipe which has been disabled just with dpms we
> > then guarantee that we'll keep these resources reserve and so will
> > always be able to enable the pipe again. If you disable the pipe
> > completely (i.e. set crtc->enable to false) we'll release these
> > resources. E.g. in the i915 share dpll code we have both an active
> > refcount (does the ppl need to be running) and a reference mask
> > (which crtc is referencing this pll, even when the crtc is disabled
> > with dpms).
> 
> ahh, ok, "reserved but not enabled" makes a lot more sense.. that was
> the distinction that I was missing.  That probably deserves to be in
> headerdoc somewhere..

A rename would be nice too; it's very misleading.  Though with a move
to a boolean DPMS internal state, it should be possible to drop it and
just re-alloc all the resources on DPMS on (iow treat both DPMS off and
on as mode sets).  But that's not something that should block these
changes by any means.

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

Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Move the .global_resources() hook call into modeset_update_crtc_power_domains()

[Jesse Barnes]

On Mon, 10 Nov 2014 19:24:37 +0200
Ville Syrjälä  wrote:

> On Mon, Nov 10, 2014 at 09:14:11AM -0800, Jesse Barnes wrote:
> > On Thu, 6 Nov 2014 14:10:49 +0100
> > Daniel Vetter  wrote:
> > 
> > > On Thu, Nov 06, 2014 at 02:49:12PM +0200,
> > > ville.syrj...@linux.intel.com wrote:
> > > > From: Ville Syrjälä 
> > > > 
> > > > We may need to access various hardware bits in
> > > > the .global_resources() hook, so move the call to occur after
> > > > enabling all the newly required power wells, but before
> > > > disabling all the now unneeded wells. This should guarantee
> > > > that we have all the sufficient hardware resources available
> > > > during the .global_resources() call. And if not, any additional
> > > > resources must be explicitly acquired by the .global_resorces()
> > > > hook.
> > > > 
> > > > For instance on VLV/CHV we need to access the gunit mailbox so
> > > > that we can talk to punit/cck over sideband. In addition some
> > > > PFI credit reprogramming may need to be addes as well, which
> > > > may require the disp2d well.
> > > > 
> > > > This should also make the power domain refcounts consistent on
> > > > platforms which don't have a .global_resource() hook since now
> > > > they too will call modeset_update_crtc_power_domains() which
> > > > will drop the init power. Previously init power was just left
> > > > enabled for such platforms.
> > > > 
> > > > Cc: Imre Deak 
> > > > Signed-off-by: Ville Syrjälä 
> > > 
> > > Yeah I think that's a lot saner and hopefully allows us to unify
> > > the power domain more. Thanks for the patch, queued for next.
> > 
> > As a cleanup later it might be good to pull it out into a separate
> > function.  The global resources can be related to power domains, but
> > not everything we do there is (e.g. re-clocking cdclk), so it may
> > be a little confusing for readers in the future.
> 
> Well, we can't pull it out. That's the whole point with the patch. Or
> maybe I misunderstood what you want to pull and where?
> 
> But we can certainly rename modeset_update_crtc_power_domains()
> itself. But I suck at names anyway so didn't even bother to try :P
> Now that I actually think about it, I guess I could have just called
> it intel_global_resources() or something.

Yeah that would do what I'd like, something that wraps both the power
domain stuff and the global resources call into something that's named
more accurately.

Thanks,
Jesse
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Mark fastboot as unsafe

[Jesse Barnes]

On Fri, 7 Nov 2014 18:41:16 +0100
Daniel Vetter  wrote:

> On Tue, Nov 04, 2014 at 03:29:57PM +0100, Daniel Vetter wrote:
> > Fastboot in its current incarnation assumes that the pfit isn't
> > relevatn for the state and that it can be disabled without
> > restarting the crtc. Unfortunately that's not the case on gen2/3 -
> > it upsets the hw and results in a black screen.
> > 
> > Worse, the way the current fastboot hack is structure we can't
> > detect and work around this in the code, since the fastboot smashes
> > the adjusted mode into crtc->mode. Which means the higher levels
> > can't correctly figure out that this is a lie and act accordingly.
> > 
> > Since fastboot is just a tech demo let's mark the module option as
> > experimental and close the coresponding reports as wontfix.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84682
> > Signed-off-by: Daniel Vetter 
> 
> Jesse expressed concerns in private about this patch, so I've dropped
> it and the other fastboot patch.

I don't think this patch addresses the referenced bug, and it also
implies that we don't care about ever making fastboot the default, so
can ignore any related buts.  The latter surely isn't true in my mind
at least, which is why I've been pushing additional fixes, as recently
as the same day as this patch, so I'm a bit confused about the summary.

As for pfit handling issues, can you be more specific about the case
you want to support that we don't today?  The recent patch for checking
whether pfit requires a full mode set should address your first point,
but I don't know what exactly you mean in your second paragraph...

Thanks,
Jesse
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/dp: Don't stop the link when retraining

[Jesse Barnes]

On Mon,  3 Nov 2014 11:39:24 +0100
Daniel Vetter  wrote:

> On pre-ddi platforms we don't shut down the link when changing link
> training parameters. Except when clock recovery fails too hard and we
> restart with channel eq training. Which doesn't make a lot of sense
> really, since just stopping/restarting the DP port at this point
> violates the modeset sequence documented in the Bspec.
> 
> So let's tempt fate and try this.
> 
> This patch is motivated by a WARN_ON triggered by
> 
> commit bc76e320f21f8bd790a72bd5dc06909617432352
> Author: Daniel Vetter 
> Date:   Tue May 20 22:46:50 2014 +0200
> 
> drm/i915: Drop now misleading DDI comment from dp_link_down
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=85670
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c index f6a3fdd5589e..e48ca3a87199
> 100644 --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3598,7 +3598,6 @@ intel_dp_complete_link_train(struct intel_dp
> *intel_dp) 
>   /* Try 5 times, then try clock recovery if that
> fails */ if (tries > 5) {
> - intel_dp_link_down(intel_dp);
>   intel_dp_start_link_train(intel_dp);
>   intel_dp_set_link_train(intel_dp, &DP,
>   training_pattern |


Didn't look like it helped the reporter?  Or at least I didn't see it
tried in the bug above...

I'm a bit worried about this because istr the spec indicating that we
do need to down the link when retrying clock recovery.  I guess I'll
need to check again.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 2/3] drm/i915: Move the .global_resources() hook call into modeset_update_crtc_power_domains()

[Jesse Barnes]

On Thu, 6 Nov 2014 14:10:49 +0100
Daniel Vetter  wrote:

> On Thu, Nov 06, 2014 at 02:49:12PM +0200,
> ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> > 
> > We may need to access various hardware bits in
> > the .global_resources() hook, so move the call to occur after
> > enabling all the newly required power wells, but before disabling
> > all the now unneeded wells. This should guarantee that we have all
> > the sufficient hardware resources available during
> > the .global_resources() call. And if not, any additional resources
> > must be explicitly acquired by the .global_resorces() hook.
> > 
> > For instance on VLV/CHV we need to access the gunit mailbox so that
> > we can talk to punit/cck over sideband. In addition some PFI credit
> > reprogramming may need to be addes as well, which may require the
> > disp2d well.
> > 
> > This should also make the power domain refcounts consistent on
> > platforms which don't have a .global_resource() hook since now they
> > too will call modeset_update_crtc_power_domains() which will drop
> > the init power. Previously init power was just left enabled for
> > such platforms.
> > 
> > Cc: Imre Deak 
> > Signed-off-by: Ville Syrjälä 
> 
> Yeah I think that's a lot saner and hopefully allows us to unify the
> power domain more. Thanks for the patch, queued for next.

As a cleanup later it might be good to pull it out into a separate
function.  The global resources can be related to power domains, but
not everything we do there is (e.g. re-clocking cdclk), so it may be a
little confusing for readers in the future.

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

Re: [Intel-gfx] [PATCH] drm/i915: intel_backlight scale() math WA v2

[Jesse Barnes]

On Mon, 10 Nov 2014 16:15:57 +0200
Jani Nikula  wrote:

> On Mon, 10 Nov 2014, Jani Nikula  wrote:
> > On Sat, 08 Nov 2014, "Eoff, Ullysses A" 
> > wrote:
> >> On 09/24/2014 10:42 AM, Eoff, Ullysses A wrote:
>  -Original Message-
>  From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org]
>  On Behalf Of Jani Nikula Sent: Wednesday, September 24, 2014
>  10:08 AM To: Hans de Goede; Joe Konno;
>  intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH]
>  drm/i915: intel_backlight scale() math WA v2
> 
>  On Wed, 24 Sep 2014, Hans de Goede  wrote:
> > Hi,
> >
> > On 09/24/2014 05:54 PM, Joe Konno wrote:
> >> From: Joe Konno 
> >>
> >> Improper truncated integer division in the scale() function
> >> causes actual_brightness != brightness. This (partial)
> >> work-around should be sufficient for a majority of use-cases,
> >> but it is by no means a complete solution.
> >>
> >> TODO: Determine how best to scale "user" values to "hw"
> >> values, and vice-versa, when the ranges are of different
> >> sizes. That would be a buggy scenario even with this
> >> work-around.
> >>
> >> The issue was introduced in the following (v3.17-rc1) commit:
> >>
> >> 6dda730 drm/i915: respect the VBT minimum backlight
> >> brightness
> >>
> >> v2: (thanks to Chris Wilson) clarify commit message, use
> >> rounded division macro
> > I wonder why do scaling at all, why not simply shift hw_min -
> > hw_max range to 0 - (hw_max - hw_min) range and set
> > max_brightness as seen by userspace to (hw_max - hw_min) ?
>  Mostly in preparation for a future where we expose an arbitrary
>  range, say 0..100 or 0..255 to the userspace.
> 
> >>> The problem with this scaling method is that scaling from user
> >>> level to hw level and back to user level is ambiguous since there
> >>> isn't a 1:1 mapping between the user range and the hw range.
> >>>
> >>> On the other hand, this patch does fix a bug in the currently
> >>> used method (scaling). That, at least, is an improvement
> >>> nonetheless.
> >>>
> >>> U. Artie
> >> Apologies for resurrecting an old thread.  But I think we still
> >> need to address
> >> this issue about not having a 1:1 mapping between user and hw
> >> levels.
> >>
> >> Right now, the problem is that the user range is larger than the hw
> >> range which
> >> results in one or more user levels mapping to the same hw level.
> >> And when userspace requests one of those levels, the result that
> >> is reported back to userspace might not be the same as what was
> >> requested.  Take for example, on my system the hw range is [398,
> >> 7812] and the user range is [0, 7812]. Suppose
> >> userspace requests level 7017.  This maps to hw level 7058.  And
> >> when userspace requests the current level, 7018 is reported back
> >> (+1 from what was originally requested).  In fact, with these
> >> particular ranges, there are exactly
> >> 398 values that this occurs.
> >>
> >> This problem will be compounded the larger the difference in
> >> length of the discrete ranges; so long as user range > hw range.
> >>
> >> Hans' solution would fix this problem, giving 1:1 mapping from hw
> >> to user levels.
> >>
> >> Jani's [future] solution would work too, since exposing a smaller
> >> range to userspace than the hw range would isolate the non 1:1
> >> mapping inside the driver.
> >
> > I think we should just pick an arbitrary range, say 0..100, and be
> > done with it. It's not like you'd be able to get much more than 100
> > distinct brightness levels out of the backlight anyway, no matter
> > what the PWM settings.
> >
> > BR,
> > Jani.
> 
> PS. This (totally untested) patch should do it:
> 
> diff --git a/drivers/gpu/drm/i915/intel_panel.c
> b/drivers/gpu/drm/i915/intel_panel.c index b001c90312e7..a6680081415b
> 100644 --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1035,7 +1035,7 @@ static int
> intel_backlight_device_register(struct intel_connector *connector)
>* Note: Everything should work even if the backlight device
> max
>* presented to the userspace is arbitrarily chosen.
>*/
> - props.max_brightness = panel->backlight.max;
> + props.max_brightness = 100;
>   props.brightness = scale_hw_to_user(connector,
>   panel->backlight.level,
>   props.max_brightness);

100% agreed on exposing a fixed range.  But iirc Keith did some playing
around with fading in and out of backlights and found that we needed
about 1000 levels to make it smooth (definitely possible on some
platforms, though not all).  So my only nitpick would be that we have a
range that allows a bit more precision.

Thanks,
Jesse
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.f

Re: [Intel-gfx] [PATCH 6/6] drm/i915: calculate pfit changes in set_config v2

[Jesse Barnes]

On Mon, 10 Nov 2014 18:20:56 +0200
Ander Conselvan de Oliveira  wrote:

> On 11/06/2014 12:26 AM, Jesse Barnes wrote:
> > This should allow us to avoid mode sets for some panel fitter config
> > changes.
> >
> > v2:
> >- fixup pfit comment (Ander)
> >
> > Signed-off-by: Jesse Barnes 
> > ---
> >   drivers/gpu/drm/i915/intel_display.c | 61 
> > +---
> >   1 file changed, 50 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 3f1515d..49281d7 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2835,17 +2835,8 @@ static void intel_update_pipe_size(struct intel_crtc 
> > *crtc)
> > return;
> >
> > /*
> > -* Update pipe size and adjust fitter if needed: the reason for this is
> > -* that in compute_mode_changes we check the native mode (not the pfit
> > -* mode) to see if we can flip rather than do a full mode set. In the
> > -* fastboot case, we'll flip, but if we don't update the pipesrc and
> > -* pfit state, we'll end up with a big fb scanned out into the wrong
> > -* sized surface.
> > -*
> > -* To fix this properly, we need to hoist the checks up into
> > -* compute_mode_changes (or above), check the actual pfit state and
> > -* whether the platform allows pfit disable with pipe active, and only
> > -* then update the pipesrc and pfit state, even on the flip path.
> > +* See intel_pfit_changed for info on when we're allowed to
> > +* do this w/o a pipe shutdown.
> >  */
> >
> > adjusted_mode = &crtc->config.adjusted_mode;
> > @@ -11171,6 +11162,50 @@ static void disable_crtc_nofb(struct intel_crtc 
> > *crtc)
> > crtc->new_config = NULL;
> >   }
> >
> > +/* Do we need a mode set due to pfit changes? */
> > +static bool intel_pfit_changed(struct drm_device *dev,
> > +  struct intel_crtc_config *new_config,
> > +  struct intel_crtc_config *cur_config)
> > +{
> > +   bool ret = false;
> > +
> > +   if (HAS_DDI(dev) || HAS_PCH_SPLIT(dev)) {
> > +   /*
> > +* On PCH platforms we can disable pfit w/o a pipe shutdown,
> > +* otherwise we'll need a mode set.
> > +*/
> > +   if (new_config->pch_pfit.enabled &&
> > +   cur_config->pch_pfit.enabled)
> > +   ret = false;
> > +   else if (new_config->pch_pfit.enabled &&
> > +!cur_config->pch_pfit.enabled)
> > +   ret = true;
> > +   else if (!new_config->pch_pfit.enabled &&
> > +cur_config->pch_pfit.enabled)
> > +   ret = false;
> > +   else if (!new_config->pch_pfit.enabled &&
> > +!cur_config->pch_pfit.enabled)
> > +   ret = false;
> > +   } else {
> > +   bool new_enabled, old_enabled;
> > +
> > +   new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE);
> > +   old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE);
> > +
> > +   /* 9xx only needs a shutdown to disable pfit */
> > +   if (new_enabled && old_enabled)
> > +   ret = false;
> > +   else if (new_enabled && !old_enabled)
> > +   ret = false;
> > +   else if (!new_enabled && old_enabled)
> > +   ret = true;
> > +   else if (!new_enabled && !old_enabled)
> > +   ret = false;
> > +   }
> 
> Maybe I missed something, but I couldn't find anything in the 
> documentation the supports the claim above.  However, [1] and [2] read 
> that "[p]anel fitting should be enabled or disabled before the pipe is 
> enabled" on the documentation for the PFIT_CONTROL.
> 
> 
> [1] 
> https://01.org/linuxgraphics/sites/default/files/documentation/g45_vol_3_register_0_0.pdf
> [2] 
> https://01.org/linuxgraphics/sites/default/files/documentation/965_g35_vol_3_display_registers_updated_1.pdf

Yeah the docs are extra conservative on this.  But from memory, this is
what 965 allowed.  It would be good to do some extra testing though;
915/945 may allow both pfit enable and disable without a pipe shutdown.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: use compute_config in set_config v4

[Jesse Barnes]

This will allow us to consult more info before deciding whether to flip
or do a full mode set.

v2:
  - don't use uninitialized or incorrect pipe masks in set_config
failure path (Ander)
v3:
  - fixup for pipe_config changes in compute_config (Jesse)
v4:
  - drop spurious hunk in force restore path (Ander)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index a3ebab8..72123e0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11178,6 +11178,8 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
struct drm_device *dev;
struct drm_mode_set save_set;
struct intel_set_config *config;
+   struct intel_crtc_config *pipe_config;
+   unsigned modeset_pipes, prepare_pipes, disable_pipes;
int ret;
 
BUG_ON(!set);
@@ -11223,9 +11225,23 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
if (ret)
goto fail;
 
+   pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
+  set->fb,
+  &modeset_pipes,
+  &prepare_pipes,
+  &disable_pipes);
+   if (IS_ERR(pipe_config))
+   goto fail;
+
+   /* set_mode will free it in the mode_changed case */
+   if (!config->mode_changed)
+   kfree(pipe_config);
+
if (config->mode_changed) {
-   ret = intel_set_mode(set->crtc, set->mode,
-set->x, set->y, set->fb);
+   ret = intel_set_mode_pipes(set->crtc, set->mode,
+  set->x, set->y, set->fb, pipe_config,
+  modeset_pipes, prepare_pipes,
+  disable_pipes);
} else if (config->fb_changed) {
struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
 
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH 2/6] drm/i915: use compute_config in set_config v3

[Jesse Barnes]

On Fri, 07 Nov 2014 11:41:48 +0200
Ander Conselvan de Oliveira  wrote:

> On 11/06/2014 12:26 AM, Jesse Barnes wrote:
> > This will allow us to consult more info before deciding whether to flip
> > or do a full mode set.
> >
> > v2:
> >- don't use uninitialized or incorrect pipe masks in set_config
> >  failure path (Ander)
> > v3:
> >- fixup for pipe_config changes in compute_config (Jesse)
> >
> > Signed-off-by: Jesse Barnes 
> > ---
> >   drivers/gpu/drm/i915/intel_display.c | 36 
> > +---
> >   1 file changed, 33 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index a3ebab8..cb96f11 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11178,6 +11178,8 @@ static int intel_crtc_set_config(struct 
> > drm_mode_set *set)
> > struct drm_device *dev;
> > struct drm_mode_set save_set;
> > struct intel_set_config *config;
> > +   struct intel_crtc_config *pipe_config;
> > +   unsigned modeset_pipes, prepare_pipes, disable_pipes;
> > int ret;
> >
> > BUG_ON(!set);
> > @@ -11223,9 +11225,23 @@ static int intel_crtc_set_config(struct 
> > drm_mode_set *set)
> > if (ret)
> > goto fail;
> >
> > +   pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
> > +  set->fb,
> > +  &modeset_pipes,
> > +  &prepare_pipes,
> > +  &disable_pipes);
> > +   if (IS_ERR(pipe_config))
> > +   goto fail;
> > +
> > +   /* set_mode will free it in the mode_changed case */
> > +   if (!config->mode_changed)
> > +   kfree(pipe_config);
> > +
> > if (config->mode_changed) {
> > -   ret = intel_set_mode(set->crtc, set->mode,
> > -set->x, set->y, set->fb);
> > +   ret = intel_set_mode_pipes(set->crtc, set->mode,
> > +  set->x, set->y, set->fb, pipe_config,
> > +  modeset_pipes, prepare_pipes,
> > +  disable_pipes);
> > } else if (config->fb_changed) {
> > struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
> >
> > @@ -13198,6 +13214,20 @@ void intel_modeset_setup_hw_state(struct 
> > drm_device *dev,
> > for_each_pipe(dev_priv, pipe) {
> > struct drm_crtc *crtc =
> > dev_priv->pipe_to_crtc_mapping[pipe];
> > +   struct intel_crtc_config *pipe_config;
> > +   unsigned modeset_pipes, prepare_pipes, disable_pipes;
> > +
> > +   pipe_config = intel_modeset_compute_config(crtc,
> > +  &crtc->mode,
> > +  
> > crtc->primary->fb,
> > +  
> > &modeset_pipes,
> > +  
> > &prepare_pipes,
> > +  
> > &disable_pipes);
> > +   if (IS_ERR(pipe_config)) {
> > +   DRM_DEBUG_KMS("prepare failed\n");
> > +   kfree(pipe_config);
> > +   goto out;
> > +   }
> >
> > intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
> >crtc->primary->fb);
> 
> Did you mean to convert this to intel_set_mode_pipes()? Since you 
> changed intel_set_mode() to do exactly the same thing as above, the 
> whole hunk seems unnecessary unless you really care about logging that 
> it was the prepare step that failed.

Ah yeah forgot to drop the above bits.  I think it's easier to just
call set_mode here rather than duplicating more compute_config bits.
I'll do that.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 08/16] drm/i915: remove unused restore_gtt_mappings optimization during suspend

[Jesse Barnes]

On Tue, 21 Oct 2014 16:50:12 +0300
Ville Syrjälä  wrote:

> On Thu, Sep 11, 2014 at 07:15:27AM -0700, Jesse Barnes wrote:
> > On Thu, 11 Sep 2014 14:59:35 +0300
> > Imre Deak  wrote:
> > 
> > > On Thu, 2014-09-11 at 08:49 +0100, Chris Wilson wrote:
> > > > On Wed, Sep 10, 2014 at 06:17:01PM +0300, Imre Deak wrote:
> > > > > Since correctness wins over optimal code and since the
> > > > > optimization
> > > > 
> > > > Optimal code is also correct ;-) s/optimal/just plain broken/
> > > 
> > > Yes, bad wording. To clarify, since the optimization is now
> > > always off anyway (and it's also broken), I would hope that we
> > > could remove it for now and concentrate on fixing the existing
> > > s/r issues. Once we find that things are stable enough we could
> > > add back this optimization.
> > 
> > Arg, I guess we didn't test after moving to the opregion test?  Or
> > maybe it was working when it landed for S3 and not for S4?  Or
> > broke sometime after it landed?
> > 
> > Anyway this is a really valuable optimization for resume time on
> > some platforms, and really we shouldn't have other agents
> > clobbering our GTT on resume, so I'd really like to fix/re-add it
> > asap.
> 
> If/when we add this back I would suggest that we also add a sanity
> check that can be enabled with drm.debug which would verify the GTT
> mapping are what we expect. That way at least most developer would
> have it enabled and we'd catch problems earlier, and also bug
> reporters would be forced to enable it when we ask for dmesg w/
> debugs.

Yeah we had that awhile back iirc, but I think it got bikeshedded to
death.  I'd be happy if someone resurrected it; I'd give it an r-b
without suggesting more sophisticated checksum algorithms. :)

http://lists.freedesktop.org/archives/intel-gfx/2012-September/020305.html

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

[Intel-gfx] [PATCH 6/6] drm/i915: calculate pfit changes in set_config v2

[Jesse Barnes]

This should allow us to avoid mode sets for some panel fitter config
changes.

v2:
  - fixup pfit comment (Ander)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 61 +---
 1 file changed, 50 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3f1515d..49281d7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2835,17 +2835,8 @@ static void intel_update_pipe_size(struct intel_crtc 
*crtc)
return;
 
/*
-* Update pipe size and adjust fitter if needed: the reason for this is
-* that in compute_mode_changes we check the native mode (not the pfit
-* mode) to see if we can flip rather than do a full mode set. In the
-* fastboot case, we'll flip, but if we don't update the pipesrc and
-* pfit state, we'll end up with a big fb scanned out into the wrong
-* sized surface.
-*
-* To fix this properly, we need to hoist the checks up into
-* compute_mode_changes (or above), check the actual pfit state and
-* whether the platform allows pfit disable with pipe active, and only
-* then update the pipesrc and pfit state, even on the flip path.
+* See intel_pfit_changed for info on when we're allowed to
+* do this w/o a pipe shutdown.
 */
 
adjusted_mode = &crtc->config.adjusted_mode;
@@ -11171,6 +11162,50 @@ static void disable_crtc_nofb(struct intel_crtc *crtc)
crtc->new_config = NULL;
 }
 
+/* Do we need a mode set due to pfit changes? */
+static bool intel_pfit_changed(struct drm_device *dev,
+  struct intel_crtc_config *new_config,
+  struct intel_crtc_config *cur_config)
+{
+   bool ret = false;
+
+   if (HAS_DDI(dev) || HAS_PCH_SPLIT(dev)) {
+   /*
+* On PCH platforms we can disable pfit w/o a pipe shutdown,
+* otherwise we'll need a mode set.
+*/
+   if (new_config->pch_pfit.enabled &&
+   cur_config->pch_pfit.enabled)
+   ret = false;
+   else if (new_config->pch_pfit.enabled &&
+!cur_config->pch_pfit.enabled)
+   ret = true;
+   else if (!new_config->pch_pfit.enabled &&
+cur_config->pch_pfit.enabled)
+   ret = false;
+   else if (!new_config->pch_pfit.enabled &&
+!cur_config->pch_pfit.enabled)
+   ret = false;
+   } else {
+   bool new_enabled, old_enabled;
+
+   new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE);
+   old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE);
+
+   /* 9xx only needs a shutdown to disable pfit */
+   if (new_enabled && old_enabled)
+   ret = false;
+   else if (new_enabled && !old_enabled)
+   ret = false;
+   else if (!new_enabled && old_enabled)
+   ret = true;
+   else if (!new_enabled && !old_enabled)
+   ret = false;
+   }
+
+   return ret;
+}
+
 static int intel_crtc_set_config(struct drm_mode_set *set)
 {
struct drm_device *dev;
@@ -11239,6 +11274,10 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
to_intel_crtc(set->crtc)->config.has_infoframe)
config->mode_changed = true;
+
+   if (intel_pfit_changed(dev, 
to_intel_crtc(set->crtc)->new_config,
+  &to_intel_crtc(set->crtc)->config))
+   config->mode_changed = true;
}
 
/* set_mode will free it in the mode_changed case */
-- 
1.9.1

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

[Intel-gfx] [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode v3

[Jesse Barnes]

This allows us to calculate the full pipe config before we do any mode
setting work.

v2:
  - clarify comments about global vs. per-crtc mode set (Ander)
  - clean up unnecessary pipe_config = NULL setting (Ander)
v3:
  - fix pipe_config handling (alloc in compute_config, free in set_mode) (Jesse)
  - fix arg order in set_mode (Jesse)
  - fix failure path of set_config (Ander)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 105 ---
 1 file changed, 74 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index a9df85f..a3ebab8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10656,45 +10656,60 @@ static void update_scanline_offset(struct intel_crtc 
*crtc)
crtc->scanline_offset = 1;
 }
 
+static struct intel_crtc_config *
+intel_modeset_compute_config(struct drm_crtc *crtc,
+struct drm_display_mode *mode,
+struct drm_framebuffer *fb,
+unsigned *modeset_pipes,
+unsigned *prepare_pipes,
+unsigned *disable_pipes)
+{
+   struct intel_crtc_config *pipe_config = NULL;
+
+   intel_modeset_affected_pipes(crtc, modeset_pipes,
+prepare_pipes, disable_pipes);
+
+   if ((*modeset_pipes) == 0)
+   goto out;
+
+   /*
+* Note this needs changes when we start tracking multiple modes
+* and crtcs.  At that point we'll need to compute the whole config
+* (i.e. one pipe_config for each crtc) rather than just the one
+* for this crtc.
+*/
+   pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
+   if (IS_ERR(pipe_config)) {
+   goto out;
+   }
+   intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
+  "[modeset]");
+   to_intel_crtc(crtc)->new_config = pipe_config;
+
+out:
+   return pipe_config;
+}
+
 static int __intel_set_mode(struct drm_crtc *crtc,
struct drm_display_mode *mode,
-   int x, int y, struct drm_framebuffer *fb)
+   int x, int y, struct drm_framebuffer *fb,
+   struct intel_crtc_config *pipe_config,
+   unsigned modeset_pipes,
+   unsigned prepare_pipes,
+   unsigned disable_pipes)
 {
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_display_mode *saved_mode;
-   struct intel_crtc_config *pipe_config = NULL;
struct intel_crtc *intel_crtc;
-   unsigned disable_pipes, prepare_pipes, modeset_pipes;
int ret = 0;
 
saved_mode = kmalloc(sizeof(*saved_mode), GFP_KERNEL);
if (!saved_mode)
return -ENOMEM;
 
-   intel_modeset_affected_pipes(crtc, &modeset_pipes,
-&prepare_pipes, &disable_pipes);
-
*saved_mode = crtc->mode;
 
-   /* Hack: Because we don't (yet) support global modeset on multiple
-* crtcs, we don't keep track of the new mode for more than one crtc.
-* Hence simply check whether any bit is set in modeset_pipes in all the
-* pieces of code that are not yet converted to deal with mutliple crtcs
-* changing their mode at the same time. */
-   if (modeset_pipes) {
-   pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
-   if (IS_ERR(pipe_config)) {
-   ret = PTR_ERR(pipe_config);
-   pipe_config = NULL;
-
-   goto out;
-   }
-   intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
-  "[modeset]");
-   to_intel_crtc(crtc)->new_config = pipe_config;
-   }
-
/*
 * See if the config requires any additional preparation, e.g.
 * to adjust global state with pipes off.  We need to do this
@@ -10719,6 +10734,10 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 
/* crtc->mode is already used by the ->mode_set callbacks, hence we need
 * to set it here already despite that we pass it down the callchain.
+*
+* Note we'll need to fix this up when we start tracking multiple
+* pipes; here we assume a single modeset_pipe and only track the
+* single crtc and mode.
 */
if (modeset_pipes) {
crtc->mode = *mode;
@@ -10787,19 +10806,23 @@ done:
if (ret && crtc->enabled)
crtc->mode = *saved_mode;
 
-out:
kfree(pipe_config);
   

[Intel-gfx] [PATCH 5/6] drm/i915: update pipe size at set_config time

[Jesse Barnes]

This only affects the fastboot path as-is.  In that case, we simply need
to make sure that we update the pipe size at the first mode set.  Rather
than putting it off until we decide to flip (if indeed we do end up
flipping), update the pipe size as appropriate a bit earlier in the
set_config call.

This sets us up for better pipe tracking in later patches.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index c86eee6..3f1515d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2906,8 +2906,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
return ret;
}
 
-   intel_update_pipe_size(intel_crtc);
-
dev_priv->display.update_primary_plane(crtc, fb, x, y);
 
if (intel_crtc->active)
@@ -11247,6 +11245,8 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
if (!config->mode_changed)
kfree(pipe_config);
 
+   intel_update_pipe_size(to_intel_crtc(set->crtc));
+
if (config->mode_changed) {
ret = intel_set_mode_pipes(set->crtc, set->mode,
   set->x, set->y, set->fb, pipe_config,
-- 
1.9.1

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

[Intel-gfx] [PATCH 2/6] drm/i915: use compute_config in set_config v3

[Jesse Barnes]

This will allow us to consult more info before deciding whether to flip
or do a full mode set.

v2:
  - don't use uninitialized or incorrect pipe masks in set_config
failure path (Ander)
v3:
  - fixup for pipe_config changes in compute_config (Jesse)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 36 +---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index a3ebab8..cb96f11 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11178,6 +11178,8 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
struct drm_device *dev;
struct drm_mode_set save_set;
struct intel_set_config *config;
+   struct intel_crtc_config *pipe_config;
+   unsigned modeset_pipes, prepare_pipes, disable_pipes;
int ret;
 
BUG_ON(!set);
@@ -11223,9 +11225,23 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
if (ret)
goto fail;
 
+   pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
+  set->fb,
+  &modeset_pipes,
+  &prepare_pipes,
+  &disable_pipes);
+   if (IS_ERR(pipe_config))
+   goto fail;
+
+   /* set_mode will free it in the mode_changed case */
+   if (!config->mode_changed)
+   kfree(pipe_config);
+
if (config->mode_changed) {
-   ret = intel_set_mode(set->crtc, set->mode,
-set->x, set->y, set->fb);
+   ret = intel_set_mode_pipes(set->crtc, set->mode,
+  set->x, set->y, set->fb, pipe_config,
+  modeset_pipes, prepare_pipes,
+  disable_pipes);
} else if (config->fb_changed) {
struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
 
@@ -13198,6 +13214,20 @@ void intel_modeset_setup_hw_state(struct drm_device 
*dev,
for_each_pipe(dev_priv, pipe) {
struct drm_crtc *crtc =
dev_priv->pipe_to_crtc_mapping[pipe];
+   struct intel_crtc_config *pipe_config;
+   unsigned modeset_pipes, prepare_pipes, disable_pipes;
+
+   pipe_config = intel_modeset_compute_config(crtc,
+  &crtc->mode,
+  
crtc->primary->fb,
+  
&modeset_pipes,
+  
&prepare_pipes,
+  
&disable_pipes);
+   if (IS_ERR(pipe_config)) {
+   DRM_DEBUG_KMS("prepare failed\n");
+   kfree(pipe_config);
+   goto out;
+   }
 
intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
   crtc->primary->fb);
@@ -13205,7 +13235,7 @@ void intel_modeset_setup_hw_state(struct drm_device 
*dev,
} else {
intel_modeset_update_staged_output_state(dev);
}
-
+out:
intel_modeset_check_state(dev);
 }
 
-- 
1.9.1

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

[Intel-gfx] [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets v2

[Jesse Barnes]

If these change (e.g. after a modeset following a fastboot), we need to
do a full mode set.

v2:
  - put under pipe_config check so we don't deref a null state (Jesse)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index cb96f11..c86eee6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11230,8 +11230,18 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
   &modeset_pipes,
   &prepare_pipes,
   &disable_pipes);
-   if (IS_ERR(pipe_config))
+   if (IS_ERR(pipe_config)) {
goto fail;
+   } else if (pipe_config) {
+   if (to_intel_crtc(set->crtc)->new_config->has_audio !=
+   to_intel_crtc(set->crtc)->config.has_audio)
+   config->mode_changed = true;
+
+   /* Force mode sets for any infoframe stuff */
+   if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
+   to_intel_crtc(set->crtc)->config.has_infoframe)
+   config->mode_changed = true;
+   }
 
/* set_mode will free it in the mode_changed case */
if (!config->mode_changed)
-- 
1.9.1

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

[Intel-gfx] [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config v2

[Jesse Barnes]

This is useful for checking things later.

v2:
  - fix hsw infoframe enabled check (Ander)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_drv.h  |  4 +++
 drivers/gpu/drm/i915/intel_hdmi.c | 62 +++
 2 files changed, 66 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d53ac23..8aa80e1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -292,6 +292,9 @@ struct intel_crtc_config {
 * between pch encoders and cpu encoders. */
bool has_pch_encoder;
 
+   /* Are we sending infoframes on the attached port */
+   bool has_infoframe;
+
/* CPU Transcoder for the pipe. Currently this can only differ from the
 * pipe on Haswell (where we have a special eDP transcoder). */
enum transcoder cpu_transcoder;
@@ -543,6 +546,7 @@ struct intel_hdmi {
void (*set_infoframes)(struct drm_encoder *encoder,
   bool enable,
   struct drm_display_mode *adjusted_mode);
+   bool (*infoframe_enabled)(struct drm_encoder *encoder);
 };
 
 struct intel_dp_mst_encoder;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 07b5ebd..994237a 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -166,6 +166,15 @@ static void g4x_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(VIDEO_DIP_CTL);
 }
 
+static bool g4x_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   u32 val = I915_READ(VIDEO_DIP_CTL);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void ibx_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -204,6 +213,17 @@ static void ibx_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(reg);
 }
 
+static bool ibx_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
+   u32 val = I915_READ(reg);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void cpt_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -245,6 +265,17 @@ static void cpt_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(reg);
 }
 
+static bool cpt_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
+   u32 val = I915_READ(reg);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void vlv_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -283,6 +314,17 @@ static void vlv_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(reg);
 }
 
+static bool vlv_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   int reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
+   u32 val = I915_READ(reg);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void hsw_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -320,6 +362,18 @@ static void hsw_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(ctl_reg);
 }
 
+static bool hsw_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config.cpu_transcoder);
+   u32 val = I915_READ(ctl_reg);
+
+   return val & (VIDEO_DIP_ENABLE_AVI_HSW | VIDEO_DIP_ENABLE_SPD_HSW |
+ VIDEO_DIP_ENABLE_VS_HSW);
+}
+
 /*
  * The data we write to the DIP data buffer registers is 1 byte bigger than the
  * HDMI infoframe size because of an ECC/reserved byte at position 3 (starting
@@ -732,6 +786,9 @@ static void intel_hdmi_get_config(struct intel_encoder 
*encoder,
if (tmp & HDMI_MOD

Re: [Intel-gfx] [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode v2

[Jesse Barnes]

Thanks for the review, Ander.

Daniel, in case you're tempted to merge this, don't just yet.  Shaung's
test bot found an issue with the refactoring patch I'm still tracking
down.  I'll post a v3 with the fix in reply to this...

Thanks,
Jesse

On Thu, 30 Oct 2014 11:53:59 -0700
Jesse Barnes  wrote:

> This allows us to calculate the full pipe config before we do any mode
> setting work.
> 
> v2:
>   - clarify comments about global vs. per-crtc mode set (Ander)
>   - clean up unnecessary pipe_config = NULL setting (Ander)
> 
> Signed-off-by: Jesse Barnes 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 96
> +--- 1 file changed, 68
> insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c index 6e5bc3c..f9a0963 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10910,45 +10910,61 @@ static void update_scanline_offset(struct
> intel_crtc *crtc) crtc->scanline_offset = 1;
>  }
>  
> +static int intel_modeset_compute_config(struct drm_crtc *crtc,
> + struct drm_display_mode
> *mode,
> + struct drm_framebuffer *fb,
> + unsigned *modeset_pipes,
> + unsigned *prepare_pipes,
> + unsigned *disable_pipes)
> +{
> + struct intel_crtc_config *pipe_config = NULL;
> + int ret = 0;
> +
> + intel_modeset_affected_pipes(crtc, modeset_pipes,
> +  prepare_pipes, disable_pipes);
> +
> + if (!modeset_pipes)
> + goto out;
> +
> + /*
> +  * Note this needs changes when we start tracking multiple
> modes
> +  * and crtcs.  At that point we'll need to compute the whole
> config
> +  * (i.e. one pipe_config for each crtc) rather than just the
> one
> +  * for this crtc.
> +  */
> + pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
> + if (IS_ERR(pipe_config)) {
> + ret = PTR_ERR(pipe_config);
> + goto out;
> + }
> + intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
> +"[modeset]");
> + to_intel_crtc(crtc)->new_config = pipe_config;
> +
> +out:
> + return ret;
> +}
> +
>  static int __intel_set_mode(struct drm_crtc *crtc,
>   struct drm_display_mode *mode,
> - int x, int y, struct drm_framebuffer *fb)
> + int x, int y, struct drm_framebuffer *fb,
> + unsigned disable_pipes,
> + unsigned prepare_pipes,
> + unsigned modeset_pipes)
>  {
>   struct drm_device *dev = crtc->dev;
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   struct drm_display_mode *saved_mode;
>   struct intel_crtc_config *pipe_config = NULL;
>   struct intel_crtc *intel_crtc;
> - unsigned disable_pipes, prepare_pipes, modeset_pipes;
>   int ret = 0;
>  
>   saved_mode = kmalloc(sizeof(*saved_mode), GFP_KERNEL);
>   if (!saved_mode)
>   return -ENOMEM;
>  
> - intel_modeset_affected_pipes(crtc, &modeset_pipes,
> -  &prepare_pipes, &disable_pipes);
> -
>   *saved_mode = crtc->mode;
>  
> - /* Hack: Because we don't (yet) support global modeset on
> multiple
> -  * crtcs, we don't keep track of the new mode for more than
> one crtc.
> -  * Hence simply check whether any bit is set in
> modeset_pipes in all the
> -  * pieces of code that are not yet converted to deal with
> mutliple crtcs
> -  * changing their mode at the same time. */
> - if (modeset_pipes) {
> - pipe_config = intel_modeset_pipe_config(crtc, fb,
> mode);
> - if (IS_ERR(pipe_config)) {
> - ret = PTR_ERR(pipe_config);
> - pipe_config = NULL;
> -
> - goto out;
> - }
> - intel_dump_pipe_config(to_intel_crtc(crtc),
> pipe_config,
> -"[modeset]");
> - to_intel_crtc(crtc)->new_config = pipe_config;
> - }
> -
>   /*
>* See if the config requires any additional preparation,
> e.g.
>* to adjust global state with pipes off.  We need to do this
> @@ -10973,6 +10989,10 @@ static int __intel_set_mode(struct drm_crtc
> *crtc, 
>   /* crtc->mo

Re: [Intel-gfx] i915.fastboot bug report - not working on coreboot

[Jesse Barnes]

On Thu, 23 Oct 2014 16:44:26 -0400
Charles Devereaux  wrote:

> [0.529733] [drm:intel_set_config_compute_mode_changes], modes are
> different, full mode set
> [0.529736] [drm:drm_mode_debug_printmodeline], Modeline 0:"" 0 54167
> 1024 1048 1184 1344 768 771 777 806 0x0 0xa
> [0.529740] [drm:drm_mode_debug_printmodeline], Modeline 11:"1024x768"
> 60 65000 1024 1048 1184 1344 768 771 777 806 0x48 0xa

This looks like the issue.  The BIOS programs a slightly different
1024x768 mode than what the kernel tries to apply.  Looks like reduced
vs non-reduced blanking approximately.

We could adjust the fastboot code to handle that, or change coreboot to
use the preferred mode from the EDID of the display or make the VBT
match, which is presumably what the kernel is using.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/4] drm/i915: preserve swizzle settings if necessary v4

[Jesse Barnes]

On Tue, 21 Oct 2014 16:49:12 +0200
Daniel Vetter  wrote:

> On Thu, Oct 09, 2014 at 12:57:43PM -0700, Jesse Barnes wrote:
> > Some machines (like MBAs) might use a tiled framebuffer but not
> > enable display swizzling at boot time.  We want to preserve that
> > configuration if possible to prevent a boot time mode set.  On IVB+
> > it shouldn't affect performance anyway since the memory controller
> > does internal swizzling anyway.
> > 
> > For most other configs we'll be able to enable swizzling at boot
> > time, since the initial framebuffer won't be tiled, thus we won't
> > see any corruption when we enable it.
> > 
> > v2: preserve swizzling if BIOS had it set (Daniel)
> > v3: preserve swizzling only if we inherited a tiled framebuffer
> > (Daniel) check display swizzle setting in detect_bit_6_swizzle
> > (Daniel) use gen6 as cutoff point (Daniel)
> > v4: fixup swizzle preserve again, had wrong init order (Daniel)
> > 
> > Reported-by: Kristian Høgsberg 
> > Signed-off-by: Jesse Barnes 
> 
> lgtm. Queued for -next, thanks for the patch.

Can you pick up the SSC one too (earlier in this thread)?  I applied
Jani's requested change which should address your concerns about the
BIOS and kernel disagreeing.

I'll ping Kristian again about the not-quite-preferred mode on his MBA.

Thanks,
Jesse
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: don't try using training pattern 3 on pre-haswell

[Jesse Barnes]

On Wed, 29 Oct 2014 11:07:32 +0200
Jani Nikula  wrote:

> On Wed, 29 Oct 2014, Ville Syrjälä  wrote:
> > On Wed, Oct 29, 2014 at 10:23:50AM +0200, Jani Nikula wrote:
> >> On Wed, 29 Oct 2014, Ville Syrjälä  wrote:
> >> > On Wed, Oct 29, 2014 at 05:02:50PM +1000, Dave Airlie wrote:
> >> >> From: Dave Airlie 
> >> >> 
> >> >> Ivybridge + 30" monitor prints a drm error on every modeset, since
> >> >> IVB doesn't support DP3 we should even bother trying to use it.
> >> >> 
> >> >> Reviewed-by: Daniel Vetter  (on irc)
> >> >> Signed-off-by: Dave Airlie 
> >> >> ---
> >> >>  drivers/gpu/drm/i915/intel_dp.c | 4 +++-
> >> >>  1 file changed, 3 insertions(+), 1 deletion(-)
> >> >> 
> >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> >> >> b/drivers/gpu/drm/i915/intel_dp.c
> >> >> index f6a3fdd..87cfb92 100644
> >> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> >> @@ -3547,13 +3547,15 @@ intel_dp_start_link_train(struct intel_dp 
> >> >> *intel_dp)
> >> >>  void
> >> >>  intel_dp_complete_link_train(struct intel_dp *intel_dp)
> >> >>  {
> >> >> +   struct drm_encoder *encoder = 
> >> >> &dp_to_dig_port(intel_dp)->base.base;
> >> >> +   struct drm_device *dev = encoder->dev;
> >> >> bool channel_eq = false;
> >> >> int tries, cr_tries;
> >> >> uint32_t DP = intel_dp->DP;
> >> >> uint32_t training_pattern = DP_TRAINING_PATTERN_2;
> >> >>  
> >> >> /* Training Pattern 3 for HBR2 ot 1.2 devices that support it*/
> >> >> -   if (intel_dp->link_bw == DP_LINK_BW_5_4 || intel_dp->use_tps3)
> >> >> +   if ((intel_dp->link_bw == DP_LINK_BW_5_4 || intel_dp->use_tps3) 
> >> >> && !HAS_DDI(dev))
> >> >
> >> > CHV has pattern 3.
> >> 
> >> Is "supports tps3" the same set of platforms as "supports 5.4 Gbps"? We
> >> should abstract the check from intel_dp_max_link_bw.
> >
> > Not quite. HSW-ULX supports pattern 3 even though it doesn't do 5.4 Gbps.
> 
> How about [1] instead? I forgot --in-reply-to, sorry.
> 
> BR,
> Jani.
> 
> 
> [1] 
> http://mid.gmane.org/1414573406-17071-1-git-send-email-jani.nik...@intel.com

Looks like we need something like that at least, assuming we're not
hitting the link_bw == DP_LINK_BW_5_4 case.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode v2

[Jesse Barnes]

This allows us to calculate the full pipe config before we do any mode
setting work.

v2:
  - clarify comments about global vs. per-crtc mode set (Ander)
  - clean up unnecessary pipe_config = NULL setting (Ander)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 96 +---
 1 file changed, 68 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 6e5bc3c..f9a0963 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10910,45 +10910,61 @@ static void update_scanline_offset(struct intel_crtc 
*crtc)
crtc->scanline_offset = 1;
 }
 
+static int intel_modeset_compute_config(struct drm_crtc *crtc,
+   struct drm_display_mode *mode,
+   struct drm_framebuffer *fb,
+   unsigned *modeset_pipes,
+   unsigned *prepare_pipes,
+   unsigned *disable_pipes)
+{
+   struct intel_crtc_config *pipe_config = NULL;
+   int ret = 0;
+
+   intel_modeset_affected_pipes(crtc, modeset_pipes,
+prepare_pipes, disable_pipes);
+
+   if (!modeset_pipes)
+   goto out;
+
+   /*
+* Note this needs changes when we start tracking multiple modes
+* and crtcs.  At that point we'll need to compute the whole config
+* (i.e. one pipe_config for each crtc) rather than just the one
+* for this crtc.
+*/
+   pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
+   if (IS_ERR(pipe_config)) {
+   ret = PTR_ERR(pipe_config);
+   goto out;
+   }
+   intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
+  "[modeset]");
+   to_intel_crtc(crtc)->new_config = pipe_config;
+
+out:
+   return ret;
+}
+
 static int __intel_set_mode(struct drm_crtc *crtc,
struct drm_display_mode *mode,
-   int x, int y, struct drm_framebuffer *fb)
+   int x, int y, struct drm_framebuffer *fb,
+   unsigned disable_pipes,
+   unsigned prepare_pipes,
+   unsigned modeset_pipes)
 {
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_display_mode *saved_mode;
struct intel_crtc_config *pipe_config = NULL;
struct intel_crtc *intel_crtc;
-   unsigned disable_pipes, prepare_pipes, modeset_pipes;
int ret = 0;
 
saved_mode = kmalloc(sizeof(*saved_mode), GFP_KERNEL);
if (!saved_mode)
return -ENOMEM;
 
-   intel_modeset_affected_pipes(crtc, &modeset_pipes,
-&prepare_pipes, &disable_pipes);
-
*saved_mode = crtc->mode;
 
-   /* Hack: Because we don't (yet) support global modeset on multiple
-* crtcs, we don't keep track of the new mode for more than one crtc.
-* Hence simply check whether any bit is set in modeset_pipes in all the
-* pieces of code that are not yet converted to deal with mutliple crtcs
-* changing their mode at the same time. */
-   if (modeset_pipes) {
-   pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
-   if (IS_ERR(pipe_config)) {
-   ret = PTR_ERR(pipe_config);
-   pipe_config = NULL;
-
-   goto out;
-   }
-   intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
-  "[modeset]");
-   to_intel_crtc(crtc)->new_config = pipe_config;
-   }
-
/*
 * See if the config requires any additional preparation, e.g.
 * to adjust global state with pipes off.  We need to do this
@@ -10973,6 +10989,10 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 
/* crtc->mode is already used by the ->mode_set callbacks, hence we need
 * to set it here already despite that we pass it down the callchain.
+*
+* Note we'll need to fix this up when we start tracking multiple
+* pipes; here we assume a single modeset_pipe and only track the
+* single crtc and mode.
 */
if (modeset_pipes) {
crtc->mode = *mode;
@@ -11042,19 +11062,22 @@ done:
if (ret && crtc->enabled)
crtc->mode = *saved_mode;
 
-out:
kfree(pipe_config);
kfree(saved_mode);
return ret;
 }
 
-static int intel_set_mode(struct drm_crtc *crtc,
- struct drm_display_mode *mode,
- 

[Intel-gfx] [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets

[Jesse Barnes]

If these change (e.g. after a modeset following a fastboot), we need to
do a full mode set.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 0f1952e..2199a9a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11482,6 +11482,16 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
if (ret)
goto fail;
 
+   if (to_intel_crtc(set->crtc)->new_config->has_audio !=
+   to_intel_crtc(set->crtc)->config.has_audio)
+   config->mode_changed = true;
+
+   /* Force mode sets for any infoframe stuff */
+   if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
+   to_intel_crtc(set->crtc)->config.has_infoframe)
+   config->mode_changed = true;
+
+
if (config->mode_changed) {
ret = intel_set_mode_pipes(set->crtc, set->mode,
   set->x, set->y, set->fb,
-- 
1.9.1

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

[Intel-gfx] [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config v2

[Jesse Barnes]

This is useful for checking things later.

v2:
  - fix hsw infoframe enabled check (Ander)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_drv.h  |  4 +++
 drivers/gpu/drm/i915/intel_hdmi.c | 62 +++
 2 files changed, 66 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5ab813c..cdace63 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -290,6 +290,9 @@ struct intel_crtc_config {
 * between pch encoders and cpu encoders. */
bool has_pch_encoder;
 
+   /* Are we sending infoframes on the attached port */
+   bool has_infoframe;
+
/* CPU Transcoder for the pipe. Currently this can only differ from the
 * pipe on Haswell (where we have a special eDP transcoder). */
enum transcoder cpu_transcoder;
@@ -541,6 +544,7 @@ struct intel_hdmi {
void (*set_infoframes)(struct drm_encoder *encoder,
   bool enable,
   struct drm_display_mode *adjusted_mode);
+   bool (*infoframe_enabled)(struct drm_encoder *encoder);
 };
 
 struct intel_dp_mst_encoder;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 8b5f3aa..d44c499 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -166,6 +166,15 @@ static void g4x_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(VIDEO_DIP_CTL);
 }
 
+static bool g4x_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   u32 val = I915_READ(VIDEO_DIP_CTL);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void ibx_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -204,6 +213,17 @@ static void ibx_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(reg);
 }
 
+static bool ibx_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
+   u32 val = I915_READ(reg);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void cpt_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -245,6 +265,17 @@ static void cpt_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(reg);
 }
 
+static bool cpt_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
+   u32 val = I915_READ(reg);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void vlv_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -283,6 +314,17 @@ static void vlv_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(reg);
 }
 
+static bool vlv_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   int reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
+   u32 val = I915_READ(reg);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void hsw_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -320,6 +362,18 @@ static void hsw_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(ctl_reg);
 }
 
+static bool hsw_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config.cpu_transcoder);
+   u32 val = I915_READ(ctl_reg);
+
+   return val & (VIDEO_DIP_ENABLE_AVI_HSW | VIDEO_DIP_ENABLE_SPD_HSW |
+ VIDEO_DIP_ENABLE_VS_HSW);
+}
+
 /*
  * The data we write to the DIP data buffer registers is 1 byte bigger than the
  * HDMI infoframe size because of an ECC/reserved byte at position 3 (starting
@@ -732,6 +786,9 @@ static void intel_hdmi_get_config(struct intel_encoder 
*encoder,
if (tmp & HDMI_MOD

[Intel-gfx] [PATCH 5/6] drm/i915: update pipe size at set_config time

[Jesse Barnes]

This only affects the fastboot path as-is.  In that case, we simply need
to make sure that we update the pipe size at the first mode set.  Rather
than putting it off until we decide to flip (if indeed we do end up
flipping), update the pipe size as appropriate a bit earlier in the
set_config call.

This sets us up for better pipe tracking in later patches.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 2199a9a..470cdac 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2902,8 +2902,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
return ret;
}
 
-   intel_update_pipe_size(intel_crtc);
-
dev_priv->display.update_primary_plane(crtc, fb, x, y);
 
if (intel_crtc->active)
@@ -11491,6 +11489,7 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
to_intel_crtc(set->crtc)->config.has_infoframe)
config->mode_changed = true;
 
+   intel_update_pipe_size(to_intel_crtc(set->crtc));
 
if (config->mode_changed) {
ret = intel_set_mode_pipes(set->crtc, set->mode,
-- 
1.9.1

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

[Intel-gfx] [PATCH 2/6] drm/i915: use compute_config in set_config v2

[Jesse Barnes]

This will allow us to consult more info before deciding whether to flip
or do a full mode set.

v2:
  - don't use uninitialized or incorrect pipe masks in set_config
failure path (Ander)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 30 ++
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f9a0963..0f1952e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11430,6 +11430,7 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
struct drm_device *dev;
struct drm_mode_set save_set;
struct intel_set_config *config;
+   unsigned modeset_pipes, prepare_pipes, disable_pipes;
int ret;
 
BUG_ON(!set);
@@ -11475,9 +11476,17 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
if (ret)
goto fail;
 
+   ret = intel_modeset_compute_config(set->crtc, set->mode, set->fb,
+  &modeset_pipes, &prepare_pipes,
+  &disable_pipes);
+   if (ret)
+   goto fail;
+
if (config->mode_changed) {
-   ret = intel_set_mode(set->crtc, set->mode,
-set->x, set->y, set->fb);
+   ret = intel_set_mode_pipes(set->crtc, set->mode,
+  set->x, set->y, set->fb,
+  modeset_pipes, prepare_pipes,
+  disable_pipes);
} else if (config->fb_changed) {
struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
 
@@ -13412,14 +13421,27 @@ void intel_modeset_setup_hw_state(struct drm_device 
*dev,
for_each_pipe(dev_priv, pipe) {
struct drm_crtc *crtc =
dev_priv->pipe_to_crtc_mapping[pipe];
+   unsigned modeset_pipes, prepare_pipes, disable_pipes;
+   int ret;
+
+   ret = intel_modeset_compute_config(crtc, &crtc->mode,
+  crtc->primary->fb,
+  &modeset_pipes,
+  &prepare_pipes,
+  &disable_pipes);
+   if (ret) {
+   DRM_DEBUG_KMS("prepare failed\n");
+   goto out;
+   }
 
__intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
-crtc->primary->fb);
+crtc->primary->fb, modeset_pipes,
+prepare_pipes, disable_pipes);
}
} else {
intel_modeset_update_staged_output_state(dev);
}
-
+out:
intel_modeset_check_state(dev);
 }
 
-- 
1.9.1

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

[Intel-gfx] [PATCH 6/6] drm/i915: calculate pfit changes in set_config v2

[Jesse Barnes]

This should allow us to avoid mode sets for some panel fitter config
changes.

v2:
  - fixup pfit comment (Ander)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 61 +---
 1 file changed, 50 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 470cdac..56154a1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2831,17 +2831,8 @@ static void intel_update_pipe_size(struct intel_crtc 
*crtc)
return;
 
/*
-* Update pipe size and adjust fitter if needed: the reason for this is
-* that in compute_mode_changes we check the native mode (not the pfit
-* mode) to see if we can flip rather than do a full mode set. In the
-* fastboot case, we'll flip, but if we don't update the pipesrc and
-* pfit state, we'll end up with a big fb scanned out into the wrong
-* sized surface.
-*
-* To fix this properly, we need to hoist the checks up into
-* compute_mode_changes (or above), check the actual pfit state and
-* whether the platform allows pfit disable with pipe active, and only
-* then update the pipesrc and pfit state, even on the flip path.
+* See intel_pfit_changed for info on when we're allowed to
+* do this w/o a pipe shutdown.
 */
 
adjusted_mode = &crtc->config.adjusted_mode;
@@ -11423,6 +11414,50 @@ static void disable_crtc_nofb(struct intel_crtc *crtc)
crtc->new_config = NULL;
 }
 
+/* Do we need a mode set due to pfit changes? */
+static bool intel_pfit_changed(struct drm_device *dev,
+  struct intel_crtc_config *new_config,
+  struct intel_crtc_config *cur_config)
+{
+   bool ret = false;
+
+   if (HAS_DDI(dev) || HAS_PCH_SPLIT(dev)) {
+   /*
+* On PCH platforms we can disable pfit w/o a pipe shutdown,
+* otherwise we'll need a mode set.
+*/
+   if (new_config->pch_pfit.enabled &&
+   cur_config->pch_pfit.enabled)
+   ret = false;
+   else if (new_config->pch_pfit.enabled &&
+!cur_config->pch_pfit.enabled)
+   ret = true;
+   else if (!new_config->pch_pfit.enabled &&
+cur_config->pch_pfit.enabled)
+   ret = false;
+   else if (!new_config->pch_pfit.enabled &&
+!cur_config->pch_pfit.enabled)
+   ret = false;
+   } else {
+   bool new_enabled, old_enabled;
+
+   new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE);
+   old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE);
+
+   /* 9xx only needs a shutdown to disable pfit */
+   if (new_enabled && old_enabled)
+   ret = false;
+   else if (new_enabled && !old_enabled)
+   ret = false;
+   else if (!new_enabled && old_enabled)
+   ret = true;
+   else if (!new_enabled && !old_enabled)
+   ret = false;
+   }
+
+   return ret;
+}
+
 static int intel_crtc_set_config(struct drm_mode_set *set)
 {
struct drm_device *dev;
@@ -11489,6 +11524,10 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
to_intel_crtc(set->crtc)->config.has_infoframe)
config->mode_changed = true;
 
+   if (intel_pfit_changed(dev, to_intel_crtc(set->crtc)->new_config,
+  &to_intel_crtc(set->crtc)->config))
+   config->mode_changed = true;
+
intel_update_pipe_size(to_intel_crtc(set->crtc));
 
if (config->mode_changed) {
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config

[Jesse Barnes]

On Thu, 30 Oct 2014 15:20:43 +0200
Ander Conselvan de Oliveira  wrote:

> On 10/23/2014 09:50 PM, Jesse Barnes wrote:
> > This is useful for checking things later.
> >
> > Signed-off-by: Jesse Barnes 
> > ---
> >   drivers/gpu/drm/i915/intel_drv.h  |  4 +++
> >   drivers/gpu/drm/i915/intel_hdmi.c | 61
> > +++ 2 files changed, 65
> > insertions(+)
> >
> 
> [...]
> 
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > b/drivers/gpu/drm/i915/intel_hdmi.c index 8b5f3aa..75efe4c 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> 
> [...]
> 
> > @@ -320,6 +362,17 @@ static void hsw_write_infoframe(struct
> > drm_encoder *encoder, POSTING_READ(ctl_reg);
> >   }
> >
> > +static bool hsw_infoframe_enabled(struct drm_encoder *encoder)
> > +{
> > +   struct drm_device *dev = encoder->dev;
> > +   struct drm_i915_private *dev_priv = dev->dev_private;
> > +   struct intel_crtc *intel_crtc =
> > to_intel_crtc(encoder->crtc);
> > +   u32 ctl_reg =
> > HSW_TVIDEO_DIP_CTL(intel_crtc->config.cpu_transcoder);
> > +   u32 val = I915_READ(ctl_reg);
> > +
> > +   return val & VIDEO_DIP_ENABLE;
> 
> Haswell docs list bit 31 of VIDEO_DIP_CTL as reserved. Looking at 
> hsw_set_infoframe(), it seems that you would have to test for the
> enable bits for the different infoframe types instead.

Ah right, fixed.

Thanks,
Jesse
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/6] drm/i915: use compute_config in set_config

[Jesse Barnes]

On Wed, 29 Oct 2014 16:31:56 +0200
Ander Conselvan de Oliveira  wrote:

> On 10/23/2014 09:50 PM, Jesse Barnes wrote:
> > This will allow us to consult more info before deciding whether to
> > flip or do a full mode set.
> >
> > Signed-off-by: Jesse Barnes 
> > ---
> >   drivers/gpu/drm/i915/intel_display.c | 36
> > ++-- 1 file changed, 30
> > insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c index d5f95e4..e031ee8 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11427,6 +11427,7 @@ static int intel_crtc_set_config(struct
> > drm_mode_set *set) struct drm_device *dev;
> > struct drm_mode_set save_set;
> > struct intel_set_config *config;
> > +   unsigned modeset_pipes, prepare_pipes, disable_pipes;
> > int ret;
> >
> > BUG_ON(!set);
> > @@ -11472,9 +11473,17 @@ static int intel_crtc_set_config(struct
> > drm_mode_set *set) if (ret)
> > goto fail;
> 
> If this fails ...
> 
> >
> > +   ret = intel_modeset_compute_config(set->crtc, set->mode,
> > set->fb,
> > +  &modeset_pipes,
> > &prepare_pipes,
> > +  &disable_pipes);
> > +   if (ret)
> > +   goto fail;
> > +
> > if (config->mode_changed) {
> > -   ret = intel_set_mode(set->crtc, set->mode,
> > -set->x, set->y, set->fb);
> > +   ret = intel_set_mode_pipes(set->crtc, set->mode,
> > +  set->x, set->y, set->fb,
> > +  modeset_pipes,
> > prepare_pipes,
> > +  disable_pipes);
> > } else if (config->fb_changed) {
> > struct intel_crtc *intel_crtc =
> > to_intel_crtc(set->crtc);
> >
> > @@ -11521,8 +11530,10 @@ fail:
> >
> > /* Try to restore the config */
> > if (config->mode_changed &&
> > -   intel_set_mode(save_set.crtc, save_set.mode,
> > -  save_set.x, save_set.y,
> > save_set.fb))
> > +   intel_set_mode_pipes(save_set.crtc,
> > save_set.mode,
> > +save_set.x, save_set.y,
> > save_set.fb,
> > +modeset_pipes,
> > prepare_pipes,
> > +disable_pipes))
> 
> ... we end up here, with *_pipes uninitialized. And in any case, they 
> were computed for the new mode, not the saved one.

Yeah I should just be using the old intel_set_mode() here on the
failure path.  That should calculate a new set of pipes and the
appropriate config and apply it.  Fixed.

Thanks,
Jesse
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode

[Jesse Barnes]

On Wed, 29 Oct 2014 16:30:43 +0200
Ander Conselvan de Oliveira  wrote:

> On 10/23/2014 09:50 PM, Jesse Barnes wrote:
> > This allows us to calculate the full pipe config before we do any
> > mode setting work.
> >
> > Signed-off-by: Jesse Barnes 
> > ---
> >   drivers/gpu/drm/i915/intel_display.c | 93
> > +--- 1 file changed, 65
> > insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c index 6e5bc3c..d5f95e4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10910,45 +10910,62 @@ static void update_scanline_offset(struct
> > intel_crtc *crtc) crtc->scanline_offset = 1;
> >   }
> >
> > +static int intel_modeset_compute_config(struct drm_crtc *crtc,
> 
> s/drm_crtc/intel_crtc/

Since 2/3 of the functions this one calls take a drm_crtc I figured I'd
leave it for a big cleanup patch (maybe cocci could do it).

> > +   /* Hack: Because we don't (yet) support global modeset on
> > multiple
> > +* crtcs, we don't keep track of the new mode for more
> > than one crtc.
> > +* Hence simply check whether any bit is set in
> > modeset_pipes in all the
> > +* pieces of code that are not yet converted to deal with
> > mutliple crtcs
> > +* changing their mode at the same time. */
> 
> This comment seems out of place here since it refers to checking for
> set bits in modeset_pipes. I guess the important part of it is to
> explain the fact that only one new pipe config is obtained here, so
> maybe it can reworded to make that clearer.
> 
> And there's another "if (modeset_pipes)" left in __intel_set_mode()
> to which this comment applies.

I tried to clear these up a bit, hopefully I didn't make things worse.

> 
> > +   pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
> > +   if (IS_ERR(pipe_config)) {
> > +   ret = PTR_ERR(pipe_config);
> > +   pipe_config = NULL;
> > +
> > +   goto out;
> 
> Nothing will use pipe_config after this point, so there's no need to
> set it to NULL.

Yep, fixed.

> 
> > +   }
> > +   intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
> > +  "[modeset]");
> > +   to_intel_crtc(crtc)->new_config = pipe_config;
> > +
> > +out:
> > +   return ret;
> 
> There's nothing being done here, so we could avoid this goto dance
> and just use returns instead.

Yeah I just tend to use the "return only in one place" style, but I
have no strong preference if someone wants to change it.

> 
> Anyway, mostly nit picks, so either way,
> 
> Reviewed-by: Ander Conselvan de Oliveira 
> 

Thanks, I'll post v2 shortly.

Jesse

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

Re: [Intel-gfx] [PATCH] drm/i915: Enable pipe-a power well on chv

[Jesse Barnes]

On Mon, 27 Oct 2014 16:07:32 +0200
ville.syrj...@linux.intel.com wrote:

> From: Ville Syrjälä 
> 
> It seems that the pipe-a power well has replaced the disp2d power well
> on chv. At least that's the case with the current punit firmware. So
> enable the pipe-a power and expand its domains to cover everything the
> disp2d well ought to cover.
> 
> The other power wells (apart from the cmnlane wells) still seem awol
> in the current punit firmware. So leave them disabled in the code.
> 
> This fixes a hilarious oops during resume on bsw where
> intel_hdmi_get_config() would read the port register and get back
> 0x and thus think the port is enabled on pipe D. It would then
> go and index the pipe_to_crtc_mapping[] array with PIPE_D and blow up
> when intel_hdmi_get_config() tries to write to crtc->config. Someone
> really ought to replace all naked pipe_to_crtc_mapping[] uses with the
> appropriate function call so we could add a warning there if the pipe
> doesn't actually exist...
> 
> We must also call the power seqeuencer state reset function from
> the pipe-a well disable just like we do from disp2d on vlv. Otherwise
> the eDP panel won't recover at resume time since the PPS has lost its
> hold on the port.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84903
> Signed-off-by: Ville Syrjälä 
> ---
> Note that this depends on my earlier power sequencer series.
> 
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c index a44046f..fdf68a1
> 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -629,6 +629,9 @@ static void chv_pipe_power_well_disable(struct
> drm_i915_private *dev_priv, power_well->data != PIPE_C);
>  
>   chv_set_pipe_power_well(dev_priv, power_well, false);
> +
> + if (power_well->data == PIPE_A)
> + vlv_power_sequencer_reset(dev_priv);
>  }
>  
>  static void check_power_well_state(struct drm_i915_private *dev_priv,
> @@ -980,12 +983,20 @@ static struct i915_power_well chv_power_wells[]
> = { .data = PUNIT_POWER_WELL_DISP2D,
>   .ops = &vlv_display_power_well_ops,
>   },
> +#endif
>   {
>   .name = "pipe-a",
> - .domains = CHV_PIPE_A_POWER_DOMAINS,
> + /*
> +  * FIXME: pipe A power well seems to be the new
> disp2d well.
> +  * At least all registers seem to be housed there.
> Figure
> +  * out if this a a temporary situation in
> pre-production
> +  * hardware or a permanent state of affairs.
> +  */
> + .domains = CHV_PIPE_A_POWER_DOMAINS |
> VLV_DISPLAY_POWER_DOMAINS, .data = PIPE_A,
>   .ops = &chv_pipe_power_well_ops,
>   },
> +#if 0
>   {
>   .name = "pipe-b",
>   .domains = CHV_PIPE_B_POWER_DOMAINS,

Moar power wells.

Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/5] drm/i915: Do vlv cmnlane toggle w/a in more cases

[Jesse Barnes]

On Thu, 16 Oct 2014 20:52:33 +0300
ville.syrj...@linux.intel.com wrote:

> From: Ville Syrjälä 
> 
> In case the cmnlane power well is down but cmnreset isn't asserted we
> would currently skip the off+on toggle for the power well. That could
> leave cmnreset deasserted while cmnlane is powered down which might
> lead to problems with the PHY.
> 
> To avoid such issues skip the cmnlane toggle only if both cmnlane and
> disp2d wells are up and cmnreset is already deasserted. In all other
> cases power down the cmnlane well which will also make sure cmnreset
> gets asserted correctly while cmnlane is powered down.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c index 36749b9..f6b4e8d
> 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1137,12 +1137,9 @@ static void vlv_cmnlane_wa(struct
> drm_i915_private *dev_priv) struct i915_power_well *disp2d =
>   lookup_power_well(dev_priv, PUNIT_POWER_WELL_DISP2D);
>  
> - /* nothing to do if common lane is already off */
> - if (!cmn->ops->is_enabled(dev_priv, cmn))
> - return;
> -
>   /* If the display might be already active skip this */
> - if (disp2d->ops->is_enabled(dev_priv, disp2d) &&
> + if (cmn->ops->is_enabled(dev_priv, cmn) &&
> + disp2d->ops->is_enabled(dev_priv, disp2d) &&
>   I915_READ(DPIO_CTL) & DPIO_CMNRST)
>   return;
>  


Yeah looks ok.  Do we have any bugs we know this fixes?  I'm hoping the
remaining VLV DP training failures are fixed either by something like
this or your panel power sequencer fixes.

Reviewed-by: Jesse Barnes 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Move flags describing VMA mappings into the VMA

[Jesse Barnes]

On Fri, 24 Oct 2014 16:44:32 +0200
Daniel Vetter  wrote:

> On Fri, Oct 24, 2014 at 04:43:08PM +0200, Daniel Vetter wrote:
> > On Fri, Oct 24, 2014 at 01:09:08PM +0100, Chris Wilson wrote:
> > > On Fri, Oct 24, 2014 at 12:42:33PM +0100, Tvrtko Ursulin wrote:
> > > > From: Tvrtko Ursulin 
> > > > 
> > > > If these flags are on the object level it will be more difficult to 
> > > > allow
> > > > for multiple VMAs per object.
> > > > 
> > > > v2: Simplification and cleanup after code review comments (Chris 
> > > > Wilson).
> > > > 
> > > > Signed-off-by: Tvrtko Ursulin 
> > > > Cc: Chris Wilson 
> > > Reviewed-by: Chris Wilson 
> > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
> > > > b/drivers/gpu/drm/i915/i915_gpu_error.c
> > > > index 053d99e..80f0c80 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > > > @@ -567,6 +567,7 @@ i915_error_object_create(struct drm_i915_private 
> > > > *dev_priv,
> > > >  struct i915_address_space *vm)
> > > >  {
> > > > struct drm_i915_error_object *dst;
> > > > +   struct i915_vma *vma = NULL;
> > > > int num_pages;
> > > > bool use_ggtt;
> > > > int i = 0;
> > > > @@ -587,16 +588,17 @@ i915_error_object_create(struct drm_i915_private 
> > > > *dev_priv,
> > > > dst->gtt_offset = -1;
> > > >  
> > > > reloc_offset = dst->gtt_offset;
> > > > +   if (i915_is_ggtt(vm))
> > > > +   vma = i915_gem_obj_to_ggtt(src);
> > > > use_ggtt = (src->cache_level == I915_CACHE_NONE &&
> > > > -   i915_is_ggtt(vm) &&
> > > > -   src->has_global_gtt_mapping &&
> > > > -   reloc_offset + num_pages * PAGE_SIZE <= 
> > > > dev_priv->gtt.mappable_end);
> > > > +  vma && (vma->bound & GLOBAL_BIND) &&
> > > > +  reloc_offset + num_pages * PAGE_SIZE <= 
> > > > dev_priv->gtt.mappable_end);
> > > >  
> > > > /* Cannot access stolen address directly, try to use the 
> > > > aperture */
> > > > if (src->stolen) {
> > > > use_ggtt = true;
> > > >  
> > > > -   if (!src->has_global_gtt_mapping)
> > > > +   if (!(vma && vma->bound & GLOBAL_BIND))
> > > > goto unwind;
> > > 
> > > This looked odd as I have the vma being passed in:
> > > 
> > > static struct drm_i915_error_object *
> > > i915_error_object_create(struct drm_i915_private *dev_priv,
> > >  struct i915_vma *vma);
> > > 
> > > but I presume the ppgtt error-capture is still outstanding.
> > 
> > Oh meh, this is probably one of those tasks that I've asked Jesse to close
> > in one of the "fuck this all" rage-quits in the past few months ... Indeed
> > both code and testcase seem to not have materialized afaics.
> > 
> > Jesse?
> 
> Actually cc Jesse ...

The only thing I'm aware of here is this task:
https://jira01.devtools.intel.com/browse/VIZ-4123

which Mika already closed.  If there's still some breakage, do we have
bugs open on them?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/5] drm/i915: factor compute_config out of __intel_mode_set

[Jesse Barnes]

This allows us to calculate the full pipe config before we do any mode
setting work.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 40 ++--
 1 file changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index a9c1c32..6aec3ae 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10976,21 +10976,18 @@ out:
 
 static int __intel_set_mode(struct drm_crtc *crtc,
struct drm_display_mode *mode,
-   int x, int y, struct drm_framebuffer *fb)
+   int x, int y, struct drm_framebuffer *fb,
+   unsigned disable_pipes,
+   unsigned prepare_pipes,
+   unsigned modeset_pipes)
 {
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_display_mode *saved_mode;
struct intel_crtc_config *pipe_config = NULL;
struct intel_crtc *intel_crtc;
-   unsigned disable_pipes, prepare_pipes, modeset_pipes;
int ret = 0;
 
-   ret = intel_modeset_compute_config(crtc, mode, fb, &modeset_pipes,
-  &prepare_pipes, &disable_pipes);
-   if (ret)
-   return ret;
-
saved_mode = kmalloc(sizeof(*saved_mode), GFP_KERNEL);
if (!saved_mode)
return -ENOMEM;
@@ -11095,13 +11092,17 @@ done:
return ret;
 }
 
-static int intel_set_mode(struct drm_crtc *crtc,
- struct drm_display_mode *mode,
- int x, int y, struct drm_framebuffer *fb)
+static int intel_set_mode_pipes(struct drm_crtc *crtc,
+   struct drm_display_mode *mode,
+   int x, int y, struct drm_framebuffer *fb,
+   unsigned modeset_pipes,
+   unsigned prepare_pipes,
+   unsigned disable_pipes)
 {
int ret;
 
-   ret = __intel_set_mode(crtc, mode, x, y, fb);
+   ret = __intel_set_mode(crtc, mode, x, y, fb, modeset_pipes,
+  prepare_pipes, disable_pipes);
 
if (ret == 0)
intel_modeset_check_state(crtc->dev);
@@ -11109,6 +0,23 @@ static int intel_set_mode(struct drm_crtc *crtc,
return ret;
 }
 
+static int intel_set_mode(struct drm_crtc *crtc,
+ struct drm_display_mode *mode,
+ int x, int y, struct drm_framebuffer *fb)
+{
+   unsigned modeset_pipes, prepare_pipes, disable_pipes;
+   int ret;
+
+   ret = intel_modeset_compute_config(crtc, mode, fb,
+  &modeset_pipes, &prepare_pipes,
+  &disable_pipes);
+   if (ret)
+   return ret;
+
+   return intel_set_mode_pipes(crtc, mode, x, y, fb, modeset_pipes,
+   prepare_pipes, disable_pipes);
+}
+
 void intel_crtc_restore_mode(struct drm_crtc *crtc)
 {
intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->primary->fb);
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH 3/4] drm: add drm_mode_same_size function

[Jesse Barnes]

On Tue, 21 Oct 2014 16:49:37 +0200
Daniel Vetter  wrote:

> On Thu, Oct 09, 2014 at 12:57:44PM -0700, Jesse Barnes wrote:
> > From: Kristian Høgsberg 
> > 
> > Like mode_equal but w/o the clock checks.  Useful for checking if modes
> > are close enough to re-use to avoid a boot time mode set for example.
> > 
> > Signed-off-by: Kristian Høgsberg 
> > Signed-off-by: Jesse Barnes 
> > ---
> >  drivers/gpu/drm/drm_modes.c | 8 
> >  include/drm/drm_modes.h | 2 ++
> >  2 files changed, 10 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index d1b7d20..51ede38 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -905,6 +905,14 @@ bool drm_mode_equal_no_clocks_no_stereo(const struct 
> > drm_display_mode *mode1,
> >  }
> >  EXPORT_SYMBOL(drm_mode_equal_no_clocks_no_stereo);
> >  
> 
> kerneldoc is missing here.

wtf I'm having deja vu here.  I can't even claim it was a failure to
"git add"... I just looked at it and then didn't do it.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/6] drm/i915: use compute_config in set_config

[Jesse Barnes]

This will allow us to consult more info before deciding whether to flip
or do a full mode set.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 36 ++--
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index d5f95e4..e031ee8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11427,6 +11427,7 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
struct drm_device *dev;
struct drm_mode_set save_set;
struct intel_set_config *config;
+   unsigned modeset_pipes, prepare_pipes, disable_pipes;
int ret;
 
BUG_ON(!set);
@@ -11472,9 +11473,17 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
if (ret)
goto fail;
 
+   ret = intel_modeset_compute_config(set->crtc, set->mode, set->fb,
+  &modeset_pipes, &prepare_pipes,
+  &disable_pipes);
+   if (ret)
+   goto fail;
+
if (config->mode_changed) {
-   ret = intel_set_mode(set->crtc, set->mode,
-set->x, set->y, set->fb);
+   ret = intel_set_mode_pipes(set->crtc, set->mode,
+  set->x, set->y, set->fb,
+  modeset_pipes, prepare_pipes,
+  disable_pipes);
} else if (config->fb_changed) {
struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
 
@@ -11521,8 +11530,10 @@ fail:
 
/* Try to restore the config */
if (config->mode_changed &&
-   intel_set_mode(save_set.crtc, save_set.mode,
-  save_set.x, save_set.y, save_set.fb))
+   intel_set_mode_pipes(save_set.crtc, save_set.mode,
+save_set.x, save_set.y, save_set.fb,
+modeset_pipes, prepare_pipes,
+disable_pipes))
DRM_ERROR("failed to restore config after modeset 
failure\n");
}
 
@@ -13409,14 +13420,27 @@ void intel_modeset_setup_hw_state(struct drm_device 
*dev,
for_each_pipe(dev_priv, pipe) {
struct drm_crtc *crtc =
dev_priv->pipe_to_crtc_mapping[pipe];
+   unsigned modeset_pipes, prepare_pipes, disable_pipes;
+   int ret;
+
+   ret = intel_modeset_compute_config(crtc, &crtc->mode,
+  crtc->primary->fb,
+  &modeset_pipes,
+  &prepare_pipes,
+  &disable_pipes);
+   if (ret) {
+   DRM_DEBUG_KMS("prepare failed\n");
+   goto out;
+   }
 
__intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
-crtc->primary->fb);
+crtc->primary->fb, modeset_pipes,
+prepare_pipes, disable_pipes);
}
} else {
intel_modeset_update_staged_output_state(dev);
}
-
+out:
intel_modeset_check_state(dev);
 }
 
-- 
1.9.1

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

[Intel-gfx] [PATCH 5/6] drm/i915: update pipe size at set_config time

[Jesse Barnes]

This only affects the fastboot path as-is.  In that case, we simply need
to make sure that we update the pipe size at the first mode set.  Rather
than putting it off until we decide to flip (if indeed we do end up
flipping), update the pipe size as appropriate a bit earlier in the
set_config call.

This sets us up for better pipe tracking in later patches.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 235933a..10468a7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2902,8 +2902,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
return ret;
}
 
-   intel_update_pipe_size(intel_crtc);
-
dev_priv->display.update_primary_plane(crtc, fb, x, y);
 
if (intel_crtc->active)
@@ -11488,6 +11486,7 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
to_intel_crtc(set->crtc)->config.has_infoframe)
config->mode_changed = true;
 
+   intel_update_pipe_size(to_intel_crtc(set->crtc));
 
if (config->mode_changed) {
ret = intel_set_mode_pipes(set->crtc, set->mode,
-- 
1.9.1

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

[Intel-gfx] [PATCH 4/6] drm/i915: check for audio and infoframe changes across mode sets

[Jesse Barnes]

If these change (e.g. after a modeset following a fastboot), we need to
do a full mode set.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index e031ee8..235933a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11479,6 +11479,16 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
if (ret)
goto fail;
 
+   if (to_intel_crtc(set->crtc)->new_config->has_audio !=
+   to_intel_crtc(set->crtc)->config.has_audio)
+   config->mode_changed = true;
+
+   /* Force mode sets for any infoframe stuff */
+   if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
+   to_intel_crtc(set->crtc)->config.has_infoframe)
+   config->mode_changed = true;
+
+
if (config->mode_changed) {
ret = intel_set_mode_pipes(set->crtc, set->mode,
   set->x, set->y, set->fb,
-- 
1.9.1

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

[Intel-gfx] [PATCH 3/6] drm/i915/hdmi: fetch infoframe status in get_config

[Jesse Barnes]

This is useful for checking things later.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_drv.h  |  4 +++
 drivers/gpu/drm/i915/intel_hdmi.c | 61 +++
 2 files changed, 65 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5ab813c..cdace63 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -290,6 +290,9 @@ struct intel_crtc_config {
 * between pch encoders and cpu encoders. */
bool has_pch_encoder;
 
+   /* Are we sending infoframes on the attached port */
+   bool has_infoframe;
+
/* CPU Transcoder for the pipe. Currently this can only differ from the
 * pipe on Haswell (where we have a special eDP transcoder). */
enum transcoder cpu_transcoder;
@@ -541,6 +544,7 @@ struct intel_hdmi {
void (*set_infoframes)(struct drm_encoder *encoder,
   bool enable,
   struct drm_display_mode *adjusted_mode);
+   bool (*infoframe_enabled)(struct drm_encoder *encoder);
 };
 
 struct intel_dp_mst_encoder;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 8b5f3aa..75efe4c 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -166,6 +166,15 @@ static void g4x_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(VIDEO_DIP_CTL);
 }
 
+static bool g4x_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   u32 val = I915_READ(VIDEO_DIP_CTL);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void ibx_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -204,6 +213,17 @@ static void ibx_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(reg);
 }
 
+static bool ibx_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
+   u32 val = I915_READ(reg);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void cpt_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -245,6 +265,17 @@ static void cpt_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(reg);
 }
 
+static bool cpt_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
+   u32 val = I915_READ(reg);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void vlv_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -283,6 +314,17 @@ static void vlv_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(reg);
 }
 
+static bool vlv_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   int reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
+   u32 val = I915_READ(reg);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void hsw_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -320,6 +362,17 @@ static void hsw_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(ctl_reg);
 }
 
+static bool hsw_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config.cpu_transcoder);
+   u32 val = I915_READ(ctl_reg);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 /*
  * The data we write to the DIP data buffer registers is 1 byte bigger than the
  * HDMI infoframe size because of an ECC/reserved byte at position 3 (starting
@@ -732,6 +785,9 @@ static void intel_hdmi_get_config(struct intel_encoder 
*encoder,
if (tmp & HDMI_MODE_SELECT_HDMI)
pipe_config->has_hdmi_sink = true;
 
+   if (intel_hdmi->infoframe_enabled(&encoder->base

[Intel-gfx] [PATCH 1/6] drm/i915: factor out compute_config from __intel_set_mode

[Jesse Barnes]

This allows us to calculate the full pipe config before we do any mode
setting work.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 93 +---
 1 file changed, 65 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 6e5bc3c..d5f95e4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10910,45 +10910,62 @@ static void update_scanline_offset(struct intel_crtc 
*crtc)
crtc->scanline_offset = 1;
 }
 
+static int intel_modeset_compute_config(struct drm_crtc *crtc,
+   struct drm_display_mode *mode,
+   struct drm_framebuffer *fb,
+   unsigned *modeset_pipes,
+   unsigned *prepare_pipes,
+   unsigned *disable_pipes)
+{
+   struct intel_crtc_config *pipe_config = NULL;
+   int ret = 0;
+
+   intel_modeset_affected_pipes(crtc, modeset_pipes,
+prepare_pipes, disable_pipes);
+
+   if (!modeset_pipes)
+   goto out;
+
+   /* Hack: Because we don't (yet) support global modeset on multiple
+* crtcs, we don't keep track of the new mode for more than one crtc.
+* Hence simply check whether any bit is set in modeset_pipes in all the
+* pieces of code that are not yet converted to deal with mutliple crtcs
+* changing their mode at the same time. */
+   pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
+   if (IS_ERR(pipe_config)) {
+   ret = PTR_ERR(pipe_config);
+   pipe_config = NULL;
+
+   goto out;
+   }
+   intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
+  "[modeset]");
+   to_intel_crtc(crtc)->new_config = pipe_config;
+
+out:
+   return ret;
+}
+
 static int __intel_set_mode(struct drm_crtc *crtc,
struct drm_display_mode *mode,
-   int x, int y, struct drm_framebuffer *fb)
+   int x, int y, struct drm_framebuffer *fb,
+   unsigned disable_pipes,
+   unsigned prepare_pipes,
+   unsigned modeset_pipes)
 {
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_display_mode *saved_mode;
struct intel_crtc_config *pipe_config = NULL;
struct intel_crtc *intel_crtc;
-   unsigned disable_pipes, prepare_pipes, modeset_pipes;
int ret = 0;
 
saved_mode = kmalloc(sizeof(*saved_mode), GFP_KERNEL);
if (!saved_mode)
return -ENOMEM;
 
-   intel_modeset_affected_pipes(crtc, &modeset_pipes,
-&prepare_pipes, &disable_pipes);
-
*saved_mode = crtc->mode;
 
-   /* Hack: Because we don't (yet) support global modeset on multiple
-* crtcs, we don't keep track of the new mode for more than one crtc.
-* Hence simply check whether any bit is set in modeset_pipes in all the
-* pieces of code that are not yet converted to deal with mutliple crtcs
-* changing their mode at the same time. */
-   if (modeset_pipes) {
-   pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
-   if (IS_ERR(pipe_config)) {
-   ret = PTR_ERR(pipe_config);
-   pipe_config = NULL;
-
-   goto out;
-   }
-   intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
-  "[modeset]");
-   to_intel_crtc(crtc)->new_config = pipe_config;
-   }
-
/*
 * See if the config requires any additional preparation, e.g.
 * to adjust global state with pipes off.  We need to do this
@@ -11042,19 +11059,22 @@ done:
if (ret && crtc->enabled)
crtc->mode = *saved_mode;
 
-out:
kfree(pipe_config);
kfree(saved_mode);
return ret;
 }
 
-static int intel_set_mode(struct drm_crtc *crtc,
- struct drm_display_mode *mode,
- int x, int y, struct drm_framebuffer *fb)
+static int intel_set_mode_pipes(struct drm_crtc *crtc,
+   struct drm_display_mode *mode,
+   int x, int y, struct drm_framebuffer *fb,
+   unsigned modeset_pipes,
+   unsigned prepare_pipes,
+   unsigned disable_pipes)
 {
int ret;
 
-   ret = __intel_set_mode(crtc,

[Intel-gfx] [PATCH 6/6] drm/i915: calculate pfit changes in set_config

[Jesse Barnes]

This should allow us to avoid mode sets for some panel fitter config
changes.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 48 
 1 file changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 10468a7..84331a3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11420,6 +11420,50 @@ static void disable_crtc_nofb(struct intel_crtc *crtc)
crtc->new_config = NULL;
 }
 
+/* Do we need a mode set due to pfit changes? */
+static bool intel_pfit_changed(struct drm_device *dev,
+  struct intel_crtc_config *new_config,
+  struct intel_crtc_config *cur_config)
+{
+   bool ret = false;
+
+   if (HAS_DDI(dev) || HAS_PCH_SPLIT(dev)) {
+   /*
+* On PCH platforms we can disable pfit w/o a pipe shutdown,
+* otherwise we'll need a mode set.
+*/
+   if (new_config->pch_pfit.enabled &&
+   cur_config->pch_pfit.enabled)
+   ret = false;
+   else if (new_config->pch_pfit.enabled &&
+!cur_config->pch_pfit.enabled)
+   ret = true;
+   else if (!new_config->pch_pfit.enabled &&
+cur_config->pch_pfit.enabled)
+   ret = false;
+   else if (!new_config->pch_pfit.enabled &&
+!cur_config->pch_pfit.enabled)
+   ret = false;
+   } else {
+   bool new_enabled, old_enabled;
+
+   new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE);
+   old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE);
+
+   /* 9xx only needs a shutdown to disable pfit */
+   if (new_enabled && old_enabled)
+   ret = false;
+   else if (new_enabled && !old_enabled)
+   ret = false;
+   else if (!new_enabled && old_enabled)
+   ret = true;
+   else if (!new_enabled && !old_enabled)
+   ret = false;
+   }
+
+   return ret;
+}
+
 static int intel_crtc_set_config(struct drm_mode_set *set)
 {
struct drm_device *dev;
@@ -11486,6 +11530,10 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
to_intel_crtc(set->crtc)->config.has_infoframe)
config->mode_changed = true;
 
+   if (intel_pfit_changed(dev, to_intel_crtc(set->crtc)->new_config,
+  &to_intel_crtc(set->crtc)->config))
+   config->mode_changed = true;
+
intel_update_pipe_size(to_intel_crtc(set->crtc));
 
if (config->mode_changed) {
-- 
1.9.1

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

Re: [Intel-gfx] [PATCH 3/5] drm/i915: use compute_config in set_config

[Jesse Barnes]

On Thu, 23 Oct 2014 19:28:18 +0100
Chris Wilson  wrote:

> On Thu, Oct 23, 2014 at 06:59:14PM +0200, Jesse Barnes wrote:
> > This will allow us to consult more info before deciding whether to flip
> > or do a full mode set.
> > 
> > Signed-off-by: Jesse Barnes 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 36 
> > ++--
> >  1 file changed, 30 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 6aec3ae..b2d4f9c 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11455,6 +11455,7 @@ static int intel_crtc_set_config(struct 
> > drm_mode_set *set)
> > struct drm_device *dev;
> > struct drm_mode_set save_set;
> > struct intel_set_config *config;
> > +   unsigned modeset_pipes, prepare_pipes, disable_pipes;
> > int ret;
> >  
> > BUG_ON(!set);
> > @@ -11500,9 +11501,17 @@ static int intel_crtc_set_config(struct 
> > drm_mode_set *set)
> > if (ret)
> > goto fail;
> >  
> > +   ret = intel_modeset_compute_config(set->crtc, set->mode, set->fb,
> > +  &modeset_pipes, &prepare_pipes,
> > +  &disable_pipes);
> > +   if (ret)
> > +   goto fail;
> > +
> > if (config->mode_changed) {
> > -   ret = intel_set_mode(set->crtc, set->mode,
> > -set->x, set->y, set->fb);
> > +   ret = intel_set_mode_pipes(set->crtc, set->mode,
> > +  set->x, set->y, set->fb,
> > +  modeset_pipes, prepare_pipes,
> > +  disable_pipes);
> > } else if (config->fb_changed) {
> > struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
> >  
> > @@ -11549,8 +11558,10 @@ fail:
> >  
> > /* Try to restore the config */
> > if (config->mode_changed &&
> > -   intel_set_mode(save_set.crtc, save_set.mode,
> > -  save_set.x, save_set.y, save_set.fb))
> > +   intel_set_mode_pipes(save_set.crtc, save_set.mode,
> > +save_set.x, save_set.y, save_set.fb,
> > +modeset_pipes, prepare_pipes,
> > +disable_pipes))
> > DRM_ERROR("failed to restore config after modeset 
> > failure\n");
> > }
> >  
> > @@ -13361,14 +13372,27 @@ void intel_modeset_setup_hw_state(struct 
> > drm_device *dev,
> > for_each_pipe(dev_priv, pipe) {
> > struct drm_crtc *crtc =
> > dev_priv->pipe_to_crtc_mapping[pipe];
> > +   unsigned modeset_pipes, prepare_pipes, disable_pipes;
> > +   int ret;
> > +
> > +   ret = intel_modeset_compute_config(crtc, &crtc->mode,
> > +  crtc->primary->fb,
> > +  &modeset_pipes,
> > +  &prepare_pipes,
> > +  &disable_pipes);
> > +   if (ret) {
> > +   DRM_DEBUG_KMS("prepare failed\n");
> > +   goto out;
> > +   }
> >  
> > __intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
> > -crtc->primary->fb);
> > +crtc->primary->fb, modeset_pipes,
> > +prepare_pipes, disable_pipes);
> 
> This doesn't make sense -- where's the change in function definition?
> I am pretty sure that as we iterate over each pipe, we only want to
> operate on that pipe here.
> 
> How I wish we didn't need force_restore.

Hm, let's see, this was a mechanical conversion leftover from an
earlier patch.  I don't think it's needed, but it should be equivalent,
given the changes to __intel_set_mode() right?

Ah I see, some patch series fail on my part.  2/5 didn't get posted and
should have been squashed with 1/5.  Let's see if I can make this
clearer.

I'll post an updated series with some pfit changes as well.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 5/5] drm/i915: check for audio and infoframe changes across mode sets

[Jesse Barnes]

If these change (e.g. after a modeset following a fastboot), we need to
do a full mode set.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index b2d4f9c..6fbe2fc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11507,6 +11507,16 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
if (ret)
goto fail;
 
+   if (to_intel_crtc(set->crtc)->new_config->has_audio !=
+   to_intel_crtc(set->crtc)->config.has_audio)
+   config->mode_changed = true;
+
+   /* Force mode sets for any infoframe stuff */
+   if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
+   to_intel_crtc(set->crtc)->config.has_infoframe)
+   config->mode_changed = true;
+
+
if (config->mode_changed) {
ret = intel_set_mode_pipes(set->crtc, set->mode,
   set->x, set->y, set->fb,
-- 
1.9.1

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

[Intel-gfx] [PATCH 1/5] drm/i915: factor out compute_config from __intel_set_mode

[Jesse Barnes]

So we can call it earlier for use in computing mode config changes.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 63 +++-
 1 file changed, 41 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 1d0425d..a9c1c32 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10938,6 +10938,42 @@ static void update_scanline_offset(struct intel_crtc 
*crtc)
crtc->scanline_offset = 1;
 }
 
+static int intel_modeset_compute_config(struct drm_crtc *crtc,
+   struct drm_display_mode *mode,
+   struct drm_framebuffer *fb,
+   unsigned *modeset_pipes,
+   unsigned *prepare_pipes,
+   unsigned *disable_pipes)
+{
+   struct intel_crtc_config *pipe_config = NULL;
+   int ret = 0;
+
+   intel_modeset_affected_pipes(crtc, modeset_pipes,
+prepare_pipes, disable_pipes);
+
+   if (!modeset_pipes)
+   goto out;
+
+   /* Hack: Because we don't (yet) support global modeset on multiple
+* crtcs, we don't keep track of the new mode for more than one crtc.
+* Hence simply check whether any bit is set in modeset_pipes in all the
+* pieces of code that are not yet converted to deal with mutliple crtcs
+* changing their mode at the same time. */
+   pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
+   if (IS_ERR(pipe_config)) {
+   ret = PTR_ERR(pipe_config);
+   pipe_config = NULL;
+
+   goto out;
+   }
+   intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
+  "[modeset]");
+   to_intel_crtc(crtc)->new_config = pipe_config;
+
+out:
+   return ret;
+}
+
 static int __intel_set_mode(struct drm_crtc *crtc,
struct drm_display_mode *mode,
int x, int y, struct drm_framebuffer *fb)
@@ -10950,33 +10986,17 @@ static int __intel_set_mode(struct drm_crtc *crtc,
unsigned disable_pipes, prepare_pipes, modeset_pipes;
int ret = 0;
 
+   ret = intel_modeset_compute_config(crtc, mode, fb, &modeset_pipes,
+  &prepare_pipes, &disable_pipes);
+   if (ret)
+   return ret;
+
saved_mode = kmalloc(sizeof(*saved_mode), GFP_KERNEL);
if (!saved_mode)
return -ENOMEM;
 
-   intel_modeset_affected_pipes(crtc, &modeset_pipes,
-&prepare_pipes, &disable_pipes);
-
*saved_mode = crtc->mode;
 
-   /* Hack: Because we don't (yet) support global modeset on multiple
-* crtcs, we don't keep track of the new mode for more than one crtc.
-* Hence simply check whether any bit is set in modeset_pipes in all the
-* pieces of code that are not yet converted to deal with mutliple crtcs
-* changing their mode at the same time. */
-   if (modeset_pipes) {
-   pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
-   if (IS_ERR(pipe_config)) {
-   ret = PTR_ERR(pipe_config);
-   pipe_config = NULL;
-
-   goto out;
-   }
-   intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
-  "[modeset]");
-   to_intel_crtc(crtc)->new_config = pipe_config;
-   }
-
/*
 * See if the config requires any additional preparation, e.g.
 * to adjust global state with pipes off.  We need to do this
@@ -11070,7 +11090,6 @@ done:
if (ret && crtc->enabled)
crtc->mode = *saved_mode;
 
-out:
kfree(pipe_config);
kfree(saved_mode);
return ret;
-- 
1.9.1

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

[Intel-gfx] [PATCH 3/5] drm/i915: use compute_config in set_config

[Jesse Barnes]

This will allow us to consult more info before deciding whether to flip
or do a full mode set.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 36 ++--
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 6aec3ae..b2d4f9c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11455,6 +11455,7 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
struct drm_device *dev;
struct drm_mode_set save_set;
struct intel_set_config *config;
+   unsigned modeset_pipes, prepare_pipes, disable_pipes;
int ret;
 
BUG_ON(!set);
@@ -11500,9 +11501,17 @@ static int intel_crtc_set_config(struct drm_mode_set 
*set)
if (ret)
goto fail;
 
+   ret = intel_modeset_compute_config(set->crtc, set->mode, set->fb,
+  &modeset_pipes, &prepare_pipes,
+  &disable_pipes);
+   if (ret)
+   goto fail;
+
if (config->mode_changed) {
-   ret = intel_set_mode(set->crtc, set->mode,
-set->x, set->y, set->fb);
+   ret = intel_set_mode_pipes(set->crtc, set->mode,
+  set->x, set->y, set->fb,
+  modeset_pipes, prepare_pipes,
+  disable_pipes);
} else if (config->fb_changed) {
struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
 
@@ -11549,8 +11558,10 @@ fail:
 
/* Try to restore the config */
if (config->mode_changed &&
-   intel_set_mode(save_set.crtc, save_set.mode,
-  save_set.x, save_set.y, save_set.fb))
+   intel_set_mode_pipes(save_set.crtc, save_set.mode,
+save_set.x, save_set.y, save_set.fb,
+modeset_pipes, prepare_pipes,
+disable_pipes))
DRM_ERROR("failed to restore config after modeset 
failure\n");
}
 
@@ -13361,14 +13372,27 @@ void intel_modeset_setup_hw_state(struct drm_device 
*dev,
for_each_pipe(dev_priv, pipe) {
struct drm_crtc *crtc =
dev_priv->pipe_to_crtc_mapping[pipe];
+   unsigned modeset_pipes, prepare_pipes, disable_pipes;
+   int ret;
+
+   ret = intel_modeset_compute_config(crtc, &crtc->mode,
+  crtc->primary->fb,
+  &modeset_pipes,
+  &prepare_pipes,
+  &disable_pipes);
+   if (ret) {
+   DRM_DEBUG_KMS("prepare failed\n");
+   goto out;
+   }
 
__intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
-crtc->primary->fb);
+crtc->primary->fb, modeset_pipes,
+prepare_pipes, disable_pipes);
}
} else {
intel_modeset_update_staged_output_state(dev);
}
-
+out:
intel_modeset_check_state(dev);
 }
 
-- 
1.9.1

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

[Intel-gfx] [PATCH 4/5] drm/i915/hdmi: fetch infoframe status in get_config

[Jesse Barnes]

This is useful for checking things later.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_drv.h  |  4 +++
 drivers/gpu/drm/i915/intel_hdmi.c | 61 +++
 2 files changed, 65 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5ab813c..cdace63 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -290,6 +290,9 @@ struct intel_crtc_config {
 * between pch encoders and cpu encoders. */
bool has_pch_encoder;
 
+   /* Are we sending infoframes on the attached port */
+   bool has_infoframe;
+
/* CPU Transcoder for the pipe. Currently this can only differ from the
 * pipe on Haswell (where we have a special eDP transcoder). */
enum transcoder cpu_transcoder;
@@ -541,6 +544,7 @@ struct intel_hdmi {
void (*set_infoframes)(struct drm_encoder *encoder,
   bool enable,
   struct drm_display_mode *adjusted_mode);
+   bool (*infoframe_enabled)(struct drm_encoder *encoder);
 };
 
 struct intel_dp_mst_encoder;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 8b5f3aa..75efe4c 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -166,6 +166,15 @@ static void g4x_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(VIDEO_DIP_CTL);
 }
 
+static bool g4x_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   u32 val = I915_READ(VIDEO_DIP_CTL);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void ibx_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -204,6 +213,17 @@ static void ibx_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(reg);
 }
 
+static bool ibx_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
+   u32 val = I915_READ(reg);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void cpt_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -245,6 +265,17 @@ static void cpt_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(reg);
 }
 
+static bool cpt_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   int reg = TVIDEO_DIP_CTL(intel_crtc->pipe);
+   u32 val = I915_READ(reg);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void vlv_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -283,6 +314,17 @@ static void vlv_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(reg);
 }
 
+static bool vlv_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   int reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
+   u32 val = I915_READ(reg);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 static void hsw_write_infoframe(struct drm_encoder *encoder,
enum hdmi_infoframe_type type,
const void *frame, ssize_t len)
@@ -320,6 +362,17 @@ static void hsw_write_infoframe(struct drm_encoder 
*encoder,
POSTING_READ(ctl_reg);
 }
 
+static bool hsw_infoframe_enabled(struct drm_encoder *encoder)
+{
+   struct drm_device *dev = encoder->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+   u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->config.cpu_transcoder);
+   u32 val = I915_READ(ctl_reg);
+
+   return val & VIDEO_DIP_ENABLE;
+}
+
 /*
  * The data we write to the DIP data buffer registers is 1 byte bigger than the
  * HDMI infoframe size because of an ECC/reserved byte at position 3 (starting
@@ -732,6 +785,9 @@ static void intel_hdmi_get_config(struct intel_encoder 
*encoder,
if (tmp & HDMI_MODE_SELECT_HDMI)
pipe_config->has_hdmi_sink = true;
 
+   if (intel_hdmi->infoframe_enabled(&encoder->base