Re: [Intel-gfx] [PATCH v6 13/20] drm/i915/guc: Provide register list to be saved/restored during engine reset

2017-04-21 Thread Michel Thierry 



On 21/04/17 13:21, Chris Wilson wrote:

On Fri, Apr 21, 2017 at 01:10:37PM -0700, Daniele Ceraolo Spurio wrote:



On 21/04/17 13:07, Michel Thierry wrote:



On 20/04/17 10:29, Michel Thierry wrote:



On 20/04/17 09:39, Daniele Ceraolo Spurio wrote:



On 20/04/17 04:33, Joonas Lahtinen wrote:

On ke, 2017-04-19 at 11:35 -0700, Michel Thierry wrote:

From: Arun Siluvery 

GuC expects a list of registers from the driver which are
saved/restored
during engine reset. The type of value to be saved is controlled by
flags. We provide a minimal set of registers that we want GuC to save
and
restore. This is not an issue in case of engine reset as driver
initializes
most of them following an engine reset, but in case of media reset
(aka
watchdog reset) which is completely internal to GuC (including
resubmission
of hung workload), it is necessary to provide this list, otherwise
GuC won't
be able to schedule further workloads after a reset. This is the
minimal
set of registers identified for things to work as expected but if we
see
any new issues, this register list can be expanded.

In order to not loose any existing workarounds, we have to let GuC
know
the registers and its values. These will be reapplied after the reset.
Note that we can't just read the current value because most of these
registers are masked (so we have a workaround for a workaround for a
workaround).

v2: REGSET_MASKED is too difficult for GuC, use
REGSET_SAVE_DEFAULT_VALUE
and current value from RING_MODE reg instead; no need to preserve
head/tail either, be extra paranoid and save whitelisted registers
(Daniele).

v3: Workarounds added only once during _init_workarounds also have to
been restored, or we risk loosing them after internal GuC reset
(Daniele).

Cc: Daniele Ceraolo Spurio 
Signed-off-by: Arun Siluvery 
Signed-off-by: Jeff McGee 
Signed-off-by: Michel Thierry 





@@ -732,15 +755,16 @@ static int gen9_init_workarounds(struct
intel_engine_cs *engine)

int ret;


/* WaConextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk */
-I915_WRITE(GEN9_CSFE_CHICKEN1_RCS,
_MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE));
+I915_GUC_REG_WRITE(GEN9_CSFE_CHICKEN1_RCS,
+
_MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE));


To make grepping easier, how about?

   I915_WRITE(GUC_REG(GEN9_CSFE_CHICKEN1_RCS),
  ...);

Regards, Joonas



GUC_REG makes it sound like it is somehow related to GuC, while it
isn't, we just want GuC to restore its value. What about
GUC_REG_RESTORE?



Honestly, I dont care about names, pick one and I add it.
Just a reminder, we not only need the reg offset, we want to save the
value too.



I915_WRITE_GUC_RESTORE(reg, value) ?

That would be inline to the others we have, e.g. I915_WRITE_FW,
I915_WRITE_CTL, I915_WRITE_HEAD/TAIL.


I915_WRITE_FW is not the same class as I915_WRITE_CTL/HEAD/TAIL, and I
can say from experience the I915_*_CTL/HEAD/TAIL were a mistake (special
casing one particular access to the ring mmio, but we often deviate from
that pattern).

Looking at the above I see you are falling for the same trap as the ring
shorthand... So are you sure the convenience will not be lost later? And
in particular avoid using I915_WRITE_*() naming style as I would rather
that was earmarked for the different mmio accessors.


Ok, then can follow the pattern of the other workarounds & whitelist reg 
code?


E.g. WA_REG_GUC_RESTORE or WA_MMIO_REG_GUC_RESTORE (to make it clearer 
that these are not registers in the ctx image).




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

Re: [Intel-gfx] [PATCH v6 13/20] drm/i915/guc: Provide register list to be saved/restored during engine reset

2017-04-21 Thread Chris Wilson 
On Fri, Apr 21, 2017 at 01:10:37PM -0700, Daniele Ceraolo Spurio wrote:
> 
> 
> On 21/04/17 13:07, Michel Thierry wrote:
> >
> >
> >On 20/04/17 10:29, Michel Thierry wrote:
> >>
> >>
> >>On 20/04/17 09:39, Daniele Ceraolo Spurio wrote:
> >>>
> >>>
> >>>On 20/04/17 04:33, Joonas Lahtinen wrote:
> On ke, 2017-04-19 at 11:35 -0700, Michel Thierry wrote:
> >From: Arun Siluvery 
> >
> >GuC expects a list of registers from the driver which are
> >saved/restored
> >during engine reset. The type of value to be saved is controlled by
> >flags. We provide a minimal set of registers that we want GuC to save
> >and
> >restore. This is not an issue in case of engine reset as driver
> >initializes
> >most of them following an engine reset, but in case of media reset
> >(aka
> >watchdog reset) which is completely internal to GuC (including
> >resubmission
> >of hung workload), it is necessary to provide this list, otherwise
> >GuC won't
> >be able to schedule further workloads after a reset. This is the
> >minimal
> >set of registers identified for things to work as expected but if we
> >see
> >any new issues, this register list can be expanded.
> >
> >In order to not loose any existing workarounds, we have to let GuC
> >know
> >the registers and its values. These will be reapplied after the reset.
> >Note that we can't just read the current value because most of these
> >registers are masked (so we have a workaround for a workaround for a
> >workaround).
> >
> >v2: REGSET_MASKED is too difficult for GuC, use
> >REGSET_SAVE_DEFAULT_VALUE
> >and current value from RING_MODE reg instead; no need to preserve
> >head/tail either, be extra paranoid and save whitelisted registers
> >(Daniele).
> >
> >v3: Workarounds added only once during _init_workarounds also have to
> >been restored, or we risk loosing them after internal GuC reset
> >(Daniele).
> >
> >Cc: Daniele Ceraolo Spurio 
> >Signed-off-by: Arun Siluvery 
> >Signed-off-by: Jeff McGee 
> >Signed-off-by: Michel Thierry 
> 
> 
> 
> >@@ -732,15 +755,16 @@ static int gen9_init_workarounds(struct
> >intel_engine_cs *engine)
> >> int ret;
> >
> > /* WaConextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk */
> >-I915_WRITE(GEN9_CSFE_CHICKEN1_RCS,
> >_MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE));
> >+I915_GUC_REG_WRITE(GEN9_CSFE_CHICKEN1_RCS,
> >+
> >_MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE));
> 
> To make grepping easier, how about?
> 
> I915_WRITE(GUC_REG(GEN9_CSFE_CHICKEN1_RCS),
>    ...);
> 
> Regards, Joonas
> 
> >>>
> >>>GUC_REG makes it sound like it is somehow related to GuC, while it
> >>>isn't, we just want GuC to restore its value. What about
> >>>GUC_REG_RESTORE?
> >>>
> >>
> >>Honestly, I dont care about names, pick one and I add it.
> >>Just a reminder, we not only need the reg offset, we want to save the
> >>value too.
> >>
> >
> >I915_WRITE_GUC_RESTORE(reg, value) ?
> >
> >That would be inline to the others we have, e.g. I915_WRITE_FW,
> >I915_WRITE_CTL, I915_WRITE_HEAD/TAIL.

I915_WRITE_FW is not the same class as I915_WRITE_CTL/HEAD/TAIL, and I
can say from experience the I915_*_CTL/HEAD/TAIL were a mistake (special
casing one particular access to the ring mmio, but we often deviate from
that pattern).

Looking at the above I see you are falling for the same trap as the ring
shorthand... So are you sure the convenience will not be lost later? And
in particular avoid using I915_WRITE_*() naming style as I would rather
that was earmarked for the different mmio accessors.
-Chris

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

Re: [Intel-gfx] [PATCH v6 13/20] drm/i915/guc: Provide register list to be saved/restored during engine reset

2017-04-21 Thread Daniele Ceraolo Spurio 



On 21/04/17 13:07, Michel Thierry wrote:



On 20/04/17 10:29, Michel Thierry wrote:



On 20/04/17 09:39, Daniele Ceraolo Spurio wrote:



On 20/04/17 04:33, Joonas Lahtinen wrote:

On ke, 2017-04-19 at 11:35 -0700, Michel Thierry wrote:

From: Arun Siluvery 

GuC expects a list of registers from the driver which are
saved/restored
during engine reset. The type of value to be saved is controlled by
flags. We provide a minimal set of registers that we want GuC to save
and
restore. This is not an issue in case of engine reset as driver
initializes
most of them following an engine reset, but in case of media reset
(aka
watchdog reset) which is completely internal to GuC (including
resubmission
of hung workload), it is necessary to provide this list, otherwise
GuC won't
be able to schedule further workloads after a reset. This is the
minimal
set of registers identified for things to work as expected but if we
see
any new issues, this register list can be expanded.

In order to not loose any existing workarounds, we have to let GuC
know
the registers and its values. These will be reapplied after the reset.
Note that we can't just read the current value because most of these
registers are masked (so we have a workaround for a workaround for a
workaround).

v2: REGSET_MASKED is too difficult for GuC, use
REGSET_SAVE_DEFAULT_VALUE
and current value from RING_MODE reg instead; no need to preserve
head/tail either, be extra paranoid and save whitelisted registers
(Daniele).

v3: Workarounds added only once during _init_workarounds also have to
been restored, or we risk loosing them after internal GuC reset
(Daniele).

Cc: Daniele Ceraolo Spurio 
Signed-off-by: Arun Siluvery 
Signed-off-by: Jeff McGee 
Signed-off-by: Michel Thierry 





@@ -732,15 +755,16 @@ static int gen9_init_workarounds(struct
intel_engine_cs *engine)

 int ret;


 /* WaConextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk */
-I915_WRITE(GEN9_CSFE_CHICKEN1_RCS,
_MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE));
+I915_GUC_REG_WRITE(GEN9_CSFE_CHICKEN1_RCS,
+
_MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE));


To make grepping easier, how about?

I915_WRITE(GUC_REG(GEN9_CSFE_CHICKEN1_RCS),
   ...);

Regards, Joonas



GUC_REG makes it sound like it is somehow related to GuC, while it
isn't, we just want GuC to restore its value. What about
GUC_REG_RESTORE?



Honestly, I dont care about names, pick one and I add it.
Just a reminder, we not only need the reg offset, we want to save the
value too.



I915_WRITE_GUC_RESTORE(reg, value) ?

That would be inline to the others we have, e.g. I915_WRITE_FW,
I915_WRITE_CTL, I915_WRITE_HEAD/TAIL.

-Michel


LGTM.

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

Re: [Intel-gfx] [PATCH v6 13/20] drm/i915/guc: Provide register list to be saved/restored during engine reset

2017-04-21 Thread Michel Thierry 



On 20/04/17 10:29, Michel Thierry wrote:



On 20/04/17 09:39, Daniele Ceraolo Spurio wrote:



On 20/04/17 04:33, Joonas Lahtinen wrote:

On ke, 2017-04-19 at 11:35 -0700, Michel Thierry wrote:

From: Arun Siluvery 

GuC expects a list of registers from the driver which are
saved/restored
during engine reset. The type of value to be saved is controlled by
flags. We provide a minimal set of registers that we want GuC to save
and
restore. This is not an issue in case of engine reset as driver
initializes
most of them following an engine reset, but in case of media reset (aka
watchdog reset) which is completely internal to GuC (including
resubmission
of hung workload), it is necessary to provide this list, otherwise
GuC won't
be able to schedule further workloads after a reset. This is the
minimal
set of registers identified for things to work as expected but if we
see
any new issues, this register list can be expanded.

In order to not loose any existing workarounds, we have to let GuC know
the registers and its values. These will be reapplied after the reset.
Note that we can't just read the current value because most of these
registers are masked (so we have a workaround for a workaround for a
workaround).

v2: REGSET_MASKED is too difficult for GuC, use
REGSET_SAVE_DEFAULT_VALUE
and current value from RING_MODE reg instead; no need to preserve
head/tail either, be extra paranoid and save whitelisted registers
(Daniele).

v3: Workarounds added only once during _init_workarounds also have to
been restored, or we risk loosing them after internal GuC reset
(Daniele).

Cc: Daniele Ceraolo Spurio 
Signed-off-by: Arun Siluvery 
Signed-off-by: Jeff McGee 
Signed-off-by: Michel Thierry 





@@ -732,15 +755,16 @@ static int gen9_init_workarounds(struct
intel_engine_cs *engine)

 int ret;


 /* WaConextSwitchWithConcurrentTLBInvalidate:skl,bxt,kbl,glk */
-I915_WRITE(GEN9_CSFE_CHICKEN1_RCS,
_MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE));
+I915_GUC_REG_WRITE(GEN9_CSFE_CHICKEN1_RCS,
+
_MASKED_BIT_ENABLE(GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE));


To make grepping easier, how about?

I915_WRITE(GUC_REG(GEN9_CSFE_CHICKEN1_RCS),
   ...);

Regards, Joonas



GUC_REG makes it sound like it is somehow related to GuC, while it
isn't, we just want GuC to restore its value. What about GUC_REG_RESTORE?



Honestly, I dont care about names, pick one and I add it.
Just a reminder, we not only need the reg offset, we want to save the
value too.



I915_WRITE_GUC_RESTORE(reg, value) ?

That would be inline to the others we have, e.g. I915_WRITE_FW, 
I915_WRITE_CTL, I915_WRITE_HEAD/TAIL.


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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Two stage watermarks for g4x

2017-04-21 Thread Patchwork 
== Series Details ==

Series: drm/i915: Two stage watermarks for g4x
URL   : https://patchwork.freedesktop.org/series/23358/
State : success

== Summary ==

Series 23358v1 drm/i915: Two stage watermarks for g4x
https://patchwork.freedesktop.org/api/1.0/series/23358/revisions/1/mbox/

fi-bdw-5557u total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  
time:433s
fi-bdw-gvtdvmtotal:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  
time:429s
fi-bsw-n3050 total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  
time:579s
fi-bxt-j4205 total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  
time:515s
fi-bxt-t5700 total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  
time:554s
fi-byt-j1900 total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  
time:486s
fi-byt-n2820 total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time:479s
fi-hsw-4770  total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time:408s
fi-hsw-4770r total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time:405s
fi-ilk-650   total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  
time:423s
fi-ivb-3520m total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:492s
fi-ivb-3770  total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:471s
fi-kbl-7500u total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:459s
fi-kbl-7560u total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  
time:567s
fi-skl-6260u total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time:450s
fi-skl-6700hqtotal:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  
time:573s
fi-skl-6700k total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  
time:457s
fi-skl-6770hqtotal:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time:498s
fi-skl-gvtdvmtotal:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  
time:430s
fi-snb-2520m total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time:535s
fi-snb-2600  total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  
time:411s

d473a03c132969f33a6f95456a4dbbfb36b99373 drm-tip: 2017y-04m-21d-15h-42m-02s UTC 
integration manifest
ec327fa drm/i915: Add support for sprites on g4x
6fdd8f0 drm/i915: Add g4x watermark tracepoint
dddf36a drm/i915: Enable HPLL watermarks on g4x
9a73d73 drm/i915: Two stage watermarks for g4x
fe292bd drm/i915: Apply the g4x TLB miss w/a to SR watermarks as well
8adede1 drm/i915: Refactor wm calculations
79a7571 drm/i915: Refactor the g4x TLB miss w/a to a helper
e680001 drm/i915: Fix the g4x watermark TLB miss workaround
76a8ff6 drm/i915: Fix cursor 'cpp' in watermark calculatins for old platforms
9df15fa drm/i915: Document CxSR
29a6f48 drm/i915: Make vlv/chv watermark debug print less cryptic
abd09b7 drm/i915: Rename bunch of vlv_ watermark structures to g4x_
d05cc7f drm/i915: s/vlv_num_wm_levels/intel_wm_num_levels/
73841ae drm/i915: Drop the debug message from vlv_get_fifo_size()
d236600 drm/i915: s/vlv_plane_wm_compute/vlv_raw_plane_wm_compute/ etc.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4531/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 08/15] drm/i915: Fix the g4x watermark TLB miss workaround

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

The g4x watermark TLB miss workaround requires that we bump up the
watermark by the difference between 8 full lines and the FIFO size.
Unfortunately the way we compute it at the moment ignores the size
of the pixels. The code also used the primary plane width as the
cursor width when computing the TLB miss w/a for the cursor.
Let's fix both problems.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7d0d1a0c4c63..09d4676419fb 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -811,7 +811,7 @@ static bool g4x_compute_wm0(struct drm_i915_private 
*dev_priv,
struct intel_crtc *crtc;
const struct drm_display_mode *adjusted_mode;
const struct drm_framebuffer *fb;
-   int htotal, hdisplay, clock, cpp;
+   int htotal, plane_width, cursor_width, clock, cpp;
int line_time_us, line_count;
int entries, tlb_miss;
 
@@ -826,12 +826,13 @@ static bool g4x_compute_wm0(struct drm_i915_private 
*dev_priv,
fb = crtc->base.primary->state->fb;
clock = adjusted_mode->crtc_clock;
htotal = adjusted_mode->crtc_htotal;
-   hdisplay = crtc->config->pipe_src_w;
+   plane_width = crtc->config->pipe_src_w;
+   cursor_width = crtc->base.cursor->state->crtc_w;
cpp = fb->format->cpp[0];
 
/* Use the small buffer method to calculate plane watermark */
entries = ((clock * cpp / 1000) * display_latency_ns) / 1000;
-   tlb_miss = display->fifo_size*display->cacheline_size - hdisplay * 8;
+   tlb_miss = display->fifo_size*display->cacheline_size - plane_width * 
cpp * 8;
if (tlb_miss > 0)
entries += tlb_miss;
entries = DIV_ROUND_UP(entries, display->cacheline_size);
@@ -842,8 +843,8 @@ static bool g4x_compute_wm0(struct drm_i915_private 
*dev_priv,
/* Use the large buffer method to calculate cursor watermark */
line_time_us = max(htotal * 1000 / clock, 1);
line_count = (cursor_latency_ns / line_time_us + 1000) / 1000;
-   entries = line_count * crtc->base.cursor->state->crtc_w * 4;
-   tlb_miss = cursor->fifo_size*cursor->cacheline_size - hdisplay * 8;
+   entries = line_count * cursor_width * 4;
+   tlb_miss = cursor->fifo_size*cursor->cacheline_size - cursor_width * 4 
* 8;
if (tlb_miss > 0)
entries += tlb_miss;
entries = DIV_ROUND_UP(entries, cursor->cacheline_size);
-- 
2.10.2

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

[Intel-gfx] [PATCH 12/15] drm/i915: Two stage watermarks for g4x

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

Implement proper two stage watermark programming for g4x. As with
other pre-SKL platforms, the watermark registers aren't double
buffered on g4x. Hence we must sequence the watermark update
carefully around plane updates.

The code is quite heavily modelled on the VLV/CHV code, with some
fairly significant differences due to the different hardware
architecture:
* g4x doesn't use inverted watermark values
* CxSR actually affects the watermarks since it controls memory self
  refresh in addition to the max FIFO mode
* A further HPLL SR mode is possible with higher memory wakeup
  latency
* g4x has FBC2 and so it also has FBC watermarks
* max FIFO mode for primary plane only (cursor is allowed, sprite is not)
* g4x has no manual FIFO repartitioning
* some TLB miss related workarounds are needed for the watermarks

Actually the hardware is quite similar to ILK+ in many ways. The
most visible differences are in the actual watermakr register
layout. ILK revamped that part quite heavily whereas g4x is still
using the layout inherited from earlier platforms.

Note that we didn't previously enable the HPLL SR on g4x. So in order
to not introduce too many functional changes in this patch I've not
actually enabled it here either, even though the code is now fully
ready for it. We'll enable it separately later on.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  12 +-
 drivers/gpu/drm/i915/i915_drv.h  |  12 +
 drivers/gpu/drm/i915/intel_display.c |  25 +-
 drivers/gpu/drm/i915/intel_drv.h |  28 ++
 drivers/gpu/drm/i915/intel_pm.c  | 942 +++
 5 files changed, 792 insertions(+), 227 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 870c470177b5..69550d31099e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3898,6 +3898,8 @@ static void wm_latency_show(struct seq_file *m, const 
uint16_t wm[8])
num_levels = 3;
else if (IS_VALLEYVIEW(dev_priv))
num_levels = 1;
+   else if (IS_G4X(dev_priv))
+   num_levels = 3;
else
num_levels = ilk_wm_max_level(dev_priv) + 1;
 
@@ -3910,8 +3912,10 @@ static void wm_latency_show(struct seq_file *m, const 
uint16_t wm[8])
 * - WM1+ latency values in 0.5us units
 * - latencies are in us on gen9/vlv/chv
 */
-   if (INTEL_GEN(dev_priv) >= 9 || IS_VALLEYVIEW(dev_priv) ||
-   IS_CHERRYVIEW(dev_priv))
+   if (INTEL_GEN(dev_priv) >= 9 ||
+   IS_VALLEYVIEW(dev_priv) ||
+   IS_CHERRYVIEW(dev_priv) ||
+   IS_G4X(dev_priv))
latency *= 10;
else if (level > 0)
latency *= 5;
@@ -3972,7 +3976,7 @@ static int pri_wm_latency_open(struct inode *inode, 
struct file *file)
 {
struct drm_i915_private *dev_priv = inode->i_private;
 
-   if (INTEL_GEN(dev_priv) < 5)
+   if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv))
return -ENODEV;
 
return single_open(file, pri_wm_latency_show, dev_priv);
@@ -4014,6 +4018,8 @@ static ssize_t wm_latency_write(struct file *file, const 
char __user *ubuf,
num_levels = 3;
else if (IS_VALLEYVIEW(dev_priv))
num_levels = 1;
+   else if (IS_G4X(dev_priv))
+   num_levels = 3;
else
num_levels = ilk_wm_max_level(dev_priv) + 1;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0a393fdc53d1..6df8bff7f5a7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1762,11 +1762,13 @@ struct ilk_wm_values {
 
 struct g4x_pipe_wm {
uint16_t plane[I915_MAX_PLANES];
+   uint16_t fbc;
 };
 
 struct g4x_sr_wm {
uint16_t plane;
uint16_t cursor;
+   uint16_t fbc;
 };
 
 struct vlv_wm_ddl_values {
@@ -1781,6 +1783,15 @@ struct vlv_wm_values {
bool cxsr;
 };
 
+struct g4x_wm_values {
+   struct g4x_pipe_wm pipe[2];
+   struct g4x_sr_wm sr;
+   struct g4x_sr_wm hpll;
+   bool cxsr;
+   bool hpll_en;
+   bool fbc_en;
+};
+
 struct skl_ddb_entry {
uint16_t start, end;/* in number of blocks, 'end' is exclusive */
 };
@@ -2410,6 +2421,7 @@ struct drm_i915_private {
struct ilk_wm_values hw;
struct skl_wm_values skl_hw;
struct vlv_wm_values vlv;
+   struct g4x_wm_values g4x;
};
 
uint8_t max_level;
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 85b9e2f521a0..0f42263c3f76 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++

[Intel-gfx] [PATCH 13/15] drm/i915: Enable HPLL watermarks on g4x

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

I don't see why we couldn't use the HPLL watermarks on g4x. So let's
enable them. Let's assume a 35 usec memory latency for the HPLL mode.
That's roughly what PNV uses.

Based on the behaviour of the ELK box I have 35 usec is probably
overkill. Actually all the current latency values used seem overkill as
I can reduce them pretty drastically before I start to see underruns.
But let's play things a bit safe for now.

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 9b0a6a4572ce..957ef10b1569 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1020,8 +1020,9 @@ static void g4x_setup_wm_latency(struct drm_i915_private 
*dev_priv)
/* all latencies in usec */
dev_priv->wm.pri_latency[G4X_WM_LEVEL_NORMAL] = 5;
dev_priv->wm.pri_latency[G4X_WM_LEVEL_SR] = 12;
+   dev_priv->wm.pri_latency[G4X_WM_LEVEL_HPLL] = 35;
 
-   dev_priv->wm.max_level = G4X_WM_LEVEL_SR;
+   dev_priv->wm.max_level = G4X_WM_LEVEL_HPLL;
 }
 
 static int g4x_plane_fifo_size(enum plane_id plane_id, int level)
-- 
2.10.2

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

[Intel-gfx] [PATCH 15/15] drm/i915: Add support for sprites on g4x

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

Now that the watermarks are in order, it should be safe to enable sprite
planes on g4x. We alreday have the code in fact, we just call it ilk_.
Let's rename to g4x_ and let it loose.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_device_info.c |  2 +-
 drivers/gpu/drm/i915/intel_display.c |  4 ++--
 drivers/gpu/drm/i915/intel_sprite.c  | 18 +-
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_device_info.c 
b/drivers/gpu/drm/i915/intel_device_info.c
index 7d01dfe7faac..3718341662c2 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -337,7 +337,7 @@ void intel_device_info_runtime_init(struct drm_i915_private 
*dev_priv)
} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
for_each_pipe(dev_priv, pipe)
info->num_sprites[pipe] = 2;
-   } else if (INTEL_GEN(dev_priv) >= 5) {
+   } else if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)) {
for_each_pipe(dev_priv, pipe)
info->num_sprites[pipe] = 1;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 0f42263c3f76..3ccc39e46f8f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1277,7 +1277,7 @@ static void assert_sprites_disabled(struct 
drm_i915_private *dev_priv,
I915_STATE_WARN(val & SPRITE_ENABLE,
 "sprite %c assertion failure, should be off on pipe %c but 
is still active\n",
 plane_name(pipe), pipe_name(pipe));
-   } else if (INTEL_GEN(dev_priv) >= 5) {
+   } else if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)) {
u32 val = I915_READ(DVSCNTR(pipe));
I915_STATE_WARN(val & DVS_ENABLE,
 "sprite %c assertion failure, should be off on pipe %c but 
is still active\n",
@@ -14429,7 +14429,7 @@ static int intel_framebuffer_init(struct 
intel_framebuffer *intel_fb,
case DRM_FORMAT_UYVY:
case DRM_FORMAT_YVYU:
case DRM_FORMAT_VYUY:
-   if (INTEL_GEN(dev_priv) < 5) {
+   if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv)) {
DRM_DEBUG_KMS("unsupported pixel format: %s\n",
  
drm_get_format_name(mode_cmd->pixel_format, _name));
goto err;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index f7d431427115..511f0e969f7a 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -629,7 +629,7 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc 
*crtc)
spin_unlock_irqrestore(_priv->uncore.lock, irqflags);
 }
 
-static u32 ilk_sprite_ctl(const struct intel_crtc_state *crtc_state,
+static u32 g4x_sprite_ctl(const struct intel_crtc_state *crtc_state,
  const struct intel_plane_state *plane_state)
 {
struct drm_i915_private *dev_priv =
@@ -683,7 +683,7 @@ static u32 ilk_sprite_ctl(const struct intel_crtc_state 
*crtc_state,
 }
 
 static void
-ilk_update_plane(struct drm_plane *plane,
+g4x_update_plane(struct drm_plane *plane,
 const struct intel_crtc_state *crtc_state,
 const struct intel_plane_state *plane_state)
 {
@@ -744,7 +744,7 @@ ilk_update_plane(struct drm_plane *plane,
 }
 
 static void
-ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
+g4x_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 {
struct drm_device *dev = plane->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
@@ -964,7 +964,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
if (ret)
return ret;
 
-   state->ctl = ilk_sprite_ctl(crtc_state, state);
+   state->ctl = g4x_sprite_ctl(crtc_state, state);
}
 
return 0;
@@ -1024,7 +1024,7 @@ int intel_sprite_set_colorkey(struct drm_device *dev, 
void *data,
return ret;
 }
 
-static const uint32_t ilk_plane_formats[] = {
+static const uint32_t g4x_plane_formats[] = {
DRM_FORMAT_XRGB,
DRM_FORMAT_YUYV,
DRM_FORMAT_YVYU,
@@ -1128,15 +1128,15 @@ intel_sprite_plane_create(struct drm_i915_private 
*dev_priv,
intel_plane->can_scale = true;
intel_plane->max_downscale = 16;
 
-   intel_plane->update_plane = ilk_update_plane;
-   intel_plane->disable_plane = ilk_disable_plane;
+   intel_plane->update_plane = g4x_update_plane;
+   intel_plane->disable_plane = g4x_disable_plane;
 
if (IS_GEN6(dev_priv)) {
plane_formats = snb_plane_formats;
num_plane_formats

[Intel-gfx] [PATCH 14/15] drm/i915: Add g4x watermark tracepoint

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

Add a tracepoint for watermark programming on g4x, similar to what we
have on vlv/chv. Should help in debugging watermark programming sequence
issues.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_trace.h | 49 +++
 drivers/gpu/drm/i915/intel_pm.c   |  5 
 2 files changed, 54 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_trace.h 
b/drivers/gpu/drm/i915/i915_trace.h
index 66404c5aee82..b24a83d43559 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -89,6 +89,55 @@ TRACE_EVENT(intel_memory_cxsr,
  __entry->frame[PIPE_C], __entry->scanline[PIPE_C])
 );
 
+TRACE_EVENT(g4x_wm,
+   TP_PROTO(struct intel_crtc *crtc, const struct g4x_wm_values *wm),
+   TP_ARGS(crtc, wm),
+
+   TP_STRUCT__entry(
+__field(enum pipe, pipe)
+__field(u32, frame)
+__field(u32, scanline)
+__field(u16, primary)
+__field(u16, sprite)
+__field(u16, cursor)
+__field(u16, sr_plane)
+__field(u16, sr_cursor)
+__field(u16, sr_fbc)
+__field(u16, hpll_plane)
+__field(u16, hpll_cursor)
+__field(u16, hpll_fbc)
+__field(bool, cxsr)
+__field(bool, hpll)
+__field(bool, fbc)
+),
+
+   TP_fast_assign(
+  __entry->pipe = crtc->pipe;
+  __entry->frame = 
crtc->base.dev->driver->get_vblank_counter(crtc->base.dev,
+   
   crtc->pipe);
+  __entry->scanline = intel_get_crtc_scanline(crtc);
+  __entry->primary = 
wm->pipe[crtc->pipe].plane[PLANE_PRIMARY];
+  __entry->sprite = 
wm->pipe[crtc->pipe].plane[PLANE_SPRITE0];
+  __entry->cursor = 
wm->pipe[crtc->pipe].plane[PLANE_CURSOR];
+  __entry->sr_plane = wm->sr.plane;
+  __entry->sr_cursor = wm->sr.cursor;
+  __entry->sr_fbc = wm->sr.fbc;
+  __entry->hpll_plane = wm->hpll.plane;
+  __entry->hpll_cursor = wm->hpll.cursor;
+  __entry->hpll_fbc = wm->hpll.fbc;
+  __entry->cxsr = wm->cxsr;
+  __entry->hpll = wm->hpll_en;
+  __entry->fbc = wm->fbc_en;
+  ),
+
+   TP_printk("pipe %c, frame=%u, scanline=%u, wm %d/%d/%d, sr 
%s/%d/%d/%d, hpll %s/%d/%d/%d, fbc %s",
+ pipe_name(__entry->pipe), __entry->frame, 
__entry->scanline,
+ __entry->primary, __entry->sprite, __entry->cursor,
+ yesno(__entry->cxsr), __entry->sr_plane, 
__entry->sr_cursor, __entry->sr_fbc,
+ yesno(__entry->hpll), __entry->hpll_plane, 
__entry->hpll_cursor, __entry->hpll_fbc,
+ yesno(__entry->fbc))
+);
+
 TRACE_EVENT(vlv_wm,
TP_PROTO(struct intel_crtc *crtc, const struct vlv_wm_values *wm),
TP_ARGS(crtc, wm),
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 957ef10b1569..ef0e9f8d4dbd 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -913,6 +913,11 @@ static int g4x_tlb_miss_wa(int fifo_size, int width, int 
cpp)
 static void g4x_write_wm_values(struct drm_i915_private *dev_priv,
const struct g4x_wm_values *wm)
 {
+   enum pipe pipe;
+
+   for_each_pipe(dev_priv, pipe)
+   trace_g4x_wm(intel_get_crtc_for_pipe(dev_priv, pipe), wm);
+
I915_WRITE(DSPFW1,
   FW_WM(wm->sr.plane, SR) |
   FW_WM(wm->pipe[PIPE_B].plane[PLANE_CURSOR], CURSORB) |
-- 
2.10.2

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

[Intel-gfx] [PATCH 10/15] drm/i915: Refactor wm calculations

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

All platforms until SKL compute their watermarks essentially
using the same method1/small buffer and method2/large buffer
formulas. Most just open code it in slightly different ways.
Let's pull it all into common helpers. This makes it a little
easier to spot the actual differences.

While at it try to add some docs explainign what the formulas
are trying to do.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c43fcd5d29b2..c07f3b2b0972 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -626,8 +626,104 @@ static const struct intel_watermark_params i845_wm_info = 
{
 };
 
 /**
+ * intel_wm_method1 - Method 1 / "small buffer" watermark formula
+ * @pixel_rate: Pipe pixel rate in kHz
+ * @cpp: Plane bytes per pixel
+ * @latency: Memory wakeup latency in 0.1us units
+ *
+ * Compute the watermark using the method 1 or "small buffer"
+ * formula. The caller may additonally add extra cachelines
+ * to account for TLB misses and clock crossings.
+ *
+ * This method is concerned with the short term drain rate
+ * of the FIFO, ie. it does not account for blanking periods
+ * which would effectively reduce the average drain rate across
+ * a longer period. The name "small" refers to the fact the
+ * FIFO is relatively small compared to the amount of data
+ * fetched.
+ *
+ * The FIFO level vs. time graph might look something like:
+ *
+ *   |\   |\
+ *   | \  | \
+ * __---__---__ (- plane active, _ blanking)
+ * -> time
+ *
+ * or perhaps like this:
+ *
+ *   |\|\  |\|\
+ * ______ (- plane active, _ blanking)
+ * -> time
+ *
+ * Returns:
+ * The watermark in bytes
+ */
+static unsigned int intel_wm_method1(unsigned int pixel_rate,
+unsigned int cpp,
+unsigned int latency)
+{
+   uint64_t ret;
+
+   ret = (uint64_t) pixel_rate * cpp * latency;
+   ret = DIV_ROUND_UP_ULL(ret, 1);
+
+   return ret;
+}
+
+/**
+ * intel_wm_method2 - Method 2 / "large buffer" watermark formula
+ * @pixel_rate: Pipe pixel rate in kHz
+ * @htotal: Pipe horizontal total
+ * @width: Plane width in pixels
+ * @cpp: Plane bytes per pixel
+ * @latency: Memory wakeup latency in 0.1us units
+ *
+ * Compute the watermark using the method 2 or "large buffer"
+ * formula. The caller may additonally add extra cachelines
+ * to account for TLB misses and clock crossings.
+ *
+ * This method is concerned with the long term drain rate
+ * of the FIFO, ie. it does account for blanking periods
+ * which effectively reduce the average drain rate across
+ * a longer period. The name "large" refers to the fact the
+ * FIFO is relatively large compared to the amount of data
+ * fetched.
+ *
+ * The FIFO level vs. time graph might look something like:
+ *
+ *|\___   |\___
+ *|\___   |\___
+ *|\  |\
+ * __ --__--__--__--__--__--__ (- plane active, _ blanking)
+ * -> time
+ *
+ * Returns:
+ * The watermark in bytes
+ */
+static unsigned int intel_wm_method2(unsigned int pixel_rate,
+unsigned int htotal,
+unsigned int width,
+unsigned int cpp,
+unsigned int latency)
+{
+   unsigned int ret;
+
+   /*
+* FIXME remove once all users are computing
+* watermarks in the correct place.
+*/
+   if (WARN_ON_ONCE(htotal == 0))
+   htotal = 1;
+
+   ret = (latency * pixel_rate) / (htotal * 1);
+   ret = (ret + 1) * width * cpp;
+
+   return ret;
+}
+
+/**
  * intel_calculate_wm - calculate watermark level
- * @clock_in_khz: pixel clock
+ * @pixel_rate: pixel clock
  * @wm: chip FIFO params
  * @cpp: bytes per pixel
  * @latency_ns: memory latency for the platform
@@ -643,12 +739,12 @@ static const struct intel_watermark_params i845_wm_info = 
{
  * past the watermark point.  If the FIFO drains completely, a FIFO underrun
  * will occur, and a display engine hang could result.
  */
-static unsigned long intel_calculate_wm(unsigned long clock_in_khz,
-   const struct intel_watermark_params *wm,
-   int fifo_size, int cpp,
-   unsigned long latency_ns)
+static unsigned int intel_calculate_wm(int pixel_rate,
+  const struct intel_watermark_params *wm,
+  int fifo_size, int cpp,
+  unsigned int latency_ns)
 {
-   long entries_required, wm_size;
+   int entries, wm_size;
 
/*
 * Note: we

[Intel-gfx] [PATCH 11/15] drm/i915: Apply the g4x TLB miss w/a to SR watermarks as well

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

The documentation I've seen doesn't actually specify which watermarks
need the TLB miss w/a. Currently we only apply the w/a to the normal
watermarks for both primary and cursor planes. Since the documentation
doesn't explicitly say anything I'm going to assume that the w/a should
equally apply to the SR/HPLL watermarks. So let's do that.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index c07f3b2b0972..61b67994c4a8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1006,7 +1006,7 @@ static bool g4x_compute_srwm(struct drm_i915_private 
*dev_priv,
struct intel_crtc *crtc;
const struct drm_display_mode *adjusted_mode;
const struct drm_framebuffer *fb;
-   int hdisplay, htotal, cpp, clock;
+   int plane_width, cursor_width, htotal, cpp, clock;
int small, large;
int entries;
 
@@ -1020,20 +1020,23 @@ static bool g4x_compute_srwm(struct drm_i915_private 
*dev_priv,
fb = crtc->base.primary->state->fb;
clock = adjusted_mode->crtc_clock;
htotal = adjusted_mode->crtc_htotal;
-   hdisplay = crtc->config->pipe_src_w;
+   plane_width = crtc->config->pipe_src_w;
+   cursor_width = crtc->base.cursor->state->crtc_w;
cpp = fb->format->cpp[0];
 
/* Use the minimum of the small and large buffer method for primary */
small = intel_wm_method1(clock, cpp, latency_ns / 100);
-   large = intel_wm_method2(clock, htotal, hdisplay, cpp,
+   large = intel_wm_method2(clock, htotal, plane_width, cpp,
 latency_ns / 100);
-   entries = DIV_ROUND_UP(min(small, large), display->cacheline_size);
+   entries = min(small, large);
+   entries += g4x_tlb_miss_wa(display->fifo_size, plane_width, cpp);
+   entries = DIV_ROUND_UP(entries, display->cacheline_size);
*display_wm = entries + display->guard_size;
 
/* calculate the self-refresh watermark for display cursor */
-   entries = intel_wm_method2(clock, htotal,
-  crtc->base.cursor->state->crtc_w, 4,
+   entries = intel_wm_method2(clock, htotal, cursor_width, 4,
   latency_ns / 100);
+   entries += g4x_tlb_miss_wa(cursor->fifo_size, cursor_width, 4);
entries = DIV_ROUND_UP(entries, cursor->cacheline_size);
*cursor_wm = entries + cursor->guard_size;
 
-- 
2.10.2

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

[Intel-gfx] [PATCH 09/15] drm/i915: Refactor the g4x TLB miss w/a to a helper

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

Pull the g4x TLB miss w/a calculation into a small helper.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 09d4676419fb..c43fcd5d29b2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -799,6 +799,23 @@ static void pineview_update_wm(struct intel_crtc 
*unused_crtc)
}
 }
 
+/*
+ * Documentation says:
+ * "If the line size is small, the TLB fetches can get in the way of the
+ *  data fetches, causing some lag in the pixel data return which is not
+ *  accounted for in the above formulas. The following adjustment only
+ *  needs to be applied if eight whole lines fit in the buffer at once.
+ *  The WM is adjusted upwards by the difference between the FIFO size
+ *  and the size of 8 whole lines. This adjustment is always performed
+ *  in the actual pixel depth regardless of whether FBC is enabled or not."
+ */
+static int g4x_tlb_miss_wa(int fifo_size, int width, int cpp)
+{
+   int tlb_miss = fifo_size * 64 - width * cpp * 8;
+
+   return max(0, tlb_miss);
+}
+
 static bool g4x_compute_wm0(struct drm_i915_private *dev_priv,
int plane,
const struct intel_watermark_params *display,
@@ -813,7 +830,7 @@ static bool g4x_compute_wm0(struct drm_i915_private 
*dev_priv,
const struct drm_framebuffer *fb;
int htotal, plane_width, cursor_width, clock, cpp;
int line_time_us, line_count;
-   int entries, tlb_miss;
+   int entries;
 
crtc = intel_get_crtc_for_plane(dev_priv, plane);
if (!intel_crtc_active(crtc)) {
@@ -832,9 +849,7 @@ static bool g4x_compute_wm0(struct drm_i915_private 
*dev_priv,
 
/* Use the small buffer method to calculate plane watermark */
entries = ((clock * cpp / 1000) * display_latency_ns) / 1000;
-   tlb_miss = display->fifo_size*display->cacheline_size - plane_width * 
cpp * 8;
-   if (tlb_miss > 0)
-   entries += tlb_miss;
+   entries += g4x_tlb_miss_wa(display->fifo_size, plane_width, cpp);
entries = DIV_ROUND_UP(entries, display->cacheline_size);
*plane_wm = entries + display->guard_size;
if (*plane_wm > (int)display->max_wm)
@@ -844,9 +859,7 @@ static bool g4x_compute_wm0(struct drm_i915_private 
*dev_priv,
line_time_us = max(htotal * 1000 / clock, 1);
line_count = (cursor_latency_ns / line_time_us + 1000) / 1000;
entries = line_count * cursor_width * 4;
-   tlb_miss = cursor->fifo_size*cursor->cacheline_size - cursor_width * 4 
* 8;
-   if (tlb_miss > 0)
-   entries += tlb_miss;
+   entries += g4x_tlb_miss_wa(cursor->fifo_size, cursor_width, 4);
entries = DIV_ROUND_UP(entries, cursor->cacheline_size);
*cursor_wm = entries + cursor->guard_size;
if (*cursor_wm > (int)cursor->max_wm)
-- 
2.10.2

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

[Intel-gfx] [PATCH 06/15] drm/i915: Document CxSR

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

Add some documentation explaining what CxSR actually is.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index da2072728df2..7bd4c8688acb 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -386,6 +386,43 @@ static bool _intel_set_memory_cxsr(struct drm_i915_private 
*dev_priv, bool enabl
return was_enabled;
 }
 
+/**
+ * intel_set_memory_cxsr - Configure CxSR state
+ * @dev_priv: i915 device
+ * @enable: Allow vs. disallow CxSR
+ *
+ * Allow or disallow the system to enter a special CxSR
+ * (C-state self refresh) state. What typically happens in CxSR mode
+ * is that several display FIFOs may get combined into a single larger
+ * FIFO for a particular plane (so called max FIFO mode) to allow the
+ * system to defer memory fetches longer, and the memory will enter
+ * self refresh.
+ *
+ * Note that enabling CxSR does not guarantee that the system enter
+ * this special mode, nor does it guarantee that the system stays
+ * in that mode once entered. So this just allows/disallows the system
+ * to autonomously utilize the CxSR mode. Other factors such as core
+ * C-states will affect when/if the system actually enters/exits the
+ * CxSR mode.
+ *
+ * Note that on VLV/CHV this actually only controls the max FIFO mode,
+ * and the system is free to enter/exit memory self refresh at any time
+ * even when the use of CxSR has been disallowed.
+ *
+ * While the system is actually in the CxSR/max FIFO mode, some plane
+ * control registers will not get latched on vblank. Thus in order to
+ * guarantee the system will respond to changes in the plane registers
+ * we must always disallow CxSR prior to making changes to those registers.
+ * Unfortunately the system will re-evaluate the CxSR conditions at
+ * frame start which happens after vblank start (which is when the plane
+ * registers would get latched), so we can't proceed with the plane update
+ * during the same frame where we disallowed CxSR.
+ *
+ * Certain platforms also have a deeper HPLL SR mode. Fortunately the
+ * HPLL SR mode depends on CxSR itself, so we don't have to hand hold
+ * the hardware w.r.t. HPLL SR when writing to plane registers.
+ * Disallowing just CxSR is sufficient.
+ */
 bool intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable)
 {
bool ret;
-- 
2.10.2

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

[Intel-gfx] [PATCH 07/15] drm/i915: Fix cursor 'cpp' in watermark calculatins for old platforms

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

The watermark code for the old platforms (g4x and older) uses the
primary plane cpp when computing cursor watermarks. To keep the fix
simple let's just hardcode cpp=4 for the cursor on those platforms
since that's all we support.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 7bd4c8688acb..7d0d1a0c4c63 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -768,7 +768,7 @@ static void pineview_update_wm(struct intel_crtc 
*unused_crtc)
/* cursor SR */
wm = intel_calculate_wm(clock, _cursor_wm,
pineview_display_wm.fifo_size,
-   cpp, latency->cursor_sr);
+   4, latency->cursor_sr);
reg = I915_READ(DSPFW3);
reg &= ~DSPFW_CURSOR_SR_MASK;
reg |= FW_WM(wm, CURSOR_SR);
@@ -786,7 +786,7 @@ static void pineview_update_wm(struct intel_crtc 
*unused_crtc)
/* cursor HPLL off SR */
wm = intel_calculate_wm(clock, _cursor_hplloff_wm,
pineview_display_hplloff_wm.fifo_size,
-   cpp, latency->cursor_hpll_disable);
+   4, latency->cursor_hpll_disable);
reg = I915_READ(DSPFW3);
reg &= ~DSPFW_HPLL_CURSOR_MASK;
reg |= FW_WM(wm, HPLL_CURSOR);
@@ -842,7 +842,7 @@ static bool g4x_compute_wm0(struct drm_i915_private 
*dev_priv,
/* Use the large buffer method to calculate cursor watermark */
line_time_us = max(htotal * 1000 / clock, 1);
line_count = (cursor_latency_ns / line_time_us + 1000) / 1000;
-   entries = line_count * crtc->base.cursor->state->crtc_w * cpp;
+   entries = line_count * crtc->base.cursor->state->crtc_w * 4;
tlb_miss = cursor->fifo_size*cursor->cacheline_size - hdisplay * 8;
if (tlb_miss > 0)
entries += tlb_miss;
@@ -930,7 +930,7 @@ static bool g4x_compute_srwm(struct drm_i915_private 
*dev_priv,
*display_wm = entries + display->guard_size;
 
/* calculate the self-refresh watermark for display cursor */
-   entries = line_count * cpp * crtc->base.cursor->state->crtc_w;
+   entries = line_count * 4 * crtc->base.cursor->state->crtc_w;
entries = DIV_ROUND_UP(entries, cursor->cacheline_size);
*cursor_wm = entries + cursor->guard_size;
 
@@ -1736,7 +1736,7 @@ static void i965_update_wm(struct intel_crtc *unused_crtc)
  entries, srwm);
 
entries = (((sr_latency_ns / line_time_us) + 1000) / 1000) *
-   cpp * crtc->base.cursor->state->crtc_w;
+   4 * crtc->base.cursor->state->crtc_w;
entries = DIV_ROUND_UP(entries,
  i965_cursor_wm_info.cacheline_size);
cursor_sr = i965_cursor_wm_info.fifo_size -
-- 
2.10.2

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

[Intel-gfx] [PATCH 03/15] drm/i915: s/vlv_num_wm_levels/intel_wm_num_levels/

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

Rename the VLV/CHV max_level->num_levels helper to have an intel_
prefix since it's not VLV/CHV specific and I'll want to use it on
other platforms as well.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 2c63abe2039c..ee045be2a5e0 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -648,6 +648,11 @@ static unsigned long intel_calculate_wm(unsigned long 
clock_in_khz,
return wm_size;
 }
 
+static int intel_wm_num_levels(struct drm_i915_private *dev_priv)
+{
+   return dev_priv->wm.max_level + 1;
+}
+
 static bool intel_wm_plane_visible(const struct intel_crtc_state *crtc_state,
   const struct intel_plane_state *plane_state)
 {
@@ -1136,18 +1141,13 @@ static int vlv_compute_fifo(struct intel_crtc_state 
*crtc_state)
return 0;
 }
 
-static int vlv_num_wm_levels(struct drm_i915_private *dev_priv)
-{
-   return dev_priv->wm.max_level + 1;
-}
-
 /* mark all levels starting from 'level' as invalid */
 static void vlv_invalidate_wms(struct intel_crtc *crtc,
   struct vlv_wm_state *wm_state, int level)
 {
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
-   for (; level < vlv_num_wm_levels(dev_priv); level++) {
+   for (; level < intel_wm_num_levels(dev_priv); level++) {
enum plane_id plane_id;
 
for_each_plane_id_on_crtc(crtc, plane_id)
@@ -1174,7 +1174,7 @@ static bool vlv_raw_plane_wm_set(struct intel_crtc_state 
*crtc_state,
 int level, enum plane_id plane_id, u16 value)
 {
struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
-   int num_levels = vlv_num_wm_levels(dev_priv);
+   int num_levels = intel_wm_num_levels(dev_priv);
bool dirty = false;
 
for (; level < num_levels; level++) {
@@ -1192,7 +1192,7 @@ static bool vlv_raw_plane_wm_compute(struct 
intel_crtc_state *crtc_state,
 {
struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
enum plane_id plane_id = plane->id;
-   int num_levels = vlv_num_wm_levels(to_i915(plane->base.dev));
+   int num_levels = intel_wm_num_levels(to_i915(plane->base.dev));
int level;
bool dirty = false;
 
@@ -1306,7 +1306,7 @@ static int vlv_compute_pipe_wm(struct intel_crtc_state 
*crtc_state)
}
 
/* initially allow all levels */
-   wm_state->num_levels = vlv_num_wm_levels(dev_priv);
+   wm_state->num_levels = intel_wm_num_levels(dev_priv);
/*
 * Note that enabling cxsr with no primary/sprite planes
 * enabled can wedge the pipe. Hence we only allow cxsr
-- 
2.10.2

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

[Intel-gfx] [PATCH 05/15] drm/i915: Make vlv/chv watermark debug print less cryptic

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

The magic numbers 0,1,2 aren't all that interesting for users perhaps.
Since we know what these watermark levels mean for VLV/CHV let's print
their names.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1cc13ccadee6..da2072728df2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1218,7 +1218,7 @@ static bool vlv_raw_plane_wm_compute(struct 
intel_crtc_state *crtc_state,
 
 out:
if (dirty)
-   DRM_DEBUG_KMS("%s wms: [0]=%d,[1]=%d,[2]=%d\n",
+   DRM_DEBUG_KMS("%s watermarks: PM2=%d, PM5=%d, DDR DVFS=%d\n",
  plane->base.name,
  
crtc_state->wm.vlv.raw[VLV_WM_LEVEL_PM2].plane[plane_id],
  
crtc_state->wm.vlv.raw[VLV_WM_LEVEL_PM5].plane[plane_id],
-- 
2.10.2

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

[Intel-gfx] [PATCH 04/15] drm/i915: Rename bunch of vlv_ watermark structures to g4x_

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

We'll be wanting to share some of these watermark structures on g4x,
so let's rename them to have a g4x_ prefix instead of vlv_.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.h  |  8 
 drivers/gpu/drm/i915/intel_drv.h |  6 +++---
 drivers/gpu/drm/i915/intel_pm.c  | 14 +++---
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 357b6c6c2f04..0a393fdc53d1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1760,11 +1760,11 @@ struct ilk_wm_values {
enum intel_ddb_partitioning partitioning;
 };
 
-struct vlv_pipe_wm {
+struct g4x_pipe_wm {
uint16_t plane[I915_MAX_PLANES];
 };
 
-struct vlv_sr_wm {
+struct g4x_sr_wm {
uint16_t plane;
uint16_t cursor;
 };
@@ -1774,8 +1774,8 @@ struct vlv_wm_ddl_values {
 };
 
 struct vlv_wm_values {
-   struct vlv_pipe_wm pipe[3];
-   struct vlv_sr_wm sr;
+   struct g4x_pipe_wm pipe[3];
+   struct g4x_sr_wm sr;
struct vlv_wm_ddl_values ddl[3];
uint8_t level;
bool cxsr;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 54f3ff840812..d1cdd10998fa 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -512,8 +512,8 @@ enum vlv_wm_level {
 };
 
 struct vlv_wm_state {
-   struct vlv_pipe_wm wm[NUM_VLV_WM_LEVELS];
-   struct vlv_sr_wm sr[NUM_VLV_WM_LEVELS];
+   struct g4x_pipe_wm wm[NUM_VLV_WM_LEVELS];
+   struct g4x_sr_wm sr[NUM_VLV_WM_LEVELS];
uint8_t num_levels;
bool cxsr;
 };
@@ -549,7 +549,7 @@ struct intel_crtc_wm_state {
 
struct {
/* "raw" watermarks (not inverted) */
-   struct vlv_pipe_wm raw[NUM_VLV_WM_LEVELS];
+   struct g4x_pipe_wm raw[NUM_VLV_WM_LEVELS];
/* intermediate watermarks (inverted) */
struct vlv_wm_state intermediate;
/* optimal watermarks (inverted) */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index ee045be2a5e0..1cc13ccadee6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1062,7 +1062,7 @@ static bool vlv_need_sprite0_fifo_workaround(unsigned int 
active_planes)
 static int vlv_compute_fifo(struct intel_crtc_state *crtc_state)
 {
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-   const struct vlv_pipe_wm *raw =
+   const struct g4x_pipe_wm *raw =
_state->wm.vlv.raw[VLV_WM_LEVEL_PM2];
struct vlv_fifo_state *fifo_state = _state->wm.vlv.fifo_state;
unsigned int active_planes = crtc_state->active_planes & 
~BIT(PLANE_CURSOR);
@@ -1178,7 +1178,7 @@ static bool vlv_raw_plane_wm_set(struct intel_crtc_state 
*crtc_state,
bool dirty = false;
 
for (; level < num_levels; level++) {
-   struct vlv_pipe_wm *raw = _state->wm.vlv.raw[level];
+   struct g4x_pipe_wm *raw = _state->wm.vlv.raw[level];
 
dirty |= raw->plane[plane_id] != value;
raw->plane[plane_id] = value;
@@ -1202,7 +1202,7 @@ static bool vlv_raw_plane_wm_compute(struct 
intel_crtc_state *crtc_state,
}
 
for (level = 0; level < num_levels; level++) {
-   struct vlv_pipe_wm *raw = _state->wm.vlv.raw[level];
+   struct g4x_pipe_wm *raw = _state->wm.vlv.raw[level];
int wm = vlv_compute_wm_level(crtc_state, plane_state, level);
int max_wm = plane_id == PLANE_CURSOR ? 63 : 511;
 
@@ -1230,7 +1230,7 @@ static bool vlv_raw_plane_wm_compute(struct 
intel_crtc_state *crtc_state,
 static bool vlv_raw_plane_wm_is_valid(const struct intel_crtc_state 
*crtc_state,
  enum plane_id plane_id, int level)
 {
-   const struct vlv_pipe_wm *raw =
+   const struct g4x_pipe_wm *raw =
_state->wm.vlv.raw[level];
const struct vlv_fifo_state *fifo_state =
_state->wm.vlv.fifo_state;
@@ -1315,7 +1315,7 @@ static int vlv_compute_pipe_wm(struct intel_crtc_state 
*crtc_state)
wm_state->cxsr = crtc->pipe != PIPE_C && num_active_planes == 1;
 
for (level = 0; level < wm_state->num_levels; level++) {
-   const struct vlv_pipe_wm *raw = _state->wm.vlv.raw[level];
+   const struct g4x_pipe_wm *raw = _state->wm.vlv.raw[level];
const int sr_fifo_size = INTEL_INFO(dev_priv)->num_pipes * 512 
- 1;
 
if (!vlv_raw_crtc_wm_is_valid(crtc_state, level))
@@ -4785,7 +4785,7 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
active->cxsr = wm->cxsr;
 
for (level = 0; level < active->num_levels; level++) {
-   struct

[Intel-gfx] [PATCH 01/15] drm/i915: s/vlv_plane_wm_compute/vlv_raw_plane_wm_compute/ etc.

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

Rename some of the vlv wm functions to reflect the fact that they
operate on the "raw" watermarks.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index cacb65fa2dd5..23fff9167d77 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1194,8 +1194,8 @@ static bool vlv_raw_plane_wm_set(struct intel_crtc_state 
*crtc_state,
return dirty;
 }
 
-static bool vlv_plane_wm_compute(struct intel_crtc_state *crtc_state,
-const struct intel_plane_state *plane_state)
+static bool vlv_raw_plane_wm_compute(struct intel_crtc_state *crtc_state,
+const struct intel_plane_state 
*plane_state)
 {
struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
enum plane_id plane_id = plane->id;
@@ -1234,8 +1234,8 @@ static bool vlv_plane_wm_compute(struct intel_crtc_state 
*crtc_state,
return dirty;
 }
 
-static bool vlv_plane_wm_is_valid(const struct intel_crtc_state *crtc_state,
- enum plane_id plane_id, int level)
+static bool vlv_raw_plane_wm_is_valid(const struct intel_crtc_state 
*crtc_state,
+ enum plane_id plane_id, int level)
 {
const struct vlv_pipe_wm *raw =
_state->wm.vlv.raw[level];
@@ -1245,12 +1245,12 @@ static bool vlv_plane_wm_is_valid(const struct 
intel_crtc_state *crtc_state,
return raw->plane[plane_id] <= fifo_state->plane[plane_id];
 }
 
-static bool vlv_crtc_wm_is_valid(const struct intel_crtc_state *crtc_state, 
int level)
+static bool vlv_raw_crtc_wm_is_valid(const struct intel_crtc_state 
*crtc_state, int level)
 {
-   return vlv_plane_wm_is_valid(crtc_state, PLANE_PRIMARY, level) &&
-   vlv_plane_wm_is_valid(crtc_state, PLANE_SPRITE0, level) &&
-   vlv_plane_wm_is_valid(crtc_state, PLANE_SPRITE1, level) &&
-   vlv_plane_wm_is_valid(crtc_state, PLANE_CURSOR, level);
+   return vlv_raw_plane_wm_is_valid(crtc_state, PLANE_PRIMARY, level) &&
+   vlv_raw_plane_wm_is_valid(crtc_state, PLANE_SPRITE0, level) &&
+   vlv_raw_plane_wm_is_valid(crtc_state, PLANE_SPRITE1, level) &&
+   vlv_raw_plane_wm_is_valid(crtc_state, PLANE_CURSOR, level);
 }
 
 static int vlv_compute_pipe_wm(struct intel_crtc_state *crtc_state)
@@ -1279,7 +1279,7 @@ static int vlv_compute_pipe_wm(struct intel_crtc_state 
*crtc_state)
old_plane_state->base.crtc != >base)
continue;
 
-   if (vlv_plane_wm_compute(crtc_state, plane_state))
+   if (vlv_raw_plane_wm_compute(crtc_state, plane_state))
dirty |= BIT(plane->id);
}
 
@@ -1325,7 +1325,7 @@ static int vlv_compute_pipe_wm(struct intel_crtc_state 
*crtc_state)
const struct vlv_pipe_wm *raw = _state->wm.vlv.raw[level];
const int sr_fifo_size = INTEL_INFO(dev_priv)->num_pipes * 512 
- 1;
 
-   if (!vlv_crtc_wm_is_valid(crtc_state, level))
+   if (!vlv_raw_crtc_wm_is_valid(crtc_state, level))
break;
 
for_each_plane_id_on_crtc(crtc, plane_id) {
-- 
2.10.2

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

[Intel-gfx] [PATCH 02/15] drm/i915: Drop the debug message from vlv_get_fifo_size()

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

Seeing the display FIFO sizes at driver load time doesn't really provide
anything useful for us, so let's just drop the debug message. One can
always use eg. intel_watermarks to dump out the hardware settings prior
to loading the driver.

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

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 23fff9167d77..2c63abe2039c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -454,13 +454,6 @@ static void vlv_get_fifo_size(struct intel_crtc_state 
*crtc_state)
fifo_state->plane[PLANE_SPRITE0] = sprite1_start - sprite0_start;
fifo_state->plane[PLANE_SPRITE1] = 511 - sprite1_start;
fifo_state->plane[PLANE_CURSOR] = 63;
-
-   DRM_DEBUG_KMS("Pipe %c FIFO size: %d/%d/%d/%d\n",
- pipe_name(pipe),
- fifo_state->plane[PLANE_PRIMARY],
- fifo_state->plane[PLANE_SPRITE0],
- fifo_state->plane[PLANE_SPRITE1],
- fifo_state->plane[PLANE_CURSOR]);
 }
 
 static int i9xx_get_fifo_size(struct drm_i915_private *dev_priv, int plane)
-- 
2.10.2

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

[Intel-gfx] [PATCH 00/15] drm/i915: Two stage watermarks for g4x

2017-04-21 Thread ville . syrjala 
From: Ville Syrjälä 

This makes g4x follow the two stage watermark programming approach as
well, and as a bonus exposes the video sprites on g4x.

There is one slight problem with merging the wms from multiple pipes;
If one pipe is currently enabled and we're about to enabled another one,
we should turn off CxSR before the second pipe gets enabled as the FIFO
will get repartitioned. This could also happen when there's a parallel
watermark update on the first pipe, so a dumb approach of just disabling
CxSR in the modeset path doesn't really work. I think the proper fix
will involve some shuffling of code in the modeset path because it's
currently a bit of a mess. So with the current code there could be an
occasional underrun reported when enabling the second pipe.

Entire series available here:
git://github.com/vsyrjala/linux.git g4x_atomic_wm_8

Ville Syrjälä (15):
  drm/i915: s/vlv_plane_wm_compute/vlv_raw_plane_wm_compute/ etc.
  drm/i915: Drop the debug message from vlv_get_fifo_size()
  drm/i915: s/vlv_num_wm_levels/intel_wm_num_levels/
  drm/i915: Rename bunch of vlv_ watermark structures to g4x_
  drm/i915: Make vlv/chv watermark debug print less cryptic
  drm/i915: Document CxSR
  drm/i915: Fix cursor 'cpp' in watermark calculatins for old platforms
  drm/i915: Fix the g4x watermark TLB miss workaround
  drm/i915: Refactor the g4x TLB miss w/a to a helper
  drm/i915: Refactor wm calculations
  drm/i915: Apply the g4x TLB miss w/a to SR watermarks as well
  drm/i915: Two stage watermarks for g4x
  drm/i915: Enable HPLL watermarks on g4x
  drm/i915: Add g4x watermark tracepoint
  drm/i915: Add support for sprites on g4x

 drivers/gpu/drm/i915/i915_debugfs.c  |   12 +-
 drivers/gpu/drm/i915/i915_drv.h  |   20 +-
 drivers/gpu/drm/i915/i915_trace.h|   49 ++
 drivers/gpu/drm/i915/intel_device_info.c |2 +-
 drivers/gpu/drm/i915/intel_display.c |   29 +-
 drivers/gpu/drm/i915/intel_drv.h |   34 +-
 drivers/gpu/drm/i915/intel_pm.c  | 1254 ++
 drivers/gpu/drm/i915/intel_sprite.c  |   18 +-
 8 files changed, 1081 insertions(+), 337 deletions(-)

-- 
2.10.2

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

Re: [Intel-gfx] [PULL] drm-misc-next-fixes

2017-04-21 Thread Sean Paul 
On Thu, Apr 20, 2017 at 11:50:02PM +0200, Daniel Vetter wrote:
> On Thu, Apr 20, 2017 at 10:11 PM, Sean Paul  wrote:
> > Hi Dave,
> > A few fixes for you to pick up. The driver changes are trivial, and the
> > maintainer change was necessitated by the sti fix. The headliner here is the
> > dma_buf_ops rename, since it touches so many drivers. Everything looks sane 
> > and
> > builds with that change, so it shouldn't cause problems.
> >
> >
> > drm-misc-next-fixes-2017-04-20:
> >
> > Core changes:
> > - Maintain sti via drm-misc (Vincent)
> > - Rename dma_buf_ops->kmap_* to avoid naming collision (Logan)
> 
> This one touches v4l and ion and is acked by the corresponding
> maintainers (but Sumit forgot to record that when applying the patch,
> and Sean didn't highlight it in the summary).
> 
> Sean, should we augment the template that cross-subsystem stuff
> (outside of what's on-topic for drm-misc) needs to be highlighted
> specifically in the pull summary?

Yeah, I think that's reasonable. I'd like to think that I would have done a
better job highlighting this had it been a standard header. Going forward, I'll
add UABI and cross-sub headers to force myself to be more careful :)

> 
> Off for vacation for real now.

Enjoy!

Sean

> 
> Cheers, Daniel
> 
> >
> > Driver changes:
> > - Fix UHD displays on stih407 (Vincent)
> > - Fix uninitialized var return in atmel-hlcdc (Dan)
> >
> > Take care, Sean
> >
> >
> > The following changes since commit 8cb68c83ab99a474ae847602f0c514d0fe17cc82:
> >
> >   drm: Fix get_property logic fumble (2017-04-12 18:11:32 +0200)
> >
> > are available in the git repository at:
> >
> >   git://anongit.freedesktop.org/git/drm-misc 
> > tags/drm-misc-next-fixes-2017-04-20
> >
> > for you to fetch changes up to f9b67f0014cba18f1aabb6fa9272335a043eb6fd:
> >
> >   dma-buf: Rename dma-ops to prevent conflict with kunmap_atomic macro 
> > (2017-04-20 13:47:46 +0530)
> >
> > 
> > drm-misc-next-fixes-2017-04-20
> >
> > Core changes:
> > - Maintain sti via drm-misc (Vincent)
> > - Rename dma_buf_ops->kmap_* to avoid naming collision (Logan)
> >
> > Driver changes:
> > - Fix UHD displays on stih407 (Vincent)
> > - Fix uninitialized var return in atmel-hlcdc (Dan)
> >
> > 
> > Dan Carpenter (1):
> >   drm: atmel-hlcdc: Uninitialized return in atmel_hlcdc_create_outputs()
> >
> > Logan Gunthorpe (1):
> >   dma-buf: Rename dma-ops to prevent conflict with kunmap_atomic macro
> >
> > Vincent Abriou (2):
> >   MAINTAINERS: add drm/sti driver into drm-misc
> >   drm/sti: fix GDP size to support up to UHD resolution
> >
> >  MAINTAINERS  |  2 +-
> >  drivers/dma-buf/dma-buf.c| 16 
> >  drivers/gpu/drm/armada/armada_gem.c  |  8 
> >  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c |  5 ++---
> >  drivers/gpu/drm/drm_prime.c  |  8 
> >  drivers/gpu/drm/i915/i915_gem_dmabuf.c   |  8 
> >  drivers/gpu/drm/i915/selftests/mock_dmabuf.c |  8 
> >  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c|  8 
> >  drivers/gpu/drm/sti/sti_gdp.c| 12 +++-
> >  drivers/gpu/drm/tegra/gem.c  |  8 
> >  drivers/gpu/drm/udl/udl_dmabuf.c |  8 
> >  drivers/gpu/drm/vmwgfx/vmwgfx_prime.c|  8 
> >  drivers/media/v4l2-core/videobuf2-dma-contig.c   |  4 ++--
> >  drivers/media/v4l2-core/videobuf2-dma-sg.c   |  4 ++--
> >  drivers/media/v4l2-core/videobuf2-vmalloc.c  |  4 ++--
> >  drivers/staging/android/ion/ion.c|  8 
> >  include/linux/dma-buf.h  | 22 
> > +++---
> >  17 files changed, 71 insertions(+), 70 deletions(-)
> >
> > --
> > Sean Paul, Software Engineer, Google / Chromium OS
> 
> 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] tests/pm_sseu: Re-enable the test

2017-04-21 Thread Michał Winiarski 
On Tue, Apr 18, 2017 at 04:45:29PM -0700, Oscar Mateo wrote:
> This test got inadvertently disabled by commit 83884e97 (Restore
> "lib: Open debugfs files for the given DRM device").
> 
> Cc: Jeff McGee 
> Cc: Chris Wilson 
> Signed-off-by: Oscar Mateo 

Reviewed-by: Michał Winiarski 

-Michał

> ---
>  tests/pm_sseu.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio (rev2)

2017-04-21 Thread Chris Wilson 
On Fri, Apr 21, 2017 at 02:26:14PM -, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio (rev2)
> URL   : https://patchwork.freedesktop.org/series/23340/
> State : success
> 
> == Summary ==
> 
> Series 23340v2 drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio
> https://patchwork.freedesktop.org/api/1.0/series/23340/revisions/2/mbox/
> 
> Test drv_module_reload:
> Subgroup basic-reload-inject:
> incomplete -> PASS   (fi-bdw-5557u) fdo#100750
> Test gem_exec_suspend:
> Subgroup basic-s4-devices:
> pass   -> DMESG-WARN (fi-kbl-7560u) fdo#100125
> Test kms_flip:
> Subgroup basic-flip-vs-wf_vblank:
> pass   -> FAIL   (fi-skl-6700hq) fdo#99739
> 
> fdo#100750 https://bugs.freedesktop.org/show_bug.cgi?id=100750
> fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
> fdo#99739 https://bugs.freedesktop.org/show_bug.cgi?id=99739

Applied, thanks for the review and testing. As it is in principle a
revert of the change to intel_wait_for_register() it seems safe enough
to apply for extended testing.
-Chris

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

Re: [Intel-gfx] [PULL] drm-misc-next-fixes

2017-04-21 Thread Sumit Semwal 
On 21 April 2017 at 03:20, Daniel Vetter  wrote:

>> - Rename dma_buf_ops->kmap_* to avoid naming collision (Logan)
>
> This one touches v4l and ion and is acked by the corresponding
> maintainers (but Sumit forgot to record that when applying the patch,
> and Sean didn't highlight it in the summary).
>
Hmm - I assumed that the mbox I downloaded (after all the acks got
registered in patchworks) would've had the Acks? Didn't cross-check,
my bad.
Will of course take care from next time on.

> Sean, should we augment the template that cross-subsystem stuff
> (outside of what's on-topic for drm-misc) needs to be highlighted
> specifically in the pull summary?
>
> Off for vacation for real now.
>
> Cheers, Daniel
>
Best,
Sumit.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t v5] benchmarks/gem_wsim: Command submission workload simulator

2017-04-21 Thread Tvrtko Ursulin 
From: Tvrtko Ursulin 

Tool which emits batch buffers to engines with configurable
sequences, durations, contexts, dependencies and userspace waits.

Unfinished but shows promise so sending out for early feedback.

v2:
 * Load workload descriptors from files. (also -w)
 * Help text.
 * Calibration control if needed. (-t)
 * NORELOC | LUT to eb flags.
 * Added sample workload to wsim/workload1.

v3:
 * Multiple parallel different workloads (-w -w ...).
 * Multi-context workloads.
 * Variable (random) batch length.
 * Load balancing (round robin and queue depth estimation).
 * Workloads delays and explicit sync steps.
 * Workload frequency (period) control.

v4:
 * Fixed queue-depth estimation by creating separate batches
   per engine when qd load balancing is on.
 * Dropped separate -s cmd line option. It can turn itself on
   automatically when needed.
 * Keep a single status page and lie about the write hazard
   as suggested by Chris.
 * Use batch_start_offset for controlling the batch duration.
   (Chris)
 * Set status page object cache level. (Chris)
 * Moved workload description to a README.
 * Tidied example workloads.
 * Some other cleanups and refactorings.

v5:
 * Master and background workloads (-W / -w).
 * Single batch per step is enough even when balancing. (Chris)
 * Use hars_petruska_f54_1_random IGT functions and see to zero
   at start. (Chris)
 * Use WC cache domain when WC mapping. (Chris)
 * Keep seqnos 64-bytes apart in the status page. (Chris)
 * Add workload throttling and queue-depth throttling commands.
   (Chris)

TODO list:

 * Fence support.
 * Better error handling.
 * Less 1980's workload parsing.
 * Proper workloads.
 * Threads?
 * ... ?

Signed-off-by: Tvrtko Ursulin 
Cc: Chris Wilson 
Cc: "Rogozhkin, Dmitry V" 
---
 benchmarks/Makefile.sources  |1 +
 benchmarks/gem_wsim.c| 1189 ++
 benchmarks/wsim/README   |   56 ++
 benchmarks/wsim/media_17i7.wsim  |7 +
 benchmarks/wsim/media_load_balance_17i7.wsim |7 +
 benchmarks/wsim/vcs1.wsim|   26 +
 benchmarks/wsim/vcs_balanced.wsim|   26 +
 lib/igt_core.c   |   26 +
 lib/igt_core.h   |1 +
 9 files changed, 1339 insertions(+)
 create mode 100644 benchmarks/gem_wsim.c
 create mode 100644 benchmarks/wsim/README
 create mode 100644 benchmarks/wsim/media_17i7.wsim
 create mode 100644 benchmarks/wsim/media_load_balance_17i7.wsim
 create mode 100644 benchmarks/wsim/vcs1.wsim
 create mode 100644 benchmarks/wsim/vcs_balanced.wsim

diff --git a/benchmarks/Makefile.sources b/benchmarks/Makefile.sources
index 3af54ebe36f2..3a941150abb3 100644
--- a/benchmarks/Makefile.sources
+++ b/benchmarks/Makefile.sources
@@ -14,6 +14,7 @@ benchmarks_prog_list =\
gem_prw \
gem_set_domain  \
gem_syslatency  \
+   gem_wsim\
kms_vblank  \
prime_lookup\
vgem_mmap   \
diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
new file mode 100644
index ..3d6670fdb815
--- /dev/null
+++ b/benchmarks/gem_wsim.c
@@ -0,0 +1,1189 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+#include "intel_chipset.h"
+#include "drm.h"
+#include "ioctl_wrappers.h"
+#include "drmtest.h"
+#include "intel_io.h"
+#include "igt_rand.h"
+
+enum

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio (rev2)

2017-04-21 Thread Patchwork 
== Series Details ==

Series: drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio (rev2)
URL   : https://patchwork.freedesktop.org/series/23340/
State : success

== Summary ==

Series 23340v2 drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio
https://patchwork.freedesktop.org/api/1.0/series/23340/revisions/2/mbox/

Test drv_module_reload:
Subgroup basic-reload-inject:
incomplete -> PASS   (fi-bdw-5557u) fdo#100750
Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass   -> DMESG-WARN (fi-kbl-7560u) fdo#100125
Test kms_flip:
Subgroup basic-flip-vs-wf_vblank:
pass   -> FAIL   (fi-skl-6700hq) fdo#99739

fdo#100750 https://bugs.freedesktop.org/show_bug.cgi?id=100750
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#99739 https://bugs.freedesktop.org/show_bug.cgi?id=99739

fi-bdw-5557u total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  
time:435s
fi-bdw-gvtdvmtotal:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  
time:430s
fi-bsw-n3050 total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  
time:573s
fi-bxt-j4205 total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  
time:515s
fi-bxt-t5700 total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  
time:566s
fi-byt-j1900 total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  
time:484s
fi-byt-n2820 total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time:481s
fi-hsw-4770  total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time:414s
fi-hsw-4770r total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time:405s
fi-ilk-650   total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  
time:427s
fi-ivb-3520m total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:488s
fi-ivb-3770  total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:466s
fi-kbl-7500u total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:452s
fi-kbl-7560u total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  
time:567s
fi-skl-6260u total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time:455s
fi-skl-6700hqtotal:278  pass:260  dwarn:0   dfail:0   fail:1   skip:17  
time:572s
fi-skl-6700k total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  
time:461s
fi-skl-6770hqtotal:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time:488s
fi-skl-gvtdvmtotal:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  
time:428s
fi-snb-2520m total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time:533s
fi-snb-2600  total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  
time:402s

0fb03930a694adb823f77a954d79905fdbe6d727 drm-tip: 2017y-04m-21d-08h-59m-17s UTC 
integration manifest
16d4e52 drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4530/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2] drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio

2017-04-21 Thread Chris Wilson 
The busy-spin, as the first stage of intel_wait_for_register(), is
currently under suspicion for causing:

[   62.034926] NMI watchdog: Watchdog detected hard LOCKUP on cpu 1
[   62.034928] Modules linked in: i2c_dev i915 intel_gtt drm_kms_helper 
prime_numbers
[   62.034932] CPU: 1 PID: 183 Comm: kworker/1:2 Not tainted 4.11.0-rc7+ #471
[   62.034933] Hardware name:  /, BIOS 
PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[   62.034934] Workqueue: pm pm_runtime_work
[   62.034936] task: 880275a04ec0 task.stack: c92d8000
[   62.034936] RIP: 0010:__intel_wait_for_register_fw+0x77/0x1a0 [i915]
[   62.034937] RSP: 0018:c92dbc38 EFLAGS: 0082
[   62.034939] RAX: c90003530094 RBX: 00130094 RCX: 0001
[   62.034940] RDX: 00a1 RSI: 88027fd15e58 RDI: 
[   62.034941] RBP: c92dbc78 R08: 0002 R09: 
[   62.034942] R10: c92dbc18 R11: 880276429dd0 R12: 8802707c
[   62.034943] R13: 00a0 R14:  R15: fffefc10
[   62.034945] FS:  () GS:88027fd0() 
knlGS:
[   62.034945] CS:  0010 DS:  ES:  CR0: 80050033
[   62.034947] CR2: 7ffd3cd98ff8 CR3: 000274c19000 CR4: 001006e0
[   62.034947] Call Trace:
[   62.034948]  intel_wait_for_register+0x77/0x140 [i915]
[   62.034949]  vlv_suspend_complete+0x23/0x5b0 [i915]
[   62.034950]  intel_runtime_suspend+0x16c/0x2a0 [i915]
[   62.034950]  pci_pm_runtime_suspend+0x50/0x180
[   62.034951]  ? pci_pm_runtime_resume+0xa0/0xa0
[   62.034952]  __rpm_callback+0xc5/0x210
[   62.034953]  rpm_callback+0x1f/0x80
[   62.034953]  ? pci_pm_runtime_resume+0xa0/0xa0
[   62.034954]  rpm_suspend+0x118/0x580
[   62.034955]  pm_runtime_work+0x64/0x90
[   62.034956]  process_one_work+0x1bb/0x3e0
[   62.034956]  worker_thread+0x46/0x4f0
[   62.034957]  ? __schedule+0x18b/0x610
[   62.034958]  kthread+0xff/0x140
[   62.034958]  ? process_one_work+0x3e0/0x3e0
[   62.034959]  ? kthread_create_on_node+

and related hard lockups in CI for byt and bsw.

Note this effectively reverts commits 41ce405e6894 and b27366958869
("drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()")

v2: Convert bool allow into a u32 mask for clarity and repeat the
comment on vlv rc6 timing to justify the 3ms timeout used for the wait (Ville)

Fixes: 41ce405e6894 ("drm/i915: Convert wait_for(I915_READ(reg)) to 
intel_wait_for_register()")
Fixes: b27366958869 ("drm/i915: Convert wait_for(I915_READ(reg)) to 
intel_wait_for_register()")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100718
Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: Ville Syrjälä 
Cc: Tomi Sarvela 
Reviewed-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.c | 46 +
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e4f902f61e57..dc48eae4072c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2179,6 +2179,20 @@ static void vlv_restore_gunit_s0ix_state(struct 
drm_i915_private *dev_priv)
I915_WRITE(VLV_GUNIT_CLOCK_GATE2,   s->clock_gate_dis2);
 }
 
+static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv,
+ u32 mask, u32 val)
+{
+   /* The HW does not like us polling for PW_STATUS frequently, so
+* use the sleeping loop rather than risk the busy spin within
+* intel_wait_for_register().
+*
+* Transitioning between RC6 states should be at most 2ms (see
+* valleyview_enable_rps) so use a 3ms timeout.
+*/
+   return wait_for((I915_READ_NOTRACE(VLV_GTLC_PW_STATUS) & mask) == val,
+   3);
+}
+
 int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
 {
u32 val;
@@ -2207,8 +2221,9 @@ int vlv_force_gfx_clock(struct drm_i915_private 
*dev_priv, bool force_on)
 
 static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow)
 {
+   u32 mask;
u32 val;
-   int err = 0;
+   int err;
 
val = I915_READ(VLV_GTLC_WAKE_CTRL);
val &= ~VLV_GTLC_ALLOWWAKEREQ;
@@ -2217,45 +2232,32 @@ static int vlv_allow_gt_wake(struct drm_i915_private 
*dev_priv, bool allow)
I915_WRITE(VLV_GTLC_WAKE_CTRL, val);
POSTING_READ(VLV_GTLC_WAKE_CTRL);
 
-   err = intel_wait_for_register(dev_priv,
- VLV_GTLC_PW_STATUS,
- VLV_GTLC_ALLOWWAKEACK,
- allow,
- 1);
+   mask = VLV_GTLC_ALLOWWAKEACK;
+   val = allow ? mask : 0;
+
+   err =

Re: [Intel-gfx] [PATCH] drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio

2017-04-21 Thread Ville Syrjälä 
On Fri, Apr 21, 2017 at 02:00:40PM +0100, Chris Wilson wrote:
> The busy-spin, as the first stage of intel_wait_for_register, is
> currently under suspicion for causing:
> 
> [   62.034926] NMI watchdog: Watchdog detected hard LOCKUP on cpu 1
> [   62.034928] Modules linked in: i2c_dev i915 intel_gtt drm_kms_helper 
> prime_numbers
> [   62.034932] CPU: 1 PID: 183 Comm: kworker/1:2 Not tainted 4.11.0-rc7+ #471
> [   62.034933] Hardware name:  /, BIOS 
> PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> [   62.034934] Workqueue: pm pm_runtime_work
> [   62.034936] task: 880275a04ec0 task.stack: c92d8000
> [   62.034936] RIP: 0010:__intel_wait_for_register_fw+0x77/0x1a0 [i915]
> [   62.034937] RSP: 0018:c92dbc38 EFLAGS: 0082
> [   62.034939] RAX: c90003530094 RBX: 00130094 RCX: 
> 0001
> [   62.034940] RDX: 00a1 RSI: 88027fd15e58 RDI: 
> 
> [   62.034941] RBP: c92dbc78 R08: 0002 R09: 
> 
> [   62.034942] R10: c92dbc18 R11: 880276429dd0 R12: 
> 8802707c
> [   62.034943] R13: 00a0 R14:  R15: 
> fffefc10
> [   62.034945] FS:  () GS:88027fd0() 
> knlGS:
> [   62.034945] CS:  0010 DS:  ES:  CR0: 80050033
> [   62.034947] CR2: 7ffd3cd98ff8 CR3: 000274c19000 CR4: 
> 001006e0
> [   62.034947] Call Trace:
> [   62.034948]  intel_wait_for_register+0x77/0x140 [i915]
> [   62.034949]  vlv_suspend_complete+0x23/0x5b0 [i915]
> [   62.034950]  intel_runtime_suspend+0x16c/0x2a0 [i915]
> [   62.034950]  pci_pm_runtime_suspend+0x50/0x180
> [   62.034951]  ? pci_pm_runtime_resume+0xa0/0xa0
> [   62.034952]  __rpm_callback+0xc5/0x210
> [   62.034953]  rpm_callback+0x1f/0x80
> [   62.034953]  ? pci_pm_runtime_resume+0xa0/0xa0
> [   62.034954]  rpm_suspend+0x118/0x580
> [   62.034955]  pm_runtime_work+0x64/0x90
> [   62.034956]  process_one_work+0x1bb/0x3e0
> [   62.034956]  worker_thread+0x46/0x4f0
> [   62.034957]  ? __schedule+0x18b/0x610
> [   62.034958]  kthread+0xff/0x140
> [   62.034958]  ? process_one_work+0x3e0/0x3e0
> [   62.034959]  ? kthread_create_on_node+
> 
> and related hard lockups in CI for byt and bsw.
> 
> Note this effectively reverts commits 41ce405e6894 and b27366958869
> ("drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()")
> 
> Fixes: 41ce405e6894 ("drm/i915: Convert wait_for(I915_READ(reg)) to 
> intel_wait_for_register()")
> Fixes: b27366958869 ("drm/i915: Convert wait_for(I915_READ(reg)) to 
> intel_wait_for_register()")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100718
> Signed-off-by: Chris Wilson 
> Cc: Tvrtko Ursulin 
> Cc: Ville Syrjälä 
> Cc: Tomi Sarvela 
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 39 +--
>  1 file changed, 17 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e4f902f61e57..89b517c478e8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2179,6 +2179,17 @@ static void vlv_restore_gunit_s0ix_state(struct 
> drm_i915_private *dev_priv)
>   I915_WRITE(VLV_GUNIT_CLOCK_GATE2,   s->clock_gate_dis2);
>  }
>  
> +static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv,
> +   u32 mask, u32 val)
> +{
> + /* The HW does not like us polling for PW_STATUS frequently, so
> +  * use the sleeping loop rather than risk the busy spin within
> +  * intel_wait_for_register().
> +  */
> + return wait_for((I915_READ_NOTRACE(VLV_GTLC_PW_STATUS) & mask) == val,
> + 3);
> +}
> +
>  int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
>  {
>   u32 val;
> @@ -2208,7 +2219,7 @@ int vlv_force_gfx_clock(struct drm_i915_private 
> *dev_priv, bool force_on)
>  static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow)
>  {
>   u32 val;
> - int err = 0;
> + int err;
>  
>   val = I915_READ(VLV_GTLC_WAKE_CTRL);
>   val &= ~VLV_GTLC_ALLOWWAKEREQ;
> @@ -2217,45 +2228,29 @@ static int vlv_allow_gt_wake(struct drm_i915_private 
> *dev_priv, bool allow)
>   I915_WRITE(VLV_GTLC_WAKE_CTRL, val);
>   POSTING_READ(VLV_GTLC_WAKE_CTRL);
>  
> - err = intel_wait_for_register(dev_priv,
> -   VLV_GTLC_PW_STATUS,
> -   VLV_GTLC_ALLOWWAKEACK,
> -   allow,
> -   1);
> + err = vlv_wait_for_pw_status(dev_priv, VLV_GTLC_ALLOWWAKEACK, allow);

Looks a bit funny to wait for a boolean. Maybe 'allow ? VLV_GTLC_ALLOWWAKEACK : 
0'
to make this a little less confusing? 

>   if (err)
>

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio

2017-04-21 Thread Patchwork 
== Series Details ==

Series: drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio
URL   : https://patchwork.freedesktop.org/series/23340/
State : failure

== Summary ==

Series 23340v1 drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio
https://patchwork.freedesktop.org/api/1.0/series/23340/revisions/1/mbox/

Test drv_module_reload:
Subgroup basic-reload-inject:
incomplete -> PASS   (fi-bdw-5557u) fdo#100750
Test gem_exec_suspend:
Subgroup basic-s4-devices:
pass   -> DMESG-WARN (fi-kbl-7560u) fdo#100125
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
pass   -> FAIL   (fi-ivb-3520m)
Subgroup suspend-read-crc-pipe-b:
pass   -> FAIL   (fi-ivb-3520m)
Subgroup suspend-read-crc-pipe-c:
pass   -> DMESG-WARN (fi-bsw-n3050) fdo#100651
pass   -> FAIL   (fi-ivb-3520m)
Test pm_rpm:
Subgroup basic-pci-d3-state:
pass   -> DMESG-WARN (fi-bsw-n3050) fdo#100718

fdo#100750 https://bugs.freedesktop.org/show_bug.cgi?id=100750
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#100651 https://bugs.freedesktop.org/show_bug.cgi?id=100651
fdo#100718 https://bugs.freedesktop.org/show_bug.cgi?id=100718

fi-bdw-5557u total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  
time:432s
fi-bdw-gvtdvmtotal:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  
time:430s
fi-bsw-n3050 total:278  pass:240  dwarn:2   dfail:0   fail:0   skip:36  
time:562s
fi-bxt-j4205 total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  
time:512s
fi-bxt-t5700 total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  
time:567s
fi-byt-j1900 total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  
time:487s
fi-byt-n2820 total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time:482s
fi-hsw-4770  total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time:407s
fi-hsw-4770r total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time:413s
fi-ilk-650   total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  
time:421s
fi-ivb-3520m total:278  pass:257  dwarn:0   dfail:0   fail:3   skip:18  
time:442s
fi-ivb-3770  total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:484s
fi-kbl-7500u total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:457s
fi-kbl-7560u total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  
time:573s
fi-skl-6260u total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time:450s
fi-skl-6700hqtotal:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  
time:574s
fi-skl-6700k total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  
time:462s
fi-skl-6770hqtotal:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time:490s
fi-skl-gvtdvmtotal:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  
time:428s
fi-snb-2520m total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time:534s
fi-snb-2600  total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  
time:410s

0fb03930a694adb823f77a954d79905fdbe6d727 drm-tip: 2017y-04m-21d-08h-59m-17s UTC 
integration manifest
2d36703 drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4529/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Avoid busy-spinning on VLV_GLTC_PW_STATUS mmio

2017-04-21 Thread Chris Wilson 
The busy-spin, as the first stage of intel_wait_for_register, is
currently under suspicion for causing:

[   62.034926] NMI watchdog: Watchdog detected hard LOCKUP on cpu 1
[   62.034928] Modules linked in: i2c_dev i915 intel_gtt drm_kms_helper 
prime_numbers
[   62.034932] CPU: 1 PID: 183 Comm: kworker/1:2 Not tainted 4.11.0-rc7+ #471
[   62.034933] Hardware name:  /, BIOS 
PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[   62.034934] Workqueue: pm pm_runtime_work
[   62.034936] task: 880275a04ec0 task.stack: c92d8000
[   62.034936] RIP: 0010:__intel_wait_for_register_fw+0x77/0x1a0 [i915]
[   62.034937] RSP: 0018:c92dbc38 EFLAGS: 0082
[   62.034939] RAX: c90003530094 RBX: 00130094 RCX: 0001
[   62.034940] RDX: 00a1 RSI: 88027fd15e58 RDI: 
[   62.034941] RBP: c92dbc78 R08: 0002 R09: 
[   62.034942] R10: c92dbc18 R11: 880276429dd0 R12: 8802707c
[   62.034943] R13: 00a0 R14:  R15: fffefc10
[   62.034945] FS:  () GS:88027fd0() 
knlGS:
[   62.034945] CS:  0010 DS:  ES:  CR0: 80050033
[   62.034947] CR2: 7ffd3cd98ff8 CR3: 000274c19000 CR4: 001006e0
[   62.034947] Call Trace:
[   62.034948]  intel_wait_for_register+0x77/0x140 [i915]
[   62.034949]  vlv_suspend_complete+0x23/0x5b0 [i915]
[   62.034950]  intel_runtime_suspend+0x16c/0x2a0 [i915]
[   62.034950]  pci_pm_runtime_suspend+0x50/0x180
[   62.034951]  ? pci_pm_runtime_resume+0xa0/0xa0
[   62.034952]  __rpm_callback+0xc5/0x210
[   62.034953]  rpm_callback+0x1f/0x80
[   62.034953]  ? pci_pm_runtime_resume+0xa0/0xa0
[   62.034954]  rpm_suspend+0x118/0x580
[   62.034955]  pm_runtime_work+0x64/0x90
[   62.034956]  process_one_work+0x1bb/0x3e0
[   62.034956]  worker_thread+0x46/0x4f0
[   62.034957]  ? __schedule+0x18b/0x610
[   62.034958]  kthread+0xff/0x140
[   62.034958]  ? process_one_work+0x3e0/0x3e0
[   62.034959]  ? kthread_create_on_node+

and related hard lockups in CI for byt and bsw.

Note this effectively reverts commits 41ce405e6894 and b27366958869
("drm/i915: Convert wait_for(I915_READ(reg)) to intel_wait_for_register()")

Fixes: 41ce405e6894 ("drm/i915: Convert wait_for(I915_READ(reg)) to 
intel_wait_for_register()")
Fixes: b27366958869 ("drm/i915: Convert wait_for(I915_READ(reg)) to 
intel_wait_for_register()")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100718
Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: Ville Syrjälä 
Cc: Tomi Sarvela 
---
 drivers/gpu/drm/i915/i915_drv.c | 39 +--
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e4f902f61e57..89b517c478e8 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2179,6 +2179,17 @@ static void vlv_restore_gunit_s0ix_state(struct 
drm_i915_private *dev_priv)
I915_WRITE(VLV_GUNIT_CLOCK_GATE2,   s->clock_gate_dis2);
 }
 
+static int vlv_wait_for_pw_status(struct drm_i915_private *dev_priv,
+ u32 mask, u32 val)
+{
+   /* The HW does not like us polling for PW_STATUS frequently, so
+* use the sleeping loop rather than risk the busy spin within
+* intel_wait_for_register().
+*/
+   return wait_for((I915_READ_NOTRACE(VLV_GTLC_PW_STATUS) & mask) == val,
+   3);
+}
+
 int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
 {
u32 val;
@@ -2208,7 +2219,7 @@ int vlv_force_gfx_clock(struct drm_i915_private 
*dev_priv, bool force_on)
 static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow)
 {
u32 val;
-   int err = 0;
+   int err;
 
val = I915_READ(VLV_GTLC_WAKE_CTRL);
val &= ~VLV_GTLC_ALLOWWAKEREQ;
@@ -2217,45 +2228,29 @@ static int vlv_allow_gt_wake(struct drm_i915_private 
*dev_priv, bool allow)
I915_WRITE(VLV_GTLC_WAKE_CTRL, val);
POSTING_READ(VLV_GTLC_WAKE_CTRL);
 
-   err = intel_wait_for_register(dev_priv,
- VLV_GTLC_PW_STATUS,
- VLV_GTLC_ALLOWWAKEACK,
- allow,
- 1);
+   err = vlv_wait_for_pw_status(dev_priv, VLV_GTLC_ALLOWWAKEACK, allow);
if (err)
DRM_ERROR("timeout disabling GT waking\n");
 
return err;
 }
 
-static int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv,
-bool wait_for_on)
+static void vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv,
+ bool wait_for_on)
 {
u32 mask;
u32 val;
-

Re: [Intel-gfx] [PATCH v8 1/2] ACPI / bus: Introduce a list of ids for "always present" devices

2017-04-21 Thread Hans de Goede 

Hi,

On 21-04-17 13:38, Andy Shevchenko wrote:

On Fri, 2017-04-21 at 12:47 +0200, Hans de Goede wrote:

Several Bay / Cherry Trail devices (all of which ship with Windows 10)
hide
the LPSS PWM controller in ACPI, typically the _STA method looks like
this:

 Method (_STA, 0, NotSerialized)  // _STA: Status
 {
 If (OSID == One)
 {
 Return (Zero)
 }

 Return (0x0F)
 }

Where OSID is some dark magic seen in all Cherry Trail ACPI tables
making
the machine behave differently depending on which OS it *thinks* it is
booting, this gets set in a number of ways which we cannot control, on
some newer machines it simple hardcoded to "One" aka win10.

This causes the PWM controller to get hidden, which means Linux cannot
control the backlight level on cht based tablets / laptops.

Since loading the driver for this does no harm (the only in kernel
user
of it is the i915 driver, which will only uses it when it needs it),
this
commit makes acpi_bus_get_status() always set status to
ACPI_STA_DEFAULT
for the LPSS PWM device, fixing the lack of backlight control.




  drivers/acpi/Makefile|  1 +
  drivers/acpi/bus.c   |  5 +++
  drivers/acpi/x86/x86_utils.c | 85



Perhaps .../x86/utils.c ?


I thought that utils.c would be too generic,
but that was mainly thinking about module kernel
cmdline options which do not apply here, still
having a somewhat unique basename seems useful.




  include/acpi/acpi_bus.h  |  9 +



diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 34fbe02..784bda6 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -114,6 +114,11 @@ int acpi_bus_get_status(struct acpi_device
*device)
acpi_status status;
unsigned long long sta;
  
+	if (acpi_device_always_present(device)) {

+   acpi_set_device_status(device, ACPI_STA_DEFAULT);
+   return 0;
+   }
+
status = acpi_bus_get_status_handle(device->handle, );
if (ACPI_FAILURE(status))
return -ENODEV;




+#define ICPU(model){ X86_VENDOR_INTEL, 6, model,
X86_FEATURE_ANY, }
+



+#define ENTRY(hid, uid, cpu_models) {  


cpu_models -> cpu_model ?

Or I missed that it's an array?


It may be an array, e.g. :

ENTRY("INT0002", "1", (ICPU(INTEL_FAM6_ATOM_SILVERMONT1), 
ICPU(INTEL_FAM6_ATOM_AIRMONT)) ),

Note this is a theoretical example (for now).

Regards,

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

Re: [Intel-gfx] [PATCH v8 1/2] ACPI / bus: Introduce a list of ids for "always present" devices

2017-04-21 Thread Andy Shevchenko 
On Fri, 2017-04-21 at 12:47 +0200, Hans de Goede wrote:
> Several Bay / Cherry Trail devices (all of which ship with Windows 10)
> hide
> the LPSS PWM controller in ACPI, typically the _STA method looks like
> this:
> 
> Method (_STA, 0, NotSerialized)  // _STA: Status
> {
> If (OSID == One)
> {
> Return (Zero)
> }
> 
> Return (0x0F)
> }
> 
> Where OSID is some dark magic seen in all Cherry Trail ACPI tables
> making
> the machine behave differently depending on which OS it *thinks* it is
> booting, this gets set in a number of ways which we cannot control, on
> some newer machines it simple hardcoded to "One" aka win10.
> 
> This causes the PWM controller to get hidden, which means Linux cannot
> control the backlight level on cht based tablets / laptops.
> 
> Since loading the driver for this does no harm (the only in kernel
> user
> of it is the i915 driver, which will only uses it when it needs it),
> this
> commit makes acpi_bus_get_status() always set status to
> ACPI_STA_DEFAULT
> for the LPSS PWM device, fixing the lack of backlight control.
> 

>  drivers/acpi/Makefile|  1 +
>  drivers/acpi/bus.c   |  5 +++
>  drivers/acpi/x86/x86_utils.c | 85
> 

Perhaps .../x86/utils.c ?

>  include/acpi/acpi_bus.h  |  9 +

> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 34fbe02..784bda6 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -114,6 +114,11 @@ int acpi_bus_get_status(struct acpi_device
> *device)
>   acpi_status status;
>   unsigned long long sta;
>  
> + if (acpi_device_always_present(device)) {
> + acpi_set_device_status(device, ACPI_STA_DEFAULT);
> + return 0;
> + }
> +
>   status = acpi_bus_get_status_handle(device->handle, );
>   if (ACPI_FAILURE(status))
>   return -ENODEV;
> 

> +#define ICPU(model)  { X86_VENDOR_INTEL, 6, model,
> X86_FEATURE_ANY, }
> +

> +#define ENTRY(hid, uid, cpu_models) {

cpu_models -> cpu_model ?

Or I missed that it's an array?

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

Re: [Intel-gfx] [PATCH v7 1/2] ACPI / bus: Introduce a list of ids for "always present" devices

2017-04-21 Thread Hans de Goede 

Hi,

On 21-04-17 12:40, Rafael J. Wysocki wrote:

On Friday, April 21, 2017 12:42:31 PM Hans de Goede wrote:

HI,



[cut]


And in that path, which I seem to have overlooked before, the
acpi_set_device_status() call is too early for invoking
acpi_device_always_present(adev), so the latter should be called
directly from acpi_add_single_object() like

   acpi_init_device_object(device, handle, type, sta);
   if (acpi_device_always_present(adev))
   acpi_set_device_status(adev, ACPI_STA_DEFAULT);


That is not necessary, the place(s) where we care about status being
fixed-up for always-present devices, so that they get scanned / their
platform device initiated, is in acpi_bus_attach() which
already calls acpi_bus_get_status() and thus gets the
acpi_device_always_present() check applied before checking.

For hotplugged devices there are acpi_scan_bus_check and
acpi_scan_device_check but those both also already call
acpi_bus_get_status() before checking adev->status.


OK

Which probably means that we don't need to initialize adev->status
in acpi_init_device_object() to anything meaningful, do we?


Right, I don't think that is necessary. But maybe it is necessary
for some special cases (e.g. power resources) ?


For power resources _STA is defined differently at all and we don't
even call acpi_add_single_object() for them. :-)

Are there any other special cases in which that may matter?


Not that I'm aware of, but I'm in no way an expert when it comes
to the ACPI subsystem.

Regards,

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

[Intel-gfx] [PATCH v8 1/2] ACPI / bus: Introduce a list of ids for "always present" devices

2017-04-21 Thread Hans de Goede 
Several Bay / Cherry Trail devices (all of which ship with Windows 10) hide
the LPSS PWM controller in ACPI, typically the _STA method looks like this:

Method (_STA, 0, NotSerialized)  // _STA: Status
{
If (OSID == One)
{
Return (Zero)
}

Return (0x0F)
}

Where OSID is some dark magic seen in all Cherry Trail ACPI tables making
the machine behave differently depending on which OS it *thinks* it is
booting, this gets set in a number of ways which we cannot control, on
some newer machines it simple hardcoded to "One" aka win10.

This causes the PWM controller to get hidden, which means Linux cannot
control the backlight level on cht based tablets / laptops.

Since loading the driver for this does no harm (the only in kernel user
of it is the i915 driver, which will only uses it when it needs it), this
commit makes acpi_bus_get_status() always set status to ACPI_STA_DEFAULT
for the LPSS PWM device, fixing the lack of backlight control.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-Use pr_debug instead of ACPI_DEBUG_PRINT
Changes in v3:
-Un-inline acpi_set_device_status and do the always_present_device_ids
 table check inside the un-inlined version of it
Changes in v4:
-Use dev_info instead of pr_debug
-Not only check for ACPI HID but also for CPU (SoC) model so as to not
 for devices present on other models then for which the quirk is intended and
 to avoid enabling unrelated ACPI devices which happen to use the same HID
Changes in v5:
-Only do the dev_info once per device (acpi_set_device_status gets called
 multiple times per device during boot)
Changes in v6:
-Allow specifying more then one CPU-model for a single HID
-Not only match the HID but also the UID, like on Cherry Trail, on some Bay
 Trail Windows 10 tablets we need to enable the PWM controller to get working
 backlight even though _STA returns 0. The Bay Trail SoC has 2 PWM controllers
 and we only need the first one. UID matching will allows adding an entry for
 Bay Trail which only enables the first PWM controller
Changes in v7:
-Put the actual x86 specific matching code and table with always present
 device HID + UID + CPU model entries in a new x86/x86_utils.c file which
 provides an acpi_device_always_present() helper function on x86, on
 non x86 a stub which always returns false is provided
-Squash in the addition of the Bay Trail PWM entry in the table as it
 is there for the same reasons as the Cherry Trail entry is there
Changes in v8:
-Move the acpi_device_always_present() check to acpi_bus_get_status(),
 leave acpi_set_device_status unchanged
---
 drivers/acpi/Makefile|  1 +
 drivers/acpi/bus.c   |  5 +++
 drivers/acpi/x86/x86_utils.c | 85 
 include/acpi/acpi_bus.h  |  9 +
 4 files changed, 100 insertions(+)
 create mode 100644 drivers/acpi/x86/x86_utils.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index d78065c..f3940ac 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -50,6 +50,7 @@ acpi-$(CONFIG_ACPI_REDUCED_HARDWARE_ONLY) += evged.o
 acpi-y += sysfs.o
 acpi-y += property.o
 acpi-$(CONFIG_X86) += acpi_cmos_rtc.o
+acpi-$(CONFIG_X86) += x86/x86_utils.o
 acpi-$(CONFIG_DEBUG_FS)+= debugfs.o
 acpi-$(CONFIG_ACPI_NUMA)   += numa.o
 acpi-$(CONFIG_ACPI_PROCFS_POWER) += cm_sbs.o
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 34fbe02..784bda6 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -114,6 +114,11 @@ int acpi_bus_get_status(struct acpi_device *device)
acpi_status status;
unsigned long long sta;
 
+   if (acpi_device_always_present(device)) {
+   acpi_set_device_status(device, ACPI_STA_DEFAULT);
+   return 0;
+   }
+
status = acpi_bus_get_status_handle(device->handle, );
if (ACPI_FAILURE(status))
return -ENODEV;
diff --git a/drivers/acpi/x86/x86_utils.c b/drivers/acpi/x86/x86_utils.c
new file mode 100644
index 000..74f1237
--- /dev/null
+++ b/drivers/acpi/x86/x86_utils.c
@@ -0,0 +1,85 @@
+/*
+ *  X86 ACPI Utility Functions
+ *
+ * Copyright (C) 2017 Hans de Goede 
+ *
+ * Based on various non upstream patches to support the CHT Whiskey Cove PMIC:
+ * Copyright (C) 2013-2015 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include "../internal.h"
+
+/*
+ * Some ACPI devices are hidden (status == 0x0) in recent BIOS-es because
+ * some recent Windows drivers bind to one device but poke at multiple
+ * devices at the same time, so the others get hidden.
+ * We work around this by always reporting ACPI_STA_DEFAULT for

[Intel-gfx] [PATCH v8 2/2] ACPI / bus: Add INT0002 to list of always-present devices

2017-04-21 Thread Hans de Goede 
The INT0002 device is necessary to clear wakeup interrupt sources
on Cherry Trail devices, without it we get nobody cared IRQ msgs
and some systems don't properly resume at all without it.

Signed-off-by: Hans de Goede 
---
Changes in v6:
-This is a new patch in v6 of this patch-set
Changes in v7:
-Adjust for the always present devices table being moved to
 drivers/acpi/x86/x86_utils.c
---
 drivers/acpi/x86/x86_utils.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/acpi/x86/x86_utils.c b/drivers/acpi/x86/x86_utils.c
index 74f1237..98d875a 100644
--- a/drivers/acpi/x86/x86_utils.c
+++ b/drivers/acpi/x86/x86_utils.c
@@ -49,6 +49,11 @@ static const struct always_present_id always_present_ids[] = 
{
 */
ENTRY("80860F09", "1", ICPU(INTEL_FAM6_ATOM_SILVERMONT1)),
ENTRY("80862288", "1", ICPU(INTEL_FAM6_ATOM_AIRMONT)),
+   /*
+* The INT0002 device is necessary to clear wakeup interrupt sources
+* on Cherry Trail devices, without it we get nobody cared IRQ msgs.
+*/
+   ENTRY("INT0002", "1", ICPU(INTEL_FAM6_ATOM_AIRMONT)),
 };
 
 bool acpi_device_always_present(struct acpi_device *adev)
-- 
2.9.3

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

Re: [Intel-gfx] [PATCH v7 1/2] ACPI / bus: Introduce a list of ids for "always present" devices

2017-04-21 Thread Rafael J. Wysocki 
On Friday, April 21, 2017 12:42:31 PM Hans de Goede wrote:
> HI,
> 

[cut]

> >>> And in that path, which I seem to have overlooked before, the
> >>> acpi_set_device_status() call is too early for invoking
> >>> acpi_device_always_present(adev), so the latter should be called
> >>> directly from acpi_add_single_object() like
> >>>
> >>>   acpi_init_device_object(device, handle, type, sta);
> >>>   if (acpi_device_always_present(adev))
> >>>   acpi_set_device_status(adev, ACPI_STA_DEFAULT);
> >>
> >> That is not necessary, the place(s) where we care about status being
> >> fixed-up for always-present devices, so that they get scanned / their
> >> platform device initiated, is in acpi_bus_attach() which
> >> already calls acpi_bus_get_status() and thus gets the
> >> acpi_device_always_present() check applied before checking.
> >>
> >> For hotplugged devices there are acpi_scan_bus_check and
> >> acpi_scan_device_check but those both also already call
> >> acpi_bus_get_status() before checking adev->status.
> > 
> > OK
> > 
> > Which probably means that we don't need to initialize adev->status
> > in acpi_init_device_object() to anything meaningful, do we?
> 
> Right, I don't think that is necessary. But maybe it is necessary
> for some special cases (e.g. power resources) ?

For power resources _STA is defined differently at all and we don't
even call acpi_add_single_object() for them. :-)

Are there any other special cases in which that may matter?

Thanks,
Rafael

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

Re: [Intel-gfx] [PATCH v7 1/2] ACPI / bus: Introduce a list of ids for "always present" devices

2017-04-21 Thread Rafael J. Wysocki 
On Friday, April 21, 2017 11:59:34 AM Hans de Goede wrote:
> Hi,
> 
> On 19-04-17 22:14, Rafael J. Wysocki wrote:
> > On Wed, Apr 19, 2017 at 2:04 PM, Hans de Goede  wrote:
> >> Several Bay / Cherry Trail devices (all of which ship with Windows 10) hide
> >> the LPSS PWM controller in ACPI, typically the _STA method looks like this:
> >>
> >>  Method (_STA, 0, NotSerialized)  // _STA: Status
> >>  {
> >>  If (OSID == One)
> >>  {
> >>  Return (Zero)
> >>  }
> >>
> >>  Return (0x0F)
> >>  }
> >>
> >> Where OSID is some dark magic seen in all Cherry Trail ACPI tables making
> >> the machine behave differently depending on which OS it *thinks* it is
> >> booting, this gets set in a number of ways which we cannot control, on
> >> some newer machines it simple hardcoded to "One" aka win10.
> >>
> >> This causes the PWM controller to get hidden, which means Linux cannot
> >> control the backlight level on cht based tablets / laptops.
> >>
> >> Since loading the driver for this does no harm (the only in kernel user
> >> of it is the i915 driver, which will only uses it when it needs it), this
> >> commit makes acpi_bus_get_status() always set status to ACPI_STA_DEFAULT
> >> for the LPSS PWM device, fixing the lack of backlight control.
> >>
> >> Signed-off-by: Hans de Goede 
> >> ---
> >> Changes in v2:
> >> -Use pr_debug instead of ACPI_DEBUG_PRINT
> >> Changes in v3:
> >> -Un-inline acpi_set_device_status and do the always_present_device_ids
> >>   table check inside the un-inlined version of it
> >> Changes in v4:
> >> -Use dev_info instead of pr_debug
> >> -Not only check for ACPI HID but also for CPU (SoC) model so as to not
> >>   for devices present on other models then for which the quirk is intended 
> >> and
> >>   to avoid enabling unrelated ACPI devices which happen to use the same HID
> >> Changes in v5:
> >> -Only do the dev_info once per device (acpi_set_device_status gets called
> >>   multiple times per device during boot)
> >> Changes in v6:
> >> -Allow specifying more then one CPU-model for a single HID
> >> -Not only match the HID but also the UID, like on Cherry Trail, on some Bay
> >>   Trail Windows 10 tablets we need to enable the PWM controller to get 
> >> working
> >>   backlight even though _STA returns 0. The Bay Trail SoC has 2 PWM 
> >> controllers
> >>   and we only need the first one. UID matching will allows adding an entry 
> >> for
> >>   Bay Trail which only enables the first PWM controller
> >> Changes in v7:
> >> -Put the actual x86 specific matching code and table with always present
> >>   device HID + UID + CPU model entries in a new x86/x86_utils.c file which
> >>   provides an acpi_device_always_present() helper function on x86, on
> >>   non x86 a stub which always returns false is provided
> >> -Squash in the addition of the Bay Trail PWM entry in the table as it
> >>   is there for the same reasons as the Cherry Trail entry is there
> 
> 
> 
> >> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> >> index 34fbe02..4d952cc 100644
> >> --- a/drivers/acpi/bus.c
> >> +++ b/drivers/acpi/bus.c
> >> @@ -132,6 +132,16 @@ int acpi_bus_get_status(struct acpi_device *device)
> >>   }
> >>   EXPORT_SYMBOL(acpi_bus_get_status);
> >>
> >> +void acpi_set_device_status(struct acpi_device *adev, u32 sta)
> >> +{
> >> +   u32 *status = (u32 *)>status;
> >> +
> >> +   if (acpi_device_always_present(adev))
> >> +   *status = ACPI_STA_DEFAULT;
> >> +   else
> >> +   *status = sta;
> > 
> > *((u32 *)>status) = acpi_device_always_present(adev) ?
> > ACPI_STA_DEFAULT : sta;
> > 
> > and I guess it could still be static inline?
> > 
> > But that said, evaluating _STA for the always present devices is
> > pointless (as Lukas noticed), so why not to modify
> > acpi_bus_get_status() to do something like:
> > 
> >  if (acpi_device_always_present(adev)) {
> >  acpi_set_device_status(adev, ACPI_STA_DEFAULT);
> >  return 0;
> >  }
> > 
> > upfront
> 
> Hehe, my v1 of this patch actually did the check in acpi_bus_get_status()
> I moved it to acpi_set_device_status() on your request. Moving it back
> is fine with me and indeed seems cleaner.
> 
> > and modify the other path invoking acpi_set_device_status() accordingly.
> > 
> > And in that path, which I seem to have overlooked before, the
> > acpi_set_device_status() call is too early for invoking
> > acpi_device_always_present(adev), so the latter should be called
> > directly from acpi_add_single_object() like
> > 
> >  acpi_init_device_object(device, handle, type, sta);
> >  if (acpi_device_always_present(adev))
> >  acpi_set_device_status(adev, ACPI_STA_DEFAULT);
> 
> That is not necessary, the place(s) where we care about status being
> fixed-up for always-present devices, so that they get scanned / their
> platform device initiated, is in acpi_bus_attach() which
>

Re: [Intel-gfx] [PATCH v7 1/2] ACPI / bus: Introduce a list of ids for "always present" devices

2017-04-21 Thread Hans de Goede 

HI,

On 21-04-17 12:33, Rafael J. Wysocki wrote:

On Friday, April 21, 2017 11:59:34 AM Hans de Goede wrote:

Hi,

On 19-04-17 22:14, Rafael J. Wysocki wrote:

On Wed, Apr 19, 2017 at 2:04 PM, Hans de Goede  wrote:

Several Bay / Cherry Trail devices (all of which ship with Windows 10) hide
the LPSS PWM controller in ACPI, typically the _STA method looks like this:

  Method (_STA, 0, NotSerialized)  // _STA: Status
  {
  If (OSID == One)
  {
  Return (Zero)
  }

  Return (0x0F)
  }

Where OSID is some dark magic seen in all Cherry Trail ACPI tables making
the machine behave differently depending on which OS it *thinks* it is
booting, this gets set in a number of ways which we cannot control, on
some newer machines it simple hardcoded to "One" aka win10.

This causes the PWM controller to get hidden, which means Linux cannot
control the backlight level on cht based tablets / laptops.

Since loading the driver for this does no harm (the only in kernel user
of it is the i915 driver, which will only uses it when it needs it), this
commit makes acpi_bus_get_status() always set status to ACPI_STA_DEFAULT
for the LPSS PWM device, fixing the lack of backlight control.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-Use pr_debug instead of ACPI_DEBUG_PRINT
Changes in v3:
-Un-inline acpi_set_device_status and do the always_present_device_ids
   table check inside the un-inlined version of it
Changes in v4:
-Use dev_info instead of pr_debug
-Not only check for ACPI HID but also for CPU (SoC) model so as to not
   for devices present on other models then for which the quirk is intended and
   to avoid enabling unrelated ACPI devices which happen to use the same HID
Changes in v5:
-Only do the dev_info once per device (acpi_set_device_status gets called
   multiple times per device during boot)
Changes in v6:
-Allow specifying more then one CPU-model for a single HID
-Not only match the HID but also the UID, like on Cherry Trail, on some Bay
   Trail Windows 10 tablets we need to enable the PWM controller to get working
   backlight even though _STA returns 0. The Bay Trail SoC has 2 PWM controllers
   and we only need the first one. UID matching will allows adding an entry for
   Bay Trail which only enables the first PWM controller
Changes in v7:
-Put the actual x86 specific matching code and table with always present
   device HID + UID + CPU model entries in a new x86/x86_utils.c file which
   provides an acpi_device_always_present() helper function on x86, on
   non x86 a stub which always returns false is provided
-Squash in the addition of the Bay Trail PWM entry in the table as it
   is there for the same reasons as the Cherry Trail entry is there





diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 34fbe02..4d952cc 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -132,6 +132,16 @@ int acpi_bus_get_status(struct acpi_device *device)
   }
   EXPORT_SYMBOL(acpi_bus_get_status);

+void acpi_set_device_status(struct acpi_device *adev, u32 sta)
+{
+   u32 *status = (u32 *)>status;
+
+   if (acpi_device_always_present(adev))
+   *status = ACPI_STA_DEFAULT;
+   else
+   *status = sta;


*((u32 *)>status) = acpi_device_always_present(adev) ?
ACPI_STA_DEFAULT : sta;

and I guess it could still be static inline?

But that said, evaluating _STA for the always present devices is
pointless (as Lukas noticed), so why not to modify
acpi_bus_get_status() to do something like:

  if (acpi_device_always_present(adev)) {
  acpi_set_device_status(adev, ACPI_STA_DEFAULT);
  return 0;
  }

upfront


Hehe, my v1 of this patch actually did the check in acpi_bus_get_status()
I moved it to acpi_set_device_status() on your request. Moving it back
is fine with me and indeed seems cleaner.


and modify the other path invoking acpi_set_device_status() accordingly.

And in that path, which I seem to have overlooked before, the
acpi_set_device_status() call is too early for invoking
acpi_device_always_present(adev), so the latter should be called
directly from acpi_add_single_object() like

  acpi_init_device_object(device, handle, type, sta);
  if (acpi_device_always_present(adev))
  acpi_set_device_status(adev, ACPI_STA_DEFAULT);


That is not necessary, the place(s) where we care about status being
fixed-up for always-present devices, so that they get scanned / their
platform device initiated, is in acpi_bus_attach() which
already calls acpi_bus_get_status() and thus gets the
acpi_device_always_present() check applied before checking.

For hotplugged devices there are acpi_scan_bus_check and
acpi_scan_device_check but those both also already call
acpi_bus_get_status() before checking adev->status.


OK

Which probably means that we don't need to initialize adev->status
in acpi_init_device_object() to anything meaningful, do we?


Right, I

Re: [Intel-gfx] [PATCH v7 1/2] ACPI / bus: Introduce a list of ids for "always present" devices

2017-04-21 Thread Hans de Goede 

Hi,

On 19-04-17 22:14, Rafael J. Wysocki wrote:

On Wed, Apr 19, 2017 at 2:04 PM, Hans de Goede  wrote:

Several Bay / Cherry Trail devices (all of which ship with Windows 10) hide
the LPSS PWM controller in ACPI, typically the _STA method looks like this:

 Method (_STA, 0, NotSerialized)  // _STA: Status
 {
 If (OSID == One)
 {
 Return (Zero)
 }

 Return (0x0F)
 }

Where OSID is some dark magic seen in all Cherry Trail ACPI tables making
the machine behave differently depending on which OS it *thinks* it is
booting, this gets set in a number of ways which we cannot control, on
some newer machines it simple hardcoded to "One" aka win10.

This causes the PWM controller to get hidden, which means Linux cannot
control the backlight level on cht based tablets / laptops.

Since loading the driver for this does no harm (the only in kernel user
of it is the i915 driver, which will only uses it when it needs it), this
commit makes acpi_bus_get_status() always set status to ACPI_STA_DEFAULT
for the LPSS PWM device, fixing the lack of backlight control.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-Use pr_debug instead of ACPI_DEBUG_PRINT
Changes in v3:
-Un-inline acpi_set_device_status and do the always_present_device_ids
  table check inside the un-inlined version of it
Changes in v4:
-Use dev_info instead of pr_debug
-Not only check for ACPI HID but also for CPU (SoC) model so as to not
  for devices present on other models then for which the quirk is intended and
  to avoid enabling unrelated ACPI devices which happen to use the same HID
Changes in v5:
-Only do the dev_info once per device (acpi_set_device_status gets called
  multiple times per device during boot)
Changes in v6:
-Allow specifying more then one CPU-model for a single HID
-Not only match the HID but also the UID, like on Cherry Trail, on some Bay
  Trail Windows 10 tablets we need to enable the PWM controller to get working
  backlight even though _STA returns 0. The Bay Trail SoC has 2 PWM controllers
  and we only need the first one. UID matching will allows adding an entry for
  Bay Trail which only enables the first PWM controller
Changes in v7:
-Put the actual x86 specific matching code and table with always present
  device HID + UID + CPU model entries in a new x86/x86_utils.c file which
  provides an acpi_device_always_present() helper function on x86, on
  non x86 a stub which always returns false is provided
-Squash in the addition of the Bay Trail PWM entry in the table as it
  is there for the same reasons as the Cherry Trail entry is there





diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 34fbe02..4d952cc 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -132,6 +132,16 @@ int acpi_bus_get_status(struct acpi_device *device)
  }
  EXPORT_SYMBOL(acpi_bus_get_status);

+void acpi_set_device_status(struct acpi_device *adev, u32 sta)
+{
+   u32 *status = (u32 *)>status;
+
+   if (acpi_device_always_present(adev))
+   *status = ACPI_STA_DEFAULT;
+   else
+   *status = sta;


*((u32 *)>status) = acpi_device_always_present(adev) ?
ACPI_STA_DEFAULT : sta;

and I guess it could still be static inline?

But that said, evaluating _STA for the always present devices is
pointless (as Lukas noticed), so why not to modify
acpi_bus_get_status() to do something like:

 if (acpi_device_always_present(adev)) {
 acpi_set_device_status(adev, ACPI_STA_DEFAULT);
 return 0;
 }

upfront


Hehe, my v1 of this patch actually did the check in acpi_bus_get_status()
I moved it to acpi_set_device_status() on your request. Moving it back
is fine with me and indeed seems cleaner.


and modify the other path invoking acpi_set_device_status() accordingly.

And in that path, which I seem to have overlooked before, the
acpi_set_device_status() call is too early for invoking
acpi_device_always_present(adev), so the latter should be called
directly from acpi_add_single_object() like

 acpi_init_device_object(device, handle, type, sta);
 if (acpi_device_always_present(adev))
 acpi_set_device_status(adev, ACPI_STA_DEFAULT);


That is not necessary, the place(s) where we care about status being
fixed-up for always-present devices, so that they get scanned / their
platform device initiated, is in acpi_bus_attach() which
already calls acpi_bus_get_status() and thus gets the
acpi_device_always_present() check applied before checking.

For hotplugged devices there are acpi_scan_bus_check and
acpi_scan_device_check but those both also already call
acpi_bus_get_status() before checking adev->status.

So just adding the check to acpi_bus_get_status(), as I did in
v1 is sufficient.

Note that the end result is still significantly cleaner then v1
as we're now using the acpi_device_always_present() which makes
the drivers/acpi/bus.c changes a lot cleaner.

I'll go and test this and

Re: [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Stop touching hangcheck.seqno from intel_engine_init_global_seqno()

2017-04-21 Thread Chris Wilson 
On Fri, Apr 21, 2017 at 08:50:26AM -, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Stop touching hangcheck.seqno from 
> intel_engine_init_global_seqno()
> URL   : https://patchwork.freedesktop.org/series/23320/
> State : success
> 
> == Summary ==
> 
> Series 23320v1 drm/i915: Stop touching hangcheck.seqno from 
> intel_engine_init_global_seqno()
> https://patchwork.freedesktop.org/api/1.0/series/23320/revisions/1/mbox/
> 
> Test gem_exec_flush:
> Subgroup basic-batch-kernel-default-uc:
> pass   -> FAIL   (fi-snb-2600) fdo#17
> Test kms_pipe_crc_basic:
> Subgroup suspend-read-crc-pipe-a:
> fail   -> PASS   (fi-skl-6700k) fdo#100367
> Test pm_rpm:
> Subgroup basic-rte:
> incomplete -> PASS   (fi-bsw-n3050) fdo#100718
> 
> fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
> fdo#100367 https://bugs.freedesktop.org/show_bug.cgi?id=100367
> fdo#100718 https://bugs.freedesktop.org/show_bug.cgi?id=100718

Pushed, thanks for the review.
-Chris

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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Stop touching hangcheck.seqno from intel_engine_init_global_seqno()

2017-04-21 Thread Patchwork 
== Series Details ==

Series: drm/i915: Stop touching hangcheck.seqno from 
intel_engine_init_global_seqno()
URL   : https://patchwork.freedesktop.org/series/23320/
State : success

== Summary ==

Series 23320v1 drm/i915: Stop touching hangcheck.seqno from 
intel_engine_init_global_seqno()
https://patchwork.freedesktop.org/api/1.0/series/23320/revisions/1/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass   -> FAIL   (fi-snb-2600) fdo#17
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
fail   -> PASS   (fi-skl-6700k) fdo#100367
Test pm_rpm:
Subgroup basic-rte:
incomplete -> PASS   (fi-bsw-n3050) fdo#100718

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
fdo#100367 https://bugs.freedesktop.org/show_bug.cgi?id=100367
fdo#100718 https://bugs.freedesktop.org/show_bug.cgi?id=100718

fi-bdw-5557u total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  
time:431s
fi-bdw-gvtdvmtotal:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  
time:430s
fi-bsw-n3050 total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  
time:576s
fi-bxt-j4205 total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  
time:508s
fi-bxt-t5700 total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  
time:561s
fi-byt-j1900 total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  
time:492s
fi-byt-n2820 total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time:479s
fi-hsw-4770  total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time:406s
fi-hsw-4770r total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time:409s
fi-ilk-650   total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  
time:423s
fi-ivb-3520m total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:483s
fi-ivb-3770  total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:461s
fi-kbl-7500u total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:459s
fi-kbl-7560u total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  
time:571s
fi-skl-6260u total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time:461s
fi-skl-6700hqtotal:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  
time:569s
fi-skl-6700k total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  
time:467s
fi-skl-6770hqtotal:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time:491s
fi-skl-gvtdvmtotal:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  
time:430s
fi-snb-2520m total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time:529s
fi-snb-2600  total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  
time:402s

9bb844000d9412c2758b861ce7aee1e9546fccb9 drm-tip: 2017y-04m-20d-14h-46m-59s UTC 
integration manifest
a7239e1 drm/i915: Stop touching hangcheck.seqno from 
intel_engine_init_global_seqno()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4528/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Stop touching hangcheck.seqno from intel_engine_init_global_seqno()

2017-04-21 Thread Chris Wilson 
On Fri, Apr 21, 2017 at 11:12:21AM +0300, Mika Kuoppala wrote:
> Chris Wilson  writes:
> 
> > The hangcheck runs independently to the main flow of seqno through the
> > driver. However, we have an odd coupling of the seqno reset that is
> > unwelcome, and if poked at just the right rate can cause spurious hangs
> > (e.g. gem_exec_whisper) on an apparently idle engine.
> >
> > Signed-off-by: Chris Wilson 
> > Cc: Mika Kuoppala 
> 
> Reviewed-by: Mika Kuoppala 

Ta, I'll send this by itself to CI to confirm that it is safe :)
-Chris

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

[Intel-gfx] [CI] drm/i915: Stop touching hangcheck.seqno from intel_engine_init_global_seqno()

2017-04-21 Thread Chris Wilson 
The hangcheck runs independently to the main flow of seqno through the
driver. However, we have an odd coupling of the seqno reset that is
unwelcome, and if poked at just the right rate can cause spurious hangs
(e.g. gem_exec_whisper) on an apparently idle engine.

Signed-off-by: Chris Wilson 
Cc: Mika Kuoppala 
Reviewed-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/intel_engine_cs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
b/drivers/gpu/drm/i915/intel_engine_cs.c
index 402769d9d840..82a274b336c5 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -265,6 +265,7 @@ void intel_engine_init_global_seqno(struct intel_engine_cs 
*engine, u32 seqno)
struct drm_i915_private *dev_priv = engine->i915;
 
GEM_BUG_ON(!intel_engine_is_idle(engine));
+   GEM_BUG_ON(i915_gem_active_isset(>timeline->last_request));
 
/* Our semaphore implementation is strictly monotonic (i.e. we proceed
 * so long as the semaphore value in the register/page is greater
@@ -296,9 +297,6 @@ void intel_engine_init_global_seqno(struct intel_engine_cs 
*engine, u32 seqno)
intel_write_status_page(engine, I915_GEM_HWS_INDEX, seqno);
clear_bit(ENGINE_IRQ_BREADCRUMB, >irq_posted);
 
-   GEM_BUG_ON(i915_gem_active_isset(>timeline->last_request));
-   engine->hangcheck.seqno = seqno;
-
/* After manually advancing the seqno, fake the interrupt in case
 * there are any waiters for that seqno.
 */
-- 
2.11.0

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

Re: [Intel-gfx] [PATCH 1/2] drm/i915: Stop touching hangcheck.seqno from intel_engine_init_global_seqno()

2017-04-21 Thread Mika Kuoppala 
Chris Wilson  writes:

> The hangcheck runs independently to the main flow of seqno through the
> driver. However, we have an odd coupling of the seqno reset that is
> unwelcome, and if poked at just the right rate can cause spurious hangs
> (e.g. gem_exec_whisper) on an apparently idle engine.
>
> Signed-off-by: Chris Wilson 
> Cc: Mika Kuoppala 

Reviewed-by: Mika Kuoppala 

> ---
>  drivers/gpu/drm/i915/intel_engine_cs.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c 
> b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 7681d17ce454..f3cb7e931317 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -265,6 +265,7 @@ void intel_engine_init_global_seqno(struct 
> intel_engine_cs *engine, u32 seqno)
>   struct drm_i915_private *dev_priv = engine->i915;
>  
>   GEM_BUG_ON(!intel_engine_is_idle(engine));
> + GEM_BUG_ON(i915_gem_active_isset(>timeline->last_request));
>  
>   /* Our semaphore implementation is strictly monotonic (i.e. we proceed
>* so long as the semaphore value in the register/page is greater
> @@ -284,9 +285,6 @@ void intel_engine_init_global_seqno(struct 
> intel_engine_cs *engine, u32 seqno)
>   intel_write_status_page(engine, I915_GEM_HWS_INDEX, seqno);
>   clear_bit(ENGINE_IRQ_BREADCRUMB, >irq_posted);
>  
> - GEM_BUG_ON(i915_gem_active_isset(>timeline->last_request));
> - engine->hangcheck.seqno = seqno;
> -
>   /* After manually advancing the seqno, fake the interrupt in case
>* there are any waiters for that seqno.
>*/
> -- 
> 2.11.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for Adding driver-private objects to atomic state

2017-04-21 Thread Patchwork 
== Series Details ==

Series: Adding driver-private objects to atomic state
URL   : https://patchwork.freedesktop.org/series/23308/
State : success

== Summary ==

Series 23308v1 Adding driver-private objects to atomic state
https://patchwork.freedesktop.org/api/1.0/series/23308/revisions/1/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass   -> FAIL   (fi-snb-2600) fdo#17
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
fail   -> PASS   (fi-skl-6700k) fdo#100367
Test pm_rpm:
Subgroup basic-rte:
incomplete -> PASS   (fi-bsw-n3050) fdo#100718

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17
fdo#100367 https://bugs.freedesktop.org/show_bug.cgi?id=100367
fdo#100718 https://bugs.freedesktop.org/show_bug.cgi?id=100718

fi-bdw-5557u total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  
time:437s
fi-bdw-gvtdvmtotal:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  
time:427s
fi-bsw-n3050 total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  
time:588s
fi-bxt-j4205 total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  
time:511s
fi-bxt-t5700 total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  
time:542s
fi-byt-j1900 total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  
time:481s
fi-byt-n2820 total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time:487s
fi-hsw-4770  total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time:414s
fi-hsw-4770r total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time:406s
fi-ilk-650   total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  
time:423s
fi-ivb-3520m total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:492s
fi-ivb-3770  total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:486s
fi-kbl-7500u total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time:454s
fi-kbl-7560u total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  
time:568s
fi-skl-6260u total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time:448s
fi-skl-6700hqtotal:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  
time:568s
fi-skl-6700k total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  
time:456s
fi-skl-6770hqtotal:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time:499s
fi-skl-gvtdvmtotal:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  
time:428s
fi-snb-2520m total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time:541s
fi-snb-2600  total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  
time:405s

9bb844000d9412c2758b861ce7aee1e9546fccb9 drm-tip: 2017y-04m-20d-14h-46m-59s UTC 
integration manifest
3347ee1 drm/dp: Track MST link bandwidth
8fef895b4 drm/dp: Add DP MST helpers to atomically find and release vcpi slots
403e426 drm/dp: Introduce MST topology state to track available link bandwidth
4eb3832 drm: Add driver-private objects to atomic state

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4527/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx