Re: [Intel-gfx] [PATCH 2/5] drm/i915: Read the CCK fuse register from CCK

[Deepak S]

On Saturday 08 November 2014 01:03 AM, ville.syrj...@linux.intel.com wrote:

From: Ville Syrjälä 

When reading a CCK register we should obviously read it from CCK not
Punit. This problem has been present ever since this of code was
introduced in

  commit 67c3bf6f55a97a0915a0f9ea07278a3073cc9601
  Author: Deepak S 
  Date:   Thu Jul 10 13:16:24 2014 +0530

 drm/i915: populate mem_freq/cz_clock for chv

The problem was raised during review by Mika [1] but somehow slipped
through the cracks, and the patch got applied with the problem unfixed.

[1] http://lists.freedesktop.org/archives/intel-gfx/2014-July/048937.html

Cc: Deepak S 
Cc: Mika Kuoppala 
Signed-off-by: Ville Syrjälä 
---
  drivers/gpu/drm/i915/intel_pm.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9285dee..befad36 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5253,7 +5253,10 @@ static void cherryview_init_gt_powersave(struct 
drm_device *dev)
  
  	mutex_lock(&dev_priv->rps.hw_lock);

-   val = vlv_punit_read(dev_priv, CCK_FUSE_REG);
+   mutex_lock(&dev_priv->dpio_lock);
+   val = vlv_cck_read(dev_priv, CCK_FUSE_REG);
+   mutex_unlock(&dev_priv->dpio_lock);
+
switch ((val >> 2) & 0x7) {
case 0:
case 1:



Oops i missed the  comment.
Reviewed-by: Deepak S 

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

Re: [Intel-gfx] [PATCH v7] drm/i915: Add tracepoints to track a vm

[shuang . he]

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=277/348->276/348
PNV: pass/total=323/328->325/328
ILK: pass/total=328/330->329/330
IVB: pass/total=545/546->544/546
SNB: pass/total=558/563->559/563
HSW: pass/total=577/581->574/581
BDW: pass/total=435/435->434/435
-Detailed-
test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, 
machine_id...)...->result_with_patch_applied(count, machine_id)...
BYT: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc-lut, NSPT(1, M29)PASS(9, 
M36M29) -> NSPT(1, M29)PASS(3, M29)
PNV: Intel_gpu_tools, igt_gem_mmap_offset_exhaustion, DMESG_WARN(2, 
M23M24)PASS(14, M24M23M7) -> PASS(4, M7)
PNV: Intel_gpu_tools, igt_gem_unref_active_buffers, DMESG_WARN(2, M23)PASS(14, 
M24M23M7) -> PASS(4, M7)
ILK: Intel_gpu_tools, igt_kms_render_gpu-blit, DMESG_WARN(1, M26)PASS(15, 
M6M26M37) -> PASS(4, M26)
ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-rmfb, PASS(4, M26) -> DMESG_WARN(2, 
M26)PASS(2, M26)
ILK: Intel_gpu_tools, igt_kms_flip_wf_vblank-vs-dpms-interruptible, 
DMESG_WARN(1, M26)PASS(15, M6M26M37) -> PASS(4, M26)
IVB: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, NSPT(4, M4M34)PASS(9, 
M34M4) -> NSPT(2, M34)PASS(2, M34)
SNB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-sliding, DMESG_WARN(3, 
M35M22)PASS(7, M22M35) -> PASS(4, M22)
HSW: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, NSPT(4, 
M39M20M40)PASS(9, M40M39M20) -> NSPT(2, M39)PASS(2, M39)
HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-64x64-random, FAIL(1, 
M39)PASS(3, M40M39) -> FAIL(1, M39)DMESG_WARN(2, M39)PASS(1, M39)
HSW: Intel_gpu_tools, igt_kms_rotation_crc_primary-rotation, PASS(4, M40M39) -> 
DMESG_WARN(1, M39)PASS(3, M39)
BDW: Intel_gpu_tools, igt_gem_reset_stats_ban-bsd, PASS(16, M30M28M42) -> 
DMESG_WARN(1, M42)PASS(3, M42)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4.1] drm/i915: WARN if we receive any gen9 rps

[shuang . he]

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=277/348->277/348
PNV: pass/total=323/328->326/328
ILK: pass/total=328/330->330/330
IVB: pass/total=545/546->545/546
SNB: pass/total=558/563->559/563
HSW: pass/total=574/578->572/578
BDW: pass/total=435/435->434/435
-Detailed-
test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, 
machine_id...)...->result_with_patch_applied(count, machine_id)...
PNV: Intel_gpu_tools, igt_gem_linear_blits_normal, NSPT(1, M23)PASS(6, M24) -> 
PASS(4, M24)
PNV: Intel_gpu_tools, igt_gem_mmap_offset_exhaustion, DMESG_WARN(2, 
M23M24)PASS(14, M24M23M7) -> DMESG_WARN(1, M24)PASS(3, M24)
PNV: Intel_gpu_tools, igt_gem_unref_active_buffers, DMESG_WARN(2, M23)PASS(14, 
M24M23M7) -> PASS(4, M24)
ILK: Intel_gpu_tools, igt_kms_render_gpu-blit, DMESG_WARN(1, M26)PASS(15, 
M6M26M37) -> PASS(4, M37)
ILK: Intel_gpu_tools, igt_kms_flip_wf_vblank-vs-dpms-interruptible, 
DMESG_WARN(1, M26)PASS(15, M6M26M37) -> PASS(4, M37)
IVB: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, NSPT(3, M4M34)PASS(7, 
M34M4) -> NSPT(2, M4)PASS(2, M4)
IVB: Intel_gpu_tools, igt_kms_plane_plane-position-covered-pipe-A-plane-1, 
TIMEOUT(1, M34)PASS(6, M4) -> PASS(4, M4)
SNB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-sliding, DMESG_WARN(2, 
M35)PASS(5, M22M35) -> DMESG_WARN(1, M35)PASS(3, M35)
HSW: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, NSPT(4, 
M39M20M40)PASS(9, M40M39M20) -> NSPT(2, M40)PASS(2, M40)
HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-64x64-offscreen, DMESG_WARN(1, 
M39)PASS(6, M40M39) -> DMESG_WARN(1, M40)PASS(3, M40)
BDW: Intel_gpu_tools, igt_gem_reset_stats_ban-bsd, PASS(16, M30M28M42) -> 
DMESG_WARN(1, M28)PASS(3, M28)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld()

[Daniel Vetter]

On Tue, Oct 28, 2014 at 04:20:48PM +0200, Jani Nikula wrote:
> The Baseline_ELD_Len field does not include ELD Header Block size.
> 
> From High Definition Audio Specification, Revision 1.0a:
> 
>   The header block is a fixed size of 4 bytes. The baseline block
>   is variable size in multiple of 4 bytes, and its size is defined
>   in the header block Baseline_ELD_Len field (in number of
>   DWords).
> 
> Do not include the header size in Baseline_ELD_Len field. Fix all known
> users of eld[2].
> 
> While at it, switch to DIV_ROUND_UP instead of open coding it.
> 
> Signed-off-by: Jani Nikula 

Queued for -next with a pile of acks and one fixup to make it compile,
thanks for the patch.
-Daniel
> 
> ---
> 
> This is based on an audio rework series which is mid-way being merged to
> i915. I don't think this should be cc: stable worthy, as, AFAICT, we
> don't use the vendor block, and anyone reading SADs respecting SAD_Count
> should stop at the same offset regardless of this patch. So I propose
> this gets eventually merged via i915 without a rush.
> ---
>  drivers/gpu/drm/drm_edid.c |  7 +--
>  drivers/gpu/drm/i915/intel_audio.c | 16 
>  drivers/gpu/drm/nouveau/nv50_display.c |  3 ++-
>  3 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 3bf999134bcc..45aaa6f5ef36 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3128,9 +3128,12 @@ void drm_edid_to_eld(struct drm_connector *connector, 
> struct edid *edid)
>   }
>   }
>   eld[5] |= sad_count << 4;
> - eld[2] = (20 + mnl + sad_count * 3 + 3) / 4;
>  
> - DRM_DEBUG_KMS("ELD size %d, SAD count %d\n", (int)eld[2], sad_count);
> + eld[DRM_ELD_BASELINE_ELD_LEN] =
> + DIV_ROUND_UP(drm_eld_calc_baseline_block_size(eld), 4);
> +
> + DRM_DEBUG_KMS("ELD size %d, SAD count %d\n",
> +   drm_eld_size(eld), sad_count);
>  }
>  EXPORT_SYMBOL(drm_edid_to_eld);
>  
> diff --git a/drivers/gpu/drm/i915/intel_audio.c 
> b/drivers/gpu/drm/i915/intel_audio.c
> index 20af973d7cba..439fa4afa18b 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -107,7 +107,7 @@ static bool intel_eld_uptodate(struct drm_connector 
> *connector,
>   tmp &= ~bits_elda;
>   I915_WRITE(reg_elda, tmp);
>  
> - for (i = 0; i < eld[2]; i++)
> + for (i = 0; i < drm_eld_size(eld) / 4; i++)
>   if (I915_READ(reg_edid) != *((uint32_t *)eld + i))
>   return false;
>  
> @@ -162,7 +162,7 @@ static void g4x_audio_codec_enable(struct drm_connector 
> *connector,
>   len = (tmp >> 9) & 0x1f;/* ELD buffer size */
>   I915_WRITE(G4X_AUD_CNTL_ST, tmp);
>  
> - len = min_t(int, eld[2], len);
> + len = min(drm_eld_size(eld) / 4, len);
>   DRM_DEBUG_DRIVER("ELD size %d\n", len);
>   for (i = 0; i < len; i++)
>   I915_WRITE(G4X_HDMIW_HDMIEDID, *((uint32_t *)eld + i));
> @@ -209,7 +209,7 @@ static void hsw_audio_codec_enable(struct drm_connector 
> *connector,
>   int len, i;
>  
>   DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
> -   pipe_name(pipe), eld[2]);
> +   pipe_name(pipe), drm_eld_size(eld));
>  
>   /* Enable audio presence detect, invalidate ELD */
>   tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> @@ -225,8 +225,8 @@ static void hsw_audio_codec_enable(struct drm_connector 
> *connector,
>   I915_WRITE(HSW_AUD_DIP_ELD_CTRL(pipe), tmp);
>  
>   /* Up to 84 bytes of hw ELD buffer */
> - len = min_t(int, eld[2], 21);
> - for (i = 0; i < len; i++)
> + len = min(drm_eld_size(eld), 84);
> + for (i = 0; i < len / 4; i++)
>   I915_WRITE(HSW_AUD_EDID_DATA(pipe), *((uint32_t *)eld + i));
>  
>   /* ELD valid */
> @@ -315,7 +315,7 @@ static void ilk_audio_codec_enable(struct drm_connector 
> *connector,
>   int aud_cntrl_st2;
>  
>   DRM_DEBUG_KMS("Enable audio codec on port %c, pipe %c, %u bytes ELD\n",
> -   port_name(port), pipe_name(pipe), eld[2]);
> +   port_name(port), pipe_name(pipe), drm_eld_size(eld));
>  
>   /* XXX: vblank wait here */
>  
> @@ -354,8 +354,8 @@ static void ilk_audio_codec_enable(struct drm_connector 
> *connector,
>   I915_WRITE(aud_cntl_st, tmp);
>  
>   /* Up to 84 bytes of hw ELD buffer */
> - len = min_t(int, eld[2], 21);
> - for (i = 0; i < len; i++)
> + len = min(drm_eld_size(eld), 84);
> + for (i = 0; i < len / 4; i++)
>   I915_WRITE(hdmiw_hdmiedid, *((uint32_t *)eld + i));
>  
>   /* ELD valid */
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c 
> b/drivers/gpu/drm/nouveau/nv50_display.c
> index ae873d1a8d46..d92c11484bd9 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -

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

[Bob Paauwe]

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

Signed-off-by: Bob Paauwe 
CC: Ander Conselvan de Oliveira 
CC: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_display.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index ff071a7..601641d 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) &&
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;
}
 }
 
-- 
1.8.3.1

___
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 initialize pipe config after choosing DPLLs.

[Bob Paauwe]

On Mon, 10 Nov 2014 12:40:47 +0200
Ville Syrjälä  wrote:

> On Fri, Nov 07, 2014 at 04:07:50PM -0800, Bob Paauwe wrote:
> > The pipe config needs to be initialized before calling crtc_compute_clock
> > since this will update the new_config structure DPLL values. Initializing
> > the new_config structure after calling crtc_compute_clock can result in
> > incorrect timing values.
> > 
> > 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
> > 
> > Signed-off-by: Bob Paauwe 
> > CC: Ander Conselvan de Oliveira 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 10 +-
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index ff071a7..53f3d3a 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10774,7 +10774,11 @@ static int __intel_set_mode(struct drm_crtc *crtc,
> > }
> > intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
> >"[modeset]");
> > -   to_intel_crtc(crtc)->new_config = pipe_config;
> 
> new_config _is_ initialized here.
> 
> > +
> > +   /* mode_set/enable/disable functions rely on a correct pipe
> > +* config. */
> > +   to_intel_crtc(crtc)->config = *pipe_config;
> > +   to_intel_crtc(crtc)->new_config = &to_intel_crtc(crtc)->config;
> 
> And this will clobber the old config before we even know if the modeset
> will succeed. That's not what we want.

Ahh, I was looking at this wrong before. For some reason I was thinking
that when this was done below, it was overwriting something that was
set in new_config/pipe_config.
> 
> You didn't really describe the problem you're seeing, so coming up with
> theories is a bit hard. I guess one problem could be that some piece of
> code is still looking at crtc->config when it should be looking at
> crtc->new_config. In any case, I suggest you tell us a bit more before
> anyone spends too much time guessing.

With the series that changes this to choose DPLLs before disabling
CRTCs, my 945 system fails to set the initial mode (no display) and I
get this error:


Nov 11 03:47:07 localhost kernel: [2.086190] [drm:intel_pipe_config_compare 
[i915]] *ERROR* mismatch in adjusted_mode.crtc_clock (expected 148500, found 
57600)
Nov 11 03:47:07 localhost kernel: [2.086191] [ cut here 
]
Nov 11 03:47:07 localhost kernel: [2.086238] WARNING: CPU: 1 PID: 56 at 
/home/bpaauwe/git/otc/drm-intel/drivers/gpu/drm/i915/intel_display.c:10650 
check_crtc_state+0x244/0x2ac [i915]()
Nov 11 03:47:07 localhost kernel: [2.086239] pipe state doesn't match!

I bisected it back to the commit referenced above.

I had been thinking that something was not getting set property, but your
insight that maybe something was using the old values is right.  I found 
where it's doing that.  I'll send out a new patch shortly.

> 
> > }
> >  
> > /*
> > @@ -10820,10 +10824,6 @@ static int __intel_set_mode(struct drm_crtc *crtc,
> >  */
> > if (modeset_pipes) {
> > crtc->mode = *mode;
> > -   /* mode_set/enable/disable functions rely on a correct pipe
> > -* config. */
> > -   to_intel_crtc(crtc)->config = *pipe_config;
> > -   to_intel_crtc(crtc)->new_config = &to_intel_crtc(crtc)->config;
> >  
> > /*
> >  * Calculate and store various constants which
> > -- 
> > 1.8.3.1
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

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

[Intel-gfx] [PATCH 0/4] drm/i915: Additional CHV RPS fixes

[ville . syrjala]

From: Ville Syrjälä 

I was staring at the GPU frquencies my BSW reports and they didn't seem
to match the docs exactly, so I set out to fix a few things. Now things
should match what the docs. No idea if the docs are correct anymore
though, but at least the date on the spreadsheet I used was fairly recent.

And I ended up doing a bit of refactoring on the way, so the LOC went
down a bit, which is always nice :)

Ville Syrjälä (4):
  drm/i915: Refactor vlv/chv GPU frequency divider setup
  drm/i915: Fix chv GPU freq<->opcode conversions
  drm/i915: Add missing newline to 'DDR speed' debug messages
  drm/i915: Change CHV SKU400 GPU freq divider to 10

 drivers/gpu/drm/i915/intel_pm.c | 109 ++--
 1 file changed, 38 insertions(+), 71 deletions(-)

-- 
2.0.4

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

Re: [Intel-gfx] [Regression] 83f45fc turns machine's screen off

[Daniel Vetter]

Adding relevant mailing lists.


On Sat, Nov 8, 2014 at 7:34 PM, Emmanuel Benisty  wrote:
> Hi,
>
> The following commit permanently turns my screen off when x server is
> started (i3 330M Ironlake):
>
> [83f45fc360c8e16a330474860ebda872d1384c8c] drm: Don't grab an fb
> reference for the idr
>
> Reverting this commit fixed the issue.

This is definitely unexpected. I think we need a bit more data to
figure out what's going on here:
- Please boot with drm.debug=0xe added to your kernel cmdline and grab
the dmesg right after boot-up for both a working or broken kernel.
- Are you using any special i915 cmdline options?

Everything else should be in the debug logs I hope.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 4/4] drm/i915: Change CHV SKU400 GPU freq divider to 10

[ville . syrjala]

From: Ville Syrjälä 

According to "Cherryview_GFXclocks_y14w36d1.xlsx" the GPU frequency
divider should be 10 in when the CZ clock is 400 MHz. Change the code
to agree so that we report the correct frequencies.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_pm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0f5c391..b73506f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7292,8 +7292,9 @@ static int vlv_gpu_freq_div(unsigned int czclk_freq)
return 12;
case 320:
case 333:
-   case 400:
return 16;
+   case 400:
+   return 20;
default:
return -1;
}
-- 
2.0.4

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

[Intel-gfx] [PATCH 2/4] drm/i915: Fix chv GPU freq<->opcode conversions

[ville . syrjala]

From: Ville Syrjälä 

Currently we miscalculate the GPU frequency on chv. This causes us to
report the GPU frequency as half of what it really is. Drop the extra
factor of 2 from the calculations to get the correct answer.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_pm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 03fbb45..74e4293 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7329,7 +7329,7 @@ static int chv_gpu_freq(struct drm_i915_private 
*dev_priv, int val)
if (div < 0)
return div;
 
-   return DIV_ROUND_CLOSEST(czclk_freq * val, 2 * div) / 2;
+   return DIV_ROUND_CLOSEST(czclk_freq * val, 2 * div);
 }
 
 static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val)
@@ -7341,7 +7341,7 @@ static int chv_freq_opcode(struct drm_i915_private 
*dev_priv, int val)
return mul;
 
/* CHV needs even values */
-   return DIV_ROUND_CLOSEST(val * 2 * mul, czclk_freq) * 2;
+   return DIV_ROUND_CLOSEST(val * mul, czclk_freq) * 2;
 }
 
 int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val)
-- 
2.0.4

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

[Intel-gfx] [PATCH 3/4] drm/i915: Add missing newline to 'DDR speed' debug messages

[ville . syrjala]

From: Ville Syrjälä 

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_pm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 74e4293..0f5c391 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5217,7 +5217,7 @@ static void valleyview_init_gt_powersave(struct 
drm_device *dev)
dev_priv->mem_freq = 1333;
break;
}
-   DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
+   DRM_DEBUG_DRIVER("DDR speed: %d MHz\n", dev_priv->mem_freq);
 
dev_priv->rps.max_freq = valleyview_rps_max_freq(dev_priv);
dev_priv->rps.rp0_freq = dev_priv->rps.max_freq;
@@ -5286,7 +5286,7 @@ static void cherryview_init_gt_powersave(struct 
drm_device *dev)
dev_priv->mem_freq = 1600;
break;
}
-   DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
+   DRM_DEBUG_DRIVER("DDR speed: %d MHz\n", dev_priv->mem_freq);
 
dev_priv->rps.max_freq = cherryview_rps_max_freq(dev_priv);
dev_priv->rps.rp0_freq = dev_priv->rps.max_freq;
-- 
2.0.4

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

[Intel-gfx] [PATCH 1/4] drm/i915: Refactor vlv/chv GPU frequency divider setup

[ville . syrjala]

From: Ville Syrjälä 

The divider used in the GPU frequency calculations is compatible between
vlv and chv. vlv just wants doubled values compared to chv.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_pm.c | 104 ++--
 1 file changed, 35 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ef8e055..03fbb45 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -7283,99 +7283,65 @@ int sandybridge_pcode_write(struct drm_i915_private 
*dev_priv, u8 mbox, u32 val)
return 0;
 }
 
-static int byt_gpu_freq(struct drm_i915_private *dev_priv, int val)
+static int vlv_gpu_freq_div(unsigned int czclk_freq)
 {
-   int div;
-
-   /* 4 x czclk */
-   switch (dev_priv->mem_freq) {
-   case 800:
-   div = 10;
-   break;
-   case 1066:
-   div = 12;
-   break;
-   case 1333:
-   div = 16;
-   break;
+   switch (czclk_freq) {
+   case 200:
+   return 10;
+   case 267:
+   return 12;
+   case 320:
+   case 333:
+   case 400:
+   return 16;
default:
return -1;
}
+}
 
-   return DIV_ROUND_CLOSEST(dev_priv->mem_freq * (val + 6 - 0xbd), 4 * 
div);
+static int byt_gpu_freq(struct drm_i915_private *dev_priv, int val)
+{
+   int div, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->mem_freq, 4);
+
+   div = vlv_gpu_freq_div(czclk_freq);
+   if (div < 0)
+   return div;
+
+   return DIV_ROUND_CLOSEST(czclk_freq * (val + 6 - 0xbd), div);
 }
 
 static int byt_freq_opcode(struct drm_i915_private *dev_priv, int val)
 {
-   int mul;
+   int mul, czclk_freq = DIV_ROUND_CLOSEST(dev_priv->mem_freq, 4);
 
-   /* 4 x czclk */
-   switch (dev_priv->mem_freq) {
-   case 800:
-   mul = 10;
-   break;
-   case 1066:
-   mul = 12;
-   break;
-   case 1333:
-   mul = 16;
-   break;
-   default:
-   return -1;
-   }
+   mul = vlv_gpu_freq_div(czclk_freq);
+   if (mul < 0)
+   return mul;
 
-   return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6;
+   return DIV_ROUND_CLOSEST(mul * val, czclk_freq) + 0xbd - 6;
 }
 
 static int chv_gpu_freq(struct drm_i915_private *dev_priv, int val)
 {
-   int div, freq;
-
-   switch (dev_priv->rps.cz_freq) {
-   case 200:
-   div = 5;
-   break;
-   case 267:
-   div = 6;
-   break;
-   case 320:
-   case 333:
-   case 400:
-   div = 8;
-   break;
-   default:
-   return -1;
-   }
+   int div, czclk_freq = dev_priv->rps.cz_freq;
 
-   freq = (DIV_ROUND_CLOSEST((dev_priv->rps.cz_freq * val), 2 * div) / 2);
+   div = vlv_gpu_freq_div(czclk_freq) / 2;
+   if (div < 0)
+   return div;
 
-   return freq;
+   return DIV_ROUND_CLOSEST(czclk_freq * val, 2 * div) / 2;
 }
 
 static int chv_freq_opcode(struct drm_i915_private *dev_priv, int val)
 {
-   int mul, opcode;
+   int mul, czclk_freq = dev_priv->rps.cz_freq;
 
-   switch (dev_priv->rps.cz_freq) {
-   case 200:
-   mul = 5;
-   break;
-   case 267:
-   mul = 6;
-   break;
-   case 320:
-   case 333:
-   case 400:
-   mul = 8;
-   break;
-   default:
-   return -1;
-   }
+   mul = vlv_gpu_freq_div(czclk_freq) / 2;
+   if (mul < 0)
+   return mul;
 
/* CHV needs even values */
-   opcode = (DIV_ROUND_CLOSEST((val * 2 * mul), dev_priv->rps.cz_freq) * 
2);
-
-   return opcode;
+   return DIV_ROUND_CLOSEST(val * 2 * mul, czclk_freq) * 2;
 }
 
 int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val)
-- 
2.0.4

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

Re: [Intel-gfx] [PATCH 3/3] drm/i915: Splitting PPS functions based on

[shuang . he]

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=277/348->276/348
PNV: pass/total=323/328->324/328
ILK: pass/total=328/330->330/330
IVB: pass/total=545/546->544/546
SNB: pass/total=558/563->558/563
HSW: pass/total=574/578->573/578
BDW: pass/total=435/435->434/435
-Detailed-
test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, 
machine_id...)...->result_with_patch_applied(count, machine_id)...
BYT: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc-lut, NSPT(1, M29)PASS(9, 
M36M29) -> NSPT(1, M36)PASS(3, M36)
PNV: Intel_gpu_tools, igt_gem_concurrent_blit_gpu-bcs-early-read, PASS(7, 
M23M24) -> DMESG_WARN(1, M23)PASS(3, M23)
PNV: Intel_gpu_tools, igt_gem_mmap_offset_exhaustion, DMESG_WARN(2, 
M23M24)PASS(14, M24M23M7) -> PASS(4, M23)
PNV: Intel_gpu_tools, igt_gem_unref_active_buffers, DMESG_WARN(2, M23)PASS(14, 
M24M23M7) -> PASS(4, M23)
ILK: Intel_gpu_tools, igt_kms_render_gpu-blit, DMESG_WARN(1, M26)PASS(15, 
M6M26M37) -> PASS(4, M6)
ILK: Intel_gpu_tools, igt_kms_flip_wf_vblank-vs-dpms-interruptible, 
DMESG_WARN(1, M26)PASS(15, M6M26M37) -> PASS(4, M6)
IVB: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, NSPT(2, M4M34)PASS(5, 
M34M4) -> NSPT(2, M34)PASS(2, M34)
SNB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-random, PASS(4, M35M22) 
-> DMESG_WARN(1, M22)PASS(3, M22)
SNB: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-sliding, DMESG_WARN(1, 
M35)PASS(3, M22) -> PASS(4, M22)
HSW: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, NSPT(4, 
M39M20M40)PASS(9, M40M39M20) -> NSPT(2, M20)PASS(2, M20)
BDW: Intel_gpu_tools, igt_gem_reset_stats_ban-bsd, PASS(10, M30M28) -> 
DMESG_WARN(1, M30)PASS(3, M30)
___
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

[Intel-gfx] [PATCH 5/8] drm/i915: Add ioctl to set per-context parameters

[Rodrigo Vivi]

From: Chris Wilson 

Sometimes we wish to tweak how an individual context behaves. Since we
always create a context for every filp, this means that individual
processes can fine tune their behaviour even if they do not explicitly
create a context.

The first example parameter here is to enable multi-process GPU testing,
but the interface should be able to cope with passing arbitrarily complex
parameters.

Signed-off-by: Chris Wilson 
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/i915_dma.c |  2 +
 drivers/gpu/drm/i915/i915_drv.h |  4 ++
 drivers/gpu/drm/i915/i915_gem_context.c | 69 +
 include/uapi/drm/i915_drm.h | 12 ++
 4 files changed, 87 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 1c145ed..04a6f77 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -2042,6 +2042,8 @@ 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),
+   DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, 
i915_gem_context_getparam_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, 
i915_gem_context_setparam_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 };
 
 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 cddecaf..6d99ddf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2749,6 +2749,10 @@ int i915_gem_context_create_ioctl(struct drm_device 
*dev, void *data,
  struct drm_file *file);
 int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file);
+int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
+   struct drm_file *file_priv);
+int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
+   struct drm_file *file_priv);
 
 /* i915_gem_evict.c */
 int __must_check i915_gem_evict_something(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 29150a4..15c1602 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -743,3 +743,72 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, 
void *data,
DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
return 0;
 }
+
+int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
+   struct drm_file *file)
+{
+   struct drm_i915_file_private *file_priv = file->driver_priv;
+   struct drm_i915_gem_context_param *args = data;
+   struct intel_context *ctx;
+   int ret;
+
+   ret = i915_mutex_lock_interruptible(dev);
+   if (ret)
+   return ret;
+
+   ctx = i915_gem_context_get(file_priv, args->ctx_id);
+   if (IS_ERR(ctx)) {
+   mutex_unlock(&dev->struct_mutex);
+   return PTR_ERR(ctx);
+   }
+
+   args->size = 0;
+   switch (args->param) {
+   case I915_CONTEXT_PARAM_BAN_PERIOD:
+   args->value = ctx->hang_stats.ban_period_seconds;
+   break;
+   default:
+   ret = -EINVAL;
+   break;
+   }
+   mutex_unlock(&dev->struct_mutex);
+
+   return ret;
+}
+
+int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
+   struct drm_file *file)
+{
+   struct drm_i915_file_private *file_priv = file->driver_priv;
+   struct drm_i915_gem_context_param *args = data;
+   struct intel_context *ctx;
+   int ret;
+
+   ret = i915_mutex_lock_interruptible(dev);
+   if (ret)
+   return ret;
+
+   ctx = i915_gem_context_get(file_priv, args->ctx_id);
+   if (IS_ERR(ctx)) {
+   mutex_unlock(&dev->struct_mutex);
+   return PTR_ERR(ctx);
+   }
+
+   switch (args->param) {
+   case I915_CONTEXT_PARAM_BAN_PERIOD:
+   if (args->size)
+   ret = -EINVAL;
+   else if (args->value < ctx->hang_stats.ban_period_seconds &&
+!capable(CAP_SYS_ADMIN))
+   ret = -EPERM;
+   else
+   ctx->hang_stats.ban_period_seconds = args->value;
+   break;
+   default:
+   ret = -EINVAL;
+   break;
+   }
+   mutex_unlock(&dev->struct_mutex);
+
+   return ret;
+}
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i9

[Intel-gfx] [PATCH 4/8] drm/i915: Move the ban period onto the context

[Rodrigo Vivi]

From: Chris Wilson 

This will allow us to set per-file, or even per-context, periods in the
future.

Signed-off-by: Chris Wilson 
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/i915_drv.h | 5 +
 drivers/gpu/drm/i915/i915_gem.c | 3 ++-
 drivers/gpu/drm/i915/i915_gem_context.c | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1219282..cddecaf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -622,6 +622,11 @@ struct i915_ctx_hang_stats {
/* Time when this context was last blamed for a GPU reset */
unsigned long guilty_ts;
 
+   /* If the contexts causes a second GPU hang within this time,
+* it is permanently banned from submitting any more work.
+*/
+   unsigned long ban_period_seconds;
+
/* This context is banned to submit more work */
bool banned;
 };
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 86cf428..2eb66a6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2540,7 +2540,8 @@ static bool i915_context_is_banned(struct 
drm_i915_private *dev_priv,
if (ctx->hang_stats.banned)
return true;
 
-   if (elapsed <= DRM_I915_CTX_BAN_PERIOD) {
+   if (ctx->hang_stats.ban_period_seconds &&
+   elapsed <= ctx->hang_stats.ban_period_seconds) {
if (!i915_gem_context_is_default(ctx)) {
DRM_DEBUG("context hanging too fast, banning!\n");
return true;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 7d32571..29150a4 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -219,6 +219,8 @@ __create_hw_context(struct drm_device *dev,
 * is no remap info, it will be a NOP. */
ctx->remap_slice = (1 << NUM_L3_SLICES(dev)) - 1;
 
+   ctx->hang_stats.ban_period_seconds = DRM_I915_CTX_BAN_PERIOD;
+
return ctx;
 
 err_out:
-- 
1.9.3

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

[Intel-gfx] [PATCH 3/8] drm/i915: add I915_PARAM_HAS_BSD2 to i915_getparam

[Rodrigo Vivi]

From: Zhipeng Gong 

This will let userland only try to use the new ring
when the appropriate kernel is present

Signed-off-by: Zhipeng Gong 
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/i915_dma.c | 3 +++
 include/uapi/drm/i915_drm.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5dc37f0..1c145ed 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -979,6 +979,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
case I915_PARAM_HAS_VEBOX:
value = intel_ring_initialized(&dev_priv->ring[VECS]);
break;
+   case I915_PARAM_HAS_BSD2:
+   value = intel_ring_initialized(&dev_priv->ring[VCS2]);
+   break;
case I915_PARAM_HAS_RELAXED_FENCING:
value = 1;
break;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index fcb16bf..fa99129 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -341,6 +341,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_WT   27
 #define I915_PARAM_CMD_PARSER_VERSION   28
 #define I915_PARAM_HAS_COHERENT_PHYS_GTT 29
+#define I915_PARAM_HAS_BSD2 30
 
 typedef struct drm_i915_getparam {
int param;
-- 
1.9.3

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

[Intel-gfx] [PATCH 0/8] drm-intel-collector - update

[Rodrigo Vivi]

This is another drm-intel-collector updated notice:
http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=drm-intel-collector

Here goes the update list in order for better reviewers assignment:

Patch drm/i915: Make the physical object coherent with GTT - Reviewed-by: 
Ville
Patch drm/i915: Specify bsd rings through exec flag - Reviewer: Rodrigo
Patch drm/i915: add I915_PARAM_HAS_BSD2 to i915_getparam - Reviewer: Rodrigo
Patch drm/i915: Move the ban period onto the context - Reviewer: Mika
Patch drm/i915: Add ioctl to set per-context parameters - Reviewer: Rodrigo
Patch drm/i915: Put logical pipe_control emission into a helper. - 
Reviewer: Mika
Patch drm/i915: Add WaCsStallBeforeStateCacheInvalidate:bdw, chv to logical 
ring - Reviewer: Mika
Patch drm/i915: Wait thread status on gen8+ fw sequence - Reviewer: Ville

Hi all,

First of all thanks for Nacks and coments on previous round. 

I still need to investigate why so big difference on PRTS when puting testing x 
collector.
But meanwhile let me do another round. This time I just collected one more and 
removed the Nacks.

This round coverred the gap on drm-intel-testing updates from Oct 03 to Oct 24.

I volunteered myself, Ville and Mika to some Reviews here. Please let me know 
if this list is ok or other
people should be reviewing them.

Thanks in advance,
Rodrigo.


Chris Wilson (3):
  drm/i915: Make the physical object coherent with GTT
  drm/i915: Move the ban period onto the context
  drm/i915: Add ioctl to set per-context parameters

Mika Kuoppala (1):
  drm/i915: Wait thread status on gen8+ fw sequence

Rodrigo Vivi (2):
  drm/i915: Put logical pipe_control emission into a helper.
  drm/i915: Add WaCsStallBeforeStateCacheInvalidate:bdw, chv to logical
ring

Zhipeng Gong (2):
  drm/i915: Specify bsd rings through exec flag
  drm/i915: add I915_PARAM_HAS_BSD2 to i915_getparam

 drivers/gpu/drm/i915/i915_dma.c|   8 ++
 drivers/gpu/drm/i915/i915_drv.h|  15 ++-
 drivers/gpu/drm/i915/i915_gem.c| 210 -
 drivers/gpu/drm/i915/i915_gem_context.c|  71 ++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  19 ++-
 drivers/gpu/drm/i915/intel_dp.c|   3 +
 drivers/gpu/drm/i915/intel_lrc.c   |  41 --
 drivers/gpu/drm/i915/intel_uncore.c|   3 +-
 include/uapi/drm/i915_drm.h|  22 ++-
 9 files changed, 307 insertions(+), 85 deletions(-)

-- 
1.9.3

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

[Intel-gfx] [PATCH 8/8] drm/i915: Wait thread status on gen8+ fw sequence

[Rodrigo Vivi]

From: Mika Kuoppala 

As per latest pm guide, we need to do this also on
past hsw.

Cc: Ville Syrjälä 
Cc: Chris Wilson 
Cc: Damien Lespiau 
Signed-off-by: Mika Kuoppala 
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_uncore.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
b/drivers/gpu/drm/i915/intel_uncore.c
index 6a0c3fb..86a755a 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -120,8 +120,7 @@ static void __gen7_gt_force_wake_mt_get(struct 
drm_i915_private *dev_priv,
DRM_ERROR("Timed out waiting for forcewake to ack request.\n");
 
/* WaRsForcewakeWaitTC0:ivb,hsw */
-   if (INTEL_INFO(dev_priv->dev)->gen < 8)
-   __gen6_gt_wait_for_thread_c0(dev_priv);
+   __gen6_gt_wait_for_thread_c0(dev_priv);
 }
 
 static void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv)
-- 
1.9.3

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

[Intel-gfx] [PATCH 2/8] drm/i915: Specify bsd rings through exec flag

[Rodrigo Vivi]

From: Zhipeng Gong 

On Broadwell GT3 we have 2 Video Command Streamers (VCS), but userspace
has no control when using VCS1 or VCS2. This patch introduces a mechanism
to avoid the default ping-pong mode and use one specific ring through
execution flag.

v2: fix whitespace (Rodrigo)

Signed-off-by: Zhipeng Gong 
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 19 +--
 drivers/gpu/drm/i915/intel_dp.c|  3 +++
 include/uapi/drm/i915_drm.h|  8 +++-
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index e1ed85a..d9081ec 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1273,8 +1273,23 @@ i915_gem_do_execbuffer(struct drm_device *dev, void 
*data,
else if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_BSD) {
if (HAS_BSD2(dev)) {
int ring_id;
-   ring_id = gen8_dispatch_bsd_ring(dev, file);
-   ring = &dev_priv->ring[ring_id];
+
+   switch (args->flags & I915_EXEC_BSD_MASK) {
+   case I915_EXEC_BSD_DEFAULT:
+   ring_id = gen8_dispatch_bsd_ring(dev, file);
+   ring = &dev_priv->ring[ring_id];
+   break;
+   case I915_EXEC_BSD_RING1:
+   ring = &dev_priv->ring[VCS];
+   break;
+   case I915_EXEC_BSD_RING2:
+   ring = &dev_priv->ring[VCS2];
+   break;
+   default:
+   DRM_DEBUG("execbuf with unknown bsd ring: %d\n",
+ (int)(args->flags & 
I915_EXEC_BSD_MASK));
+   return -EINVAL;
+   }
} else
ring = &dev_priv->ring[VCS];
} else
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ceb528f..a8c9e47 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4802,6 +4802,9 @@ intel_dp_hpd_pulse(struct intel_digital_port 
*intel_dig_port, bool long_hpd)
if (HAS_PCH_SPLIT(dev)) {
if (!ibx_digital_port_connected(dev_priv, 
intel_dig_port))
goto mst_fail;
+   if (!intel_dp->output_reg)
+   goto mst_fail;
+
} else {
if (g4x_digital_port_connected(dev, intel_dig_port) != 
1)
goto mst_fail;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 2502622..fcb16bf 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -737,7 +737,13 @@ struct drm_i915_gem_execbuffer2 {
  */
 #define I915_EXEC_HANDLE_LUT   (1<<12)
 
-#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_HANDLE_LUT<<1)
+/** Used for switching BSD rings on the platforms with two BSD rings */
+#define I915_EXEC_BSD_MASK (3<<13)
+#define I915_EXEC_BSD_DEFAULT  (0<<13) /* default ping-pong mode */
+#define I915_EXEC_BSD_RING1(1<<13)
+#define I915_EXEC_BSD_RING2(2<<13)
+
+#define __I915_EXEC_UNKNOWN_FLAGS -(1<<15)
 
 #define I915_EXEC_CONTEXT_ID_MASK  (0x)
 #define i915_execbuffer2_set_context_id(eb2, context) \
-- 
1.9.3

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

[Intel-gfx] [PATCH 6/8] drm/i915: Put logical pipe_control emission into a helper.

[Rodrigo Vivi]

To be used for a Workaroud. Similar to:

commit 884ceacee308f0e4616d0c933518af2639f7b1d8
Author: Kenneth Graunke 
Date:   Sat Jun 28 02:04:20 2014 +0300

drm/i915: Refactor Broadwell PIPE_CONTROL emission into a helper.

Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_lrc.c | 35 +--
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 6025ac7..666c000 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1126,6 +1126,26 @@ static int gen8_emit_flush(struct intel_ringbuffer 
*ringbuf,
return 0;
 }
 
+static int gen8_emit_pipe_control(struct intel_ringbuffer *ringbuf,
+ u32 flags, u32 scratch_addr)
+{
+   int ret;
+
+   ret = intel_logical_ring_begin(ringbuf, 6);
+   if (ret)
+   return ret;
+
+   intel_logical_ring_emit(ringbuf, GFX_OP_PIPE_CONTROL(6));
+   intel_logical_ring_emit(ringbuf, flags);
+   intel_logical_ring_emit(ringbuf, scratch_addr);
+   intel_logical_ring_emit(ringbuf, 0);
+   intel_logical_ring_emit(ringbuf, 0);
+   intel_logical_ring_emit(ringbuf, 0);
+   intel_logical_ring_advance(ringbuf);
+
+   return 0;
+}
+
 static int gen8_emit_flush_render(struct intel_ringbuffer *ringbuf,
  u32 invalidate_domains,
  u32 flush_domains)
@@ -1133,7 +1153,6 @@ static int gen8_emit_flush_render(struct intel_ringbuffer 
*ringbuf,
struct intel_engine_cs *ring = ringbuf->ring;
u32 scratch_addr = ring->scratch.gtt_offset + 2 * CACHELINE_BYTES;
u32 flags = 0;
-   int ret;
 
flags |= PIPE_CONTROL_CS_STALL;
 
@@ -1153,19 +1172,7 @@ static int gen8_emit_flush_render(struct 
intel_ringbuffer *ringbuf,
flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
}
 
-   ret = intel_logical_ring_begin(ringbuf, 6);
-   if (ret)
-   return ret;
-
-   intel_logical_ring_emit(ringbuf, GFX_OP_PIPE_CONTROL(6));
-   intel_logical_ring_emit(ringbuf, flags);
-   intel_logical_ring_emit(ringbuf, scratch_addr);
-   intel_logical_ring_emit(ringbuf, 0);
-   intel_logical_ring_emit(ringbuf, 0);
-   intel_logical_ring_emit(ringbuf, 0);
-   intel_logical_ring_advance(ringbuf);
-
-   return 0;
+   return gen8_emit_pipe_control(ringbuf, flags, scratch_addr);
 }
 
 static u32 gen8_get_seqno(struct intel_engine_cs *ring, bool lazy_coherency)
-- 
1.9.3

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

[Intel-gfx] [PATCH 7/8] drm/i915: Add WaCsStallBeforeStateCacheInvalidate:bdw, chv to logical ring

[Rodrigo Vivi]

Similar to:

commit 02c9f7e3cfe76a7f54ef03438c36aade86cc1c8b
Author: Kenneth Graunke 
Date:   Mon Jan 27 14:20:16 2014 -0800

drm/i915: Add the WaCsStallBeforeStateCacheInvalidate:bdw workaround.

On Broadwell, any PIPE_CONTROL with the "State Cache Invalidate" bit set
must be preceded by a PIPE_CONTROL with the "CS Stall" bit set.

Documented on the BSpec 3D workarounds page.

Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_lrc.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 666c000..54bf724 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1153,6 +1153,7 @@ static int gen8_emit_flush_render(struct intel_ringbuffer 
*ringbuf,
struct intel_engine_cs *ring = ringbuf->ring;
u32 scratch_addr = ring->scratch.gtt_offset + 2 * CACHELINE_BYTES;
u32 flags = 0;
+   int ret;
 
flags |= PIPE_CONTROL_CS_STALL;
 
@@ -1170,6 +1171,15 @@ static int gen8_emit_flush_render(struct 
intel_ringbuffer *ringbuf,
flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE;
flags |= PIPE_CONTROL_QW_WRITE;
flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
+
+
+   /* WaCsStallBeforeStateCacheInvalidate:bdw,chv */
+   ret = gen8_emit_pipe_control(ring,
+PIPE_CONTROL_CS_STALL |
+PIPE_CONTROL_STALL_AT_SCOREBOARD,
+0);
+   if (ret)
+   return ret;
}
 
return gen8_emit_pipe_control(ringbuf, flags, scratch_addr);
-- 
1.9.3

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

[Intel-gfx] [PATCH 1/8] drm/i915: Make the physical object coherent with GTT

[Rodrigo Vivi]

From: Chris Wilson 

Currently objects for which the hardware needs a contiguous physical
address are allocated a shadow backing storage to satisfy the contraint.
This shadow buffer is not wired into the normal obj->pages and so the
physical object is incoherent with accesses via the GPU, GTT and CPU. By
setting up the appropriate scatter-gather table, we can allow userspace
to access the physical object via either a GTT mmaping of or by rendering
into the GEM bo. However, keeping the CPU mmap of the shmemfs backing
storage coherent with the contiguous shadow is not yet possible.
Fortuituously, CPU mmaps of objects requiring physical addresses are not
expected to be coherent anyway.

This allows the physical constraint of the GEM object to be transparent
to userspace and allow it to efficiently render into or update them via
the GTT and GPU.

v2: Fix leak of pci handle spotted by Ville
v3: Remove the now duplicate call to detach_phys_object during free.
v4: Wait for rendering before pwrite. As this patch makes it possible to
render into the phys object, we should make it correct as well!

Signed-off-by: Chris Wilson 
Cc: Ville Syrjälä 
Reviewed-by: Ville Syrjälä 
Signed-off-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/i915_dma.c |   3 +
 drivers/gpu/drm/i915/i915_drv.h |   6 +-
 drivers/gpu/drm/i915/i915_gem.c | 207 +++-
 include/uapi/drm/i915_drm.h |   1 +
 4 files changed, 150 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 9a73533..5dc37f0 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1027,6 +1027,9 @@ static int i915_getparam(struct drm_device *dev, void 
*data,
case I915_PARAM_CMD_PARSER_VERSION:
value = i915_cmd_parser_get_version();
break;
+   case I915_PARAM_HAS_COHERENT_PHYS_GTT:
+   value = 1;
+   break;
default:
DRM_DEBUG("Unknown parameter %d\n", param->param);
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8fb8eba..1219282 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1957,10 +1957,10 @@ struct drm_i915_gem_object {
unsigned long user_pin_count;
struct drm_file *pin_filp;
 
-   /** for phy allocated objects */
-   struct drm_dma_handle *phys_handle;
-
union {
+   /** for phy allocated objects */
+   struct drm_dma_handle *phys_handle;
+
struct i915_gem_userptr {
uintptr_t ptr;
unsigned read_only :1;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3e0cabe..86cf428 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -208,40 +208,137 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void 
*data,
return 0;
 }
 
-static void i915_gem_object_detach_phys(struct drm_i915_gem_object *obj)
+static int
+i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
 {
-   drm_dma_handle_t *phys = obj->phys_handle;
+   struct address_space *mapping = file_inode(obj->base.filp)->i_mapping;
+   char *vaddr = obj->phys_handle->vaddr;
+   struct sg_table *st;
+   struct scatterlist *sg;
+   int i;
 
-   if (!phys)
-   return;
+   if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
+   return -EINVAL;
+
+   for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
+   struct page *page;
+   char *src;
+
+   page = shmem_read_mapping_page(mapping, i);
+   if (IS_ERR(page))
+   return PTR_ERR(page);
+
+   src = kmap_atomic(page);
+   memcpy(vaddr, src, PAGE_SIZE);
+   drm_clflush_virt_range(vaddr, PAGE_SIZE);
+   kunmap_atomic(src);
+
+   page_cache_release(page);
+   vaddr += PAGE_SIZE;
+   }
+
+   i915_gem_chipset_flush(obj->base.dev);
+
+   st = kmalloc(sizeof(*st), GFP_KERNEL);
+   if (st == NULL)
+   return -ENOMEM;
+
+   if (sg_alloc_table(st, 1, GFP_KERNEL)) {
+   kfree(st);
+   return -ENOMEM;
+   }
+
+   sg = st->sgl;
+   sg->offset = 0;
+   sg->length = obj->base.size;
 
-   if (obj->madv == I915_MADV_WILLNEED) {
+   sg_dma_address(sg) = obj->phys_handle->busaddr;
+   sg_dma_len(sg) = obj->base.size;
+
+   obj->pages = st;
+   obj->has_dma_mapping = true;
+   return 0;
+}
+
+static void
+i915_gem_object_put_pages_phys(struct drm_i915_gem_object *obj)
+{
+   int ret;
+
+   BUG_ON(obj->madv == __I915_MADV_PURGED);
+
+   ret = i915_gem_object_set_to_cpu_domain(obj, true);
+   if (ret) {
+   /* In the event of a disaster, abandon all caches and
+

Re: [Intel-gfx] [PATCH 72/89] drm/i915/skl: Enable/disable power well for aux transaction

[Imre Deak]

On Fri, 2014-11-07 at 12:08 +, Damien Lespiau wrote:
> On Tue, Sep 16, 2014 at 04:19:07PM +0300, Imre Deak wrote:
> > > @@ -1233,6 +1233,9 @@ static bool edp_panel_vdd_on(struct intel_dp 
> > > *intel_dp)
> > >   power_domain = intel_display_port_power_domain(intel_encoder);
> > >   intel_display_power_get(dev_priv, power_domain);
> > >  
> > > + power_domain = intel_display_aux_power_domain(intel_encoder);
> > > + intel_display_power_get(dev_priv, power_domain);
> > > +
> > 
> > The AUX power domains were added to save power when only AUX
> > functionality is needed, since then we don't need to power on the power
> > domain needed for full port functionality.
> 
> Hum I'm not sure about this. It seems to me that the value of the AUX
> power domain is to be able to shutdown the AUX hardware when AUX is not
> needed. That's slightly different from what you're saying;

Ok, I didn't describe all uses cases where the AUX domains are useful,
your description here was more complete. To summarize that for later
reference:

1. only AUX (output inactive, we only do a connector detection)
2. only main lanes (most of the time when the output is active)
3. AUX+main lanes (link training/re-training)
4. no AUX, no main lanes (output is inactive)

> The cases where "only AUX functionality is needed" seem very transient
> to me, while having the main lanes working and no need for AUX sounds
> like the case where it's interesting to have the AUX hw powered down.
> Of course, with PSR we can do both.

Perhaps, if case 1. above isn't very frequent. But my arguments were
valid even for case 2. and 3.

> > With the above change and everywhere else below we'll end up enabling
> > both power domains, though we only need AUX functionality.
> 
> If we're powering up the panel that's probably to use it very soon, so I
> don't really see the value not powering the main lanes at the same time,
> they are going to be used for training very soon? I'm probably missing
> something.

Again depends how important case 1. is. But my point was that in all the
functions where this patch takes the AUX power domain (after rebasing
the edp vdd on/off and the pps_lock/unlock functions) we only want to
enable the resources needed for AUX communication. The power domain
needed for main port functionality (that is the port power domain) is
already taken in display.modeset_global_resources() for case 2. and 3.
above.

> > The power wells needed for AUX are a subset of those needed for full
> > port functionality on all platforms (at least atm), so this patch won't
> > change anything. The patch would make sense, if you requested only the
> > AUX domains.
> 
> I think it's fine if this patch is not changing anything, at least for
> now, until we get to use this power domain to good ends?

Well I'm ok not to change the functionality for now, but I'd still do
this by taking here only the AUX power domain. Then by having the same
power domain->power well mapping in intel_runtime_pm.c for the AUX
domains as for the port domains we keep the existing behavior. This is
actually done already for all existing platforms in patch 69/89 in your
SKL patchset (except for CHV). Patch 71/89 adds the mappings for SKL and
it doesn't include the SKL DDI_A_E,B,C,D power wells in the AUX power
domains. I think this would need to be tested if it really works
(triggering case 1. above), but you could also just do the easier thing
for now and set the AUX mappings to be identical to the corresponding
port mappings, as it's done for the other platforms.

--Imre

> This patch still need the reworks you mentionned in the previous mail,
> of course.


___
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

[Ville Syrjälä]

On Mon, Nov 10, 2014 at 10:01:56AM -0800, Jesse Barnes wrote:
> 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.

I had a similar notion in my head. Can't recall where I saw it exactly.

But the current code is a bit inconistent anyway since it doesn't call
intel_dp_link_down() in all the failure cases. Which combined with an
initially stuck power sequencer on chv sometimes resulted in success
on the first retry and sometimes all the retries would just fail,
entirely depending on which codepath it used when restarting the link
training. I never bothered figuring out what really made it pick one
path over the other, but IIRC it was totally consistent in its
choice for a given combination of modeset operations.

Anyway, I suppose if we really want to follow the correct restart
procedure we might have to shut down _everything_ and start again from
the top. But that seems rather difficult to do given that the link
training isn't driving the entire modeset sequence for us.

-- 
Ville Syrjälä
Intel OTC
___
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] drm-i915-mst + Ubuntu 14.04 + HP 840

[Michal Nazarewicz]

On Fri, Nov 07 2014, David Airlie  wrote:
> Just try a 3.17 based kernel if you can.

I've tried with 3.17.2 and have similar results.  I got no BUGs during
boot and even:

  Console: switching to colour frame buffer device 240x67
  i915 :00:02.0: fb0: inteldrmfb frame buffer device
  i915 :00:02.0: registered panic notifier

but trying to startx gives:

  $ fgrep -e intel -e i915 -e '(EE)' /var/log/Xorg.0.log.old 
  (==) Matched intel as autoconfigured driver 0
  (==) Matched intel as autoconfigured driver 1
  (II) LoadModule: "intel"
  (II) Loading /usr/lib/xorg/modules/drivers/intel_drv.so
  (II) Module intel: vendor="X.Org Foundation"
  (II) intel: Driver for Intel(R) Integrated Graphics Chipsets:
   E7221 (i915), 915GM, 945G, 945GM, 945GME, Pineview GM,
  (II) intel: Driver for Intel(R) HD Graphics: 2000-6000
  (II) intel: Driver for Intel(R) Iris(TM) Graphics: 5100, 6100
  (II) intel: Driver for Intel(R) Iris(TM) Pro Graphics: 5200, 6200, P6300
  (II) intel(0): SNA compiled: xserver-xorg-video-intel 2:2.99.910-0ubuntu1.2 
(Maarten Lankhorst )
  (--) intel(0): Integrated Graphics Chipset: Intel(R) HD Graphics 4400
  (--) intel(0): CPU: x86-64, sse2, sse3, ssse3, sse4.1, sse4.2, avx, avx2
  (II) intel(0): Creating default Display subsection in Screen section
  (==) intel(0): Depth 24, (--) framebuffer bpp 32
  (==) intel(0): RGB weight 888
  (==) intel(0): Default visual is TrueColor
  (**) intel(0): Framebuffer tiled
  (**) intel(0): Pixmaps tiled
  (**) intel(0): "Tear free" disabled
  (**) intel(0): Forcing per-crtc-pixmaps? no
  (II) intel(0): Output eDP1 has no monitor section
  (--) intel(0): found backlight control interface intel_backlight (type 'raw')
  (II) intel(0): Output DP1 has no monitor section
  (II) intel(0): Output HDMI1 has no monitor section
  (II) intel(0): Output DP2 has no monitor section
  (II) intel(0): Output HDMI2 has no monitor section
  (EE) intel(0): No outputs and no modes.
  (II) UnloadModule: "intel"
  (EE) Screen(s) found, but none have a usable configuration.
  (EE) 
  (EE) no screens found(EE) 
  (EE) 
  (EE) Please also check the log file at "/var/log/Xorg.0.log" for additional 
information.
  (EE) 
  (EE) Server terminated with error (1). Closing log file.

If I disconnect the laptop from docking station X starts, but then
connecting it back (while X is running) gives [1] and xrandr -q shows
only eDP1 (i.e. the built in laptop display).

So the bug seems to be present in 3.17 as well.


[1] - >8 ---
  i915 :00:02.0: BAR 6: [??? 0x flags 0x2] has bogus alignment
  i915 :00:02.0: BAR 6: [??? 0x flags 0x2] has bogus alignment
  [ cut here ]
  WARNING: CPU: 0 PID: 57 at drivers/gpu/drm/drm_dp_mst_topology.c:1242 
process_single_tx_qlock+0x48c/0x510 [drm_kms_helper]()
  fail
  Modules linked in: xt_NFLOG(E) nfnetlink_log(E) nfnetlink(E) xt_comment(E) 
xt_multiport(E) xt_connmark(E) xt_mark(E) ctr(E) ccm(E) ip6t_REJECT(E) 
nf_log_ipv6(E) xt_hl(E) ip6t_rt(E) ipt_REJECT(E) nf_log_ipv4(E) 
nf_log_common(E) xt_LOG(E) nf_conntrack_ipv6(E) nf_defrag_ipv6(E) xt_recent(E) 
xt_limit(E) xt_tcpudp(E) nf_conntrack_ipv4(E) nf_defrag_ipv4(E) xt_addrtype(E) 
arc4(E) xt_owner(E) hp_wmi(E) sparse_keymap(E) xt_conntrack(E) dm_multipath(E) 
scsi_dh(E) ip6table_filter(E) ip6_tables(E) xt_state(E) xt_helper(E) 
nf_nat_tftp(E) intel_rapl(E) nf_conntrack_tftp(E) x86_pkg_temp_thermal(E) 
intel_powerclamp(E) coretemp(E) nf_nat_irc(E) kvm_intel(E) iwlmvm(E) 
nf_conntrack_irc(E) kvm(E) mac80211(E) nf_nat_ftp(E) nf_nat(E) 
nf_conntrack_ftp(E) nf_conntrack(E) iptable_filter(E) ip_tables(E) 
rtsx_pci_ms(E) serio_raw(E) uvcvideo(E) x_tables(E) iwlwifi(E) memstick(E) 
videobuf2_vmalloc(E) cfg80211(E) joydev(E) videobuf2_memops(E) 
videobuf2_core(E) v4l2_common(E) lpc_ich(E) videodev(E) btusb(E) hp_accel(E) 
lis3lv02d(E) input_polldev(E) intel_smartconnect(E) hp_wireless(E) 
tpm_infineon(E) snd_hda_codec_idt(E) mac_hid(E) mei_me(E) 
snd_hda_codec_generic(E) mei(E) snd_hda_codec_hdmi(E) snd_hda_intel(E) 
snd_hda_controller(E) snd_hda_codec(E) snd_hwdep(E) snd_pcm(E) snd_seq_midi(E) 
snd_seq_midi_event(E) snd_rawmidi(E) snd_seq(E) snd_seq_device(E) snd_timer(E) 
snd(E) soundcore(E) parport_pc(E) ppdev(E) lp(E) parport(E) bnep(E) rfcomm(E) 
bluetooth(E) btrfs(E) xor(E) raid6_pq(E) dm_crypt(E) dm_mirror(E) 
dm_region_hash(E) dm_log(E) e1000e(E) crct10dif_pclmul(E) crc32_pclmul(E) 
ghash_clmulni_intel(E) i915(E) hid_generic(E) i2c_algo_bit(E) ptp(E) 
drm_kms_helper(E) rtsx_pci_sdmmc(E) aesni_intel(E) aes_x86_64(E) usbhid(E) 
lrw(E) gf128mul(E) glue_helper(E) ablk_helper(E) cryptd(E) hid(E) pps_core(E) 
psmouse(E) ahci(E) rtsx_pci(E) libahci(E) drm(E) wmi(E) video(E)
  CPU: 0 PID: 57 Comm: kworker/u16:1 Tainted: GE  3.17.2-mpn+ #1
  Hardware name: Hewlett-Packard HP EliteBook 840 G1/198F, BIOS L71 Ver. 01.11 
04/29/2014
  Workqueue: i915-dp i915_digport_work_func [i915]
   0

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

[Ville Syrjälä]

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.

-- 
Ville Syrjälä
Intel OTC
___
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: use the correct obj when preparing the sprite plane

[Ville Syrjälä]

On Mon, Nov 10, 2014 at 02:47:30PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni 
> 
> Commit "drm/i915: create a prepare phase for sprite plane updates"
> changed the old_obj pointer we use when committing sprite planes,
> which caused a WARN() and a BUG() to be triggered. Later, commit
> "drm/i915: use intel_fb_obj() macros to assign gem objects" introduced
> the same problem to function intel_commit_sprite_plane().
> 
> Regression introduced by:
> commit ec82cb793c9224e0692eed904f43490cf70e8258
> Author: Gustavo Padovan 
> Date:   Fri Oct 24 14:51:32 2014 +0100
> drm/i915: create a prepare phase for sprite plane updates
> and:
> commit 77cde95217484e845743818691df026cec2534f4
> Author: Gustavo Padovan 
> Date:   Fri Oct 24 14:51:33 2014 +0100
> drm/i915: use intel_fb_obj() macros to assign gem objects
> 
> Credits to Imre Deak for pointing out the exact lines that were wrong.
> 
> v2: Also fix intel_commit_sprite_plane() (Ville)
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85634
> Testcase: igt/pm_rpm/legacy-planes
> Testcase: igt/pm_rpm/legacy-planes-dpms
> Testcase: igt/pm_rpm/universal-planes
> Testcase: igt/pm_rpm/universal-planes-dpms
> Credits-to: Imre Deak 
> Cc: Gustavo Padovan 
> Cc: Ville Syrjälä 
> Signed-off-by: Paulo Zanoni 

Yep, I believe that should do it.

Reviewed-by: Ville Syrjälä 

As a side note if someone is looking for stuff to do, then the pin/unpin
logic might be good thing to look at. We're currently a bit inconsistent
whether we have the buffer pinned when the plane is disabled, or just
otherwise invisible, or when the crtc itself is disabled. And I guess
cooking up some tests to poke at planes with disabled crtcs might be in
order too, as well as all kinds of variations on the
crtc_enable->plane_enable->crtc_disable->plane_disable theme.

> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 64076555..7d9c340 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1264,10 +1264,11 @@ intel_prepare_sprite_plane(struct drm_plane *plane,
>   struct drm_device *dev = plane->dev;
>   struct drm_crtc *crtc = state->crtc;
>   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_plane *intel_plane = to_intel_plane(plane);
>   enum pipe pipe = intel_crtc->pipe;
>   struct drm_framebuffer *fb = state->fb;
>   struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
> + struct drm_i915_gem_object *old_obj = intel_plane->obj;
>   int ret;
>  
>   if (old_obj != obj) {
> @@ -1302,7 +1303,7 @@ intel_commit_sprite_plane(struct drm_plane *plane,
>   enum pipe pipe = intel_crtc->pipe;
>   struct drm_framebuffer *fb = state->fb;
>   struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> - struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
> + struct drm_i915_gem_object *old_obj = intel_plane->obj;
>   int crtc_x, crtc_y;
>   unsigned int crtc_w, crtc_h;
>   uint32_t src_x, src_y, src_w, src_h;
> -- 
> 2.1.1

-- 
Ville Syrjälä
Intel OTC
___
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

[Intel-gfx] [PATCH] drm/i915: use the correct obj when preparing the sprite plane

[Paulo Zanoni]

From: Paulo Zanoni 

Commit "drm/i915: create a prepare phase for sprite plane updates"
changed the old_obj pointer we use when committing sprite planes,
which caused a WARN() and a BUG() to be triggered. Later, commit
"drm/i915: use intel_fb_obj() macros to assign gem objects" introduced
the same problem to function intel_commit_sprite_plane().

Regression introduced by:
commit ec82cb793c9224e0692eed904f43490cf70e8258
Author: Gustavo Padovan 
Date:   Fri Oct 24 14:51:32 2014 +0100
drm/i915: create a prepare phase for sprite plane updates
and:
commit 77cde95217484e845743818691df026cec2534f4
Author: Gustavo Padovan 
Date:   Fri Oct 24 14:51:33 2014 +0100
drm/i915: use intel_fb_obj() macros to assign gem objects

Credits to Imre Deak for pointing out the exact lines that were wrong.

v2: Also fix intel_commit_sprite_plane() (Ville)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85634
Testcase: igt/pm_rpm/legacy-planes
Testcase: igt/pm_rpm/legacy-planes-dpms
Testcase: igt/pm_rpm/universal-planes
Testcase: igt/pm_rpm/universal-planes-dpms
Credits-to: Imre Deak 
Cc: Gustavo Padovan 
Cc: Ville Syrjälä 
Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_sprite.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 64076555..7d9c340 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1264,10 +1264,11 @@ intel_prepare_sprite_plane(struct drm_plane *plane,
struct drm_device *dev = plane->dev;
struct drm_crtc *crtc = state->crtc;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+   struct intel_plane *intel_plane = to_intel_plane(plane);
enum pipe pipe = intel_crtc->pipe;
struct drm_framebuffer *fb = state->fb;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-   struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
+   struct drm_i915_gem_object *old_obj = intel_plane->obj;
int ret;
 
if (old_obj != obj) {
@@ -1302,7 +1303,7 @@ intel_commit_sprite_plane(struct drm_plane *plane,
enum pipe pipe = intel_crtc->pipe;
struct drm_framebuffer *fb = state->fb;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-   struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
+   struct drm_i915_gem_object *old_obj = intel_plane->obj;
int crtc_x, crtc_y;
unsigned int crtc_w, crtc_h;
uint32_t src_x, src_y, src_w, src_h;
-- 
2.1.1

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

Re: [Intel-gfx] [PATCH] drm/i915: Initialize workarounds in logical ring mode too

[Mika Kuoppala]

Arun Siluvery  writes:

> From: Michel Thierry 
>
> Following the legacy ring submission example, update the
> ring->init_context() hook to support the execlist submission mode.
>
> v2: update to use the new workaround macros and cleanup unused code.
> This takes care of both bdw and chv workarounds.
>
> v2.1: Add missing call to init_context() during deferred context creation.
>
> Issue: VIZ-4092
> Issue: GMIN-3475
> Change-Id: Ie3d093b2542ab0e2a44b90460533e2f979788d6c
> Cc: Deepak S 
> Signed-off-by: Michel Thierry 
> Signed-off-by: Arun Siluvery 
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c |  2 +-
>  drivers/gpu/drm/i915/intel_lrc.c| 46 
> -
>  drivers/gpu/drm/i915/intel_ringbuffer.c |  5 ++--
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  4 ++-
>  4 files changed, 52 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index a5221d8..a37668f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -629,7 +629,7 @@ done:
>  
>   if (uninitialized) {
>   if (ring->init_context) {
> - ret = ring->init_context(ring);
> + ret = ring->init_context(ring->buffer);
>   if (ret)
>   DRM_ERROR("ring init context: %d\n", ret);
>   }
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index cd74e5c..f3efdbd 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -991,6 +991,43 @@ int intel_logical_ring_begin(struct intel_ringbuffer 
> *ringbuf, int num_dwords)
>   return 0;
>  }
>  
> +static int intel_logical_ring_workarounds_emit(struct intel_ringbuffer 
> *ringbuf)
> +{
> + int ret, i;
> + struct intel_engine_cs *ring = ringbuf->ring;
> + struct drm_device *dev = ring->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct i915_workarounds *w = &dev_priv->workarounds;
> +
> + if (WARN_ON(w->count == 0))
> + return 0;
> +
> + ring->gpu_caches_dirty = true;
> + ret = logical_ring_flush_all_caches(ringbuf);
> + if (ret)
> + return ret;
> +
> + ret = intel_logical_ring_begin(ringbuf, w->count * 2 + 2);
> + if (ret)
> + return ret;
> +
> + intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(w->count));
> + for (i = 0; i < w->count; i++) {
> + intel_logical_ring_emit(ringbuf, w->reg[i].addr);
> + intel_logical_ring_emit(ringbuf, w->reg[i].value);
> + }
> + intel_logical_ring_emit(ringbuf, MI_NOOP);
> +
> + intel_logical_ring_advance(ringbuf);
> +
> + ring->gpu_caches_dirty = true;
> + ret = logical_ring_flush_all_caches(ringbuf);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
>  static int gen8_init_common_ring(struct intel_engine_cs *ring)
>  {
>   struct drm_device *dev = ring->dev;
> @@ -1034,7 +1071,7 @@ static int gen8_init_render_ring(struct intel_engine_cs 
> *ring)
>  
>   I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING));
>  
> - return ret;
> + return init_workarounds_ring(ring);
>  }
>  
>  static int gen8_emit_bb_start(struct intel_ringbuffer *ringbuf,
> @@ -1282,6 +1319,7 @@ static int logical_render_ring_init(struct drm_device 
> *dev)
>   ring->irq_keep_mask |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
>  
>   ring->init = gen8_init_render_ring;
> + ring->init_context = intel_logical_ring_workarounds_emit;
>   ring->cleanup = intel_fini_pipe_control;
>   ring->get_seqno = gen8_get_seqno;
>   ring->set_seqno = gen8_set_seqno;
> @@ -1745,6 +1783,12 @@ int intel_lr_context_deferred_create(struct 
> intel_context *ctx,
>   }
>  
>   if (ring->id == RCS && !ctx->rcs_initialized) {
> + if (ring->init_context) {
> + ret = ring->init_context(ringbuf);
> + if (ret)
> + DRM_ERROR("ring init context: %d\n", ret);
> + }
> +
>   ret = intel_lr_context_render_state_init(ring, ctx);
>   if (ret) {
>   DRM_ERROR("Init render state failed: %d\n", ret);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 368b20a..70442fa 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -665,9 +665,10 @@ err:
>   return ret;
>  }
>  
> -static int intel_ring_workarounds_emit(struct intel_engine_cs *ring)
> +static int intel_ring_workarounds_emit(struct intel_ringbuffer *ringbuf)
>  {
>   int ret, i;
> + struct intel_engine_cs *ring = ringbuf->ring;
>   struct drm_device *dev = ring->dev;
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   struct i915_worka

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

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

[Ander Conselvan de Oliveira]

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



Ander


+
+   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 */



___
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

[Ander Conselvan de Oliveira]

Reviewed-by: Ander Conselvan de Oliveira 

On 11/06/2014 12:26 AM, 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)
> + config->mode_changed = true;
> + }
>   
>   /* set_mode will free it in the mode_changed case */
>   if (!config->mode_changed)
> 

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

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

[Ander Conselvan de Oliveira]

Reviewed-by: Ander Conselvan de Oliveira 

On 11/06/2014 12:26 AM, Jesse Barnes wrote:
> 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,
> 

___
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

[Ander Conselvan de Oliveira]

Reviewed-by: Ander Conselvan de Oliveira 

On 11/06/2014 12:26 AM, 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;
> +
>   /* 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 
> th

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

[Ander Conselvan de Oliveira]

Reviewed-by: Ander Conselvan de Oliveira 

On 11/06/2014 12:26 AM, 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)
> 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)

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

[Ander Conselvan de Oliveira]

Reviewed-by: Ander Conselvan de Oliveira 

On 11/07/2014 11:11 PM, 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)
> 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);
>   
> 

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

[Intel-gfx] [RFC PATCH 4/4] drm/i915: Expose PMU for Observation Architecture

[Robert Bragg]

Gen graphics hardware can be set up to periodically write snapshots of
performance counters into a circular buffer and this patch exposes that
capability to userspace via the perf interface.

Only Haswell is supported currently.

Signed-off-by: Robert Bragg 
---
 drivers/gpu/drm/i915/Makefile   |   1 +
 drivers/gpu/drm/i915/i915_dma.c |   2 +
 drivers/gpu/drm/i915/i915_drv.h |  33 ++
 drivers/gpu/drm/i915/i915_oa_perf.c | 649 
 drivers/gpu/drm/i915/i915_reg.h |  68 
 include/uapi/drm/i915_drm.h |  21 ++
 6 files changed, 774 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/i915_oa_perf.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 75fd7de..02c0f49 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -16,6 +16,7 @@ i915-y := i915_drv.o \
 
 i915-$(CONFIG_COMPAT)   += i915_ioc32.o
 i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
+i915-$(CONFIG_PERF_EVENTS) += i915_oa_perf.o
 
 # GEM code
 i915-y += i915_cmd_parser.o \
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 1b39807..efc5ceb 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1792,6 +1792,7 @@ int i915_driver_load(struct drm_device *dev, unsigned 
long flags)
intel_gpu_ips_init(dev_priv);
 
intel_runtime_pm_enable(dev_priv);
+   i915_oa_pmu_register(dev);
 
return 0;
 
@@ -1839,6 +1840,7 @@ int i915_driver_unload(struct drm_device *dev)
return ret;
}
 
+   i915_oa_pmu_unregister(dev);
intel_power_domains_fini(dev_priv);
 
intel_gpu_ips_teardown();
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index acaf76c..3c5dac5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -48,6 +48,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /* General customization:
@@ -1693,6 +1694,29 @@ struct drm_i915_private {
 
uint32_t bios_vgacntr;
 
+#ifdef CONFIG_PERF_EVENTS
+   struct {
+   struct pmu pmu;
+   spinlock_t lock;
+   struct hrtimer timer;
+   struct pt_regs dummy_regs;
+
+   struct perf_event *exclusive_event;
+   struct intel_context *specific_ctx;
+
+   struct {
+   struct kref refcount;
+   struct drm_i915_gem_object *obj;
+   u32 gtt_offset;
+   u8 *addr;
+   u32 head;
+   u32 tail;
+   int format;
+   int format_size;
+   } oa_buffer;
+   } oa_pmu;
+#endif
+
/* Old dri1 support infrastructure, beware the dragons ya fools entering
 * here! */
struct i915_dri1_state dri1;
@@ -2773,6 +2797,15 @@ int i915_parse_cmds(struct intel_engine_cs *ring,
u32 batch_start_offset,
bool is_master);
 
+/* i915_oa_perf.c */
+#ifdef CONFIG_PERF_EVENTS
+extern void i915_oa_pmu_register(struct drm_device *dev);
+extern void i915_oa_pmu_unregister(struct drm_device *dev);
+#else
+static inline void i915_oa_pmu_register(struct drm_device *dev) {}
+static inline void i915_oa_pmu_unregister(struct drm_device *dev) {}
+#endif
+
 /* i915_suspend.c */
 extern int i915_save_state(struct drm_device *dev);
 extern int i915_restore_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_oa_perf.c 
b/drivers/gpu/drm/i915/i915_oa_perf.c
new file mode 100644
index 000..b42bfa5c3
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_oa_perf.c
@@ -0,0 +1,649 @@
+#include 
+#include 
+
+#include "i915_drv.h"
+#include "intel_ringbuffer.h"
+
+/* Must be a power of two */
+#define OA_BUFFER_SIZE  SZ_16M
+#define OA_TAKEN(tail, head) ((tail - head) & (OA_BUFFER_SIZE - 1))
+
+#define FREQUENCY 200
+#define PERIOD max_t(u64, 1, NSEC_PER_SEC / FREQUENCY)
+
+static int hsw_perf_format_sizes[] = {
+   64,  /* A13_HSW */
+   128, /* A29_HSW */
+   128, /* A13_B8_C8_HSW */
+
+   /* XXX: If we were to disallow this format we could avoid needing to
+* handle snapshots being split in two when they don't factor into
+* the buffer size... */
+   192, /* A29_B8_C8_HSW */
+   64,  /* B4_C8_HSW */
+   256, /* A45_B8_C8_HSW */
+   128, /* B4_C8_A16_HSW */
+   64   /* C4_B8_HSW */
+};
+
+static void forward_one_oa_snapshot_to_event(struct drm_i915_private *dev_priv,
+u8 *snapshot,
+struct perf_event *event)
+{
+   struct perf_sample_data data;
+   int snapshot_size = dev_priv->oa_pmu.oa_buffer.format_size;
+   struct perf_raw_record raw;
+
+   perf_sample_data_init(&data, 0, event->hw.last_period);
+
+   /* XXX: It seems strange that kernel/events/core.c only initialises
+* data->type if event->attr.sample_id_all is set
+*
+   

[Intel-gfx] [RFC PATCH 1/4] perf: export perf_event_overflow

[Robert Bragg]

To support pmu drivers in loadable modules, such as the i915 driver

Signed-off-by: Robert Bragg 
---
 kernel/events/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 1425d07..3abb368 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5640,6 +5640,7 @@ int perf_event_overflow(struct perf_event *event,
 {
return __perf_event_overflow(event, 1, data, regs);
 }
+EXPORT_SYMBOL_GPL(perf_event_overflow);
 
 /*
  * Generic software event infrastructure
-- 
2.1.3

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

[Intel-gfx] [RFC PATCH 0/4] perf pmu driver for OA counters

[Robert Bragg]

This is still a work in progress, but as I've started to get some useful
results with some simple igt tools and integration into Mesa too, others
could be interested in this proof-of-concept perf pmu driver to forward
Haswell Observation Architecture counters to userspace...

This has some similarities to the pmu driver Chris wrote back in
September last year, except that in this case the driver is configuring
a unit of the gpu to write out periodic counter snapshots to a circular
buffer that then get forwarded to userspace as perf samples, instead of
using hrtimer based sampling. I think if there is general agreement on
leveraging perf pmus for gpu metrics that we'll likely also want some
separate hrtimer based pmus later too.

As well as this RFC, I also recently sent an RFC to the LKML since I
wanted to see if the core perf maintainers would be happy with us
exposing device metrics via perf pmus (currently perf is very cpu
centric).  Initial feedback seems positive and receptive to the idea
which is reassuring:

  https://lkml.org/lkml/2014/10/22/462

When I started looking at this, it wasn't entirely clear that perf would
be a good fit for covering the tooling and Mesa use cases we have and in
particular there was a big question mark over how the permission model
would work considering that GL apps shouldn't need to run as root to use
INTEL_performance_query. Having got something working though I do feel
much more confident now that it can work for us to build on perf
infrastructure here.

For the permission issue, the current driver depends on a small change
to kernel/events/core so it can flag that its pmu relates to a device,
as opposed to the cpu which lets the driver also handle its own
authentication. In this case a drm file descriptor and context id can be
passed as part of the config when opening an event so it's only possible
to profile a specific context that you have a corresponding open drm
file descriptor for. You have to be running as root to profile across
all contexts.

For reference the PRM for these counters can be found here:

  https://01.org/linuxgraphics/sites/default/files/documentation/
  observability_performance_counters_haswell.pdf

To test the driver out I have two igt tools; intel_oacounter_top_pmu, 
which is something like intel_gpu_top but based on our observability
counters, and intel_gpu_trace_pmu which gives me a way to log all of the
pmu sample data in detail in a json format that can be visualised using
the chrome://tracing ui. These tools can be found here:

  https://github.com/rib/intel-gpu-tools/tree/wip/rib/intel-i915-oa-pmu

I've also been experimenting with using the driver from Mesa, and
that can be seen here:

  https://github.com/rib/mesa/tree/wip/rib/i915_oa_perf

Currently the mesa driver is limited to only using the i915_oa driver
as a way to configure the OA unit and still needs updating to also
collect periodic counter snapshots for detecting counter wrapping.

For anyone interested in an introduction to how the lower level perf
interface works this may be a useful reference:

  http://web.eece.maine.edu/~vweaver/projects/perf_events/
  perf_event_open.html

For convenience these patches can also be pulled from here:

  https://github.com/rib/linux/tree/wip/rib/i915_oa_perf

Regards,
- Robert

Robert Bragg (4):
  perf: export perf_event_overflow
  perf: Add PERF_PMU_CAP_IS_DEVICE flag
  drm/i915: add api to pin/unpin context state
  drm/i915: Expose PMU for Observation Architecture

 drivers/gpu/drm/i915/Makefile   |   1 +
 drivers/gpu/drm/i915/i915_dma.c |   2 +
 drivers/gpu/drm/i915/i915_drv.h |  37 ++
 drivers/gpu/drm/i915/i915_gem_context.c |  30 +-
 drivers/gpu/drm/i915/i915_oa_perf.c | 649 
 drivers/gpu/drm/i915/i915_reg.h |  68 
 include/linux/perf_event.h  |   1 +
 include/uapi/drm/i915_drm.h |  21 ++
 kernel/events/core.c|  40 +-
 9 files changed, 834 insertions(+), 15 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_oa_perf.c

-- 
2.1.3

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

[Intel-gfx] [RFC PATCH 3/4] drm/i915: add api to pin/unpin context state

[Robert Bragg]

This adds i915_gem_context_pin/unpin_state functions so that code
outside i915_gem_context.c can pin/unpin a context without duplicating
knowledge about the alignment constraints.

Signed-off-by: Robert Bragg 
---
 drivers/gpu/drm/i915/i915_drv.h |  4 
 drivers/gpu/drm/i915/i915_gem_context.c | 30 +-
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3212d62..acaf76c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2675,6 +2675,10 @@ int i915_gem_context_create_ioctl(struct drm_device 
*dev, void *data,
 int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file);
 
+int i915_gem_context_pin_state(struct drm_device *dev,
+  struct intel_context *ctx);
+void i915_gem_context_unpin_state(struct intel_context *ctx);
+
 /* i915_gem_evict.c */
 int __must_check i915_gem_evict_something(struct drm_device *dev,
  struct i915_address_space *vm,
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index a5221d8..feb1a23 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -132,6 +132,20 @@ static int get_context_size(struct drm_device *dev)
return ret;
 }
 
+int i915_gem_context_pin_state(struct drm_device *dev,
+  struct intel_context *ctx)
+{
+   BUG_ON(!mutex_is_locked(&dev->struct_mutex));
+
+   return i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
+get_context_alignment(dev), 0);
+}
+
+void i915_gem_context_unpin_state(struct intel_context *ctx)
+{
+   i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
+}
+
 void i915_gem_context_free(struct kref *ctx_ref)
 {
struct intel_context *ctx = container_of(ctx_ref,
@@ -253,8 +267,7 @@ i915_gem_create_context(struct drm_device *dev,
 * be available. To avoid this we always pin the default
 * context.
 */
-   ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
-   get_context_alignment(dev), 0);
+   ret = i915_gem_context_pin_state(dev, ctx);
if (ret) {
DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret);
goto err_destroy;
@@ -278,7 +291,7 @@ i915_gem_create_context(struct drm_device *dev,
 
 err_unpin:
if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state)
-   i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state);
+   i915_gem_context_unpin_state(ctx);
 err_destroy:
i915_gem_context_unreference(ctx);
return ERR_PTR(ret);
@@ -375,12 +388,12 @@ void i915_gem_context_fini(struct drm_device *dev)
if (dev_priv->ring[RCS].last_context == dctx) {
/* Fake switch to NULL context */
WARN_ON(dctx->legacy_hw_ctx.rcs_state->active);
-   
i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
+   i915_gem_context_unpin_state(dctx);
i915_gem_context_unreference(dctx);
dev_priv->ring[RCS].last_context = NULL;
}
 
-   i915_gem_object_ggtt_unpin(dctx->legacy_hw_ctx.rcs_state);
+   i915_gem_context_unpin_state(dctx);
}
 
for (i = 0; i < I915_NUM_RINGS; i++) {
@@ -534,8 +547,7 @@ static int do_switch(struct intel_engine_cs *ring,
 
/* Trying to pin first makes error handling easier. */
if (ring == &dev_priv->ring[RCS]) {
-   ret = i915_gem_obj_ggtt_pin(to->legacy_hw_ctx.rcs_state,
-   get_context_alignment(ring->dev), 
0);
+   ret = i915_gem_context_pin_state(ring->dev, to);
if (ret)
return ret;
}
@@ -616,7 +628,7 @@ static int do_switch(struct intel_engine_cs *ring,
BUG_ON(from->legacy_hw_ctx.rcs_state->ring != ring);
 
/* obj is kept alive until the next request by its active ref */
-   i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
+   i915_gem_context_unpin_state(from);
i915_gem_context_unreference(from);
}
 
@@ -643,7 +655,7 @@ done:
 
 unpin_out:
if (ring->id == RCS)
-   i915_gem_object_ggtt_unpin(to->legacy_hw_ctx.rcs_state);
+   i915_gem_context_unpin_state(to);
return ret;
 }
 
-- 
2.1.3

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

[Intel-gfx] [RFC PATCH 2/4] perf: Add PERF_PMU_CAP_IS_DEVICE flag

[Robert Bragg]

The PERF_PMU_CAP_IS_DEVICE flag provides pmu drivers a way to declare
that they only monitor device specific metrics and since they don't
monitor any cpu metrics then perf should bypass any cpu centric security
checks, as well as disallow cpu centric attributes.

Signed-off-by: Robert Bragg 
---
 include/linux/perf_event.h |  1 +
 kernel/events/core.c   | 39 +--
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 893a0d0..c2ae5bd 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -171,6 +171,7 @@ struct perf_event;
  * pmu::capabilities flags
  */
 #define PERF_PMU_CAP_NO_INTERRUPT  0x01
+#define PERF_PMU_CAP_IS_DEVICE 0x02
 
 /**
  * struct pmu - generic performance monitoring unit
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 3abb368..0a3b3cf 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3209,7 +3209,8 @@ find_get_context(struct pmu *pmu, struct task_struct 
*task, int cpu)
 
if (!task) {
/* Must be root to operate on a CPU event: */
-   if (perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN))
+   if (!(pmu->capabilities & PERF_PMU_CAP_IS_DEVICE) &&
+   perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN))
return ERR_PTR(-EACCES);
 
/*
@@ -7253,11 +7254,6 @@ SYSCALL_DEFINE5(perf_event_open,
if (err)
return err;
 
-   if (!attr.exclude_kernel) {
-   if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
-   return -EACCES;
-   }
-
if (attr.freq) {
if (attr.sample_freq > sysctl_perf_event_sample_rate)
return -EINVAL;
@@ -7316,6 +7312,37 @@ SYSCALL_DEFINE5(perf_event_open,
goto err_cpus;
}
 
+   if (event->pmu->capabilities & PERF_PMU_CAP_IS_DEVICE) {
+
+   /* Don't allow cpu centric attributes... */
+   if (event->attr.exclude_user ||
+   event->attr.exclude_callchain_user ||
+   event->attr.exclude_kernel ||
+   event->attr.exclude_callchain_kernel ||
+   event->attr.exclude_hv ||
+   event->attr.exclude_idle ||
+   event->attr.exclude_host ||
+   event->attr.exclude_guest ||
+   event->attr.mmap ||
+   event->attr.comm ||
+   event->attr.task)
+   return -EINVAL;
+
+   if (attr.sample_type &
+   (PERF_SAMPLE_IP |
+PERF_SAMPLE_TID |
+PERF_SAMPLE_ADDR |
+PERF_SAMPLE_CALLCHAIN |
+PERF_SAMPLE_CPU |
+PERF_SAMPLE_BRANCH_STACK |
+PERF_SAMPLE_REGS_USER |
+PERF_SAMPLE_STACK_USER))
+   return -EINVAL;
+   } else if (!attr.exclude_kernel) {
+   if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
+   return -EACCES;
+   }
+
if (flags & PERF_FLAG_PID_CGROUP) {
err = perf_cgroup_connect(pid, event, &attr, group_leader);
if (err) {
-- 
2.1.3

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

Re: [Intel-gfx] [PATCH] drm/mst: use kref_get_unless_zero for looking

[shuang . he]

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=249/348->248/348
PNV: pass/total=323/328->325/328
ILK: pass/total=328/330->330/330
IVB: pass/total=545/546->545/546
SNB: pass/total=551/556->550/556
HSW: pass/total=577/581->570/581
BDW: pass/total=435/435->434/435
-Detailed-
test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, 
machine_id...)...->result_with_patch_applied(count, machine_id)...
BYT: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc-lut, PASS(4, M36) -> 
NSPT(1, M36)PASS(3, M36)
PNV: Intel_gpu_tools, igt_gem_concurrent_blit_gpu-bcs-early-read, PASS(6, 
M23M24) -> DMESG_WARN(1, M24)PASS(3, M24)
PNV: Intel_gpu_tools, igt_gem_linear_blits_normal, NSPT(1, M23)PASS(3, M24) -> 
PASS(4, M24)
PNV: Intel_gpu_tools, igt_gem_mmap_offset_exhaustion, DMESG_WARN(2, 
M23M24)PASS(2, M24) -> PASS(4, M24)
PNV: Intel_gpu_tools, igt_gem_unref_active_buffers, DMESG_WARN(1, M23)PASS(3, 
M24) -> PASS(4, M24)
ILK: Intel_gpu_tools, igt_kms_render_gpu-blit, DMESG_WARN(1, M26)PASS(3, M6) -> 
PASS(4, M6)
ILK: Intel_gpu_tools, igt_kms_flip_wf_vblank-vs-dpms-interruptible, 
DMESG_WARN(1, M26)PASS(3, M6) -> PASS(4, M6)
IVB: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, NSPT(1, M4)PASS(3, 
M34M4) -> NSPT(2, M4)PASS(2, M4)
IVB: Intel_gpu_tools, igt_kms_plane_plane-position-covered-pipe-A-plane-1, 
TIMEOUT(1, M34)PASS(3, M4) -> PASS(4, M4)
SNB: Intel_gpu_tools, igt_gem_concurrent_blit_gpuX-bcs-overwrite-source-forked, 
PASS(4, M35M22) -> FAIL(1, M22)PASS(3, M22)
HSW: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc, NSPT(1, M39)PASS(3, 
M40M39) -> NSPT(2, M39)PASS(2, M39)
HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-128x128-onscreen, PASS(4, 
M40M39) -> DMESG_WARN(2, M39)PASS(2, M39)
HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-256x256-offscreen, PASS(4, 
M40M39) -> DMESG_WARN(2, M39)PASS(2, M39)
HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-64x64-offscreen, DMESG_WARN(1, 
M39)PASS(3, M40M39) -> DMESG_WARN(2, M39)PASS(2, M39)
HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-64x64-onscreen, DMESG_WARN(1, 
M39)PASS(3, M40M39) -> DMESG_WARN(1, M39)PASS(3, M39)
HSW: Intel_gpu_tools, igt_kms_cursor_crc_cursor-64x64-sliding, DMESG_WARN(1, 
M39)PASS(3, M40M39) -> DMESG_WARN(1, M39)PASS(3, M39)
HSW: Intel_gpu_tools, igt_kms_mmio_vs_cs_flip_setplane_vs_cs_flip, PASS(4, 
M40M39) -> DMESG_WARN(1, M39)PASS(3, M39)
BDW: Intel_gpu_tools, igt_gem_reset_stats_ban-bsd, PASS(4, M30) -> 
DMESG_WARN(1, M30)PASS(3, M30)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 6/8] drm/i915: sanitize rps irq enabling

[Imre Deak]

On Mon, 2014-11-10 at 14:15 +, Chris Wilson wrote:
> On Mon, Nov 10, 2014 at 04:13:02PM +0200, Imre Deak wrote:
> > On Mon, 2014-11-10 at 13:45 +, Chris Wilson wrote:
> > > On Mon, Nov 10, 2014 at 03:41:05PM +0200, Imre Deak wrote:
> > > > Atm we first enable the RPS interrupts then we clear any pending ones.
> > > > By this we could lose an interrupt arriving after we unmasked it. This
> > > > may not be a problem as the caller should handle such a race, but logic
> > > > still calls for the opposite order. Also we can delay enabling the
> > > > interrupts until after all the RPS initialization is ready with the
> > > > following order:
> > > > 
> > > 
> > > 0. disable left-over RPS
> > 
> > Isn't enough relying on
> > intel_uncore_sanitize()->intel_disable_gt_powersave()?
> 
> That should be enough. It's an important step to remember though :)

Ok. Btw, I also thought of clearing the interrupts right before enabling
them which would've made things simpler. I wasn't sure though if we
could lose some interrupt that the init step would trigger; though that
may not be an issue. In any case this looked like the more robust order.

> > > > 1. clear any pending RPS interrupts
> > > > 2. initialize RPS
> > > > 3. enable RPS interrupts
> -Chris
> 


___
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

[Jani Nikula]

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);

>
>
>
>
>>
>> U. Artie
 BR,
 Jani.


> Regards,
>
> Hans
>
>
>> Signed-off-by: Joe Konno 
>> ---
>>  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 f17ada3..dcdfbb3 100644
>> --- a/drivers/gpu/drm/i915/intel_panel.c
>> +++ b/drivers/gpu/drm/i915/intel_panel.c
>> @@ -421,7 +421,7 @@ static uint32_t scale(uint32_t source_val,
>>  /* avoid overflows */
>>  target_val = (uint64_t)(source_val - source_min) *
>>  (target

Re: [Intel-gfx] [PATCH v2 6/8] drm/i915: sanitize rps irq enabling

[Chris Wilson]

On Mon, Nov 10, 2014 at 04:13:02PM +0200, Imre Deak wrote:
> On Mon, 2014-11-10 at 13:45 +, Chris Wilson wrote:
> > On Mon, Nov 10, 2014 at 03:41:05PM +0200, Imre Deak wrote:
> > > Atm we first enable the RPS interrupts then we clear any pending ones.
> > > By this we could lose an interrupt arriving after we unmasked it. This
> > > may not be a problem as the caller should handle such a race, but logic
> > > still calls for the opposite order. Also we can delay enabling the
> > > interrupts until after all the RPS initialization is ready with the
> > > following order:
> > > 
> > 
> > 0. disable left-over RPS
> 
> Isn't enough relying on
> intel_uncore_sanitize()->intel_disable_gt_powersave()?

That should be enough. It's an important step to remember though :)

> > > 1. clear any pending RPS interrupts
> > > 2. initialize RPS
> > > 3. enable RPS interrupts
-Chris

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

Re: [Intel-gfx] [PATCH v2 6/8] drm/i915: sanitize rps irq enabling

[Imre Deak]

On Mon, 2014-11-10 at 13:45 +, Chris Wilson wrote:
> On Mon, Nov 10, 2014 at 03:41:05PM +0200, Imre Deak wrote:
> > Atm we first enable the RPS interrupts then we clear any pending ones.
> > By this we could lose an interrupt arriving after we unmasked it. This
> > may not be a problem as the caller should handle such a race, but logic
> > still calls for the opposite order. Also we can delay enabling the
> > interrupts until after all the RPS initialization is ready with the
> > following order:
> > 
> 
> 0. disable left-over RPS

Isn't enough relying on
intel_uncore_sanitize()->intel_disable_gt_powersave()?

> > 1. clear any pending RPS interrupts
> > 2. initialize RPS
> > 3. enable RPS interrupts
> -Chris
> 


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

[Intel-gfx] [PATCH v7] drm/i915: Add tracepoints to track a vm during its lifetime

[daniele . ceraolospurio]

From: Daniele Ceraolo Spurio 

- ppgtt init/release: these tracepoints are useful for observing the
  creation and destruction of Full PPGTTs.

- ctx create/free: we can use the ctx_free trace in combination with the
  ppgtt_release one to be sure that the ppgtt doesn't stay alive for too
  long after the ctx is destroyed. ctx_create is there for simmetry

- switch_mm: important point in the lifetime of the vm

v4: add DOC information
v5: pull the DOC in drm.tmpl
v6: clean ppgtt init/release traces + add ctx create/free and switch_mm
tracepoints (Chris)
v7: drop execlist_submit_context tracepoint

Cc: Chris Wilson 

Signed-off-by: Daniele Ceraolo Spurio 
Reviewed-by: Chris Wilson 
---
 Documentation/DocBook/drm.tmpl  |  21 +++
 drivers/gpu/drm/i915/i915_gem_context.c |   6 ++
 drivers/gpu/drm/i915/i915_gem_gtt.c |   4 ++
 drivers/gpu/drm/i915/i915_trace.h   | 104 
 4 files changed, 135 insertions(+)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 7277a7f..0f485a0 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -3994,6 +3994,27 @@ int num_ioctls;
 !Idrivers/gpu/drm/i915/intel_lrc.c
   
 
+
+
+   Tracing 
+  
+This sections covers all things related to the tracepoints implemented in
+the i915 driver.
+  
+  
+ i915_ppgtt_create and i915_ppgtt_release 
+!Pdrivers/gpu/drm/i915/i915_trace.h i915_ppgtt_create and i915_ppgtt_release 
tracepoints
+  
+  
+ i915_context_create and i915_context_free 
+!Pdrivers/gpu/drm/i915/i915_trace.h i915_context_create and i915_context_free 
tracepoints
+  
+  
+ switch_mm 
+!Pdrivers/gpu/drm/i915/i915_trace.h switch_mm tracepoint
+  
+
+
   
 !Cdrivers/gpu/drm/i915/i915_irq.c
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 7d32571..1fb 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -88,6 +88,7 @@
 #include 
 #include 
 #include "i915_drv.h"
+#include "i915_trace.h"
 
 /* This is a HW constraint. The value below is the largest known requirement
  * I've seen in a spec to date, and that was a workaround for a non-shipping
@@ -137,6 +138,8 @@ void i915_gem_context_free(struct kref *ctx_ref)
struct intel_context *ctx = container_of(ctx_ref,
 typeof(*ctx), ref);
 
+   trace_i915_context_free(ctx);
+
if (i915.enable_execlists)
intel_lr_context_free(ctx);
 
@@ -274,6 +277,8 @@ i915_gem_create_context(struct drm_device *dev,
ctx->ppgtt = ppgtt;
}
 
+   trace_i915_context_create(ctx);
+
return ctx;
 
 err_unpin:
@@ -549,6 +554,7 @@ static int do_switch(struct intel_engine_cs *ring,
from = ring->last_context;
 
if (to->ppgtt) {
+   trace_switch_mm(ring, to);
ret = to->ppgtt->switch_mm(to->ppgtt, ring);
if (ret)
goto unpin_out;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index c1cf332..7737e55 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1174,6 +1174,8 @@ i915_ppgtt_create(struct drm_device *dev, struct 
drm_i915_file_private *fpriv)
 
ppgtt->file_priv = fpriv;
 
+   trace_i915_ppgtt_create(&ppgtt->base);
+
return ppgtt;
 }
 
@@ -1182,6 +1184,8 @@ void  i915_ppgtt_release(struct kref *kref)
struct i915_hw_ppgtt *ppgtt =
container_of(kref, struct i915_hw_ppgtt, ref);
 
+   trace_i915_ppgtt_release(&ppgtt->base);
+
/* vmas should already be unbound */
WARN_ON(!list_empty(&ppgtt->base.active_list));
WARN_ON(!list_empty(&ppgtt->base.inactive_list));
diff --git a/drivers/gpu/drm/i915/i915_trace.h 
b/drivers/gpu/drm/i915/i915_trace.h
index f5aa006..751d4ad 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -587,6 +587,110 @@ TRACE_EVENT(intel_gpu_freq_change,
TP_printk("new_freq=%u", __entry->freq)
 );
 
+/**
+ * DOC: i915_ppgtt_create and i915_ppgtt_release tracepoints
+ *
+ * With full ppgtt enabled each process using drm will allocate at least one
+ * translation table. With these traces it is possible to keep track of the
+ * allocation and of the lifetime of the tables; this can be used during
+ * testing/debug to verify that we are not leaking ppgtts.
+ * These traces identify the ppgtt through the vm pointer, which is also 
printed
+ * by the i915_vma_bind and i915_vma_unbind tracepoints.
+ */
+DECLARE_EVENT_CLASS(i915_ppgtt,
+   TP_PROTO(struct i915_address_space *vm),
+   TP_ARGS(vm),
+
+   TP_STRUCT__entry(
+   __field(struct i915_address_space *, vm)
+   __field(u32, dev)
+   ),
+
+   TP_fas

Re: [Intel-gfx] [PATCH v2 6/8] drm/i915: sanitize rps irq enabling

[Chris Wilson]

On Mon, Nov 10, 2014 at 03:41:05PM +0200, Imre Deak wrote:
> Atm we first enable the RPS interrupts then we clear any pending ones.
> By this we could lose an interrupt arriving after we unmasked it. This
> may not be a problem as the caller should handle such a race, but logic
> still calls for the opposite order. Also we can delay enabling the
> interrupts until after all the RPS initialization is ready with the
> following order:
> 

0. disable left-over RPS
> 1. clear any pending RPS interrupts
> 2. initialize RPS
> 3. enable RPS interrupts
-Chris

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

[Intel-gfx] [PATCH v2 7/8] drm/i915: sanitize rps irq disabling

[Imre Deak]

When disabling the RPS interrupts there is a tricky dependency between
the thread disabling the interrupts, the RPS interrupt handler and the
corresponding RPS work. The RPS work can reenable the interrupts, so
there is no straightforward order in the disabling thread to (1) make
sure that any RPS work is flushed and to (2) disable all RPS
interrupts. Currently this is solved by masking the interrupts using two
separate mask registers (first level display IMR and PM IMR) and doing
the disabling when all first level interrupts are disabled.

This works, but the requirement to run with all first level interrupts
disabled is unnecessary making the suspend / unload time ordering of RPS
disabling wrt. other unitialization steps difficult and error prone.
Removing this restriction allows us to disable RPS early during suspend
/ unload and forget about it for the rest of the sequence. By adding a
more explicit method for avoiding the above race, it also becomes easier
to prove its correctness. Finally currently we can hit the WARN in
snb_update_pm_irq(), when a final RPS work runs with the first level
interrupts already disabled. This won't lead to any problem (due to the
separate interrupt masks), but with the change in this and the next
patch we can get rid of the WARN, while leaving it in place for other
scenarios.

To address the above points, add a new RPS interrupts_enabled flag and
use this during RPS disabling to avoid requeuing the RPS work and
reenabling of the RPS interrupts. Since the interrupt disabling happens
now in intel_suspend_gt_powersave(), we will disable RPS interrupts
explicitly during suspend (and not just through the first level mask),
but there is no problem doing so, it's also more consistent and allows
us to unify more of the RPS disabling during suspend and unload time in
the next patch.

v2:
- rebase on v2 of patch "drm/i915: move rps irq disable one level up"

Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/i915_drv.h |  6 +-
 drivers/gpu/drm/i915/i915_irq.c | 23 ---
 drivers/gpu/drm/i915/intel_pm.c | 17 +
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f830596..14e8f82 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -978,8 +978,12 @@ struct intel_rps_ei {
 };
 
 struct intel_gen6_power_mgmt {
-   /* work and pm_iir are protected by dev_priv->irq_lock */
+   /*
+* work, interrupts_enabled and pm_iir are protected by
+* dev_priv->irq_lock
+*/
struct work_struct work;
+   bool interrupts_enabled;
u32 pm_iir;
 
/* Frequencies are stored in potentially platform dependent multiples.
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 89a7be1..2677760 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -271,6 +271,7 @@ void gen6_enable_rps_interrupts(struct drm_device *dev)
 
spin_lock_irq(&dev_priv->irq_lock);
WARN_ON(dev_priv->rps.pm_iir);
+   dev_priv->rps.interrupts_enabled = true;
gen6_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
spin_unlock_irq(&dev_priv->irq_lock);
 }
@@ -279,14 +280,16 @@ void gen6_disable_rps_interrupts(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
 
+   spin_lock_irq(&dev_priv->irq_lock);
+   dev_priv->rps.interrupts_enabled = false;
+   spin_unlock_irq(&dev_priv->irq_lock);
+
+   cancel_work_sync(&dev_priv->rps.work);
+
I915_WRITE(GEN6_PMINTRMSK, INTEL_INFO(dev_priv)->gen >= 8 ?
   ~GEN8_PMINTR_REDIRECT_TO_NON_DISP : ~0);
I915_WRITE(gen6_pm_ier(dev_priv), I915_READ(gen6_pm_ier(dev_priv)) &
~dev_priv->pm_rps_events);
-   /* Complete PM interrupt masking here doesn't race with the rps work
-* item again unmasking PM interrupts because that is using a different
-* register (PMIMR) to mask PM interrupts. The only risk is in leaving
-* stale bits in PMIIR and PMIMR which gen6_enable_rps will clean up. */
 
spin_lock_irq(&dev_priv->irq_lock);
dev_priv->rps.pm_iir = 0;
@@ -1133,6 +1136,11 @@ static void gen6_pm_rps_work(struct work_struct *work)
int new_delay, adj;
 
spin_lock_irq(&dev_priv->irq_lock);
+   /* Speed up work cancelation during disabling rps interrupts. */
+   if (!dev_priv->rps.interrupts_enabled) {
+   spin_unlock_irq(&dev_priv->irq_lock);
+   return;
+   }
pm_iir = dev_priv->rps.pm_iir;
dev_priv->rps.pm_iir = 0;
/* Make sure not to corrupt PMIMR state used by ringbuffer on GEN6 */
@@ -1706,11 +1714,12 @@ static void gen6_rps_irq_handler(struct 
drm_i915_private *dev_priv, u32 pm_iir)
 
if (pm_iir & dev_priv->pm_rps_events) {
spin_lock(&dev_priv->irq_lock);

[Intel-gfx] [PATCH v2 6/8] drm/i915: sanitize rps irq enabling

[Imre Deak]

Atm we first enable the RPS interrupts then we clear any pending ones.
By this we could lose an interrupt arriving after we unmasked it. This
may not be a problem as the caller should handle such a race, but logic
still calls for the opposite order. Also we can delay enabling the
interrupts until after all the RPS initialization is ready with the
following order:

1. clear any pending RPS interrupts
2. initialize RPS
3. enable RPS interrupts

This also allows us to do the 1. and 3. step the same way for all
platforms, so let's follow this order to simplifying things.

Also make sure any queued interrupts are also cleared.

v2:
- rebase on the GEN9 patches where we don't support RPS yet, so we
  musn't enable RPS interrupts on it (Paulo)

Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/i915_irq.c  | 11 ++-
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c  | 19 +++
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 729e9a3..89a7be1 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -255,6 +255,16 @@ void gen6_disable_pm_irq(struct drm_i915_private 
*dev_priv, uint32_t mask)
snb_update_pm_irq(dev_priv, mask, 0);
 }
 
+void gen6_reset_rps_interrupts(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+
+   spin_lock_irq(&dev_priv->irq_lock);
+   I915_WRITE(gen6_pm_iir(dev_priv), dev_priv->pm_rps_events);
+   I915_WRITE(gen6_pm_iir(dev_priv), dev_priv->pm_rps_events);
+   spin_unlock_irq(&dev_priv->irq_lock);
+}
+
 void gen6_enable_rps_interrupts(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -262,7 +272,6 @@ void gen6_enable_rps_interrupts(struct drm_device *dev)
spin_lock_irq(&dev_priv->irq_lock);
WARN_ON(dev_priv->rps.pm_iir);
gen6_enable_pm_irq(dev_priv, dev_priv->pm_rps_events);
-   I915_WRITE(gen6_pm_iir(dev_priv), dev_priv->pm_rps_events);
spin_unlock_irq(&dev_priv->irq_lock);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2499348..fb2cf27 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -784,6 +784,7 @@ void gen5_enable_gt_irq(struct drm_i915_private *dev_priv, 
uint32_t mask);
 void gen5_disable_gt_irq(struct drm_i915_private *dev_priv, uint32_t mask);
 void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask);
 void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask);
+void gen6_reset_rps_interrupts(struct drm_device *dev);
 void gen6_enable_rps_interrupts(struct drm_device *dev);
 void gen6_disable_rps_interrupts(struct drm_device *dev);
 void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8d164bc..f555810 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4753,8 +4753,6 @@ static void gen8_enable_rps(struct drm_device *dev)
 
gen6_set_rps(dev, (I915_READ(GEN6_GT_PERF_STATUS) & 0xff00) >> 8);
 
-   gen6_enable_rps_interrupts(dev);
-
gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
 }
 
@@ -4851,8 +4849,6 @@ static void gen6_enable_rps(struct drm_device *dev)
dev_priv->rps.power = HIGH_POWER; /* force a reset */
gen6_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
 
-   gen6_enable_rps_interrupts(dev);
-
rc6vids = 0;
ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, 
&rc6vids);
if (IS_GEN6(dev) && ret) {
@@ -5344,8 +5340,6 @@ static void cherryview_enable_rps(struct drm_device *dev)
 
valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
 
-   gen6_enable_rps_interrupts(dev);
-
gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
 }
 
@@ -5424,8 +5418,6 @@ static void valleyview_enable_rps(struct drm_device *dev)
 
valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
 
-   gen6_enable_rps_interrupts(dev);
-
gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
 }
 
@@ -6239,6 +6231,13 @@ static void intel_gen6_powersave_work(struct work_struct 
*work)
 
mutex_lock(&dev_priv->rps.hw_lock);
 
+   /*
+* TODO: reset/enable RPS interrupts on GEN9 too, once RPS support is
+* added for it.
+*/
+   if (INTEL_INFO(dev)->gen != 9)
+   gen6_reset_rps_interrupts(dev);
+
if (IS_CHERRYVIEW(dev)) {
cherryview_enable_rps(dev);
} else if (IS_VALLEYVIEW(dev)) {
@@ -6253,6 +6252,10 @@ static void intel_gen6_powersave_work(struct work_struct 
*work)
__gen6_update_ring_freq(dev);
}
dev_priv->rps.enabled = true;
+
+   if (INTEL_INFO(dev)->gen != 9)
+   gen6_enable_rps_interrupts(dev);

Re: [Intel-gfx] [PATCH 2/2] drm/edid: fix Baseline_ELD_Len field in drm_edid_to_eld()

[Daniel Vetter]

Hi Ben,

The below patch from Jani also touches nouveau, can you please take a
look at it an ack? The core part + nouveau apply on top of drm-next,
the i915 part needs stuff from my next queue. So I'd prefer if we can
get this in through drm-intel-next.

Hi Dave,

Ack on that from your side?

Cheers, Daniel

On Tue, Oct 28, 2014 at 3:20 PM, Jani Nikula  wrote:
> The Baseline_ELD_Len field does not include ELD Header Block size.
>
> From High Definition Audio Specification, Revision 1.0a:
>
> The header block is a fixed size of 4 bytes. The baseline block
> is variable size in multiple of 4 bytes, and its size is defined
> in the header block Baseline_ELD_Len field (in number of
> DWords).
>
> Do not include the header size in Baseline_ELD_Len field. Fix all known
> users of eld[2].
>
> While at it, switch to DIV_ROUND_UP instead of open coding it.
>
> Signed-off-by: Jani Nikula 
>
> ---
>
> This is based on an audio rework series which is mid-way being merged to
> i915. I don't think this should be cc: stable worthy, as, AFAICT, we
> don't use the vendor block, and anyone reading SADs respecting SAD_Count
> should stop at the same offset regardless of this patch. So I propose
> this gets eventually merged via i915 without a rush.
> ---
>  drivers/gpu/drm/drm_edid.c |  7 +--
>  drivers/gpu/drm/i915/intel_audio.c | 16 
>  drivers/gpu/drm/nouveau/nv50_display.c |  3 ++-
>  3 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 3bf999134bcc..45aaa6f5ef36 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3128,9 +3128,12 @@ void drm_edid_to_eld(struct drm_connector *connector, 
> struct edid *edid)
> }
> }
> eld[5] |= sad_count << 4;
> -   eld[2] = (20 + mnl + sad_count * 3 + 3) / 4;
>
> -   DRM_DEBUG_KMS("ELD size %d, SAD count %d\n", (int)eld[2], sad_count);
> +   eld[DRM_ELD_BASELINE_ELD_LEN] =
> +   DIV_ROUND_UP(drm_eld_calc_baseline_block_size(eld), 4);
> +
> +   DRM_DEBUG_KMS("ELD size %d, SAD count %d\n",
> + drm_eld_size(eld), sad_count);
>  }
>  EXPORT_SYMBOL(drm_edid_to_eld);
>
> diff --git a/drivers/gpu/drm/i915/intel_audio.c 
> b/drivers/gpu/drm/i915/intel_audio.c
> index 20af973d7cba..439fa4afa18b 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -107,7 +107,7 @@ static bool intel_eld_uptodate(struct drm_connector 
> *connector,
> tmp &= ~bits_elda;
> I915_WRITE(reg_elda, tmp);
>
> -   for (i = 0; i < eld[2]; i++)
> +   for (i = 0; i < drm_eld_size(eld) / 4; i++)
> if (I915_READ(reg_edid) != *((uint32_t *)eld + i))
> return false;
>
> @@ -162,7 +162,7 @@ static void g4x_audio_codec_enable(struct drm_connector 
> *connector,
> len = (tmp >> 9) & 0x1f;/* ELD buffer size */
> I915_WRITE(G4X_AUD_CNTL_ST, tmp);
>
> -   len = min_t(int, eld[2], len);
> +   len = min(drm_eld_size(eld) / 4, len);
> DRM_DEBUG_DRIVER("ELD size %d\n", len);
> for (i = 0; i < len; i++)
> I915_WRITE(G4X_HDMIW_HDMIEDID, *((uint32_t *)eld + i));
> @@ -209,7 +209,7 @@ static void hsw_audio_codec_enable(struct drm_connector 
> *connector,
> int len, i;
>
> DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes ELD\n",
> - pipe_name(pipe), eld[2]);
> + pipe_name(pipe), drm_eld_size(eld));
>
> /* Enable audio presence detect, invalidate ELD */
> tmp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> @@ -225,8 +225,8 @@ static void hsw_audio_codec_enable(struct drm_connector 
> *connector,
> I915_WRITE(HSW_AUD_DIP_ELD_CTRL(pipe), tmp);
>
> /* Up to 84 bytes of hw ELD buffer */
> -   len = min_t(int, eld[2], 21);
> -   for (i = 0; i < len; i++)
> +   len = min(drm_eld_size(eld), 84);
> +   for (i = 0; i < len / 4; i++)
> I915_WRITE(HSW_AUD_EDID_DATA(pipe), *((uint32_t *)eld + i));
>
> /* ELD valid */
> @@ -315,7 +315,7 @@ static void ilk_audio_codec_enable(struct drm_connector 
> *connector,
> int aud_cntrl_st2;
>
> DRM_DEBUG_KMS("Enable audio codec on port %c, pipe %c, %u bytes 
> ELD\n",
> - port_name(port), pipe_name(pipe), eld[2]);
> + port_name(port), pipe_name(pipe), drm_eld_size(eld));
>
> /* XXX: vblank wait here */
>
> @@ -354,8 +354,8 @@ static void ilk_audio_codec_enable(struct drm_connector 
> *connector,
> I915_WRITE(aud_cntl_st, tmp);
>
> /* Up to 84 bytes of hw ELD buffer */
> -   len = min_t(int, eld[2], 21);
> -   for (i = 0; i < len; i++)
> +   len = min(drm_eld_size(eld), 84);
> +   for (i = 0; i < len / 4; i++)
> I915_WRITE(hdmiw_hdmiedid, *((uint32_t *)eld + 

[Intel-gfx] [PATCH v2 5/8] drm/i915: move rps irq disable one level up

[Imre Deak]

We disable the RPS interrupts for all platforms at the same spot, so
move it one level up in the callstack to simplify things.

No functional change.

v2:
- rebase on the GEN9 patches where RPS isn't supported yet, so we don't
  need to disable RPS interrupts on it (Paulo)

Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/intel_pm.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9f67400..8d164bc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4532,8 +4532,6 @@ static void gen6_disable_rps(struct drm_device *dev)
 
I915_WRITE(GEN6_RC_CONTROL, 0);
I915_WRITE(GEN6_RPNSWREQ, 1 << 31);
-
-   gen6_disable_rps_interrupts(dev);
 }
 
 static void cherryview_disable_rps(struct drm_device *dev)
@@ -4541,8 +4539,6 @@ static void cherryview_disable_rps(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
 
I915_WRITE(GEN6_RC_CONTROL, 0);
-
-   gen6_disable_rps_interrupts(dev);
 }
 
 static void valleyview_disable_rps(struct drm_device *dev)
@@ -4556,8 +4552,6 @@ static void valleyview_disable_rps(struct drm_device *dev)
I915_WRITE(GEN6_RC_CONTROL, 0);
 
gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
-
-   gen6_disable_rps_interrupts(dev);
 }
 
 static void intel_print_rc6_info(struct drm_device *dev, u32 mode)
@@ -6223,6 +6217,14 @@ void intel_disable_gt_powersave(struct drm_device *dev)
valleyview_disable_rps(dev);
else
gen6_disable_rps(dev);
+
+   /*
+* TODO: disable RPS interrupts on GEN9 too once RPS support
+* is added for it.
+*/
+   if (INTEL_INFO(dev)->gen != 9)
+   gen6_disable_rps_interrupts(dev);
+
dev_priv->rps.enabled = false;
mutex_unlock(&dev_priv->rps.hw_lock);
}
-- 
1.8.4

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

[Intel-gfx] [PATCH 4.1] drm/i915: WARN if we receive any gen9 rps interrupts

[Imre Deak]

Paulo noticed that we don't support RPS on GEN9 yet, so WARN for and
ignore any RPS interrupts on that platform.

Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/i915/i915_irq.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 96d150f..729e9a3 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1690,6 +1690,11 @@ static void i9xx_pipe_crc_irq_handler(struct drm_device 
*dev, enum pipe pipe)
  * the work queue. */
 static void gen6_rps_irq_handler(struct drm_i915_private *dev_priv, u32 pm_iir)
 {
+   /* TODO: RPS on GEN9 is not supported yet. */
+   if (WARN_ONCE(INTEL_INFO(dev_priv)->gen == 9,
+ "GEN9: unexpected RPS IRQ\n"))
+   return;
+
if (pm_iir & dev_priv->pm_rps_events) {
spin_lock(&dev_priv->irq_lock);
dev_priv->rps.pm_iir |= pm_iir & dev_priv->pm_rps_events;
-- 
1.8.4

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

Re: [Intel-gfx] [PATCH v6] drm/i915: Add tracepoints to track a vm during its lifetime

[Chris Wilson]

On Mon, Nov 10, 2014 at 12:28:11PM +, Ceraolo Spurio, Daniele wrote:
> On 11/10/2014 11:54 AM, Chris Wilson wrote:
> >On Mon, Nov 10, 2014 at 11:40:40AM +, Ceraolo Spurio, Daniele wrote:
> >>On 11/8/2014 8:44 AM, Chris Wilson wrote:
> >>>On Fri, Nov 07, 2014 at 05:45:01PM +, daniele.ceraolospu...@intel.com 
> >>>wrote:
> +/**
> + * DOC: execlist_submit_context tracepoint
> + *
> + * These tracepoint are used to track the contexts that are submitted to 
> the
> + * ring. An mm switch is automatically performed by the GPU during the 
> context
> + * switch. Given the fact that the mm switch is an important point in the
> + * lifetime of a vm, the vm assigned to the context is also printed by 
> the
> + * tracepoint when full ppgtt is enabled.
> + */
> +TRACE_EVENT(execlists_submit_context,
> + TP_PROTO(struct intel_engine_cs *ring, u32 to_num, struct intel_context 
> *to),
> +
> + TP_ARGS(ring, to_num, to),
> +
> + TP_STRUCT__entry(
> + __field(u32, ring)
> + __field(u32, to_num)
> + __field(struct intel_context *, to)
> + __field(struct i915_address_space *, vm)
> + __field(u32, dev)
> + ),
> +
> + TP_fast_assign(
> + __entry->ring = ring->id;
> + __entry->to_num = to_num;
> + __entry->to = to;
> + __entry->vm = to->ppgtt ? &to->ppgtt->base : NULL;
> + __entry->dev = ring->dev->primary->index;
> + ),
> +
> + TP_printk("dev=%u, ring=%u, ctx%d=%p, ctx_vm=%p",
> +   __entry->dev, __entry->ring,
> +   __entry->to_num, __entry->to, __entry->vm)
> +);
> +
>   #endif /* _I915_TRACE_H_ */
> 
>   /* This part must be outside protection */
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c 
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 6025ac7..e72759d 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -135,6 +135,7 @@
>   #include 
>   #include 
>   #include "i915_drv.h"
> +#include "i915_trace.h"
> 
>   #define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
>   #define GEN8_LR_CONTEXT_OTHER_SIZE (2 * PAGE_SIZE)
> @@ -367,6 +368,7 @@ static void execlists_submit_contexts(struct 
> intel_engine_cs *ring,
>   BUG_ON(!ctx_obj0);
>   WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));
> 
> + trace_execlists_submit_context(ring, 0, to0);
>   execlists_ctx_write_tail(ctx_obj0, tail0);
> >>>
> >>>This is very tenuous. This is not part of the context lifetime but of
> >>>the request.
> >>>-Chris
> >>>
> >>
> >>The aim wasn't to track the context or the request but to track the
> >>switch_mm. considering that in execlist mode that is automatically
> >>done by the GPU when the ctx is moved on the ring, this looked like
> >>a good place to put the trace. What were you exactly concerned
> >>about?
> >
> >I have a different pattern for the lifetimes in my head. In terms of
> >usage tracking, when the context is submitted is to hardware is more or
> >less irrelevant to the lifetime per se - it is interesting, but only
> >in the context of tracking the execlists/scheduler.
> >
> >For the context, the important point is when a new request (e.g.
> >execbuffer) is created from that context, which will then keep that
> >context alive until the request is complete. That's my
> >switch_mm/switch_context equivalent. (I think I have refined my stance a
> >lot since working with the contexts and requests).
> >
> >The other perspective, is that I want the context tracepoints to be
> >actions on the contexts themselves, rather than actions on the hardware
> >which deserve to be in a different domain. (Obviously in the overlap,
> >there will be some arguing as to which domain it best fits in.)
> >I am trying to keep the tracepoints somewhat logically organised. :|
> >-Chris
> >
> 
> + intel-gfx, which I've inadvertently removed in my previous reply.
> 
> I now understand what you meant and I can see your point. However,
> since I'm still getting familiar with the handling of contexts in
> the driver (execlists etc), I'll have to dig a bit more to find the
> right places for tracepoints. Would it be ok for you if I were to
> drop the submit_context trace for now and just go on with the
> others? I'll get back to this part when I feel more comfortable with
> my understanding of the code.

Yes, all the others looked good to me. Please just drop the contentious
tracepoint inside execlists, and slap on my

Reviewed-by: Chris Wilson 

(we will end up one in execlists, just probably as part of a different
set of tracepoints ;)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@li

Re: [Intel-gfx] [C220] Display goes blank with PCH PWM1 enabled warning in intel_display.c:7376/6900

[Catalin Hritcu]

On Mon, Nov 10, 2014 at 12:58 PM, Jani Nikula
 wrote:
> On Mon, 10 Nov 2014, Catalin Hritcu  wrote:
>> On Mon, Nov 10, 2014 at 11:47 AM, Jani Nikula
>>  wrote:
>>> On Mon, 10 Nov 2014, Catalin Hritcu  wrote:
 Dear Intel graphics driver community,

 I'm a user experiencing display problems and joined this mailing list
 to get some help with filing a proper bug report. In particular,
 I'm unsure against which component I should file the following problem.
>>>
>>> Please file a new bug against DRM/Intel at [1].
>>>
>>> Reference your mail on the mailing list.
>>>
>>> Smells like pm refcounting problems.
>>
>> I've filed a new bug as instructed:
>> https://bugs.freedesktop.org/show_bug.cgi?id=86105
>>
>>> Attach dmesg with
>>> drm.debug=14 module parameter set, from boot to the problem.
>>
>> I'll try to produce a dmesg log with drm.debug=14 set from the boot.
>> This will take a while since restarting causes the problem to go away
>> and it takes days or even weeks for the problem to reappear. The
>> problem is happening right now though, so if someone can suggest a way
>> to pass drm.debug=14 without rebooting the machine this I could
>> provide some preliminary information right away. I've tried this:
>
> # echo 14 > /sys/module/drm/parameters/debug
>
> Unfortunately setting that now likely does not help in figuring out how
> we got into that situation to begin with. My guess is still the same;
> there's some sequence that leads to one pm put too many.
>
Understood, I'll reboot now and try my best to produce a full log with
drm.debug=14 as soon as this happens again. Until then the end of
https://bugs.freedesktop.org/attachment.cgi?id=109217 has drm.debug=14
set.

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

Re: [Intel-gfx] [PATCH v6] drm/i915: Add tracepoints to track a vm during its lifetime

[Ceraolo Spurio, Daniele]

On 11/10/2014 11:54 AM, Chris Wilson wrote:

On Mon, Nov 10, 2014 at 11:40:40AM +, Ceraolo Spurio, Daniele wrote:

On 11/8/2014 8:44 AM, Chris Wilson wrote:

On Fri, Nov 07, 2014 at 05:45:01PM +, daniele.ceraolospu...@intel.com wrote:

+/**
+ * DOC: execlist_submit_context tracepoint
+ *
+ * These tracepoint are used to track the contexts that are submitted to the
+ * ring. An mm switch is automatically performed by the GPU during the context
+ * switch. Given the fact that the mm switch is an important point in the
+ * lifetime of a vm, the vm assigned to the context is also printed by the
+ * tracepoint when full ppgtt is enabled.
+ */
+TRACE_EVENT(execlists_submit_context,
+   TP_PROTO(struct intel_engine_cs *ring, u32 to_num, struct intel_context 
*to),
+
+   TP_ARGS(ring, to_num, to),
+
+   TP_STRUCT__entry(
+   __field(u32, ring)
+   __field(u32, to_num)
+   __field(struct intel_context *, to)
+   __field(struct i915_address_space *, vm)
+   __field(u32, dev)
+   ),
+
+   TP_fast_assign(
+   __entry->ring = ring->id;
+   __entry->to_num = to_num;
+   __entry->to = to;
+   __entry->vm = to->ppgtt ? &to->ppgtt->base : NULL;
+   __entry->dev = ring->dev->primary->index;
+   ),
+
+   TP_printk("dev=%u, ring=%u, ctx%d=%p, ctx_vm=%p",
+ __entry->dev, __entry->ring,
+ __entry->to_num, __entry->to, __entry->vm)
+);
+
  #endif /* _I915_TRACE_H_ */

  /* This part must be outside protection */
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 6025ac7..e72759d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -135,6 +135,7 @@
  #include 
  #include 
  #include "i915_drv.h"
+#include "i915_trace.h"

  #define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
  #define GEN8_LR_CONTEXT_OTHER_SIZE (2 * PAGE_SIZE)
@@ -367,6 +368,7 @@ static void execlists_submit_contexts(struct 
intel_engine_cs *ring,
BUG_ON(!ctx_obj0);
WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));

+   trace_execlists_submit_context(ring, 0, to0);
execlists_ctx_write_tail(ctx_obj0, tail0);


This is very tenuous. This is not part of the context lifetime but of
the request.
-Chris



The aim wasn't to track the context or the request but to track the
switch_mm. considering that in execlist mode that is automatically
done by the GPU when the ctx is moved on the ring, this looked like
a good place to put the trace. What were you exactly concerned
about?


I have a different pattern for the lifetimes in my head. In terms of
usage tracking, when the context is submitted is to hardware is more or
less irrelevant to the lifetime per se - it is interesting, but only
in the context of tracking the execlists/scheduler.

For the context, the important point is when a new request (e.g.
execbuffer) is created from that context, which will then keep that
context alive until the request is complete. That's my
switch_mm/switch_context equivalent. (I think I have refined my stance a
lot since working with the contexts and requests).

The other perspective, is that I want the context tracepoints to be
actions on the contexts themselves, rather than actions on the hardware
which deserve to be in a different domain. (Obviously in the overlap,
there will be some arguing as to which domain it best fits in.)
I am trying to keep the tracepoints somewhat logically organised. :|
-Chris



+ intel-gfx, which I've inadvertently removed in my previous reply.

I now understand what you meant and I can see your point. However, since 
I'm still getting familiar with the handling of contexts in the driver 
(execlists etc), I'll have to dig a bit more to find the right places 
for tracepoints. Would it be ok for you if I were to drop the 
submit_context trace for now and just go on with the others? I'll get 
back to this part when I feel more comfortable with my understanding of 
the code.


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

Re: [Intel-gfx] [C220] Display goes blank with PCH PWM1 enabled warning in intel_display.c:7376/6900

[Jani Nikula]

On Mon, 10 Nov 2014, Catalin Hritcu  wrote:
> On Mon, Nov 10, 2014 at 11:47 AM, Jani Nikula
>  wrote:
>> On Mon, 10 Nov 2014, Catalin Hritcu  wrote:
>>> Dear Intel graphics driver community,
>>>
>>> I'm a user experiencing display problems and joined this mailing list
>>> to get some help with filing a proper bug report. In particular,
>>> I'm unsure against which component I should file the following problem.
>>
>> Please file a new bug against DRM/Intel at [1].
>>
>> Reference your mail on the mailing list.
>>
>> Smells like pm refcounting problems.
>
> I've filed a new bug as instructed:
> https://bugs.freedesktop.org/show_bug.cgi?id=86105
>
>> Attach dmesg with
>> drm.debug=14 module parameter set, from boot to the problem.
>
> I'll try to produce a dmesg log with drm.debug=14 set from the boot.
> This will take a while since restarting causes the problem to go away
> and it takes days or even weeks for the problem to reappear. The
> problem is happening right now though, so if someone can suggest a way
> to pass drm.debug=14 without rebooting the machine this I could
> provide some preliminary information right away. I've tried this:

# echo 14 > /sys/module/drm/parameters/debug

Unfortunately setting that now likely does not help in figuring out how
we got into that situation to begin with. My guess is still the same;
there's some sequence that leads to one pm put too many.

BR,
Jani.


>
> [hritcu@detained i915]$ sudo sysctl drm.debug=14
> [sudo] password for hritcu:
> sysctl: cannot stat /proc/sys/drm/debug: No such file or directory
> [hritcu@detained i915]$ ls /proc/sys
> abi  debug  dev  fs  kernel  net  vm
>
> but I have no drm directory in /proc/sys.
>
> Many thanks,
> Catalin

-- 
Jani Nikula, 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] [C220] Display goes blank with PCH PWM1 enabled warning in intel_display.c:7376/6900

[Catalin Hritcu]

On Mon, Nov 10, 2014 at 11:47 AM, Jani Nikula
 wrote:
> On Mon, 10 Nov 2014, Catalin Hritcu  wrote:
>> Dear Intel graphics driver community,
>>
>> I'm a user experiencing display problems and joined this mailing list
>> to get some help with filing a proper bug report. In particular,
>> I'm unsure against which component I should file the following problem.
>
> Please file a new bug against DRM/Intel at [1].
>
> Reference your mail on the mailing list.
>
> Smells like pm refcounting problems.

I've filed a new bug as instructed:
https://bugs.freedesktop.org/show_bug.cgi?id=86105

> Attach dmesg with
> drm.debug=14 module parameter set, from boot to the problem.

I'll try to produce a dmesg log with drm.debug=14 set from the boot.
This will take a while since restarting causes the problem to go away
and it takes days or even weeks for the problem to reappear. The
problem is happening right now though, so if someone can suggest a way
to pass drm.debug=14 without rebooting the machine this I could
provide some preliminary information right away. I've tried this:

[hritcu@detained i915]$ sudo sysctl drm.debug=14
[sudo] password for hritcu:
sysctl: cannot stat /proc/sys/drm/debug: No such file or directory
[hritcu@detained i915]$ ls /proc/sys
abi  debug  dev  fs  kernel  net  vm

but I have no drm directory in /proc/sys.

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

Re: [Intel-gfx] [PATCH v6] drm/i915: Add tracepoints to track a vm during its lifetime

[Ceraolo Spurio, Daniele]

On 11/10/2014 11:40 AM, Ceraolo Spurio, Daniele wrote:

On 11/8/2014 8:44 AM, Chris Wilson wrote:

On Fri, Nov 07, 2014 at 05:45:01PM +,
daniele.ceraolospu...@intel.com wrote:

+/**
+ * DOC: execlist_submit_context tracepoint
+ *
+ * These tracepoint are used to track the contexts that are
submitted to the
+ * ring. An mm switch is automatically performed by the GPU during
the context
+ * switch. Given the fact that the mm switch is an important point
in the
+ * lifetime of a vm, the vm assigned to the context is also printed
by the
+ * tracepoint when full ppgtt is enabled.
+ */
+TRACE_EVENT(execlists_submit_context,
+TP_PROTO(struct intel_engine_cs *ring, u32 to_num, struct
intel_context *to),
+
+TP_ARGS(ring, to_num, to),
+
+TP_STRUCT__entry(
+__field(u32, ring)
+__field(u32, to_num)
+__field(struct intel_context *, to)
+__field(struct i915_address_space *, vm)
+__field(u32, dev)
+),
+
+TP_fast_assign(
+__entry->ring = ring->id;
+__entry->to_num = to_num;
+__entry->to = to;
+__entry->vm = to->ppgtt ? &to->ppgtt->base : NULL;
+__entry->dev = ring->dev->primary->index;
+),
+
+TP_printk("dev=%u, ring=%u, ctx%d=%p, ctx_vm=%p",
+  __entry->dev, __entry->ring,
+  __entry->to_num, __entry->to, __entry->vm)
+);
+
  #endif /* _I915_TRACE_H_ */

  /* This part must be outside protection */
diff --git a/drivers/gpu/drm/i915/intel_lrc.c
b/drivers/gpu/drm/i915/intel_lrc.c
index 6025ac7..e72759d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -135,6 +135,7 @@
  #include 
  #include 
  #include "i915_drv.h"
+#include "i915_trace.h"

  #define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
  #define GEN8_LR_CONTEXT_OTHER_SIZE (2 * PAGE_SIZE)
@@ -367,6 +368,7 @@ static void execlists_submit_contexts(struct
intel_engine_cs *ring,
  BUG_ON(!ctx_obj0);
  WARN_ON(!i915_gem_obj_is_pinned(ctx_obj0));

+trace_execlists_submit_context(ring, 0, to0);
  execlists_ctx_write_tail(ctx_obj0, tail0);


This is very tenuous. This is not part of the context lifetime but of
the request.
-Chris



The aim wasn't to track the context or the request but to track the
switch_mm. considering that in execlist mode that is automatically done
by the GPU when the ctx is moved on the ring, this looked like a good
place to put the trace. What were you exactly concerned about?

Thanks,
Daniele



+ intel-gfx
___
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

[Jani Nikula]

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.




>
> U. Artie
>>> BR,
>>> Jani.
>>>
>>>
 Regards,

 Hans


> Signed-off-by: Joe Konno 
> ---
>  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 f17ada3..dcdfbb3 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -421,7 +421,7 @@ static uint32_t scale(uint32_t source_val,
>   /* avoid overflows */
>   target_val = (uint64_t)(source_val - source_min) *
>   (target_max - target_min);
> - do_div(target_val, source_max - source_min);
> + target_val = DIV_ROUND_CLOSEST(target_val, source_max - source_min);
>   target_val += target_min;
>
>   return target_val;
>
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>> --
>>> Jani Nikula, Intel Open Source Technology Center
>>> ___
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop

[Intel-gfx] [PATCH 1/3] drm/i915: Move PPS calls to edp_init

[Vandana Kannan]

Calls to setup eDP panel power sequencer were there in dp_init_connector()
function. Moving these calls to edp_init_connector() to keep all PPS calls
together and under edp init.

Signed-off-by: Vandana Kannan 
---
 drivers/gpu/drm/i915/intel_dp.c | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ceb528f..34d8105 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5258,6 +5258,11 @@ static bool intel_edp_init_connector(struct intel_dp 
*intel_dp,
 
/* We now know it's not a ghost, init power sequence regs. */
pps_lock(intel_dp);
+   intel_dp_init_panel_power_timestamps(intel_dp);
+   if (IS_VALLEYVIEW(dev))
+   vlv_initial_power_sequencer_setup(intel_dp);
+   else
+   intel_dp_init_panel_power_sequencer(dev, intel_dp);
intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
pps_unlock(intel_dp);
 
@@ -5402,16 +5407,6 @@ intel_dp_init_connector(struct intel_digital_port 
*intel_dig_port,
BUG();
}
 
-   if (is_edp(intel_dp)) {
-   pps_lock(intel_dp);
-   intel_dp_init_panel_power_timestamps(intel_dp);
-   if (IS_VALLEYVIEW(dev))
-   vlv_initial_power_sequencer_setup(intel_dp);
-   else
-   intel_dp_init_panel_power_sequencer(dev, intel_dp);
-   pps_unlock(intel_dp);
-   }
-
intel_dp_aux_init(intel_dp, intel_connector);
 
/* init MST on ports that can support it */
-- 
2.0.1

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

[Intel-gfx] [PATCH 0/3] Rearranging eDP PPS code

[Vandana Kannan]

In the earlier RFC patch series, PPS related code was moved to intel_panel.c
to make it usable for all internal panels. In this patch series, the PPS
related functions are split based on platform in intel_dp.c itself. 
This avoids having code split across intel_dp.c and intel_panel.c w.r.t 
pps_lock/unlock, other pps delay functions. 
Also, the code rearrangement is for eDP alone.

Vandana Kannan (3):
  drm/i915: Move PPS calls to edp_init
  drm/i915: Use vlv_power_sequencer_pipe() only to get pipe
  drm/i915: Splitting PPS functions based on platform

 drivers/gpu/drm/i915/intel_dp.c  | 226 +--
 drivers/gpu/drm/i915/intel_drv.h |   3 +
 2 files changed, 147 insertions(+), 82 deletions(-)

-- 
2.0.1

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

[Intel-gfx] [PATCH 3/3] drm/i915: Splitting PPS functions based on platform

[Vandana Kannan]

Modifying PPS functions in intel_dp.c to avoid using too many conditional
statements based on platform.
Calling vlv_initial_power_sequencer_setup() from vlv specific pps functions
to just initialize vlv specific data and continue with the rest of the generic
code.

Signed-off-by: Vandana Kannan 
---
 drivers/gpu/drm/i915/intel_dp.c  | 215 ++-
 drivers/gpu/drm/i915/intel_drv.h |   3 +
 2 files changed, 146 insertions(+), 72 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1d4bf78..1c6b4b3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -520,8 +520,6 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp)
DRM_DEBUG_KMS("initial power sequencer for port %c: pipe %c\n",
  port_name(port), pipe_name(intel_dp->pps_pipe));
 
-   intel_dp_init_panel_power_sequencer(dev, intel_dp);
-   intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
 }
 
 void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv)
@@ -4714,6 +4712,7 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp 
*intel_dp)
 
 static void intel_dp_encoder_reset(struct drm_encoder *encoder)
 {
+   struct drm_device *dev = encoder->dev;
struct intel_dp *intel_dp;
 
if (to_intel_encoder(encoder)->type != INTEL_OUTPUT_EDP)
@@ -4727,8 +4726,10 @@ static void intel_dp_encoder_reset(struct drm_encoder 
*encoder)
 * Read out the current power sequencer assignment,
 * in case the BIOS did something with it.
 */
-   if (IS_VALLEYVIEW(encoder->dev))
-   vlv_initial_power_sequencer_setup(intel_dp);
+   if (IS_VALLEYVIEW(dev)) {
+   intel_dp_init_panel_power_sequencer(dev, intel_dp);
+   intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
+   }
 
intel_edp_panel_vdd_sanitize(intel_dp);
 
@@ -4917,45 +4918,26 @@ static void intel_dp_init_panel_power_timestamps(struct 
intel_dp *intel_dp)
intel_dp->last_backlight_off = jiffies;
 }
 
-static void
-intel_dp_init_panel_power_sequencer(struct drm_device *dev,
-   struct intel_dp *intel_dp)
+static struct edp_power_seq pch_get_pps_registers(
+   struct intel_dp *intel_dp,
+   u32 pp_ctrl_reg, u32 pp_on_reg,
+   u32 pp_off_reg, u32 pp_div_reg)
 {
+   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+   struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
-   struct edp_power_seq cur, vbt, spec,
-   *final = &intel_dp->pps_delays;
u32 pp_on, pp_off, pp_div, pp;
-   int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg;
-
-   lockdep_assert_held(&dev_priv->pps_mutex);
+   struct edp_power_seq cur;
 
-   /* already initialized? */
-   if (final->t11_t12 != 0)
-   return;
-
-   if (HAS_PCH_SPLIT(dev)) {
-   pp_ctrl_reg = PCH_PP_CONTROL;
-   pp_on_reg = PCH_PP_ON_DELAYS;
-   pp_off_reg = PCH_PP_OFF_DELAYS;
-   pp_div_reg = PCH_PP_DIVISOR;
-   } else {
-   enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
-
-   pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
-   pp_on_reg = VLV_PIPE_PP_ON_DELAYS(pipe);
-   pp_off_reg = VLV_PIPE_PP_OFF_DELAYS(pipe);
-   pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe);
-   }
+   pp_on = I915_READ(pp_on_reg);
+   pp_off = I915_READ(pp_off_reg);
+   pp_div = I915_READ(pp_div_reg);
 
/* Workaround: Need to write PP_CONTROL with the unlock key as
 * the very first thing. */
pp = ironlake_get_pp_control(intel_dp);
I915_WRITE(pp_ctrl_reg, pp);
 
-   pp_on = I915_READ(pp_on_reg);
-   pp_off = I915_READ(pp_off_reg);
-   pp_div = I915_READ(pp_div_reg);
-
/* Pull timing values out of registers */
cur.t1_t3 = (pp_on & PANEL_POWER_UP_DELAY_MASK) >>
PANEL_POWER_UP_DELAY_SHIFT;
@@ -4970,10 +4952,61 @@ intel_dp_init_panel_power_sequencer(struct drm_device 
*dev,
PANEL_POWER_DOWN_DELAY_SHIFT;
 
cur.t11_t12 = ((pp_div & PANEL_POWER_CYCLE_DELAY_MASK) >>
-  PANEL_POWER_CYCLE_DELAY_SHIFT) * 1000;
+   PANEL_POWER_CYCLE_DELAY_SHIFT) * 1000;
 
DRM_DEBUG_KMS("cur t1_t3 %d t8 %d t9 %d t10 %d t11_t12 %d\n",
- cur.t1_t3, cur.t8, cur.t9, cur.t10, cur.t11_t12);
+   cur.t1_t3, cur.t8, cur.t9, cur.t10, cur.t11_t12);
+
+   return cur;
+}
+
+
+static struct edp_power_seq pch_setup_pps(struct intel_dp *intel_dp)
+{
+   int pp_ctrl_reg, pp_on_reg, pp_off_reg, pp_div_reg;
+
+   pp_ctrl_reg = PCH_PP_CONTROL;
+   pp_on_reg = PCH_PP_ON_DELAYS;
+   pp_off_re

[Intel-gfx] [PATCH 2/3] drm/i915: Use vlv_power_sequencer_pipe() only to get pipe

[Vandana Kannan]

vlv_power_sequencer_pipe() calls into init PPS functions. Changing this
function to make it only return pipe and not call PPS init.
This is because PPS init calls into this function to get a pipe ID and all
other callers just need the pipe ID.

Signed-off-by: Vandana Kannan 
---
 drivers/gpu/drm/i915/intel_dp.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 34d8105..1d4bf78 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -434,10 +434,6 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp)
  pipe_name(intel_dp->pps_pipe),
  port_name(intel_dig_port->port));
 
-   /* init power sequencer on this pipe and port */
-   intel_dp_init_panel_power_sequencer(dev, intel_dp);
-   intel_dp_init_panel_power_sequencer_registers(dev, intel_dp);
-
/*
 * Even vdd force doesn't work until we've made
 * the power sequencer lock in on the port.
-- 
2.0.1

___
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 initialize pipe config after choosing DPLLs.

[Ville Syrjälä]

On Fri, Nov 07, 2014 at 04:07:50PM -0800, Bob Paauwe wrote:
> The pipe config needs to be initialized before calling crtc_compute_clock
> since this will update the new_config structure DPLL values. Initializing
> the new_config structure after calling crtc_compute_clock can result in
> incorrect timing values.
> 
> 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
> 
> Signed-off-by: Bob Paauwe 
> CC: Ander Conselvan de Oliveira 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index ff071a7..53f3d3a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10774,7 +10774,11 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>   }
>   intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
>  "[modeset]");
> - to_intel_crtc(crtc)->new_config = pipe_config;

new_config _is_ initialized here.

> +
> + /* mode_set/enable/disable functions rely on a correct pipe
> +  * config. */
> + to_intel_crtc(crtc)->config = *pipe_config;
> + to_intel_crtc(crtc)->new_config = &to_intel_crtc(crtc)->config;

And this will clobber the old config before we even know if the modeset
will succeed. That's not what we want.

You didn't really describe the problem you're seeing, so coming up with
theories is a bit hard. I guess one problem could be that some piece of
code is still looking at crtc->config when it should be looking at
crtc->new_config. In any case, I suggest you tell us a bit more before
anyone spends too much time guessing.

>   }
>  
>   /*
> @@ -10820,10 +10824,6 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>*/
>   if (modeset_pipes) {
>   crtc->mode = *mode;
> - /* mode_set/enable/disable functions rely on a correct pipe
> -  * config. */
> - to_intel_crtc(crtc)->config = *pipe_config;
> - to_intel_crtc(crtc)->new_config = &to_intel_crtc(crtc)->config;
>  
>   /*
>* Calculate and store various constants which
> -- 
> 1.8.3.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: avoid deadlock on failure paths in __intel_framebuffer_create()

[Alexey Khoroshilov]

Since a8bb6818270c __intel_framebuffer_create() is called
with struct_mutex held, so it should use drm_gem_object_unreference()
instead of drm_gem_object_unreference_unlocked().

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov 
---
 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 f0a1a56406eb..80f3e5a03059 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8561,7 +8561,7 @@ __intel_framebuffer_create(struct drm_device *dev,
 
intel_fb = kzalloc(sizeof(*intel_fb), GFP_KERNEL);
if (!intel_fb) {
-   drm_gem_object_unreference_unlocked(&obj->base);
+   drm_gem_object_unreference(&obj->base);
return ERR_PTR(-ENOMEM);
}
 
@@ -8571,7 +8571,7 @@ __intel_framebuffer_create(struct drm_device *dev,
 
return &intel_fb->base;
 err:
-   drm_gem_object_unreference_unlocked(&obj->base);
+   drm_gem_object_unreference(&obj->base);
kfree(intel_fb);
 
return ERR_PTR(ret);
-- 
1.9.1

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

[Intel-gfx] drm-i915-mst + Ubuntu 14.04 + HP 840

[Michal Nazarewicz]

Hi Dave,

I'm trying to get your drm-i915-mst-v3.16[1] branch applied on top of
Ubuntu[2] kernel[3] to get HP's 840 notebook to handle two DPs port on
the docking station but I'm hitting some problems with your patches.  Or
so I think.

If I start and boot with the laptop connected to the docking station and
the screens, I'm getting BUGs (full dmesg attached) and X refuses to
start at all.  E.g.:

 >8 
[   12.493151] [drm:intel_pipe_config_compare] *ERROR* mismatch in 
adjusted_mode.crtc_clock (expected 119000, found 0)
[   12.493171] WARNING: CPU: 0 PID: 61 at 
/home/mpn/code/linux/drivers/gpu/drm/i915/intel_display.c:10221 
check_crtc_state+0x235/0x350 [i915]()

[   12.503272] [drm:intel_pipe_config_compare] *ERROR* mismatch in 
adjusted_mode.crtc_clock (expected 119000, found 0)
[   12.503284] WARNING: CPU: 0 PID: 61 at 
/home/mpn/code/linux/drivers/gpu/drm/i915/intel_display.c:10221 
check_crtc_state+0x235/0x350 [i915]()

[   12.503449] [drm:intel_pipe_config_compare] *ERROR* mismatch in 
adjusted_mode.crtc_clock (expected 119000, found 0)
[   12.503460] WARNING: CPU: 0 PID: 61 at 
/home/mpn/code/linux/drivers/gpu/drm/i915/intel_display.c:10221 
check_crtc_state+0x235/0x350 [i915]()
 >8 

If I boot up with laptop disconnected from the docking station and
connect it later on, I'm getting the following errors:

 >8 
[  248.615216] i915 :00:02.0: BAR 6: [??? 0x flags 0x2] has bogus 
alignment
[  248.629843] i915 :00:02.0: BAR 6: [??? 0x flags 0x2] has bogus 
alignment
[  248.630412] acpi PNP0501:00: Still not present
[  252.261562] usb 3-1: new SuperSpeed USB device number 5 using xhci_hcd
[  252.277626] usb 3-1: New USB device found, idVendor=0424, idProduct=5534
[  252.277630] usb 3-1: New USB device strings: Mfr=2, Product=3, SerialNumber=0
[  252.277632] usb 3-1: Product: USB5534B
[  252.277633] usb 3-1: Manufacturer: SMSC
[  252.278047] hub 3-1:1.0: USB hub found
[  252.278067] hub 3-1:1.0: 4 ports detected
[  252.517255] usb 2-1: new high-speed USB device number 16 using xhci_hcd
[  252.645530] usb 2-1: New USB device found, idVendor=0424, idProduct=2134
[  252.645534] usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[  252.645536] usb 2-1: Product: USB2134B
[  252.645538] usb 2-1: Manufacturer: SMSC
[  252.645970] hub 2-1:1.0: USB hub found
[  252.646024] hub 2-1:1.0: 4 ports detected
[  252.693505] i915 :00:02.0: BAR 6: [??? 0x flags 0x2] has bogus 
alignment
[  252.704220] i915 :00:02.0: BAR 6: [??? 0x flags 0x2] has bogus 
alignment
[  252.704478] acpi PNP0501:00: Still not present
 >8 

Needless to say, “xrandr -q” did not detect any of the DisplayPort
screens.

I was wondering if you had any idea what could be a problem?  Perhaps
there is a newer version of your patchset that I should try?

[1] http://cgit.freedesktop.org/~airlied/linux/log/?h=drm-i915-mst-v3.16
[2] Which probably wouldn't by your choice of a distro. ;)
[3] 
http://kernel.ubuntu.com/git?p=ubuntu/ubuntu-utopic.git;a=shortlog;h=refs/heads/master

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +--ooO--(_)--Ooo--
[0.00] CPU0 microcode updated early to revision 0x1c, date = 2014-07-03
[0.00] Initializing cgroup subsys cpuset
[0.00] Initializing cgroup subsys cpu
[0.00] Initializing cgroup subsys cpuacct
[0.00] Linux version 3.16.0-25-generic (root@mpn-glaptop) (gcc version 
4.8.2 (Ubuntu 4.8.2-19ubuntu1) ) #33 SMP Fri Nov 7 18:45:25 CET 2014 (Ubuntu 
3.16.0-25.33-generic 3.16.7)
[0.00] Command line: BOOT_IMAGE=/vmlinuz-3.16.0-25-generic 
root=/dev/mapper/sysvg-root ro elevator=deadline acpi_backlight=vendor text
[0.00] KERNEL supported cpus:
[0.00]   Intel GenuineIntel
[0.00]   AMD AuthenticAMD
[0.00]   Centaur CentaurHauls
[0.00] e820: BIOS-provided physical RAM map:
[0.00] BIOS-e820: [mem 0x-0x0009dbff] usable
[0.00] BIOS-e820: [mem 0x0009dc00-0x0009] reserved
[0.00] BIOS-e820: [mem 0x000e-0x000f] reserved
[0.00] BIOS-e820: [mem 0x0010-0xbab7efff] usable
[0.00] BIOS-e820: [mem 0xbab7f000-0xbbe7efff] reserved
[0.00] BIOS-e820: [mem 0xbbe7f000-0xbbf7efff] ACPI NVS
[0.00] BIOS-e820: [mem 0xbbf7f000-0xbbffefff] ACPI data
[0.00] BIOS-e820: [mem 0xbbfff000-0xbbff] usable
[0.00] BIOS-e820: [mem 0xbc00-0xbf1f] reserved
[0.00] BIOS-e

Re: [Intel-gfx] [PATCH] drm/mst: use kref_get_unless_zero for looking up mst branch device

[Daniel Vetter]

On Mon, Nov 10, 2014 at 7:41 AM, Dave Airlie  wrote:
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index ce1113c..f703a5b 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -882,8 +882,10 @@ static struct drm_dp_mst_branch 
> *drm_dp_mst_get_validated_mstb_ref_locked(struct
> struct drm_dp_mst_port *port;
> struct drm_dp_mst_branch *rmstb;
> if (to_find == mstb) {
> -   kref_get(&mstb->kref);
> -   return mstb;
> +   if (!kref_get_unless_zero(&mstb->kref))
> +   return NULL;
> +   else
> +   return mstb;
> }
> list_for_each_entry(port, &mstb->ports, next) {
> if (port->mstb) {

kref_get_unless_zero only works with the lookup structure is protected
through some locking. But the list removal in
drm_dp_destroy_mst_branch_device is outside the mgr->glock, so that
needs to be moved I think. That's not directly required by
kref_get_unless_zero, just another effect of the ports list not
holding a full reference.

Aside for paranoia I'd put a mutex check next to the
kref_get_unles_zero, just to document which lock protects this weak
reference. And even more tangential: We should probably flatten the
recursion, or add a comment stating that the dp spec limits recursion
sufficiently.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx