Re: [Intel-gfx] [PATCH] drm/i915: Add ddb size field to device info structure

[Jani Nikula]

On Wed, 14 Sep 2016, Deepak M  wrote:
> Adding the ddb size into the devide info will avoid
> platform checks while computing wm.
>
> Suggested-by: Ander Conselvan de Oliveira 
> 
> Signed-off-by: Deepak M 
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  1 +
>  drivers/gpu/drm/i915/i915_pci.c |  5 +
>  drivers/gpu/drm/i915/intel_pm.c | 13 +
>  3 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1e2dda8..4518ef3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -710,6 +710,7 @@ struct intel_device_info {
>   u8 ring_mask; /* Rings supported by the HW */
>   u8 num_rings;
>   DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON);
> + u16 ddb_size;

This could use the /* in blocks */ comment.

>   /* Register offsets for the various display pipes and transcoders */
>   int pipe_offsets[I915_MAX_TRANSCODERS];
>   int trans_offsets[I915_MAX_TRANSCODERS];
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index d771870d..687c768 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -328,6 +328,7 @@ static const struct intel_device_info intel_skylake_info 
> = {
>   .gen = 9,
>   .has_csr = 1,
>   .has_guc = 1,
> + .ddb_size = 896,
>  };
>  
>  static const struct intel_device_info intel_skylake_gt3_info = {
> @@ -336,6 +337,7 @@ static const struct intel_device_info 
> intel_skylake_gt3_info = {
>   .gen = 9,
>   .has_csr = 1,
>   .has_guc = 1,
> + .ddb_size = 896,
>   .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>  };
>  
> @@ -358,6 +360,7 @@ static const struct intel_device_info intel_broxton_info 
> = {
>   .has_hw_contexts = 1,
>   .has_logical_ring_contexts = 1,
>   .has_guc = 1,
> + .ddb_size = 512,
>   GEN_DEFAULT_PIPEOFFSETS,
>   IVB_CURSOR_OFFSETS,
>   BDW_COLORS,
> @@ -369,6 +372,7 @@ static const struct intel_device_info intel_kabylake_info 
> = {
>   .gen = 9,
>   .has_csr = 1,
>   .has_guc = 1,
> + .ddb_size = 896,
>  };
>  
>  static const struct intel_device_info intel_kabylake_gt3_info = {
> @@ -377,6 +381,7 @@ static const struct intel_device_info 
> intel_kabylake_gt3_info = {
>   .gen = 9,
>   .has_csr = 1,
>   .has_guc = 1,
> + .ddb_size = 896,
>   .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>  };
>  
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 6af438f..7eeb73b 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2853,13 +2853,6 @@ bool ilk_disable_lp_wm(struct drm_device *dev)
>   return _ilk_disable_lp_wm(dev_priv, WM_DIRTY_LP_ALL);
>  }
>  
> -/*
> - * On gen9, we need to allocate Display Data Buffer (DDB) portions to the
> - * different active planes.
> - */
> -
> -#define SKL_DDB_SIZE 896 /* in blocks */
> -#define BXT_DDB_SIZE 512
>  #define SKL_SAGV_BLOCK_TIME  30 /* µs */
>  
>  /*
> @@ -3057,11 +3050,7 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device 
> *dev,
>   else
>   *num_active = hweight32(dev_priv->active_crtcs);
>  
> - if (IS_BROXTON(dev))
> - ddb_size = BXT_DDB_SIZE;
> - else
> - ddb_size = SKL_DDB_SIZE;
> -
> + ddb_size = INTEL_INFO(dev_priv)->ddb_size;

I'd perhaps stick a WARN_ON(ddb_size == 0) here.

With those fixed,

Reviewed-by: Jani Nikula 


>   ddb_size -= 4; /* 4 blocks for bypass path allocation */
>  
>   /*

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [01/18] drm/i915: Support asynchronous waits on struct fence from i915_gem_request

[Patchwork]

== Series Details ==

Series: series starting with [01/18] drm/i915: Support asynchronous waits on 
struct fence from i915_gem_request
URL   : https://patchwork.freedesktop.org/series/12433/
State : failure

== Summary ==

Series 12433v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/12433/revisions/1/mbox/

Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-c:
skip   -> PASS   (fi-hsw-4770r)
Subgroup suspend-read-crc-pipe-b:
skip   -> INCOMPLETE (fi-ivb-3770)

fi-bsw-n3050 total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-hsw-4770k total:244  pass:226  dwarn:0   dfail:0   fail:0   skip:18 
fi-hsw-4770r total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650   total:244  pass:183  dwarn:0   dfail:0   fail:1   skip:60 
fi-ivb-3520m total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770  total:202  pass:171  dwarn:0   dfail:0   fail:0   skip:30 
fi-skl-6260u total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hqtotal:244  pass:221  dwarn:0   dfail:0   fail:1   skip:22 
fi-skl-6700k total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
fi-snb-2520m total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2528/

208290026552464713d3897ab5d649f4445d5513 drm-intel-nightly: 
2016y-09m-13d-14h-45m-32s UTC integration manifest
422c489 drm/i915: Support explicit fencing for execbuf
f78943a drm/i915: Enable userspace to opt-out of implicit fencing
93acb19 drm/i915: Enable multiple timelines
7b8232a drm/i915: Reserve space in the global seqno during request allocation
c972275 drm/i915: Create a unique name for the context
6c8fa4a drm/i915: Move the global sync optimisation to the timeline
27f5fd8 drm/i915: Defer request emission
e0e3346 drm/i915: Record space required for request emission
6cbf7e2 drm/i915: Introduce a global_seqno for each request
279611d drm/i915: Wait first for submission, before waiting for request 
completion
ec2ccb1 drm/i915: Combine seqno + tracking into a global timeline struct
c5ca7a3 drm/i915: Restore nonblocking awaits for modesetting
435dc25 drm: Add reference counting to drm_atomic_state
bc1da87 drm/i915: Move GEM activity tracking into a common struct 
reservation_object
354aa03 drm/i915: Remove unused i915_gem_active_wait() in favour of _unlocked()
39f214d drm/i915: Rearrange i915_wait_request() accounting with callers
459763e drm/i915: Allow i915_sw_fence_await_sw_fence() to allocate
83aaab6 drm/i915: Support asynchronous waits on struct fence from 
i915_gem_request

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

Re: [Intel-gfx] 4.8-rc1: it is now common that machine needs re-run of xrandr after resume

[Jani Nikula]

On Wed, 14 Sep 2016, Pavel Machek  wrote:
> For the "sometimes need xrandr after resume": I don't think I can
> bisect that. It only happens sometimes :-(. But there's something
> helpful in the logs:

> [ 1856.218863] [drm:drm_edid_block_valid] *ERROR* EDID checksum is
> invalid, remainder is 130
> [ 1856.218863] Raw EDID:
> [ 1856.218863] 00 ff ff ff ff ff ff 00 ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] [drm:drm_edid_block_valid] *ERROR* EDID checksum is
> invalid, remainder is 130
> [ 1856.218863] Raw EDID:
> [ 1856.218863] 00 ff ff ff ff ff ff 00 ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] [drm:drm_edid_block_valid] *ERROR* EDID checksum is
> invalid, remainder is 130
> [ 1856.218863] Raw EDID:
> [ 1856.218863] 00 ff ff ff ff ff ff 00 ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] [drm:drm_edid_block_valid] *ERROR* EDID checksum is
> invalid, remainder is 130
> [ 1856.218863] Raw EDID:
> [ 1856.218863] 00 ff ff ff ff ff ff 00 ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> [ 1856.218863] i915 :00:02.0: HDMI-A-1: EDID block 0 invalid.

Pavel, Martin, do you always see this when the display fails to resume?
Is it HDMI/DVI for both of you?


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Only expand COND once in wait_for()

[Chris Wilson]

On Tue, Sep 13, 2016 at 08:40:19PM +0100, Chris Wilson wrote:
> I was looking at some wait_for() timeouts on a slow system, with lots of
> debug enabled (KASAN, lockdep, mmio_debug). Thinking that we were
> mishandling the timeout, I tried to ensure that we loop at least once
> after first testing COND. However, the double test of COND either side
> of the timeout check makes that unlikely. But we can do an equivalent
> loop, that keeps the COND check after testing for timeout (required so
> that we are not preempted between testing COND and then testing for a
> timeout) without expanding COND twice.
> 
> The advantage of only expanding COND once is a dramatic reduction in
> code size:
> 
>text  data bss dec hex
> 1308733  51841152 1315069  1410fd before
> 1305341  51841152 1311677  1403bd after
> 
> Signed-off-by: Chris Wilson 
> Cc: Tvrtko Ursulin 
> Cc: Ville Syrjälä 
> Cc: Daniel Vetter 
> ---
>  drivers/gpu/drm/i915/intel_drv.h | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index cb99a2540863..597899d71df9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -52,13 +52,16 @@
>   */
>  #define _wait_for(COND, US, W) ({ \
>   unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1;   \
> - int ret__ = 0;  \
> - while (!(COND)) {   \
> - if (time_after(jiffies, timeout__)) {   \
> - if (!(COND))\
> - ret__ = -ETIMEDOUT; \
> + int ret__;  \

Ok, this is the magic. Missed initializer, gcc goes wild trimming
undefined code. Patch is completely bogus.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Standardize port type for DVO encoders

[Jani Nikula]

On Wed, 14 Sep 2016, Dhinakaran Pandiyan  wrote:
> Changing the return type from 'char' to 'enum port' in
> intel_dvo_port_name() makes it easier to later move the port information to
> intel_encoder. In addition, the port type conforms to what we have
> elsewhere.
>
> Removing the last conditional that handles invalid port because dvo_reg is
> intialized to valid values for all DVO devices at definition.
>
> Signed-off-by: Dhinakaran Pandiyan 
> ---
>  drivers/gpu/drm/i915/intel_dvo.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c 
> b/drivers/gpu/drm/i915/intel_dvo.c
> index 2e452c5..1ea2627 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -412,16 +412,14 @@ intel_dvo_get_current_mode(struct drm_connector 
> *connector)
>   return mode;
>  }
>  
> -static char intel_dvo_port_name(i915_reg_t dvo_reg)
> +static char intel_dvo_port(i915_reg_t dvo_reg)

You haven't actually changed the return type to enum port.

BR,
Jani.

>  {
>   if (i915_mmio_reg_equal(dvo_reg, DVOA))
> - return 'A';
> + return PORT_A;
>   else if (i915_mmio_reg_equal(dvo_reg, DVOB))
> - return 'B';
> - else if (i915_mmio_reg_equal(dvo_reg, DVOC))
> - return 'C';
> + return PORT_B;
>   else
> - return '?';
> + return PORT_C;
>  }
>  
>  void intel_dvo_init(struct drm_device *dev)
> @@ -464,6 +462,7 @@ void intel_dvo_init(struct drm_device *dev)
>   bool dvoinit;
>   enum pipe pipe;
>   uint32_t dpll[I915_MAX_PIPES];
> + enum port port;
>  
>   /* Allow the I2C driver info to specify the GPIO to be used in
>* special cases, but otherwise default to what's defined
> @@ -511,9 +510,10 @@ void intel_dvo_init(struct drm_device *dev)
>   if (!dvoinit)
>   continue;
>  
> + port = intel_dvo_port(dvo->dvo_reg);
>   drm_encoder_init(dev, _encoder->base,
>_dvo_enc_funcs, encoder_type,
> -  "DVO %c", intel_dvo_port_name(dvo->dvo_reg));
> +  "DVO %c", port_name(port));
>  
>   intel_encoder->type = INTEL_OUTPUT_DVO;
>   intel_encoder->crtc_mask = (1 << 0) | (1 << 1);

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] 4.8-rc1: it is now common that machine needs re-run of xrandr after resume

[Jani Nikula]

On Wed, 14 Sep 2016, Jani Nikula  wrote:
> On Wed, 14 Sep 2016, Pavel Machek  wrote:
>> Hi!
>>
>>> I have
>>> 
>>> 00:02.0 VGA compatible controller: Intel Corporation 4 Series Chipset
>>> Integrated Graphics Controller (rev 03)
>>> 
>>> In previous kernels, resume worked ok. With 4.8-rc1, I quite often (1
>>> in 10 resumes?) get in state where primary monitor (DVI) is dead (in
>>> powersave) and all windows move to secondary monitor (VGA). Running
>>> "xrandr" fixes that.
>>> 
>>> I'll update to newer rc and see if it happens again, but if you have
>>> any ideas, now would be good time.
>>
>> Ok. With -rc6, X are completely broken. I got notification "could not
>> restore CRTC config for screen 63" or something like that, and window
>> manager just does not start.
>
> Ugh. Can you bisect from v4.7, assuming it worked? That's probably the
> fastest way to resolve this.

Also, if you don't mind, please file a bug at [1], attaching the logs
there. It'll be easier for me to direct attention and priority to the
bug, which will help you too in the end.

Thanks,
Jani.

[1] https://bugs.freedesktop.org/enter_bug.cgi?product=DRI=DRM/Intel

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v5 1/5] drm/i915: Fallback to lower link rate and lane count during link training

[Mika Kahola]

On Tue, 2016-09-13 at 18:08 -0700, Manasi Navare wrote:
> According to the DisplayPort Spec, in case of Clock Recovery failure
> the link training sequence should fall back to the lower link rate
> followed by lower lane count until CR succeeds.
> On CR success, the sequence proceeds with Channel EQ.
> In case of Channel EQ failures, it should fallback to
> lower link rate and lane count and start the CR phase again.
> 
> v5:
> * Reset the link rate index to the max link rate index
> before lowering the lane count (Jani Nikula)
> * Use the paradigm for loop in intel_dp_link_rate_index
> v4:
> * Fixed the link rate fallback loop (Manasi Navare)
> v3:
> * Fixed some rebase issues (Mika Kahola)
> v2:
> * Add a helper function to return index of requested link rate
> into common_rates array
> * Changed the link rate fallback loop to make use
> of common_rates array (Mika Kahola)
> * Changed INTEL_INFO to INTEL_GEN (David Weinehall)
> 
> Signed-off-by: Manasi Navare 
> ---
>  drivers/gpu/drm/i915/intel_ddi.c  | 112
> +++---
>  drivers/gpu/drm/i915/intel_dp.c   |  15 
>  drivers/gpu/drm/i915/intel_dp_link_training.c |  12 ++-
>  drivers/gpu/drm/i915/intel_drv.h  |   6 +-
>  4 files changed, 131 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 8065a5f..4d3a931 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1637,19 +1637,18 @@ void intel_ddi_clk_select(struct
> intel_encoder *encoder,
>   }
>  }
>  
> -static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> +static void intel_ddi_pre_enable_edp(struct intel_encoder *encoder,
>   int link_rate, uint32_t
> lane_count,
> - struct intel_shared_dpll *pll,
> - bool link_mst)
> + struct intel_shared_dpll *pll)
>  {
>   struct intel_dp *intel_dp = enc_to_intel_dp(>base);
>   struct drm_i915_private *dev_priv = to_i915(encoder-
> >base.dev);
>   enum port port = intel_ddi_get_encoder_port(encoder);
>  
>   intel_dp_set_link_params(intel_dp, link_rate, lane_count,
> -  link_mst);
> - if (encoder->type == INTEL_OUTPUT_EDP)
> - intel_edp_panel_on(intel_dp);
> +  false);
> +
> + intel_edp_panel_on(intel_dp);
>  
>   intel_ddi_clk_select(encoder, pll);
>   intel_prepare_dp_ddi_buffers(encoder);
> @@ -1660,6 +1659,28 @@ static void intel_ddi_pre_enable_dp(struct
> intel_encoder *encoder,
>   intel_dp_stop_link_train(intel_dp);
>  }
>  
> +static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> + int link_rate, uint32_t
> lane_count,
> + struct intel_shared_dpll *pll,
> + bool link_mst)
> +{
> + struct intel_dp *intel_dp = enc_to_intel_dp(>base);
> + struct drm_i915_private *dev_priv = to_i915(encoder-
> >base.dev);
> + struct intel_shared_dpll_config tmp_pll_config;
> +
> + /* Disable the PLL and obtain the PLL for Link Training
> +  * that starts with highest link rate and lane count.
> +  */
> + tmp_pll_config = pll->config;
> + pll->funcs.disable(dev_priv, pll);
> + pll->config.crtc_mask = 0;
> +
> + /* If Link Training fails, send a uevent to generate a
> hotplug */
> + if (!intel_ddi_link_train(intel_dp, link_rate, lane_count,
> link_mst))
> + drm_kms_helper_hotplug_event(encoder->base.dev);
> + pll->config = tmp_pll_config;
> +}
> +
>  static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>     bool has_hdmi_sink,
>     struct drm_display_mode
> *adjusted_mode,
> @@ -1693,20 +1714,26 @@ static void intel_ddi_pre_enable(struct
> intel_encoder *intel_encoder,
>   struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
>   int type = intel_encoder->type;
>  
> - if (type == INTEL_OUTPUT_DP || type == INTEL_OUTPUT_EDP) {
> + if (type == INTEL_OUTPUT_EDP)
> + intel_ddi_pre_enable_edp(intel_encoder,
> + crtc->config->port_clock,
> + crtc->config->lane_count,
> + crtc->config->shared_dpll);
> +
> + if (type == INTEL_OUTPUT_DP)
>   intel_ddi_pre_enable_dp(intel_encoder,
>   crtc->config->port_clock,
>   crtc->config->lane_count,
>   crtc->config->shared_dpll,
>   intel_crtc_has_type(crtc-
> >config,
>   INTEL_OU
> TPUT_DP_MST));
> 

[Intel-gfx] [PATCH] drm/i915: Add ddb size field to device info structure

[Deepak M]

Adding the ddb size into the devide info will avoid
platform checks while computing wm.

v2: Added comment and WARN_ON if ddb size is zero.(Jani)

Suggested-by: Ander Conselvan de Oliveira 

Signed-off-by: Deepak M 
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_pci.c |  5 +
 drivers/gpu/drm/i915/intel_pm.c | 15 +++
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1e2dda8..6014c3a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -710,6 +710,7 @@ struct intel_device_info {
u8 ring_mask; /* Rings supported by the HW */
u8 num_rings;
DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON);
+   u16 ddb_size; /* in blocks */
/* Register offsets for the various display pipes and transcoders */
int pipe_offsets[I915_MAX_TRANSCODERS];
int trans_offsets[I915_MAX_TRANSCODERS];
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index d771870d..687c768 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -328,6 +328,7 @@ static const struct intel_device_info intel_skylake_info = {
.gen = 9,
.has_csr = 1,
.has_guc = 1,
+   .ddb_size = 896,
 };
 
 static const struct intel_device_info intel_skylake_gt3_info = {
@@ -336,6 +337,7 @@ static const struct intel_device_info 
intel_skylake_gt3_info = {
.gen = 9,
.has_csr = 1,
.has_guc = 1,
+   .ddb_size = 896,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
 };
 
@@ -358,6 +360,7 @@ static const struct intel_device_info intel_broxton_info = {
.has_hw_contexts = 1,
.has_logical_ring_contexts = 1,
.has_guc = 1,
+   .ddb_size = 512,
GEN_DEFAULT_PIPEOFFSETS,
IVB_CURSOR_OFFSETS,
BDW_COLORS,
@@ -369,6 +372,7 @@ static const struct intel_device_info intel_kabylake_info = 
{
.gen = 9,
.has_csr = 1,
.has_guc = 1,
+   .ddb_size = 896,
 };
 
 static const struct intel_device_info intel_kabylake_gt3_info = {
@@ -377,6 +381,7 @@ static const struct intel_device_info 
intel_kabylake_gt3_info = {
.gen = 9,
.has_csr = 1,
.has_guc = 1,
+   .ddb_size = 896,
.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
 };
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6af438f..9c5861e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2853,13 +2853,6 @@ bool ilk_disable_lp_wm(struct drm_device *dev)
return _ilk_disable_lp_wm(dev_priv, WM_DIRTY_LP_ALL);
 }
 
-/*
- * On gen9, we need to allocate Display Data Buffer (DDB) portions to the
- * different active planes.
- */
-
-#define SKL_DDB_SIZE   896 /* in blocks */
-#define BXT_DDB_SIZE   512
 #define SKL_SAGV_BLOCK_TIME30 /* µs */
 
 /*
@@ -3057,13 +3050,11 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device 
*dev,
else
*num_active = hweight32(dev_priv->active_crtcs);
 
-   if (IS_BROXTON(dev))
-   ddb_size = BXT_DDB_SIZE;
-   else
-   ddb_size = SKL_DDB_SIZE;
-
+   ddb_size = INTEL_INFO(dev_priv)->ddb_size;
ddb_size -= 4; /* 4 blocks for bypass path allocation */
 
+   WARN_ON(ddb_size == 0);
+
/*
 * If the state doesn't change the active CRTC's, then there's
 * no need to recalculate; the existing pipe allocation limits
-- 
1.9.1

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

Re: [Intel-gfx] [CI 10/21] drm/i915: Mark up all locked waiters

[ch...@chris-wilson.co.uk]

On Tue, Sep 13, 2016 at 03:21:48PM +0300, Mika Kuoppala wrote:
> Mika Kuoppala  writes:
> > "Zanoni, Paulo R"  writes:
> >>> +#if IS_ENABLED(CONFIG_LOCKDEP)
> >>> + GEM_BUG_ON(!!lockdep_is_held(>i915->drm.struct_mutex)
> >>> !=
> >>> +    !!(flags & I915_WAIT_LOCKED));
> >>> +#endif
> >>
> >> I'm hitting this on my SKL. It usually happens right after the login,
> >> when I click the terminal icon on the toolbar in order to open it (on
> >> Cinnamon). When it doesn't happen at that time, I just open a browser
> >> and browse for like 30 seconds until it happens.
> >>
> >> I did manual reverts of this series up to this patch and obviously the
> >> problem goes away (no more GEM_BUG_ON to hit). I didn't try to do a
> >> real bisect to check if this is the first bad commit or if it's
> >> something later on the series. It could even be an older problem that
> >> just got exposed by the new BUG(). I'm available to test patches, just
> >> send them to me.
> >
> > No patches yet but there is a comment on 
> > intel_prepare_plane_fb() that says that it must be called with mutex
> > held.
> >
> > Quite likely the new GEM_BUG_ON catches this not being true.
> >
> 
> As Chris pointed out in irc, its about the reverse. Waiting
> with mutex when you shouldn't.

Fwiw, the fix is now on the list.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 04/18] drm/i915: Remove unused i915_gem_active_wait() in favour of _unlocked()

[Joonas Lahtinen]

On ke, 2016-09-14 at 07:52 +0100, Chris Wilson wrote:
> Since we only use the more generic unlocked variant, just rename it as
> the normal i915_gem_active_wait(). The temporary cost is that we need to
> always acquire the reference in a RCU safe manner, but the benefit is
> that we will combine the common paths.
> 
> Signed-off-by: Chris Wilson 

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Add ddb size field to device info structure (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Add ddb size field to device info structure (rev2)
URL   : https://patchwork.freedesktop.org/series/12427/
State : success

== Summary ==

Series 12427v2 drm/i915: Add ddb size field to device info structure
https://patchwork.freedesktop.org/api/1.0/series/12427/revisions/2/mbox/


fi-bsw-n3050 total:244  pass:202  dwarn:0   dfail:0   fail:0   skip:42 
fi-hsw-4770k total:244  pass:226  dwarn:0   dfail:0   fail:0   skip:18 
fi-hsw-4770r total:244  pass:222  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650   total:244  pass:183  dwarn:0   dfail:0   fail:1   skip:60 
fi-ivb-3520m total:244  pass:219  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 
fi-skl-6260u total:244  pass:230  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hqtotal:244  pass:221  dwarn:0   dfail:0   fail:1   skip:22 
fi-skl-6700k total:244  pass:219  dwarn:1   dfail:0   fail:0   skip:24 
fi-snb-2520m total:244  pass:208  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600  total:244  pass:207  dwarn:0   dfail:0   fail:0   skip:37 

Results at /archive/results/CI_IGT_test/Patchwork_2529/

0c7c6ebb924db723ecabc2521e2afa71beeec471 drm-intel-nightly: 
2016y-09m-14d-07h-41m-42s UTC integration manifest
67b040a drm/i915: Add ddb size field to device info structure

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

Re: [Intel-gfx] [PATCH 01/18] drm/i915: Support asynchronous waits on struct fence from i915_gem_request

[Joonas Lahtinen]

On ke, 2016-09-14 at 07:52 +0100, Chris Wilson wrote:
> +int
> +i915_gem_request_await_fence(struct drm_i915_gem_request *req,
> +  struct fence *fence)
> +{
> + struct fence_array *array;
> + int ret;
> + int i;
> +
> + if (test_bit(FENCE_FLAG_SIGNALED_BIT, >flags))
> + return 0;
> +
> + if (fence_is_i915(fence))
> + return i915_gem_request_await_request(req, to_request(fence));
> +
> + if (!fence_is_array(fence)) {
> + ret = i915_sw_fence_await_dma_fence(>submit,
> + fence, 10*HZ,

Magic 10*HZ, give it a define it is not temporary.

> + GFP_KERNEL);
> + return ret < 0 ? ret : 0;
> + }
> +
> + array = to_fence_array(fence);
> + for (i = 0; i < array->num_fences; i++) {
> + struct fence *child = array->fences[i];
> +
> + if (fence_is_i915(child))
> + ret = i915_gem_request_await_request(req,
> +  to_request(child));
> + else
> + ret = i915_sw_fence_await_dma_fence(>submit,
> + child, 10*HZ,
> + GFP_KERNEL);

This chunk repeats from above, make it a small
__i915_gem_request_await_fence function.

> + if (ret < 0)
> + return ret;

How about the failure case when we're half bound, no need to tear down?

No uses in this patch so hard to evaluate.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] 4.8-rc1: it is now common that machine needs re-run of xrandr after resume

[Jani Nikula]

On Wed, 14 Sep 2016, Pavel Machek  wrote:
> Hi!
>
>> I have
>> 
>> 00:02.0 VGA compatible controller: Intel Corporation 4 Series Chipset
>> Integrated Graphics Controller (rev 03)
>> 
>> In previous kernels, resume worked ok. With 4.8-rc1, I quite often (1
>> in 10 resumes?) get in state where primary monitor (DVI) is dead (in
>> powersave) and all windows move to secondary monitor (VGA). Running
>> "xrandr" fixes that.
>> 
>> I'll update to newer rc and see if it happens again, but if you have
>> any ideas, now would be good time.
>
> Ok. With -rc6, X are completely broken. I got notification "could not
> restore CRTC config for screen 63" or something like that, and window
> manager just does not start.

Ugh. Can you bisect from v4.7, assuming it worked? That's probably the
fastest way to resolve this.

BR,
Jani.

>
> X log is attached as delme, kernel log as delme2. Nothing too
> suspicious :-(.
>
>   Pavel

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Ignore OpRegion panel type except on select machines

[Ville Syrjälä]

On Tue, Sep 13, 2016 at 12:37:16PM +0300, Jani Nikula wrote:
> On Tue, 13 Sep 2016, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä 
> >
> > Turns out
> > commit a05628195a0d ("drm/i915: Get panel_type from OpRegion panel
> > details") has regressed quite a few machines. So it looks like we
> > can't use the panel type from OpRegion on all systems, and yet we
> > absolutely must use it on some specific systems.
> >
> > Despite trying, I was unable to find any automagic way to determine
> > if the OpRegion panel type is respectable or not. The only glimmer
> > of hope I had was bit 8 in the SCIC response, but that turned out to
> > not work either (it was always 0 on both types of systems).
> >
> > So, to fix the regressions without breaking the machine we know to need
> > the OpRegion panel type, let's just add a quirk for this. Only specific
> > machines known to require the OpRegion panel type will therefore use
> > it. Everyone else will fall bck to the VBT panel type.
> >
> > The only known machine so far is a "Conrac GmbH IX45GM2". The PCI
> > subsystem ID on this machine is just a generic 8086:2a42, so of no use.
> > Instead we'll go with a DMI match.
> >
> > I suspect we can now also revert
> > commit aeddda06c1a7 ("drm/i915: Ignore panel type from OpRegion on SKL")
> > but let's leave that to a separate patch.
> >
> > v2: Do the DMI match in the opregion code directly, as dev_priv->quirks
> > gets populated too late
> >
> > Cc: Rob Kramer 
> > Cc: Martin van Es 
> > Cc: Andrea Arcangeli 
> > Cc: Dave Airlie 
> > Cc: Marco Krüger 
> > Cc: Sean Greenslade 
> > Cc: Trudy Tective 
> > Cc: Robin Müller 
> > Cc: Alexander Kobel 
> > Cc: Alexey Shumitsky 
> > Cc: Emil Andersen Lauridsen 
> > Cc: oceans...@gmail.com
> > Cc: James Hogan 
> > Cc: James Bottomley 
> > Cc: sta...@vger.kernel.org
> > References: 
> > https://lists.freedesktop.org/archives/intel-gfx/2016-August/105545.html
> > References: 
> > https://lists.freedesktop.org/archives/dri-devel/2016-August/116888.html
> > References: 
> > https://lists.freedesktop.org/archives/intel-gfx/2016-June/098826.html
> 
> References: 
> http://patchwork.freedesktop.org/patch/msgid/1473602239-15855-1-git-send-email-adrienve...@gmail.com
> 
> Acked-by: Jani Nikula 
> 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94825
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97060
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97443
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97363
> > Fixes: a05628195a0d ("drm/i915: Get panel_type from OpRegion panel details")
> > Tested-by: Marco Krüger 
> > Tested-by: Alexey Shumitsky 
> > Tested-by: Sean Greenslade 
> > Tested-by: Emil Andersen Lauridsen 
> > Tested-by: Robin Müller 
> > Tested-by: oceans...@gmail.com
> > Signed-off-by: Ville Syrjälä 

Slapped on another tested-by and pushed to dinq. Thanks for the broad
testing, everyone.

> > ---
> >  drivers/gpu/drm/i915/intel_opregion.c | 27 +++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
> > b/drivers/gpu/drm/i915/intel_opregion.c
> > index adca262d591a..7acbbbf97833 100644
> > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > @@ -1047,6 +1047,23 @@ err_out:
> > return err;
> >  }
> >  
> > +static int intel_use_opregion_panel_type_callback(const struct 
> > dmi_system_id *id)
> > +{
> > +   DRM_INFO("Using panel type from OpRegion on %s\n", id->ident);
> > +   return 1;
> > +}
> > +
> > +static const struct dmi_system_id intel_use_opregion_panel_type[] = {
> > +   {
> > +   .callback = intel_use_opregion_panel_type_callback,
> > +   .ident = "Conrac GmbH IX45GM2",
> > +   .matches = {DMI_MATCH(DMI_SYS_VENDOR, "Conrac GmbH"),
> > +   DMI_MATCH(DMI_PRODUCT_NAME, "IX45GM2"),
> > +   },
> > +   },
> > +   { }
> > +};
> > +
> >  int
> >  intel_opregion_get_panel_type(struct drm_i915_private *dev_priv)
> >  {
> > @@ -1073,6 +1090,16 @@ intel_opregion_get_panel_type(struct 
> > drm_i915_private *dev_priv)
> > }
> >  
> > /*
> > +* So far we know that some machined must use it, others must not use 
> > it.
> > +* There doesn't seem to be any way to determine which way to go, except
> > +* via a quirk list :(
> > +*/
> > +   if (!dmi_check_system(intel_use_opregion_panel_type)) {
> > +   DRM_DEBUG_KMS("Ignoring OpRegion panel type 

Re: [Intel-gfx] 4.8-rc1: it is now common that machine needs re-run of xrandr after resume

[Pavel Machek]

On Tue 2016-09-13 22:38:45, Martin Steigerwald wrote:
> Hi.
> 
> Am Dienstag, 13. September 2016, 22:23:50 CEST schrieb Pavel Machek: 
> > I have
> > 
> > 00:02.0 VGA compatible controller: Intel Corporation 4 Series Chipset
> > Integrated Graphics Controller (rev 03)
> 
> 00:02.0 VGA compatible controller [0300]: Intel Corporation 2nd Generation 
> Core Processor Family Integrated Graphics Controller [8086:0126] (rev 09)
> 
> Phoronix Test Suite system-info:

...
> > In previous kernels, resume worked ok. With 4.8-rc1, I quite often (1
> > in 10 resumes?) get in state where primary monitor (DVI) is dead (in
> > powersave) and all windows move to secondary monitor (VGA). Running
> > "xrandr" fixes that.
> 
> I have seen this in 4.8 up to rc5 as well. I am not sure yet about rc6 which 
> I 
> am currently running.

Ok, it happened again today, with yesterdays version of 4.8-rc6. I'm
glad I'm not the only one.

Intel folks, any ideas? Can you reproduce it?

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] 4.8-rc1: it is now common that machine needs re-run of xrandr after resume

[Pavel Machek]

On Wed 2016-09-14 10:38:18, Jani Nikula wrote:
> On Wed, 14 Sep 2016, Pavel Machek  wrote:
> > Hi!
> >
> >> I have
> >> 
> >> 00:02.0 VGA compatible controller: Intel Corporation 4 Series Chipset
> >> Integrated Graphics Controller (rev 03)
> >> 
> >> In previous kernels, resume worked ok. With 4.8-rc1, I quite often (1
> >> in 10 resumes?) get in state where primary monitor (DVI) is dead (in
> >> powersave) and all windows move to secondary monitor (VGA). Running
> >> "xrandr" fixes that.
> >> 
> >> I'll update to newer rc and see if it happens again, but if you have
> >> any ideas, now would be good time.
> >
> > Ok. With -rc6, X are completely broken. I got notification "could not
> > restore CRTC config for screen 63" or something like that, and window
> > manager just does not start.
> 
> Ugh. Can you bisect from v4.7, assuming it worked? That's probably the
> fastest way to resolve this.

The "completely broken" part -- something broke in my userland, as
booting to the old kernel does not fix it. I'll have to figure it out.

For the "sometimes need xrandr after resume": I don't think I can
bisect that. It only happens sometimes :-(. But there's something
helpful in the logs:

Best regards,
Pavel

[ 1856.213154] CPU1 is up
[ 1856.213167] ACPI: Waking up from system sleep state S3
[ 1856.217998] clocksource: Switched to clocksource hpet
[ 1856.218170] uhci_hcd :00:1d.0: System wakeup disabled by ACPI
[ 1856.218470] uhci_hcd :00:1d.2: System wakeup disabled by ACPI
[ 1856.218656] uhci_hcd :00:1d.1: System wakeup disabled by ACPI
[ 1856.218665] uhci_hcd :00:1d.3: System wakeup disabled by ACPI
[ 1856.218863] ehci-pci :00:1d.7: System wakeup disabled by ACPI
[ 1856.218863] PM: noirq resume of devices complete after 19.597 msecs
[ 1856.218863] PM: early resume of devices complete after 1.092 msecs
[ 1856.218863] usb usb2: root hub lost power or was reset
[ 1856.218863] usb usb3: root hub lost power or was reset
[ 1856.218863] usb usb4: root hub lost power or was reset
[ 1856.218863] usb usb5: root hub lost power or was reset
[ 1856.218863] pcieport :00:1c.1: System wakeup disabled by ACPI
[ 1856.218863] serial 00:03: activated
[ 1856.218863] parport_pc 00:04: activated
[ 1856.218863] rtc_cmos 00:05: System wakeup disabled by ACPI
[ 1856.218863] ata2: port disabled--ignoring
[ 1856.218863] r8169 :03:00.0 eth0: link down
[ 1856.218863] sd 2:0:0:0: [sda] Starting disk
[ 1856.218863] sd 2:0:1:0: [sdb] Starting disk
[ 1856.218863] ata4.01: NODEV after polling detection
[ 1856.218863] ata3.01: ACPI cmd ef/03:45:00:00:00:b0 (SET FEATURES)
filtered out
[ 1856.218863] ata3.01: ACPI cmd ef/03:0c:00:00:00:b0 (SET FEATURES)
filtered out
[ 1856.218863] ata3.01: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[ 1856.218863] ata3.00: ACPI cmd ef/03:45:00:00:00:a0 (SET FEATURES)
filtered out
[ 1856.218863] ata3.00: ACPI cmd ef/03:0c:00:00:00:a0 (SET FEATURES)
filtered out
[ 1856.218863] ata3.00: ACPI cmd c6/00:10:00:00:00:a0 (SET MULTIPLE
MODE) succeeded
[ 1856.218863] ata3.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[ 1856.218863] ata3.00: configured for UDMA/133
[ 1856.218863] ata4.00: ACPI cmd ef/03:45:00:00:00:a0 (SET FEATURES)
filtered out
[ 1856.218863] ata4.00: ACPI cmd ef/03:0c:00:00:00:a0 (SET FEATURES)
filtered out
[ 1856.218863] ata4.00: ACPI cmd f5/00:00:00:00:00:00 (SECURITY FREEZE
LOCK) filtered out
[ 1856.218863] ata3.01: configured for UDMA/133
[ 1856.218863] ata4.00: configured for UDMA/133
[ 1856.218863] [drm:drm_edid_block_valid] *ERROR* EDID checksum is
invalid, remainder is 130
[ 1856.218863] Raw EDID:
[ 1856.218863] 00 ff ff ff ff ff ff 00 ff ff ff ff ff ff ff ff
[ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 1856.218863] [drm:drm_edid_block_valid] *ERROR* EDID checksum is
invalid, remainder is 130
[ 1856.218863] Raw EDID:
[ 1856.218863] 00 ff ff ff ff ff ff 00 ff ff ff ff ff ff ff ff
[ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 1856.218863] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[ 1856.218863] 

Re: [Intel-gfx] [PATCH] drm/i915: Ignore OpRegion panel type except on select machines

[James Hogan]

On 13 September 2016 at 10:22,   wrote:
> From: Ville Syrjälä 
>
> Turns out
> commit a05628195a0d ("drm/i915: Get panel_type from OpRegion panel
> details") has regressed quite a few machines. So it looks like we
> can't use the panel type from OpRegion on all systems, and yet we
> absolutely must use it on some specific systems.
>
> Despite trying, I was unable to find any automagic way to determine
> if the OpRegion panel type is respectable or not. The only glimmer
> of hope I had was bit 8 in the SCIC response, but that turned out to
> not work either (it was always 0 on both types of systems).
>
> So, to fix the regressions without breaking the machine we know to need
> the OpRegion panel type, let's just add a quirk for this. Only specific
> machines known to require the OpRegion panel type will therefore use
> it. Everyone else will fall bck to the VBT panel type.
>
> The only known machine so far is a "Conrac GmbH IX45GM2". The PCI
> subsystem ID on this machine is just a generic 8086:2a42, so of no use.
> Instead we'll go with a DMI match.
>
> I suspect we can now also revert
> commit aeddda06c1a7 ("drm/i915: Ignore panel type from OpRegion on SKL")
> but let's leave that to a separate patch.
>
> v2: Do the DMI match in the opregion code directly, as dev_priv->quirks
> gets populated too late
>
> Cc: Rob Kramer 
> Cc: Martin van Es 
> Cc: Andrea Arcangeli 
> Cc: Dave Airlie 
> Cc: Marco Krüger 
> Cc: Sean Greenslade 
> Cc: Trudy Tective 
> Cc: Robin Müller 
> Cc: Alexander Kobel 
> Cc: Alexey Shumitsky 
> Cc: Emil Andersen Lauridsen 
> Cc: oceans...@gmail.com
> Cc: James Hogan 
> Cc: James Bottomley 
> Cc: sta...@vger.kernel.org
> References: 
> https://lists.freedesktop.org/archives/intel-gfx/2016-August/105545.html
> References: 
> https://lists.freedesktop.org/archives/dri-devel/2016-August/116888.html
> References: 
> https://lists.freedesktop.org/archives/intel-gfx/2016-June/098826.html
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94825
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97060
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97443
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97363
> Fixes: a05628195a0d ("drm/i915: Get panel_type from OpRegion panel details")
> Tested-by: Marco Krüger 
> Tested-by: Alexey Shumitsky 
> Tested-by: Sean Greenslade 
> Tested-by: Emil Andersen Lauridsen 
> Tested-by: Robin Müller 
> Tested-by: oceans...@gmail.com
> Signed-off-by: Ville Syrjälä 

That works for me too on XPS13. Flickering screen brightness gone, and
using acpi backlight rather than intel backlight, like before
a05628195a0d ("drm/i915: Get panel_type from OpRegion panel details").

Tested-by: James Hogan 

Thanks!
James

> ---
>  drivers/gpu/drm/i915/intel_opregion.c | 27 +++
>  1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
> b/drivers/gpu/drm/i915/intel_opregion.c
> index adca262d591a..7acbbbf97833 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -1047,6 +1047,23 @@ err_out:
> return err;
>  }
>
> +static int intel_use_opregion_panel_type_callback(const struct dmi_system_id 
> *id)
> +{
> +   DRM_INFO("Using panel type from OpRegion on %s\n", id->ident);
> +   return 1;
> +}
> +
> +static const struct dmi_system_id intel_use_opregion_panel_type[] = {
> +   {
> +   .callback = intel_use_opregion_panel_type_callback,
> +   .ident = "Conrac GmbH IX45GM2",
> +   .matches = {DMI_MATCH(DMI_SYS_VENDOR, "Conrac GmbH"),
> +   DMI_MATCH(DMI_PRODUCT_NAME, "IX45GM2"),
> +   },
> +   },
> +   { }
> +};
> +
>  int
>  intel_opregion_get_panel_type(struct drm_i915_private *dev_priv)
>  {
> @@ -1073,6 +1090,16 @@ intel_opregion_get_panel_type(struct drm_i915_private 
> *dev_priv)
> }
>
> /*
> +* So far we know that some machined must use it, others must not use 
> it.
> +* There doesn't seem to be any way to determine which way to go, 
> except
> +* via a quirk list :(
> +*/
> +   if (!dmi_check_system(intel_use_opregion_panel_type)) {
> +   DRM_DEBUG_KMS("Ignoring OpRegion panel type (%d)\n", ret - 1);
> +   return -ENODEV;
> +   }
> +
> +   /*
>  * FIXME On Dell XPS 13 9350 the OpRegion panel type (0) gives us
>  * low vswing for eDP, 

Re: [Intel-gfx] [PATCH 02/18] drm/i915: Allow i915_sw_fence_await_sw_fence() to allocate

[Chris Wilson]

On Wed, Sep 14, 2016 at 10:51:12AM +0300, Joonas Lahtinen wrote:
> On ke, 2016-09-14 at 07:52 +0100, Chris Wilson wrote:
> > @@ -678,7 +678,7 @@ void __i915_add_request(struct drm_i915_gem_request 
> > *request, bool flush_caches)
> >        >i915->drm.struct_mutex);
> >     if (prev)
> >     i915_sw_fence_await_sw_fence(>submit, >submit,
> > -    >submitq);
> > +    >submitq, GFP_NOWAIT);
> 
> Wrt commit message, why do we pass both here? If one was to run
> statistic analysis, !wq is never true if you pass  here.

Only because GFP_NOWAIT was descriptive, and cleaner than say
(__force gfp_t)0

> > @@ -135,6 +135,8 @@ static int i915_sw_fence_wake(wait_queue_t *wq, 
> > unsigned mode, int flags, void *
> >     list_del(>task_list);
> >     __i915_sw_fence_complete(wq->private, key);
> >     i915_sw_fence_put(wq->private);
> > +   if (wq->flags)
> > +   kfree(wq);
> 
> This is confusing without a comment or proper flag #define.
> 
> >     INIT_LIST_HEAD(>task_list);
> > -   wq->flags = 0;
> > +   wq->flags = pending;
> 
> Why not make this look proper by using I915_SW_FENCE_FLAG_FOO name.
> 
> > +static inline void i915_sw_fence_wait(struct i915_sw_fence *fence)
> > +{
> > +   wait_event(fence->wait, i915_sw_fence_done(fence));
> > +}
> > +
> 
> This seems to be a lost-in-rebasing hunk.

I snuck in a use along the oom path to justify it here (and avoid having
to magic it out of nowhere later).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 03/18] drm/i915: Rearrange i915_wait_request() accounting with callers

[Joonas Lahtinen]

On ke, 2016-09-14 at 07:52 +0100, Chris Wilson wrote:
> +int
> +i915_gem_object_wait(struct drm_i915_gem_object *obj,
> +  unsigned int flags,
> +  long timeout,
> +  struct intel_rps_client *rps)
>  {
>  

[...]

> - return 0;
> + resv = i915_gem_object_get_dmabuf_resv(obj);
> + if (resv)
> + timeout = i915_gem_object_wait_reservation(resv,
> +    flags, timeout,
> +    rps);
> + return timeout < 0 ? timeout : timeout > 0 ? 0 : -ETIME;

Format this in a more readable manner eg.;

return timeout == 0 ? -ETIME :
       timeout < 0 ? timeout :
       0;

>  
>  static struct intel_rps_client *to_rps_client(struct drm_file *file)
> @@ -454,7 +542,13 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj,
>   /* We manually control the domain here and pretend that it
>    * remains coherent i.e. in the GTT domain, like shmem_pwrite.
>    */
> - ret = i915_gem_object_wait_rendering(obj, false);
> + lockdep_assert_held(>base.dev->struct_mutex);

Bump this before the comment to the beginning of function like
elsehwere.

> @@ -2804,17 +2923,21 @@ i915_gem_wait_ioctl(struct drm_device *dev, void 
> *data, struct drm_file *file)
>   if (!obj)
>   return -ENOENT;
>  
> - active = __I915_BO_ACTIVE(obj);
> - for_each_active(active, idx) {
> - s64 *timeout = args->timeout_ns >= 0 ? >timeout_ns : NULL;
> - ret = i915_gem_active_wait_unlocked(>last_read[idx],
> - I915_WAIT_INTERRUPTIBLE,
> - timeout, rps);
> - if (ret)
> - break;
> + start = ktime_get();
> +
> + ret = i915_gem_object_wait(obj,
> +    I915_WAIT_INTERRUPTIBLE | I915_WAIT_ALL,
> +    args->timeout_ns < 0 ? MAX_SCHEDULE_TIMEOUT 
> : nsecs_to_jiffies(args->timeout_ns),

Do break this line, plz.

Maybe just have long timeout = MAX_SCHEDULE_TIMEOUT; in the beginning
of file, then do if (args->timeout_ns >= 0) before the function, it
matches the after function if nicely.


> +    to_rps_client(file));
> +
> + if (args->timeout_ns > 0) {

And as we have this.

> + args->timeout_ns -= ktime_to_ns(ktime_sub(ktime_get(), start));
> + if (args->timeout_ns < 0)
> + args->timeout_ns = 0;
>   }
>  
>   i915_gem_object_put_unlocked(obj);
> +
>   return ret;
>  }
> 



> 
> @@ -3598,7 +3732,13 @@ i915_gem_object_set_to_cpu_domain(struct 
> drm_i915_gem_object *obj, bool write)
>   uint32_t old_write_domain, old_read_domains;
>   int ret;
>  
> - ret = i915_gem_object_wait_rendering(obj, !write);
> + lockdep_assert_held(>base.dev->struct_mutex);

I'd add a newline here like elsewhere.

> + ret = i915_gem_object_wait(obj,
> +    I915_WAIT_INTERRUPTIBLE |
> +    I915_WAIT_LOCKED |
> +    (write ? I915_WAIT_ALL : 0),
> +    MAX_SCHEDULE_TIMEOUT,
> +    NULL);
>   if (ret)
>   return ret;
>  
> @@ -3654,11 +3794,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct 
> drm_file *file)
>   struct drm_i915_file_private *file_priv = file->driver_priv;
>   unsigned long recent_enough = jiffies - DRM_I915_THROTTLE_JIFFIES;
>   struct drm_i915_gem_request *request, *target = NULL;
> - int ret;
> -
> - ret = i915_gem_wait_for_error(_priv->gpu_error);
> - if (ret)
> - return ret;

Unsure how this is related to the changes, need to explain in commit
message or I nominate this a lost hunk.

With those addressed,

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [2/9] drm/doc: Polish kerneldoc for encoders

[Pandiyan, Dhinakaran]

I guess it's too late for review now. But, I want to send this anyway.

On Mon, 2016-08-29 at 10:27 +0200, Daniel Vetter wrote:
> - Move missing bits into struct drm_encoder docs.
> - Explain that encoders are 95% internal and only 5% uapi, and that in
>   general the uapi part is broken.
> - Remove verbose comments for functions not exposed to drivers.
> 
> v2: Review from Archit:
> - Appease checkpatch in the moved code.
> - Make it clearer that bridges are not exposed to userspace.
> 
> Reviewed-by: Archit Taneja 
> Signed-off-by: Daniel Vetter 
> ---
>  Documentation/gpu/drm-kms.rst | 46 
>  drivers/gpu/drm/drm_encoder.c | 48 ++---
>  include/drm/drm_encoder.h | 70 
> +++
>  3 files changed, 101 insertions(+), 63 deletions(-)
> 
> diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
> index 7f788caebea3..47c2835b7c2d 100644
> --- a/Documentation/gpu/drm-kms.rst
> +++ b/Documentation/gpu/drm-kms.rst
> @@ -128,6 +128,12 @@ Connector Functions Reference
>  Encoder Abstraction
>  ===
>  
> +.. kernel-doc:: drivers/gpu/drm/drm_encoder.c
> +   :doc: overview
> +
> +Encoder Functions Reference
> +---
> +
>  .. kernel-doc:: include/drm/drm_encoder.h
> :internal:
>  
> @@ -207,46 +213,6 @@ future); drivers that do not wish to provide special 
> handling for
>  primary planes may make use of the helper functions described in ? to
>  create and register a primary plane with standard capabilities.
>  
> -Encoders (:c:type:`struct drm_encoder `)
> --
> -
> -An encoder takes pixel data from a CRTC and converts it to a format
> -suitable for any attached connectors. On some devices, it may be
> -possible to have a CRTC send data to more than one encoder. In that
> -case, both encoders would receive data from the same scanout buffer,
> -resulting in a "cloned" display configuration across the connectors
> -attached to each encoder.
> -
> -Encoder Initialization
> -~~
> -
> -As for CRTCs, a KMS driver must create, initialize and register at least
> -one :c:type:`struct drm_encoder ` instance. The
> -instance is allocated and zeroed by the driver, possibly as part of a
> -larger structure.
> -
> -Drivers must initialize the :c:type:`struct drm_encoder
> -` possible_crtcs and possible_clones fields before
> -registering the encoder. Both fields are bitmasks of respectively the
> -CRTCs that the encoder can be connected to, and sibling encoders
> -candidate for cloning.
> -
> -After being initialized, the encoder must be registered with a call to
> -:c:func:`drm_encoder_init()`. The function takes a pointer to the
> -encoder functions and an encoder type. Supported types are
> -
> --  DRM_MODE_ENCODER_DAC for VGA and analog on DVI-I/DVI-A
> --  DRM_MODE_ENCODER_TMDS for DVI, HDMI and (embedded) DisplayPort
> --  DRM_MODE_ENCODER_LVDS for display panels
> --  DRM_MODE_ENCODER_TVDAC for TV output (Composite, S-Video,
> -   Component, SCART)
> --  DRM_MODE_ENCODER_VIRTUAL for virtual machine displays
> -
> -Encoders must be attached to a CRTC to be used. DRM drivers leave
> -encoders unattached at initialization time. Applications (or the fbdev
> -compatibility layer when implemented) are responsible for attaching the
> -encoders they want to use to a CRTC.
> -
>  Cleanup
>  ---
>  
> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> index bce781b7bb5f..998a6743a586 100644
> --- a/drivers/gpu/drm/drm_encoder.c
> +++ b/drivers/gpu/drm/drm_encoder.c
> @@ -26,6 +26,30 @@
>  
>  #include "drm_crtc_internal.h"
>  
> +/**
> + * DOC: overview
> + *
> + * Encoders represent the connecting element between the CRTC (as the overall
> + * pixel pipeline, represented by struct _crtc) and the connectors (as 
> the
> + * generic sink entity, represented by struct _connector). Encoders are

Doesn't really say what encoders actually do apart being in between crtc
and connector. This line could have been useful here - "An encoder takes
pixel data from a CRTC and converts it to a format suitable for any
attached connectors." 

> + * objects exposed to userspace, originally to allow userspace to infer 
> cloning
> + * and connector/CRTC restrictions. Unfortunately almost all drivers get this
> + * wrong, making the uabi pretty much useless. On top of that the exposed
> + * restrictions are too simple for todays hardware, and the recommend way to
> + * infer restrictions is by using the DRM_MODE_ATOMIC_TEST_ONLY flag for the
> + * atomic IOCTL.
> + *
> + * Otherwise encoders aren't used in the uapi at all (any modeset request 
> from
> + * userspace directly connects a connector with a CRTC), drivers are 
> therefore
> + * free to use them however they wish. Modeset helper libraries make strong 
> use
> + * of encoders to facilitate