[Intel-gfx] [PATCH 0/3] drm/i915: vlv pre_enable/enable callback refactoring

2013-07-30 Thread Jani Nikula
Address Daniel's old complaints about -pre_enable and -enable called
in the same spot in valleyview_crtc_enable [1].

Rebased on top of Chris' yet-to-be-merged locking patch [2].

Cheers,
Jani.


[1] 
http://mid.gmane.org/cakmk7ufs9emvmw8bns24e5unm1d7jrfvg3sd5sdftveamgf...@mail.gmail.com

[2] 
http://mid.gmane.org/1374865055-17919-1-git-send-email-ch...@chris-wilson.co.uk


Jani Nikula (3):
  drm/i915: rearrange vlv dp enable and pre_enable callbacks
  drm/i915: rearrange vlv hdmi enable and pre_enable callbacks
  drm/i915: move encoder-enable callback later in VLV crtc enable

 drivers/gpu/drm/i915/intel_display.c |7 ++--
 drivers/gpu/drm/i915/intel_dp.c  |   73 ++
 drivers/gpu/drm/i915/intel_hdmi.c|   20 +-
 3 files changed, 53 insertions(+), 47 deletions(-)

-- 
1.7.10.4

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


[Intel-gfx] [PATCH 1/3] drm/i915: rearrange vlv dp enable and pre_enable callbacks

2013-07-30 Thread Jani Nikula
Currently -pre_enable and -enable are called back to back. Rearrange
the DP callbacks to make it possible to move -enable call later.

Basically do everything in -pre_enable on VLV, and make -enable a NOP.

There should be no functional changes.

Signed-off-by: Jani Nikula jani.nik...@intel.com
---
 drivers/gpu/drm/i915/intel_dp.c |   73 +--
 1 file changed, 39 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 928b42f..c6b38a6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1691,49 +1691,50 @@ static void intel_enable_dp(struct intel_encoder 
*encoder)
intel_dp_complete_link_train(intel_dp);
intel_dp_stop_link_train(intel_dp);
ironlake_edp_backlight_on(intel_dp);
+}
 
-   if (IS_VALLEYVIEW(dev)) {
-   struct intel_digital_port *dport =
-   enc_to_dig_port(encoder-base);
-   int channel = vlv_dport_to_channel(dport);
-
-   vlv_wait_port_ready(dev_priv, channel);
-   }
+static void vlv_enable_dp(struct intel_encoder *encoder)
+{
 }
 
 static void intel_pre_enable_dp(struct intel_encoder *encoder)
 {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base);
struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
+
+   if (dport-port == PORT_A)
+   ironlake_edp_pll_on(intel_dp);
+}
+
+static void vlv_pre_enable_dp(struct intel_encoder *encoder)
+{
+   struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base);
+   struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
struct drm_device *dev = encoder-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder-base.crtc);
+   int port = vlv_dport_to_channel(dport);
+   int pipe = intel_crtc-pipe;
+   u32 val;
 
-   if (dport-port == PORT_A  !IS_VALLEYVIEW(dev))
-   ironlake_edp_pll_on(intel_dp);
+   mutex_lock(dev_priv-dpio_lock);
 
-   if (IS_VALLEYVIEW(dev)) {
-   struct intel_crtc *intel_crtc =
-   to_intel_crtc(encoder-base.crtc);
-   int port = vlv_dport_to_channel(dport);
-   int pipe = intel_crtc-pipe;
-   u32 val;
-
-   mutex_lock(dev_priv-dpio_lock);
-   val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
-   val = 0;
-   if (pipe)
-   val |= (121);
-   else
-   val = ~(121);
-   val |= 0x001000c4;
-   vlv_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
+   val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
+   val = 0;
+   if (pipe)
+   val |= (121);
+   else
+   val = ~(121);
+   val |= 0x001000c4;
+   vlv_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
+   vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port), 0x00760018);
+   vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port), 0x00400888);
 
-   vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port),
-0x00760018);
-   vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
-0x00400888);
-   mutex_unlock(dev_priv-dpio_lock);
-   }
+   mutex_unlock(dev_priv-dpio_lock);
+
+   intel_enable_dp(encoder);
+
+   vlv_wait_port_ready(dev_priv, port);
 }
 
 static void intel_dp_pre_pll_enable(struct intel_encoder *encoder)
@@ -3513,14 +3514,18 @@ intel_dp_init(struct drm_device *dev, int output_reg, 
enum port port)
 
intel_encoder-compute_config = intel_dp_compute_config;
intel_encoder-mode_set = intel_dp_mode_set;
-   intel_encoder-enable = intel_enable_dp;
-   intel_encoder-pre_enable = intel_pre_enable_dp;
intel_encoder-disable = intel_disable_dp;
intel_encoder-post_disable = intel_post_disable_dp;
intel_encoder-get_hw_state = intel_dp_get_hw_state;
intel_encoder-get_config = intel_dp_get_config;
-   if (IS_VALLEYVIEW(dev))
+   if (IS_VALLEYVIEW(dev)) {
intel_encoder-pre_pll_enable = intel_dp_pre_pll_enable;
+   intel_encoder-pre_enable = vlv_pre_enable_dp;
+   intel_encoder-enable = vlv_enable_dp;
+   } else {
+   intel_encoder-pre_enable = intel_pre_enable_dp;
+   intel_encoder-enable = intel_enable_dp;
+   }
 
intel_dig_port-port = port;
intel_dig_port-dp.output_reg = output_reg;
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 2/3] drm/i915: rearrange vlv hdmi enable and pre_enable callbacks

2013-07-30 Thread Jani Nikula
Currently -pre_enable and -enable are called back to back. Rearrange
the HDMI callbacks to make it possible to move -enable call later.

Basically do everything in -pre_enable on VLV, and make -enable a NOP.

There should be no functional changes.

Signed-off-by: Jani Nikula jani.nik...@intel.com
---
 drivers/gpu/drm/i915/intel_hdmi.c |   20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index a2be6b2..68a27c2 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -718,14 +718,10 @@ static void intel_enable_hdmi(struct intel_encoder 
*encoder)
I915_WRITE(intel_hdmi-hdmi_reg, temp);
POSTING_READ(intel_hdmi-hdmi_reg);
}
+}
 
-   if (IS_VALLEYVIEW(dev)) {
-   struct intel_digital_port *dport =
-   enc_to_dig_port(encoder-base);
-   int channel = vlv_dport_to_channel(dport);
-
-   vlv_wait_port_ready(dev_priv, channel);
-   }
+static void vlv_enable_hdmi(struct intel_encoder *encoder)
+{
 }
 
 static void intel_disable_hdmi(struct intel_encoder *encoder)
@@ -1064,6 +1060,10 @@ static void intel_hdmi_pre_enable(struct intel_encoder 
*encoder)
vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
 0x00400888);
mutex_unlock(dev_priv-dpio_lock);
+
+   intel_enable_hdmi(encoder);
+
+   vlv_wait_port_ready(dev_priv, port);
 }
 
 static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder)
@@ -1244,14 +1244,16 @@ void intel_hdmi_init(struct drm_device *dev, int 
hdmi_reg, enum port port)
 
intel_encoder-compute_config = intel_hdmi_compute_config;
intel_encoder-mode_set = intel_hdmi_mode_set;
-   intel_encoder-enable = intel_enable_hdmi;
intel_encoder-disable = intel_disable_hdmi;
intel_encoder-get_hw_state = intel_hdmi_get_hw_state;
intel_encoder-get_config = intel_hdmi_get_config;
if (IS_VALLEYVIEW(dev)) {
-   intel_encoder-pre_enable = intel_hdmi_pre_enable;
intel_encoder-pre_pll_enable = intel_hdmi_pre_pll_enable;
+   intel_encoder-pre_enable = intel_hdmi_pre_enable;
+   intel_encoder-enable = vlv_enable_hdmi;
intel_encoder-post_disable = intel_hdmi_post_disable;
+   } else {
+   intel_encoder-enable = intel_enable_hdmi;
}
 
intel_encoder-type = INTEL_OUTPUT_HDMI;
-- 
1.7.10.4

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


[Intel-gfx] [PATCH 3/3] drm/i915: move encoder-enable callback later in VLV crtc enable

2013-07-30 Thread Jani Nikula
Encoder -pre_enable and -enable callbacks were moved back to back in
VLV crtc enable sequence, which is not very useful. Move -enable call
at the end of the sequence.

With the previously rearranged VLV DP and HDMI -pre_enable and -enable
callbacks in place, this should not cause any functional changes.

Signed-off-by: Jani Nikula jani.nik...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c |7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 7d63d8d..c3c0bf1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3663,10 +3663,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
if (encoder-pre_enable)
encoder-pre_enable(encoder);
 
-   /* VLV wants encoder enabling _before_ the pipe is up. */
-   for_each_encoder_on_crtc(dev, crtc, encoder)
-   encoder-enable(encoder);
-
i9xx_pfit_enable(intel_crtc);
 
intel_crtc_load_lut(crtc);
@@ -3677,6 +3673,9 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
intel_crtc_update_cursor(crtc, true);
 
intel_update_fbc(dev);
+
+   for_each_encoder_on_crtc(dev, crtc, encoder)
+   encoder-enable(encoder);
 }
 
 static void i9xx_crtc_enable(struct drm_crtc *crtc)
-- 
1.7.10.4

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


Re: [Intel-gfx] gpu outputs slave and cache flushing

2013-07-30 Thread Chris Wilson
On Tue, Jul 30, 2013 at 03:04:22PM +1000, Dave Airlie wrote:
 Hey,
 
 so I put a patch into intel driver a while ago to avoid doing a bo
 flush using map/unmap for output slave device if the kernel has vmap
 flushing
 
 However on reflection I realised this only works for CPU accessing
 devices like UDL but doesn't work for GPU accessing devices like
 nouveau/radeon,
 
 Going forward I'm sure we'll eventually get GPU sync via Maarten's
 patches but I'm thinking I should revert this change in the intel
 driver for now,
 so reverse optimus can work properly
 
 Anyone got any ideas for a better plan going forward, maybe a stop gap
 before Maartens patches.

I don't think it is possible to w/a this in userspace, so let's blame
Daniel^WBen for this mess and cheer on our knight in shining armour.
Go Maarten! But we need to be sure there is a similar synchronisation
point for CPU access to a foriegn dma-buf.
-Chris

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


Re: [Intel-gfx] [PATCH 1/3] drm/i915: rearrange vlv dp enable and pre_enable callbacks

2013-07-30 Thread Chris Wilson
On Tue, Jul 30, 2013 at 11:08:28AM +0300, Jani Nikula wrote:
 Currently -pre_enable and -enable are called back to back. Rearrange
 the DP callbacks to make it possible to move -enable call later.

A one sentence explanation why we want -enable later would be perfect.
If you can concisely explain why the code was originally written as is,
and why we want to do it differently in the near future, that would be
ideal.
-Chris

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


Re: [Intel-gfx] gpu outputs slave and cache flushing

2013-07-30 Thread Maarten Lankhorst
Op 30-07-13 10:13, Chris Wilson schreef:
 On Tue, Jul 30, 2013 at 03:04:22PM +1000, Dave Airlie wrote:
 Hey,

 so I put a patch into intel driver a while ago to avoid doing a bo
 flush using map/unmap for output slave device if the kernel has vmap
 flushing

 However on reflection I realised this only works for CPU accessing
 devices like UDL but doesn't work for GPU accessing devices like
 nouveau/radeon,

 Going forward I'm sure we'll eventually get GPU sync via Maarten's
 patches but I'm thinking I should revert this change in the intel
 driver for now,
 so reverse optimus can work properly

 Anyone got any ideas for a better plan going forward, maybe a stop gap
 before Maartens patches.
 I don't think it is possible to w/a this in userspace, so let's blame
 Daniel^WBen for this mess and cheer on our knight in shining armour.
 Go Maarten! But we need to be sure there is a similar synchronisation
 point for CPU access to a foriegn dma-buf.
 -Chris

It's complicated, and probably best suited to discuss at linux plumbers. :)
It probably won't work without changing how the dma-buf locking is done,
and for intel also depends on proper ww_mutex support.
I think using reservation objects and fences in intel is the right way to go,
and would allow for a less invasive integration of synchronization.

In my experimental tree I used a hack in the execbuffer before to reserve all
bo's, but the approach I took was rather hacky, and only applied to dma-bufs.
Correct handling is probably better done by someone who understands i915 better 
than me.

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


Re: [Intel-gfx] [PATCH 1/3] drm/i915: rearrange vlv dp enable and pre_enable callbacks

2013-07-30 Thread Jani Nikula
On Tue, 30 Jul 2013, Chris Wilson ch...@chris-wilson.co.uk wrote:
 On Tue, Jul 30, 2013 at 11:08:28AM +0300, Jani Nikula wrote:
 Currently -pre_enable and -enable are called back to back. Rearrange
 the DP callbacks to make it possible to move -enable call later.

 A one sentence explanation why we want -enable later would be perfect.
 If you can concisely explain why the code was originally written as is,
 and why we want to do it differently in the near future, that would be
 ideal.

Good point. How about,

for patches 12:


VLV wants encoder enabling before the pipe is up. This is currently
achieved through calling the -enable callback early, right after the
-pre_enable callback, in valleyview_crtc_enable(). This loses both the
distinction between -pre_enable and -enable on VLV and the possibility
to use a hook at the end of the modeset sequence.

Rearrange the DP callbacks to make it possible to move -enable call
later. Basically do everything in -pre_enable on VLV, and make -enable
a NOP.

There should be no functional changes.


for patch 3:


VLV wants encoder enabling before the pipe is up. With the previously
rearranged VLV DP and HDMI -pre_enable and -enable callbacks in place,
this no longer depends on the early -enable hook call. Move the
-enable call at the end of the sequence, similar to the crtc enable on
other platforms. This will be needed e.g. for moving the eDP backlight
enabling to the right place in the sequence, currently done too early on
VLV.

There should be no functional changes.


I'll send the revised patches once we agree on the wording.

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


Re: [Intel-gfx] [PATCH 1/3] drm/i915: rearrange vlv dp enable and pre_enable callbacks

2013-07-30 Thread Chris Wilson
On Tue, Jul 30, 2013 at 12:05:34PM +0300, Jani Nikula wrote:
 On Tue, 30 Jul 2013, Chris Wilson ch...@chris-wilson.co.uk wrote:
  On Tue, Jul 30, 2013 at 11:08:28AM +0300, Jani Nikula wrote:
  Currently -pre_enable and -enable are called back to back. Rearrange
  the DP callbacks to make it possible to move -enable call later.
 
  A one sentence explanation why we want -enable later would be perfect.
  If you can concisely explain why the code was originally written as is,
  and why we want to do it differently in the near future, that would be
  ideal.
 
 Good point. How about,

Thank you, that was the explanation I was looking for! :)
-Chris

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


[Intel-gfx] [PATCH v3 2/3] drm/i915: rearrange vlv hdmi enable and pre_enable callbacks

2013-07-30 Thread Jani Nikula
VLV wants encoder enabling before the pipe is up. This is currently
achieved through calling the -enable callback early, right after the
-pre_enable callback, in valleyview_crtc_enable(). This loses both the
distinction between -pre_enable and -enable on VLV and the possibility
to use a hook at the end of the modeset sequence.

Rearrange the HDMI callbacks to make it possible to move -enable call
later. Basically do everything in -pre_enable on VLV, and make -enable
a NOP.

There should be no functional changes.

v2: Rebase.

v3: Explain why this is needed in the commit message (Chris).

Signed-off-by: Jani Nikula jani.nik...@intel.com
---
 drivers/gpu/drm/i915/intel_hdmi.c |   20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index a2be6b2..68a27c2 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -718,14 +718,10 @@ static void intel_enable_hdmi(struct intel_encoder 
*encoder)
I915_WRITE(intel_hdmi-hdmi_reg, temp);
POSTING_READ(intel_hdmi-hdmi_reg);
}
+}
 
-   if (IS_VALLEYVIEW(dev)) {
-   struct intel_digital_port *dport =
-   enc_to_dig_port(encoder-base);
-   int channel = vlv_dport_to_channel(dport);
-
-   vlv_wait_port_ready(dev_priv, channel);
-   }
+static void vlv_enable_hdmi(struct intel_encoder *encoder)
+{
 }
 
 static void intel_disable_hdmi(struct intel_encoder *encoder)
@@ -1064,6 +1060,10 @@ static void intel_hdmi_pre_enable(struct intel_encoder 
*encoder)
vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
 0x00400888);
mutex_unlock(dev_priv-dpio_lock);
+
+   intel_enable_hdmi(encoder);
+
+   vlv_wait_port_ready(dev_priv, port);
 }
 
 static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder)
@@ -1244,14 +1244,16 @@ void intel_hdmi_init(struct drm_device *dev, int 
hdmi_reg, enum port port)
 
intel_encoder-compute_config = intel_hdmi_compute_config;
intel_encoder-mode_set = intel_hdmi_mode_set;
-   intel_encoder-enable = intel_enable_hdmi;
intel_encoder-disable = intel_disable_hdmi;
intel_encoder-get_hw_state = intel_hdmi_get_hw_state;
intel_encoder-get_config = intel_hdmi_get_config;
if (IS_VALLEYVIEW(dev)) {
-   intel_encoder-pre_enable = intel_hdmi_pre_enable;
intel_encoder-pre_pll_enable = intel_hdmi_pre_pll_enable;
+   intel_encoder-pre_enable = intel_hdmi_pre_enable;
+   intel_encoder-enable = vlv_enable_hdmi;
intel_encoder-post_disable = intel_hdmi_post_disable;
+   } else {
+   intel_encoder-enable = intel_enable_hdmi;
}
 
intel_encoder-type = INTEL_OUTPUT_HDMI;
-- 
1.7.10.4

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


[Intel-gfx] [PATCH v3 3/3] drm/i915: move encoder-enable callback later in VLV crtc enable

2013-07-30 Thread Jani Nikula
VLV wants encoder enabling before the pipe is up. With the previously
rearranged VLV DP and HDMI -pre_enable and -enable callbacks in place,
this no longer depends on the early -enable hook call. Move the
-enable call at the end of the sequence, similar to the crtc enable on
other platforms. This will be needed e.g. for moving the eDP backlight
enabling to the right place in the sequence, currently done too early on
VLV.

There should be no functional changes.

v2: Rebase.

v3: Explain why this is needed in the commit message (Chris).

Signed-off-by: Jani Nikula jani.nik...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c |7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 7d63d8d..c3c0bf1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3663,10 +3663,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
if (encoder-pre_enable)
encoder-pre_enable(encoder);
 
-   /* VLV wants encoder enabling _before_ the pipe is up. */
-   for_each_encoder_on_crtc(dev, crtc, encoder)
-   encoder-enable(encoder);
-
i9xx_pfit_enable(intel_crtc);
 
intel_crtc_load_lut(crtc);
@@ -3677,6 +3673,9 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
intel_crtc_update_cursor(crtc, true);
 
intel_update_fbc(dev);
+
+   for_each_encoder_on_crtc(dev, crtc, encoder)
+   encoder-enable(encoder);
 }
 
 static void i9xx_crtc_enable(struct drm_crtc *crtc)
-- 
1.7.10.4

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


[Intel-gfx] [PATCH v3 1/3] drm/i915: rearrange vlv dp enable and pre_enable callbacks

2013-07-30 Thread Jani Nikula
VLV wants encoder enabling before the pipe is up. This is currently
achieved through calling the -enable callback early, right after the
-pre_enable callback, in valleyview_crtc_enable(). This loses both the
distinction between -pre_enable and -enable on VLV and the possibility
to use a hook at the end of the modeset sequence.

Rearrange the DP callbacks to make it possible to move -enable call
later. Basically do everything in -pre_enable on VLV, and make -enable
a NOP.

There should be no functional changes.

v2: Rebase.

v3: Explain why this is needed in the commit message (Chris).

Signed-off-by: Jani Nikula jani.nik...@intel.com
---
 drivers/gpu/drm/i915/intel_dp.c |   73 +--
 1 file changed, 39 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 928b42f..c6b38a6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1691,49 +1691,50 @@ static void intel_enable_dp(struct intel_encoder 
*encoder)
intel_dp_complete_link_train(intel_dp);
intel_dp_stop_link_train(intel_dp);
ironlake_edp_backlight_on(intel_dp);
+}
 
-   if (IS_VALLEYVIEW(dev)) {
-   struct intel_digital_port *dport =
-   enc_to_dig_port(encoder-base);
-   int channel = vlv_dport_to_channel(dport);
-
-   vlv_wait_port_ready(dev_priv, channel);
-   }
+static void vlv_enable_dp(struct intel_encoder *encoder)
+{
 }
 
 static void intel_pre_enable_dp(struct intel_encoder *encoder)
 {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base);
struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
+
+   if (dport-port == PORT_A)
+   ironlake_edp_pll_on(intel_dp);
+}
+
+static void vlv_pre_enable_dp(struct intel_encoder *encoder)
+{
+   struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base);
+   struct intel_digital_port *dport = dp_to_dig_port(intel_dp);
struct drm_device *dev = encoder-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
+   struct intel_crtc *intel_crtc = to_intel_crtc(encoder-base.crtc);
+   int port = vlv_dport_to_channel(dport);
+   int pipe = intel_crtc-pipe;
+   u32 val;
 
-   if (dport-port == PORT_A  !IS_VALLEYVIEW(dev))
-   ironlake_edp_pll_on(intel_dp);
+   mutex_lock(dev_priv-dpio_lock);
 
-   if (IS_VALLEYVIEW(dev)) {
-   struct intel_crtc *intel_crtc =
-   to_intel_crtc(encoder-base.crtc);
-   int port = vlv_dport_to_channel(dport);
-   int pipe = intel_crtc-pipe;
-   u32 val;
-
-   mutex_lock(dev_priv-dpio_lock);
-   val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
-   val = 0;
-   if (pipe)
-   val |= (121);
-   else
-   val = ~(121);
-   val |= 0x001000c4;
-   vlv_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
+   val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
+   val = 0;
+   if (pipe)
+   val |= (121);
+   else
+   val = ~(121);
+   val |= 0x001000c4;
+   vlv_dpio_write(dev_priv, DPIO_DATA_CHANNEL(port), val);
+   vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port), 0x00760018);
+   vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port), 0x00400888);
 
-   vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF0(port),
-0x00760018);
-   vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
-0x00400888);
-   mutex_unlock(dev_priv-dpio_lock);
-   }
+   mutex_unlock(dev_priv-dpio_lock);
+
+   intel_enable_dp(encoder);
+
+   vlv_wait_port_ready(dev_priv, port);
 }
 
 static void intel_dp_pre_pll_enable(struct intel_encoder *encoder)
@@ -3513,14 +3514,18 @@ intel_dp_init(struct drm_device *dev, int output_reg, 
enum port port)
 
intel_encoder-compute_config = intel_dp_compute_config;
intel_encoder-mode_set = intel_dp_mode_set;
-   intel_encoder-enable = intel_enable_dp;
-   intel_encoder-pre_enable = intel_pre_enable_dp;
intel_encoder-disable = intel_disable_dp;
intel_encoder-post_disable = intel_post_disable_dp;
intel_encoder-get_hw_state = intel_dp_get_hw_state;
intel_encoder-get_config = intel_dp_get_config;
-   if (IS_VALLEYVIEW(dev))
+   if (IS_VALLEYVIEW(dev)) {
intel_encoder-pre_pll_enable = intel_dp_pre_pll_enable;
+   intel_encoder-pre_enable = vlv_pre_enable_dp;
+   intel_encoder-enable = vlv_enable_dp;
+   } else {
+   intel_encoder-pre_enable = intel_pre_enable_dp;
+   intel_encoder-enable = intel_enable_dp;
+   }
 
intel_dig_port-port = port;
intel_dig_port-dp.output_reg 

Re: [Intel-gfx] [PATCH 8/8] drm/i915: allow Package C8+ by default

2013-07-30 Thread Chris Wilson
On Mon, Jul 29, 2013 at 05:48:27PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@intel.com
 
 Things should be working, so enable it by default.
 
 Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
 index 6df516b..8f6e685 100644
 --- a/drivers/gpu/drm/i915/i915_drv.c
 +++ b/drivers/gpu/drm/i915/i915_drv.c
 @@ -141,9 +141,9 @@ module_param_named(fastboot, i915_fastboot, bool, 0600);
  MODULE_PARM_DESC(fastboot, Try to skip unnecessary mode sets at boot time 
(default: false));
  
 -int i915_allow_pc8 __read_mostly = 0;
 +int i915_allow_pc8 __read_mostly = 1;
  module_param_named(allow_pc8, i915_allow_pc8, int, 0600);
 -MODULE_PARM_DESC(allow_pc8, Allow package states  8 (default: false));
 +MODULE_PARM_DESC(allow_pc8, Allow package states  8 (default: true));

How about (after changing the variable name as earlier):
Enable support for low power package sleep states (pc8) [default: true])

Hmm, I thought we were consistent in using [default], alas not.
-Chris

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


Re: [Intel-gfx] [PATCH 5/8] drm/i915: avoid waking up from PC8 on GMBUS operations

2013-07-30 Thread Chris Wilson
On Mon, Jul 29, 2013 at 05:48:24PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@intel.com
 
 If we're already allowing PC8, just don't use the IRQs, so we won't
 need to wake from PC8. Waking up from PC8 is a slow thing, so avoid it
 when we can.

You would also need to explain that the GMBUS is outside of the display
power well.

Looks reasonable, the only bit is moving the read of forbid_count into
hsw_pc8_enabled() so that the gmbus code isn't poking around with
someone else's locks, and we can safely do an unlocked optimistic read
here.
-Chris

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


Re: [Intel-gfx] [PATCH 4/8] drm/i915: avoid waking up from PC8 on DP AUX operations

2013-07-30 Thread Chris Wilson
On Mon, Jul 29, 2013 at 05:48:23PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@intel.com
 
 If we're already allowing PC8, just don't use the IRQs, so we won't
 need to wake from PC8. Waking up from PC8 is a slow thing, so avoid it
 when we can.

This raises the question of why we where holding the power well wake
lock if we weren't using IRQs in the first place. This tells me the 
intel_aux_display_runtime_get() interface is indequate, imo. It would
need to be more expressive of why/what part of aux display runtime you
want. You could even move the state checking there so that GMBUS could
use the same interface:

unsigned long intel_aux_display_runtime_get(dev_priv, want, need)
 {
  if (need) {
 __intel_aux_display_runtime_get();
 return need;
   }

   if (want) {
if (hsw_pc8_enabled()) {
   __intel_aux_display_runtime_get();
   return want;
}
   }

   return 0;
 }
 
And pass the return value to _put for it to decide if it needs to do
anything (also it can then select its path based on whether or not it
holds a wakeref). Making the caller decide whether or not to act based
on information inside pc8 looks very fragile to me, and easy to break in
future.
-Chris

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


Re: [Intel-gfx] [PATCH 2/8] drm/i915: don't disable/reenable IVB error interrupts when not needed

2013-07-30 Thread Chris Wilson
On Mon, Jul 29, 2013 at 05:48:21PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@intel.com
 
 If the error interrupts are already disabled, don't disable and
 reenable them. This is going to be needed when we're in PC8+, where
 all the interrupts are disabled so we won't risk re-enabling
 DE_ERR_INT_IVB.

   if (IS_HASWELL(dev)) {
   spin_lock(dev_priv-irq_lock);
 - ironlake_disable_display_irq(dev_priv, DE_ERR_INT_IVB);
 + if (!(I915_READ(DEIMR)  DE_ERR_INT_IVB)) {
 + ironlake_disable_display_irq(dev_priv, DE_ERR_INT_IVB);
 + err_int_reenable = true;
 + }

Or just make ironlake_disable_display_irq() return a bool. So this then
raises the question: please justify why the deimr state tracker is out
of sync with the register. If it is possible that we write to this
register whilst under pc8 and then restore a different value, that is
scary. I would rather have a big BUG_ON(!pc8) inside
ironlake_disable_display_irq() and move the pc8 handling logic there
(i.e. if we get a request for something whilst under pc8, modify the
saved bits rather than the actual register).
-Chris

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


[Intel-gfx] [PATCH] drm/i915: make user mode sync polarity setting explicit

2013-07-30 Thread Imre Deak
Userspace can pass a mode with an unspecified vsync/hsync polarity
setting. All encoders in the Intel driver take this to mean a negative
polarity setting. The HW readout/state checker code on the other hand
needs these flags to be explicitly set, otherwise the state checker will
WARN about the mismatch.

Get rid of the WARN by making the polarity setting explicit in the
adjusted mode flags based on the requested mode flags. This will keep
the existing behavior otherwise.

Note that we could guess from the other timing parameters whether the
user wanted a VESA or other standard mode and set the polarity
accordingly. This is what the NV driver does
(drivers/gpu/drm/nouveau/dispnv04/crtc.c), but I think that's not very
exact and would change the existing behavior of the Intel driver.

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

Signed-off-by: Imre Deak imre.d...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index baaefd7..98f0fa6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8058,6 +8058,19 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
(enum transcoder) to_intel_crtc(crtc)-pipe;
pipe_config-shared_dpll = DPLL_ID_PRIVATE;
 
+   /*
+* Sanitize sync polarity flags based on requested ones. If neither
+* positive or negative polarity is requested, treat this as meaning
+* negative polarity.
+*/
+   if (!(pipe_config-adjusted_mode.flags 
+ (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_NHSYNC)))
+   pipe_config-adjusted_mode.flags |= DRM_MODE_FLAG_NHSYNC;
+
+   if (!(pipe_config-adjusted_mode.flags 
+ (DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_NVSYNC)))
+   pipe_config-adjusted_mode.flags |= DRM_MODE_FLAG_NVSYNC;
+
/* Compute a starting value for pipe_config-pipe_bpp taking the source
 * plane pixel format and any sink constraints into account. Returns the
 * source plane bpp so that dithering can be selected on mismatches
-- 
1.8.3.2

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


Re: [Intel-gfx] [PATCH] drm/i915: make user mode sync polarity setting explicit

2013-07-30 Thread Chris Wilson
On Tue, Jul 30, 2013 at 01:36:32PM +0300, Imre Deak wrote:
 Userspace can pass a mode with an unspecified vsync/hsync polarity
 setting. All encoders in the Intel driver take this to mean a negative
 polarity setting. The HW readout/state checker code on the other hand
 needs these flags to be explicitly set, otherwise the state checker will
 WARN about the mismatch.
 
 Get rid of the WARN by making the polarity setting explicit in the
 adjusted mode flags based on the requested mode flags. This will keep
 the existing behavior otherwise.
 
 Note that we could guess from the other timing parameters whether the
 user wanted a VESA or other standard mode and set the polarity
 accordingly. This is what the NV driver does
 (drivers/gpu/drm/nouveau/dispnv04/crtc.c), but I think that's not very
 exact and would change the existing behavior of the Intel driver.

Right, don't guess. If the user wanted the standard mode, then the flags
would have been taken from the standard modeline.
 
 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65442

You can add a tested-by here for qa.
 
 Signed-off-by: Imre Deak imre.d...@intel.com
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
-Chris

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


Re: [Intel-gfx] [PATCH v3 3/3] drm/i915: move encoder-enable callback later in VLV crtc enable

2013-07-30 Thread Chris Wilson
On Tue, Jul 30, 2013 at 12:20:32PM +0300, Jani Nikula wrote:
 VLV wants encoder enabling before the pipe is up. With the previously
 rearranged VLV DP and HDMI -pre_enable and -enable callbacks in place,
 this no longer depends on the early -enable hook call. Move the
 -enable call at the end of the sequence, similar to the crtc enable on
 other platforms. This will be needed e.g. for moving the eDP backlight
 enabling to the right place in the sequence, currently done too early on
 VLV.
 
 There should be no functional changes.
 
 v2: Rebase.
 
 v3: Explain why this is needed in the commit message (Chris).
 
 Signed-off-by: Jani Nikula jani.nik...@intel.com

I couldn't spot any functional changes in the 3 patches, so
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
-Chris

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


Re: [Intel-gfx] [PATCH] drm/i915: make user mode sync polarity setting explicit

2013-07-30 Thread Imre Deak
On Tue, 2013-07-30 at 15:43 +0300, Imre Deak wrote:
 On Tue, 2013-07-30 at 11:57 +0100, Chris Wilson wrote:
  On Tue, Jul 30, 2013 at 01:36:32PM +0300, Imre Deak wrote:
   Userspace can pass a mode with an unspecified vsync/hsync polarity
   setting. All encoders in the Intel driver take this to mean a negative
   polarity setting. The HW readout/state checker code on the other hand
   needs these flags to be explicitly set, otherwise the state checker will
   WARN about the mismatch.
   
   Get rid of the WARN by making the polarity setting explicit in the
   adjusted mode flags based on the requested mode flags. This will keep
   the existing behavior otherwise.
   
   Note that we could guess from the other timing parameters whether the
   user wanted a VESA or other standard mode and set the polarity
   accordingly. This is what the NV driver does
   (drivers/gpu/drm/nouveau/dispnv04/crtc.c), but I think that's not very
   exact and would change the existing behavior of the Intel driver.
  
  Right, don't guess. If the user wanted the standard mode, then the flags
  would have been taken from the standard modeline.
   
   Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65442
  
  You can add a tested-by here for qa.
 
 Tested-by: Cancan Feng cancan.f...@intel.com
  
   Signed-off-by: Imre Deak imre.d...@intel.com
  Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
 
 CC'ing people who might be interested.
 
 After some discussion with Ville, we could refine this further at the
 drm core level by enforcing the Intel behavior - defaulting to negative
 polarity and also checking/sanitizing the PHSYNC/PVSYNC flags.
 PHSYNC/PVSYNC isn't used by the Intel driver so we could still go with

Sorry, I meant PCSYNC/NCSYNC above.

--Imre

 the above patch for now and follow-up with a drm core fix.
 
 We should probably also reject modes at drm core level where both
 positive and negative flags are set, again in a separate follow-up
 patch.
 
 --Imre



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v3 3/3] drm/i915: move encoder-enable callback later in VLV crtc enable

2013-07-30 Thread Ville Syrjälä
On Tue, Jul 30, 2013 at 12:20:32PM +0300, Jani Nikula wrote:
 VLV wants encoder enabling before the pipe is up. With the previously
 rearranged VLV DP and HDMI -pre_enable and -enable callbacks in place,
 this no longer depends on the early -enable hook call. Move the
 -enable call at the end of the sequence, similar to the crtc enable on
 other platforms. This will be needed e.g. for moving the eDP backlight
 enabling to the right place in the sequence, currently done too early on
 VLV.
 
 There should be no functional changes.
 
 v2: Rebase.
 
 v3: Explain why this is needed in the commit message (Chris).
 
 Signed-off-by: Jani Nikula jani.nik...@intel.com

Looks sane to me as well. For the series:
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com

Note that I just lost my VLV board so I wasn't able to test this.

I'm assuming there will eventually be some followon patch to move the
backlight stuff to right place for VLV?

Also should we make encoder-enable optional to avoid the stubs for VLV?

And it might make sense to rename all the VLV specific dp/hdmi functions
to vlv_foo instead of the intel_foo that they are called now.

 ---
  drivers/gpu/drm/i915/intel_display.c |7 +++
  1 file changed, 3 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 7d63d8d..c3c0bf1 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -3663,10 +3663,6 @@ static void valleyview_crtc_enable(struct drm_crtc 
 *crtc)
   if (encoder-pre_enable)
   encoder-pre_enable(encoder);
  
 - /* VLV wants encoder enabling _before_ the pipe is up. */
 - for_each_encoder_on_crtc(dev, crtc, encoder)
 - encoder-enable(encoder);
 -
   i9xx_pfit_enable(intel_crtc);
  
   intel_crtc_load_lut(crtc);
 @@ -3677,6 +3673,9 @@ static void valleyview_crtc_enable(struct drm_crtc 
 *crtc)
   intel_crtc_update_cursor(crtc, true);
  
   intel_update_fbc(dev);
 +
 + for_each_encoder_on_crtc(dev, crtc, encoder)
 + encoder-enable(encoder);
  }
  
  static void i9xx_crtc_enable(struct drm_crtc *crtc)
 -- 
 1.7.10.4
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


[Intel-gfx] [edid-decode] Decode HDMI 1.4 4k VICs

2013-07-30 Thread Damien Lespiau
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 edid-decode.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/edid-decode.c b/edid-decode.c
index 9840db6..f74bbe4 100644
--- a/edid-decode.c
+++ b/edid-decode.c
@@ -625,7 +625,7 @@ cea_hdmi_block(unsigned char *x)

if (x[8]  0x20) {
int mask = 0, formats = 0;
-   int len_xx, len_3d;
+   int len_vic, len_3d;
printf(Extended HDMI video details:\n);
if (x[9 + b]  0x80)
printf(  3D present\n);
@@ -650,14 +650,17 @@ cea_hdmi_block(unsigned char *x)
printf(  Base EDID image size is in units of 5cm\n);
break;
}
-   len_xx = (x[10 + b]  0xe0)  5;
+   len_vic = (x[10 + b]  0xe0)  5;
len_3d = (x[10 + b]  0x1f)  0;
b += 2;
 
-   if (len_xx) {
-   printf(  Skipping %d bytes that HDMI refuses to publicly
-   document\n, len_xx);
-   b += len_xx;
+   if (len_vic) {
+   int i;
+
+   for (i = 0; i  len_vic; i++)
+   printf(  HDMI VIC %d\n, x[9 + b + i]);
+
+   b += len_vic;
}
 
if (len_3d) {
-- 
1.8.3.1

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


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Add async page flip support for SNB

2013-07-30 Thread Ville Syrjälä
On Thu, Jul 25, 2013 at 03:15:15PM -0700, Keith Packard wrote:
 Just copies the IVB code
 
 Signed-off-by: Keith Packard kei...@keithp.com
 ---
  drivers/gpu/drm/i915/intel_display.c | 17 +
  1 file changed, 13 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 166aa2c..4a118c3 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -7465,20 +7465,29 @@ static int intel_gen6_queue_flip(struct drm_device 
 *dev,
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
   struct intel_ring_buffer *ring = dev_priv-ring[RCS];
   uint32_t pf, pipesrc;
 + uint32_t cmd;
 + uint32_t base;
   int ret;
  
   ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
   if (ret)
   goto err;
  
 + cmd = MI_DISPLAY_FLIP | MI_DISPLAY_FLIP_PLANE(intel_crtc-plane);
 + base = i915_gem_obj_ggtt_offset(obj) + intel_crtc-dspaddr_offset;
 +
 + if (flags  DRM_MODE_PAGE_FLIP_ASYNC) {
 + cmd |= MI_DISPLAY_FLIP_ASYNC_INDICATOR;
 + base |= MI_DISPLAY_FLIP_TYPE_ASYNC;
 + }
 +
   ret = intel_ring_begin(ring, 4);
   if (ret)
   goto err_unpin;
  
 - intel_ring_emit(ring, MI_DISPLAY_FLIP |
 - MI_DISPLAY_FLIP_PLANE(intel_crtc-plane));
 + intel_ring_emit(ring, cmd);
   intel_ring_emit(ring, fb-pitches[0] | obj-tiling_mode);
 - intel_ring_emit(ring, i915_gem_obj_ggtt_offset(obj) + 
 intel_crtc-dspaddr_offset);
 + intel_ring_emit(ring, base);
  
   /* Contrary to the suggestions in the documentation,
* Enable Panel Fitter does not seem to be required when page

This PF flip stuff is a bit of a mystery. I'm not sure it exists on SNB
anymore. Some of the docs say that it's MBZ for SNB/IVB. Gen4/5 docs
say that DW3 must not be sent w/ async flips, and some SNB+ docs say
that it must not be sent with either sync or async flips.

Did you test this patch on actual hardware, and if so did it work as
expected? :)

I guess one would need to perform some empirical testing to figure out
what DW3 actually does.

 @@ -9731,7 +9740,7 @@ void intel_modeset_init(struct drm_device *dev)
   dev-mode_config.max_height = 8192;
   }
  
 - if (IS_GEN7(dev))
 + if (IS_GEN6(dev) || IS_GEN7(dev))
   dev-mode_config.async_page_flip = true;
  
   dev-mode_config.fb_base = dev_priv-gtt.mappable_base;
 -- 
 1.8.3.2
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Add async page flip support for IVB

2013-07-30 Thread Ville Syrjälä
On Thu, Jul 25, 2013 at 03:15:14PM -0700, Keith Packard wrote:
 This adds the necesary register defines for async page flipping
 through the command ring, and then hooks those up for Ivybridge (gen7)
 page flipping.

Maybe mention hsw in the patch subject/description too.

 
 Signed-off-by: Keith Packard kei...@keithp.com
 ---
  drivers/gpu/drm/i915/i915_reg.h  |  6 ++
  drivers/gpu/drm/i915/intel_display.c | 37 
 
  2 files changed, 39 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index dc3d6a7..029cfb0 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -209,6 +209,7 @@
  #define MI_LOAD_SCAN_LINES_INCL MI_INSTR(0x12, 0)
  #define MI_DISPLAY_FLIP  MI_INSTR(0x14, 2)
  #define MI_DISPLAY_FLIP_I915 MI_INSTR(0x14, 1)
 +#define   MI_DISPLAY_FLIP_ASYNC_INDICATOR(1  22)
  #define   MI_DISPLAY_FLIP_PLANE(n) ((n)  20)
  /* IVB has funny definitions for which plane to flip. */
  #define   MI_DISPLAY_FLIP_IVB_PLANE_A  (0  19)
 @@ -217,6 +218,11 @@
  #define   MI_DISPLAY_FLIP_IVB_SPRITE_B (3  19)
  #define   MI_DISPLAY_FLIP_IVB_PLANE_C  (4  19)
  #define   MI_DISPLAY_FLIP_IVB_SPRITE_C (5  19)
 +/* These go in the bottom of the base address value */
 +#define   MI_DISPLAY_FLIP_TYPE_SYNC(0  0)
 +#define   MI_DISPLAY_FLIP_TYPE_ASYNC   (1  0)
 +#define   MI_DISPLAY_FLIP_TYPE_STEREO  (2  0)
 +#define   MI_DISPLAY_FLIP_TYPE_SYNCHRONOUS   (0  0)

Duplicated bit.

  #define MI_ARB_ON_OFFMI_INSTR(0x08, 0)
  #define   MI_ARB_ENABLE  (10)
  #define   MI_ARB_DISABLE (00)
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index bdb8854..166aa2c 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -1833,8 +1833,10 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev,
   alignment = 64 * 1024;
   break;
   case I915_TILING_X:
 - /* pin() will align the object as required by fence */
 - alignment = 0;
 + /* Async page flipping requires X tiling and 32kB alignment, so 
 just
 +  * make all X tiled frame buffers aligned for that
 +  */
 + alignment = 32 * 1024;

You could limit this to gens for which you implemented async flips.

gen2/3 seem to require a 256KB alignment for async flips.

   break;
   case I915_TILING_Y:
   /* Despite that we check this in framebuffer_init userspace can
 @@ -7514,6 +7516,8 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
   struct intel_ring_buffer *ring = dev_priv-ring[BCS];
   uint32_t plane_bit = 0;
 + uint32_t cmd;
 + uint32_t base;
   int ret;
  
   ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
 @@ -7536,13 +7540,21 @@ static int intel_gen7_queue_flip(struct drm_device 
 *dev,
   goto err_unpin;
   }
  
 + cmd = MI_DISPLAY_FLIP_I915 | plane_bit;
 + base = i915_gem_obj_ggtt_offset(obj) + intel_crtc-dspaddr_offset;
 +
 + if (flags  DRM_MODE_PAGE_FLIP_ASYNC) {
 + cmd |= MI_DISPLAY_FLIP_ASYNC_INDICATOR;
 + base |= MI_DISPLAY_FLIP_TYPE_ASYNC;
 + }
 +
   ret = intel_ring_begin(ring, 4);
   if (ret)
   goto err_unpin;
  
 - intel_ring_emit(ring, MI_DISPLAY_FLIP_I915 | plane_bit);
 + intel_ring_emit(ring, cmd);
   intel_ring_emit(ring, (fb-pitches[0] | obj-tiling_mode));
 - intel_ring_emit(ring, i915_gem_obj_ggtt_offset(obj) + 
 intel_crtc-dspaddr_offset);
 + intel_ring_emit(ring, base);
   intel_ring_emit(ring, (MI_NOOP));
  
   intel_mark_page_flip_active(intel_crtc);
 @@ -7591,6 +7603,19 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
fb-pitches[0] != crtc-fb-pitches[0]))
   return -EINVAL;
  
 + /* Check tiling restrictions specific to asynchronous flips
 +  */
 + if (page_flip_flags  DRM_MODE_PAGE_FLIP_ASYNC) {
 +
 + /* New FB must be X tiled */
 + if (obj-tiling_mode != I915_TILING_X)
 + return -EINVAL;
 +
 + /* Current FB must be X tiled */
 + if (to_intel_framebuffer(old_fb)-obj-tiling_mode != 
 I915_TILING_X)
 + return -EINVAL;
 + }
 +
   work = kzalloc(sizeof *work, GFP_KERNEL);
   if (work == NULL)
   return -ENOMEM;
 @@ -9705,6 +9730,10 @@ void intel_modeset_init(struct drm_device *dev)
   dev-mode_config.max_width = 8192;
   dev-mode_config.max_height = 8192;
   }
 +
 + if (IS_GEN7(dev))
 + dev-mode_config.async_page_flip = true;
 +
   dev-mode_config.fb_base = dev_priv-gtt.mappable_base;
  
   DRM_DEBUG_KMS(%d display pipe%s available.\n,
 -- 
 1.8.3.2
 

[Intel-gfx] [PATCH 1/2] drm/i915: Use the same pte_encoding for ppgtt as for gtt

2013-07-30 Thread Chris Wilson
The PTE layouts are the same for both ppgtt and gtt, so we can simplify
the setup for ppgtt by copying the encoding function pointer from gtt.
This prevents bugs where we update one function pointer, but forget the
other.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 1294cee..0522d00 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -298,13 +298,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 * now. */
first_pd_entry_in_global_pt = gtt_total_entries(dev_priv-gtt);
 
-   if (IS_HASWELL(dev)) {
-   ppgtt-base.pte_encode = hsw_pte_encode;
-   } else if (IS_VALLEYVIEW(dev)) {
-   ppgtt-base.pte_encode = byt_pte_encode;
-   } else {
-   ppgtt-base.pte_encode = gen6_pte_encode;
-   }
+   ppgtt-base.pte_encode = dev_priv-gtt.base.pte_encode;
ppgtt-num_pd_entries = GEN6_PPGTT_PD_ENTRIES;
ppgtt-enable = gen6_ppgtt_enable;
ppgtt-base.clear_range = gen6_ppgtt_clear_range;
-- 
1.8.3.2

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


[Intel-gfx] [PATCH 2/2] drm/i915: Use Write-Through cacheing for the display plane on Iris

2013-07-30 Thread Chris Wilson
Haswell GT3e has the unique feature of supporting Write-Through cacheing
of objects within the eLLC. The purpose of this is to enable the display
plane to remain coherent whilst objects lie resident in the eLLC - so
that we in theory get the best of both worlds, perfect display and fast
access.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/i915_dma.c |  3 +++
 drivers/gpu/drm/i915/i915_drv.h |  4 +++-
 drivers/gpu/drm/i915/i915_gem.c |  3 ++-
 drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++-
 include/uapi/drm/i915_drm.h |  1 +
 5 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 8da0b3d..75989fc 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -976,6 +976,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
case I915_PARAM_HAS_LLC:
value = HAS_LLC(dev);
break;
+   case I915_PARAM_HAS_WT:
+   value = HAS_WT(dev);
+   break;
case I915_PARAM_HAS_ALIASING_PPGTT:
value = dev_priv-mm.aliasing_ppgtt ? 1 : 0;
break;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 34d2b9d..324ea14 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -452,6 +452,7 @@ enum i915_cache_level {
I915_CACHE_NONE = 0,
I915_CACHE_LLC,
I915_CACHE_LLC_MLC, /* gen6+, in docs at least! */
+   I915_CACHE_WT,
 };
 
 typedef uint32_t gen6_gtt_pte_t;
@@ -1344,7 +1345,7 @@ struct drm_i915_gem_object {
unsigned int pending_fenced_gpu_access:1;
unsigned int fenced_gpu_access:1;
 
-   unsigned int cache_level:2;
+   unsigned int cache_level:3;
 
unsigned int has_aliasing_ppgtt_mapping:1;
unsigned int has_global_gtt_mapping:1;
@@ -1547,6 +1548,7 @@ struct drm_i915_file_private {
 #define HAS_BLT(dev)(INTEL_INFO(dev)-has_blt_ring)
 #define HAS_VEBOX(dev)  (INTEL_INFO(dev)-has_vebox_ring)
 #define HAS_LLC(dev)(INTEL_INFO(dev)-has_llc)
+#define HAS_WT(dev)(IS_HASWELL(dev)  ((struct drm_i915_private 
*)(dev))-ellc_size)
 #define I915_NEED_GFX_HWS(dev) (INTEL_INFO(dev)-need_gfx_hws)
 
 #define HAS_HW_CONTEXTS(dev)   (INTEL_INFO(dev)-gen = 5)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 99362f7..cbea7f8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3565,7 +3565,8 @@ i915_gem_object_pin_to_display_plane(struct 
drm_i915_gem_object *obj,
 * of uncaching, which would allow us to flush all the LLC-cached data
 * with that bit in the PTE to main memory with just one PIPE_CONTROL.
 */
-   ret = i915_gem_object_set_cache_level(obj, I915_CACHE_NONE);
+   ret = i915_gem_object_set_cache_level(obj,
+ HAS_WT(obj-base.dev) ? 
I915_CACHE_WT : I915_CACHE_NONE);
if (ret)
return ret;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 0522d00..072a348 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -54,6 +54,7 @@
 (((bits)  0x8)  (11 - 3)))
 #define HSW_WB_LLC_AGE0HSW_CACHEABILITY_CONTROL(0x3)
 #define HSW_WB_ELLC_LLC_AGE0   HSW_CACHEABILITY_CONTROL(0xb)
+#define HSW_WT_ELLC_LLC_AGE0   HSW_CACHEABILITY_CONTROL(0x6)
 
 static gen6_gtt_pte_t gen6_pte_encode(dma_addr_t addr,
  enum i915_cache_level level)
@@ -116,8 +117,16 @@ static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr,
gen6_gtt_pte_t pte = GEN6_PTE_VALID;
pte |= HSW_PTE_ADDR_ENCODE(addr);
 
-   if (level != I915_CACHE_NONE)
+   switch (level) {
+   case I915_CACHE_NONE:
+   break;
+   case I915_CACHE_WT:
+   pte |= HSW_WT_ELLC_LLC_AGE0;
+   break;
+   default:
pte |= HSW_WB_ELLC_LLC_AGE0;
+   break;
+   }
 
return pte;
 }
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index e47cf00..e831292 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -338,6 +338,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_PINNED_BATCHES   24
 #define I915_PARAM_HAS_EXEC_NO_RELOC25
 #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
+#define I915_PARAM_HAS_WT   27
 
 typedef struct drm_i915_getparam {
int param;
-- 
1.8.3.2

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


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Use Write-Through cacheing for the display plane on Iris

2013-07-30 Thread Ville Syrjälä
On Tue, Jul 30, 2013 at 05:58:36PM +0100, Chris Wilson wrote:
 Haswell GT3e has the unique feature of supporting Write-Through cacheing
 of objects within the eLLC. The purpose of this is to enable the display
 plane to remain coherent whilst objects lie resident in the eLLC - so
 that we in theory get the best of both worlds, perfect display and fast
 access.

The description here talks about eLLC only, but you set the PTE for
WT in LLC/eLLC both.

 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Ben Widawsky b...@bwidawsk.net
 ---
  drivers/gpu/drm/i915/i915_dma.c |  3 +++
  drivers/gpu/drm/i915/i915_drv.h |  4 +++-
  drivers/gpu/drm/i915/i915_gem.c |  3 ++-
  drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++-
  include/uapi/drm/i915_drm.h |  1 +
  5 files changed, 19 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
 index 8da0b3d..75989fc 100644
 --- a/drivers/gpu/drm/i915/i915_dma.c
 +++ b/drivers/gpu/drm/i915/i915_dma.c
 @@ -976,6 +976,9 @@ static int i915_getparam(struct drm_device *dev, void 
 *data,
   case I915_PARAM_HAS_LLC:
   value = HAS_LLC(dev);
   break;
 + case I915_PARAM_HAS_WT:
 + value = HAS_WT(dev);
 + break;
   case I915_PARAM_HAS_ALIASING_PPGTT:
   value = dev_priv-mm.aliasing_ppgtt ? 1 : 0;
   break;
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 34d2b9d..324ea14 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -452,6 +452,7 @@ enum i915_cache_level {
   I915_CACHE_NONE = 0,
   I915_CACHE_LLC,
   I915_CACHE_LLC_MLC, /* gen6+, in docs at least! */
 + I915_CACHE_WT,
  };
  
  typedef uint32_t gen6_gtt_pte_t;
 @@ -1344,7 +1345,7 @@ struct drm_i915_gem_object {
   unsigned int pending_fenced_gpu_access:1;
   unsigned int fenced_gpu_access:1;
  
 - unsigned int cache_level:2;
 + unsigned int cache_level:3;
  
   unsigned int has_aliasing_ppgtt_mapping:1;
   unsigned int has_global_gtt_mapping:1;
 @@ -1547,6 +1548,7 @@ struct drm_i915_file_private {
  #define HAS_BLT(dev)(INTEL_INFO(dev)-has_blt_ring)
  #define HAS_VEBOX(dev)  (INTEL_INFO(dev)-has_vebox_ring)
  #define HAS_LLC(dev)(INTEL_INFO(dev)-has_llc)
 +#define HAS_WT(dev)(IS_HASWELL(dev)  ((struct drm_i915_private 
 *)(dev))-ellc_size)

-dev_private missing

  #define I915_NEED_GFX_HWS(dev)   (INTEL_INFO(dev)-need_gfx_hws)
  
  #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)-gen = 5)
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index 99362f7..cbea7f8 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -3565,7 +3565,8 @@ i915_gem_object_pin_to_display_plane(struct 
 drm_i915_gem_object *obj,
* of uncaching, which would allow us to flush all the LLC-cached data
* with that bit in the PTE to main memory with just one PIPE_CONTROL.
*/
 - ret = i915_gem_object_set_cache_level(obj, I915_CACHE_NONE);
 + ret = i915_gem_object_set_cache_level(obj,
 +   HAS_WT(obj-base.dev) ? 
 I915_CACHE_WT : I915_CACHE_NONE);

Don't we need to tweak the write domain like we do for UC to make sure
already dirty lines get flushed from caches?

   if (ret)
   return ret;
  
 diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
 b/drivers/gpu/drm/i915/i915_gem_gtt.c
 index 0522d00..072a348 100644
 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
 +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
 @@ -54,6 +54,7 @@
(((bits)  0x8)  (11 - 3)))
  #define HSW_WB_LLC_AGE0  HSW_CACHEABILITY_CONTROL(0x3)
  #define HSW_WB_ELLC_LLC_AGE0 HSW_CACHEABILITY_CONTROL(0xb)
 +#define HSW_WT_ELLC_LLC_AGE0 HSW_CACHEABILITY_CONTROL(0x6)
  
  static gen6_gtt_pte_t gen6_pte_encode(dma_addr_t addr,
 enum i915_cache_level level)
 @@ -116,8 +117,16 @@ static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr,
   gen6_gtt_pte_t pte = GEN6_PTE_VALID;
   pte |= HSW_PTE_ADDR_ENCODE(addr);
  
 - if (level != I915_CACHE_NONE)
 + switch (level) {
 + case I915_CACHE_NONE:
 + break;
 + case I915_CACHE_WT:
 + pte |= HSW_WT_ELLC_LLC_AGE0;
 + break;
 + default:
   pte |= HSW_WB_ELLC_LLC_AGE0;
 + break;
 + }
  
   return pte;
  }
 diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
 index e47cf00..e831292 100644
 --- a/include/uapi/drm/i915_drm.h
 +++ b/include/uapi/drm/i915_drm.h
 @@ -338,6 +338,7 @@ typedef struct drm_i915_irq_wait {
  #define I915_PARAM_HAS_PINNED_BATCHES 24
  #define I915_PARAM_HAS_EXEC_NO_RELOC  25
  #define I915_PARAM_HAS_EXEC_HANDLE_LUT   26
 +#define I915_PARAM_HAS_WT   

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Use Write-Through cacheing for the display plane on Iris

2013-07-30 Thread Chris Wilson
On Tue, Jul 30, 2013 at 08:19:28PM +0300, Ville Syrjälä wrote:
 On Tue, Jul 30, 2013 at 05:58:36PM +0100, Chris Wilson wrote:
  Haswell GT3e has the unique feature of supporting Write-Through cacheing
  of objects within the eLLC. The purpose of this is to enable the display
  plane to remain coherent whilst objects lie resident in the eLLC - so
  that we in theory get the best of both worlds, perfect display and fast
  access.
 
 The description here talks about eLLC only, but you set the PTE for
 WT in LLC/eLLC both.

s/eLLC/LLC/

For some reason, I keep telling myself that it is a magic property of
the eLLC otherwise why wouldn't they do it for all LLC!

  -   ret = i915_gem_object_set_cache_level(obj, I915_CACHE_NONE);
  +   ret = i915_gem_object_set_cache_level(obj,
  + HAS_WT(obj-base.dev) ? 
  I915_CACHE_WT : I915_CACHE_NONE);
 
 Don't we need to tweak the write domain like we do for UC to make sure
 already dirty lines get flushed from caches?

You need to explicitly do the flush, which gets ugly. I choose to ignore
the problem as unlike the LLC - UC transition, I haven't spotted any
dirt. However, it doesn't look too bad if we replace it like so:

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ac1b9cd..0e089e2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3362,27 +3362,32 @@ int i915_gem_object_set_cache_level(struct 
drm_i915_gem_object *obj,
i915_gem_obj_ggtt_set_color(obj, cache_level);
}
 
-   if (cache_level == I915_CACHE_NONE) {
-   u32 old_read_domains, old_write_domain;
-
+   if (cache_level == I915_CACHE_NONE ||
+   cache_level == I915_CACHE_WT) {
/* If we're coming from LLC cached, then we haven't
 * actually been tracking whether the data is in the
 * CPU cache or not, since we only allow one bit set
 * in obj-write_domain and have been skipping the clflushes.
-* Just set it to the CPU cache for now.
+* Do them now.
 */
-   WARN_ON(obj-base.write_domain  ~I915_GEM_DOMAIN_CPU);
-   WARN_ON(obj-base.read_domains  ~I915_GEM_DOMAIN_CPU);
+   if (obj-base.read_domains  I915_GEM_DOMAIN_CPU) {
+   u32 old_read_domains, old_write_domain;
 
-   old_read_domains = obj-base.read_domains;
-   old_write_domain = obj-base.write_domain;
+   BUG_ON(obj-pages == NULL);
 
-   obj-base.read_domains = I915_GEM_DOMAIN_CPU;
-   obj-base.write_domain = I915_GEM_DOMAIN_CPU;
+   trace_i915_gem_object_clflush(obj);
+   drm_clflush_sg(obj-pages);
 
-   trace_i915_gem_object_change_domain(obj,
-   old_read_domains,
-   old_write_domain);
+   old_read_domains = obj-base.read_domains;
+   old_write_domain = obj-base.write_domain;
+
+   obj-base.read_domains = ~I915_GEM_DOMAIN_CPU;
+   obj-base.write_domain = ~I915_GEM_DOMAIN_CPU;
+
+   trace_i915_gem_object_change_domain(obj,
+   old_read_domains,
+   old_write_domain);
+   }
}
 
obj-cache_level = cache_level;

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


Re: [Intel-gfx] [PATCH 1/2] drm/i915: Use the same pte_encoding for ppgtt as for gtt

2013-07-30 Thread Kenneth Graunke

On 07/30/2013 09:58 AM, Chris Wilson wrote:

The PTE layouts are the same for both ppgtt and gtt, so we can simplify
the setup for ppgtt by copying the encoding function pointer from gtt.
This prevents bugs where we update one function pointer, but forget the
other.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ben Widawsky b...@bwidawsk.net
---
  drivers/gpu/drm/i915/i915_gem_gtt.c | 8 +---
  1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 1294cee..0522d00 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -298,13 +298,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 * now. */
first_pd_entry_in_global_pt = gtt_total_entries(dev_priv-gtt);

-   if (IS_HASWELL(dev)) {
-   ppgtt-base.pte_encode = hsw_pte_encode;
-   } else if (IS_VALLEYVIEW(dev)) {
-   ppgtt-base.pte_encode = byt_pte_encode;
-   } else {
-   ppgtt-base.pte_encode = gen6_pte_encode;
-   }
+   ppgtt-base.pte_encode = dev_priv-gtt.base.pte_encode;
ppgtt-num_pd_entries = GEN6_PPGTT_PD_ENTRIES;
ppgtt-enable = gen6_ppgtt_enable;
ppgtt-base.clear_range = gen6_ppgtt_clear_range;


This is much nicer than my old code - thanks!

It might be worth mentioning in the commit message that in particular, 
iris_pte_encode was forgotten here.  Either way,


Reviewed-by: Kenneth Graunke kenn...@whitecape.org

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


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Use Write-Through cacheing for the display plane on Iris

2013-07-30 Thread Kenneth Graunke

On 07/30/2013 09:58 AM, Chris Wilson wrote:

Haswell GT3e has the unique feature of supporting Write-Through cacheing
of objects within the eLLC. The purpose of this is to enable the display
plane to remain coherent whilst objects lie resident in the eLLC - so
that we in theory get the best of both worlds, perfect display and fast
access.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ben Widawsky b...@bwidawsk.net


Awesome.  Thanks a ton for doing this, Chris!

Reviewed-by: Kenneth Graunke kenn...@whitecape.org

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


[Intel-gfx] [PATCH] drm/i915: Use Write-Through cacheing for the display plane on Iris

2013-07-30 Thread Chris Wilson
Haswell GT3e has the unique feature of supporting Write-Through cacheing
of objects within the eLLC/LLC. The purpose of this is to enable the display
plane to remain coherent whilst objects lie resident in the eLLC/LLC - so
that we, in theory, get the best of both worlds, perfect display and fast
access.

v2: Actually do the clflush on transition to WT, nagging by Ville.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Kenneth Graunke kenn...@whitecape.org
---
 drivers/gpu/drm/i915/i915_dma.c |  3 +++
 drivers/gpu/drm/i915/i915_drv.h |  4 +++-
 drivers/gpu/drm/i915/i915_gem.c | 29 +++--
 drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++-
 include/uapi/drm/i915_drm.h |  1 +
 5 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 8da0b3d..75989fc 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -976,6 +976,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
case I915_PARAM_HAS_LLC:
value = HAS_LLC(dev);
break;
+   case I915_PARAM_HAS_WT:
+   value = HAS_WT(dev);
+   break;
case I915_PARAM_HAS_ALIASING_PPGTT:
value = dev_priv-mm.aliasing_ppgtt ? 1 : 0;
break;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 34d2b9d..d27a82a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -452,6 +452,7 @@ enum i915_cache_level {
I915_CACHE_NONE = 0,
I915_CACHE_LLC,
I915_CACHE_LLC_MLC, /* gen6+, in docs at least! */
+   I915_CACHE_WT,
 };
 
 typedef uint32_t gen6_gtt_pte_t;
@@ -1344,7 +1345,7 @@ struct drm_i915_gem_object {
unsigned int pending_fenced_gpu_access:1;
unsigned int fenced_gpu_access:1;
 
-   unsigned int cache_level:2;
+   unsigned int cache_level:3;
 
unsigned int has_aliasing_ppgtt_mapping:1;
unsigned int has_global_gtt_mapping:1;
@@ -1547,6 +1548,7 @@ struct drm_i915_file_private {
 #define HAS_BLT(dev)(INTEL_INFO(dev)-has_blt_ring)
 #define HAS_VEBOX(dev)  (INTEL_INFO(dev)-has_vebox_ring)
 #define HAS_LLC(dev)(INTEL_INFO(dev)-has_llc)
+#define HAS_WT(dev)(IS_HASWELL(dev)  ((struct drm_i915_private 
*)(dev)-dev_private)-ellc_size)
 #define I915_NEED_GFX_HWS(dev) (INTEL_INFO(dev)-need_gfx_hws)
 
 #define HAS_HW_CONTEXTS(dev)   (INTEL_INFO(dev)-gen = 5)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 99362f7..6b33494 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3447,27 +3447,27 @@ int i915_gem_object_set_cache_level(struct 
drm_i915_gem_object *obj,
i915_gem_obj_ggtt_set_color(obj, cache_level);
}
 
-   if (cache_level == I915_CACHE_NONE) {
-   u32 old_read_domains, old_write_domain;
-
+   if (cache_level == I915_CACHE_NONE ||
+   cache_level == I915_CACHE_WT) {
/* If we're coming from LLC cached, then we haven't
 * actually been tracking whether the data is in the
 * CPU cache or not, since we only allow one bit set
 * in obj-write_domain and have been skipping the clflushes.
-* Just set it to the CPU cache for now.
+* Do them now.
 */
-   WARN_ON(obj-base.write_domain  ~I915_GEM_DOMAIN_CPU);
-   WARN_ON(obj-base.read_domains  ~I915_GEM_DOMAIN_CPU);
+   if (obj-pages  !obj-stolen) {
+   u32 old_write_domain;
 
-   old_read_domains = obj-base.read_domains;
-   old_write_domain = obj-base.write_domain;
+   trace_i915_gem_object_clflush(obj);
+   drm_clflush_sg(obj-pages);
 
-   obj-base.read_domains = I915_GEM_DOMAIN_CPU;
-   obj-base.write_domain = I915_GEM_DOMAIN_CPU;
+   old_write_domain = obj-base.write_domain;
+   obj-base.write_domain = ~I915_GEM_DOMAIN_CPU;
 
-   trace_i915_gem_object_change_domain(obj,
-   old_read_domains,
-   old_write_domain);
+   trace_i915_gem_object_change_domain(obj,
+   
obj-base.read_domains,
+   old_write_domain);
+   }
}
 
obj-cache_level = cache_level;
@@ -3565,7 +3565,8 @@ i915_gem_object_pin_to_display_plane(struct 
drm_i915_gem_object *obj,
 * of uncaching, which would allow us to flush all the LLC-cached data
 * with that bit in the PTE to main memory with 

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Use Write-Through cacheing for the display plane on Iris

2013-07-30 Thread Ville Syrjälä
On Tue, Jul 30, 2013 at 06:36:15PM +0100, Chris Wilson wrote:
 On Tue, Jul 30, 2013 at 08:19:28PM +0300, Ville Syrjälä wrote:
  On Tue, Jul 30, 2013 at 05:58:36PM +0100, Chris Wilson wrote:
   Haswell GT3e has the unique feature of supporting Write-Through cacheing
   of objects within the eLLC. The purpose of this is to enable the display
   plane to remain coherent whilst objects lie resident in the eLLC - so
   that we in theory get the best of both worlds, perfect display and fast
   access.
  
  The description here talks about eLLC only, but you set the PTE for
  WT in LLC/eLLC both.
 
 s/eLLC/LLC/
 
 For some reason, I keep telling myself that it is a magic property of
 the eLLC otherwise why wouldn't they do it for all LLC!
 
   - ret = i915_gem_object_set_cache_level(obj, I915_CACHE_NONE);
   + ret = i915_gem_object_set_cache_level(obj,
   +   HAS_WT(obj-base.dev) ? 
   I915_CACHE_WT : I915_CACHE_NONE);
  
  Don't we need to tweak the write domain like we do for UC to make sure
  already dirty lines get flushed from caches?
 
 You need to explicitly do the flush, which gets ugly.

Ah right, because i915_gem_clflush_object() checks the cache_level.

 I choose to ignore
 the problem as unlike the LLC - UC transition, I haven't spotted any
 dirt. However, it doesn't look too bad if we replace it like so:

Hmm, and what about CPU access in general? CPU doesn't know about the
WT policy, so I'm assuming we'd need to flush after all CPU writes.

 
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index ac1b9cd..0e089e2 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -3362,27 +3362,32 @@ int i915_gem_object_set_cache_level(struct 
 drm_i915_gem_object *obj,
   i915_gem_obj_ggtt_set_color(obj, cache_level);
   }
  
 - if (cache_level == I915_CACHE_NONE) {
 - u32 old_read_domains, old_write_domain;
 -
 + if (cache_level == I915_CACHE_NONE ||
 + cache_level == I915_CACHE_WT) {

   /* If we're coming from LLC cached, then we haven't
* actually been tracking whether the data is in the
* CPU cache or not, since we only allow one bit set
* in obj-write_domain and have been skipping the clflushes.
 -  * Just set it to the CPU cache for now.
 +  * Do them now.
*/
 - WARN_ON(obj-base.write_domain  ~I915_GEM_DOMAIN_CPU);
 - WARN_ON(obj-base.read_domains  ~I915_GEM_DOMAIN_CPU);
 + if (obj-base.read_domains  I915_GEM_DOMAIN_CPU) {
 + u32 old_read_domains, old_write_domain;
  
 - old_read_domains = obj-base.read_domains;
 - old_write_domain = obj-base.write_domain;
 + BUG_ON(obj-pages == NULL);
  
 - obj-base.read_domains = I915_GEM_DOMAIN_CPU;
 - obj-base.write_domain = I915_GEM_DOMAIN_CPU;
 + trace_i915_gem_object_clflush(obj);
 + drm_clflush_sg(obj-pages);
  
 - trace_i915_gem_object_change_domain(obj,
 - old_read_domains,
 - old_write_domain);
 + old_read_domains = obj-base.read_domains;
 + old_write_domain = obj-base.write_domain;
 +
 + obj-base.read_domains = ~I915_GEM_DOMAIN_CPU;
 + obj-base.write_domain = ~I915_GEM_DOMAIN_CPU;
 +
 + trace_i915_gem_object_change_domain(obj,
 + old_read_domains,
 + old_write_domain);
 + }
   }
  
   obj-cache_level = cache_level;
 
 -- 
 Chris Wilson, Intel Open Source Technology Centre

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


[Intel-gfx] [PATCH] drm/i915: Use the same pte_encoding for ppgtt as for gtt

2013-07-30 Thread Chris Wilson
The PTE layouts are the same for both ppgtt and gtt, so we can simplify
the setup for ppgtt by copying the encoding function pointer from gtt.
This prevents bugs where we update one function pointer, but forget the
other.

For instance,

commit 4d15c145a6234d999c0452eec0d275c1fbf0688c
Author: Ben Widawsky b...@bwidawsk.net
Date:   Thu Jul 4 11:02:06 2013 -0700

drm/i915: Use eLLC/LLC by default when available

only extends the gtt to use eLLC/LLC cacheing and forgets to also update
the ppgtt function pointer.

v2: Actually mention the bug being fixed (Kenneth)

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ben Widawsky b...@bwidawsk.net
Reviewed-by: Kenneth Graunke kenn...@whitecape.org
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 1294cee..0522d00 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -298,13 +298,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 * now. */
first_pd_entry_in_global_pt = gtt_total_entries(dev_priv-gtt);
 
-   if (IS_HASWELL(dev)) {
-   ppgtt-base.pte_encode = hsw_pte_encode;
-   } else if (IS_VALLEYVIEW(dev)) {
-   ppgtt-base.pte_encode = byt_pte_encode;
-   } else {
-   ppgtt-base.pte_encode = gen6_pte_encode;
-   }
+   ppgtt-base.pte_encode = dev_priv-gtt.base.pte_encode;
ppgtt-num_pd_entries = GEN6_PPGTT_PD_ENTRIES;
ppgtt-enable = gen6_ppgtt_enable;
ppgtt-base.clear_range = gen6_ppgtt_clear_range;
-- 
1.8.3.2

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


Re: [Intel-gfx] [PATCH 2/2] drm/i915: Use Write-Through cacheing for the display plane on Iris

2013-07-30 Thread Chris Wilson
On Tue, Jul 30, 2013 at 09:01:22PM +0300, Ville Syrjälä wrote:
 Hmm, and what about CPU access in general? CPU doesn't know about the
 WT policy, so I'm assuming we'd need to flush after all CPU writes.

Hmm, I had assumed the opposite. I've not yet seen signs of cache dirt.
Certainly GTT access should fine, and CPU reads. It is just those CPU
writes we need to worry about, and now I do indeed worry.
-Chris

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


Re: [Intel-gfx] [PATCH 01/35] drm/i915: Add scaled paramater to update_sprite_watermarks()

2013-07-30 Thread Paulo Zanoni
2013/7/5  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Fro calculating watermarks we want to know whether sprites are

Fro

 scaled. Pass that information to update_sprite_watermarks() so that
 eventually we may do some watermark pre-computing.

On this patch you're also renaming some variables from enable to
enabled, but not all of them. You should probably either rename them
all, or none. Example: intel_update_sprite_watermarks definition at
intel_drv.h says bool enabled, but the implementation inside
intel_pm.c says bool enable, but there are also other examples.

Besides the styling detail the patch looks correct, so if Daniel/Ville
consider my suggestion is just a bikeshed, Reviewed-by: Paulo Zanoni
paulo.r.zan...@intel.com.

And another bikeshed would be to create variables called scaled
inside the update_plane funcs :)


 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.h |  2 +-
  drivers/gpu/drm/i915/intel_drv.h|  7 ---
  drivers/gpu/drm/i915/intel_pm.c | 13 +++--
  drivers/gpu/drm/i915/intel_sprite.c | 11 +++
  4 files changed, 19 insertions(+), 14 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index fd0f589..99eb980 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -361,7 +361,7 @@ struct drm_i915_display_funcs {
 void (*update_wm)(struct drm_device *dev);
 void (*update_sprite_wm)(struct drm_device *dev, int pipe,
  uint32_t sprite_width, int pixel_size,
 -bool enable);
 +bool enable, bool scaled);
 void (*modeset_global_resources)(struct drm_device *dev);
 /* Returns the active state of the crtc, and if the crtc is active,
  * fills out the pipe-config with the hw state. */
 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index 5dfc1a0..3371ecc 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -353,7 +353,8 @@ struct intel_plane {
  * for the watermark calculations. Currently only Haswell uses this.
  */
 struct {
 -   bool enable;
 +   bool enabled;
 +   bool scaled;
 uint8_t bytes_per_pixel;
 uint32_t horiz_pixels;
 } wm;
 @@ -772,8 +773,8 @@ extern void intel_ddi_init(struct drm_device *dev, enum 
 port port);
  /* For use by IVB LP watermark workaround in intel_sprite.c */
  extern void intel_update_watermarks(struct drm_device *dev);
  extern void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
 -  uint32_t sprite_width,
 -  int pixel_size, bool enable);
 +  uint32_t sprite_width, int 
 pixel_size,
 +  bool enabled, bool scaled);

  extern unsigned long intel_gen4_compute_page_offset(int *x, int *y,
 unsigned int tiling_mode,
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 7cfd3b7..beca186 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2388,7 +2388,7 @@ static void hsw_compute_wm_parameters(struct drm_device 
 *dev,
 pipe = intel_plane-pipe;
 p = params[pipe];

 -   p-sprite_enabled = intel_plane-wm.enable;
 +   p-sprite_enabled = intel_plane-wm.enabled;
 p-spr_bytes_per_pixel = intel_plane-wm.bytes_per_pixel;
 p-spr_horiz_pixels = intel_plane-wm.horiz_pixels;

 @@ -2616,7 +2616,7 @@ static void haswell_update_wm(struct drm_device *dev)

  static void haswell_update_sprite_wm(struct drm_device *dev, int pipe,
  uint32_t sprite_width, int pixel_size,
 -bool enable)
 +bool enabled, bool scaled)
  {
 struct drm_plane *plane;

 @@ -2624,7 +2624,8 @@ static void haswell_update_sprite_wm(struct drm_device 
 *dev, int pipe,
 struct intel_plane *intel_plane = to_intel_plane(plane);

 if (intel_plane-pipe == pipe) {
 -   intel_plane-wm.enable = enable;
 +   intel_plane-wm.enabled = enabled;
 +   intel_plane-wm.scaled = scaled;
 intel_plane-wm.horiz_pixels = sprite_width + 1;
 intel_plane-wm.bytes_per_pixel = pixel_size;
 break;
 @@ -2712,7 +2713,7 @@ sandybridge_compute_sprite_srwm(struct drm_device *dev, 
 int plane,

  static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
 

Re: [Intel-gfx] [PATCH 2/2] drm/i915: Use Write-Through cacheing for the display plane on Iris

2013-07-30 Thread Chris Wilson
On Tue, Jul 30, 2013 at 07:10:03PM +0100, Chris Wilson wrote:
 On Tue, Jul 30, 2013 at 09:01:22PM +0300, Ville Syrjälä wrote:
  Hmm, and what about CPU access in general? CPU doesn't know about the
  WT policy, so I'm assuming we'd need to flush after all CPU writes.
 
 Hmm, I had assumed the opposite. I've not yet seen signs of cache dirt.
 Certainly GTT access should fine, and CPU reads. It is just those CPU
 writes we need to worry about, and now I do indeed worry.

Ok, didn't take too long to force a few updates through CPU mappings to
a WT display, and yes the CPU writes are incoherent. So we need to be
much more careful with the flushes here.
-Chris

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


Re: [Intel-gfx] [PATCH 01/35] drm/i915: Add scaled paramater to update_sprite_watermarks()

2013-07-30 Thread Ville Syrjälä
On Tue, Jul 30, 2013 at 03:26:12PM -0300, Paulo Zanoni wrote:
 2013/7/5  ville.syrj...@linux.intel.com:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
 
  Fro calculating watermarks we want to know whether sprites are
 
 Fro
 
  scaled. Pass that information to update_sprite_watermarks() so that
  eventually we may do some watermark pre-computing.
 
 On this patch you're also renaming some variables from enable to
 enabled, but not all of them. You should probably either rename them
 all, or none. Example: intel_update_sprite_watermarks definition at
 intel_drv.h says bool enabled, but the implementation inside
 intel_pm.c says bool enable, but there are also other examples.

The idea was to use enabled consistently, but it seems I messed up.
I can fix that up.

 
 Besides the styling detail the patch looks correct, so if Daniel/Ville
 consider my suggestion is just a bikeshed, Reviewed-by: Paulo Zanoni
 paulo.r.zan...@intel.com.
 
 And another bikeshed would be to create variables called scaled
 inside the update_plane funcs :)

You mean bool scaled = crtc_w != src_w || crtc_h != src_h; or so?

Yeah I suppose could make the code a bit easier to parse.

 
 
  Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
  ---
   drivers/gpu/drm/i915/i915_drv.h |  2 +-
   drivers/gpu/drm/i915/intel_drv.h|  7 ---
   drivers/gpu/drm/i915/intel_pm.c | 13 +++--
   drivers/gpu/drm/i915/intel_sprite.c | 11 +++
   4 files changed, 19 insertions(+), 14 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_drv.h 
  b/drivers/gpu/drm/i915/i915_drv.h
  index fd0f589..99eb980 100644
  --- a/drivers/gpu/drm/i915/i915_drv.h
  +++ b/drivers/gpu/drm/i915/i915_drv.h
  @@ -361,7 +361,7 @@ struct drm_i915_display_funcs {
  void (*update_wm)(struct drm_device *dev);
  void (*update_sprite_wm)(struct drm_device *dev, int pipe,
   uint32_t sprite_width, int pixel_size,
  -bool enable);
  +bool enable, bool scaled);
  void (*modeset_global_resources)(struct drm_device *dev);
  /* Returns the active state of the crtc, and if the crtc is active,
   * fills out the pipe-config with the hw state. */
  diff --git a/drivers/gpu/drm/i915/intel_drv.h 
  b/drivers/gpu/drm/i915/intel_drv.h
  index 5dfc1a0..3371ecc 100644
  --- a/drivers/gpu/drm/i915/intel_drv.h
  +++ b/drivers/gpu/drm/i915/intel_drv.h
  @@ -353,7 +353,8 @@ struct intel_plane {
   * for the watermark calculations. Currently only Haswell uses this.
   */
  struct {
  -   bool enable;
  +   bool enabled;
  +   bool scaled;
  uint8_t bytes_per_pixel;
  uint32_t horiz_pixels;
  } wm;
  @@ -772,8 +773,8 @@ extern void intel_ddi_init(struct drm_device *dev, enum 
  port port);
   /* For use by IVB LP watermark workaround in intel_sprite.c */
   extern void intel_update_watermarks(struct drm_device *dev);
   extern void intel_update_sprite_watermarks(struct drm_device *dev, int 
  pipe,
  -  uint32_t sprite_width,
  -  int pixel_size, bool enable);
  +  uint32_t sprite_width, int 
  pixel_size,
  +  bool enabled, bool scaled);
 
   extern unsigned long intel_gen4_compute_page_offset(int *x, int *y,
  unsigned int 
  tiling_mode,
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index 7cfd3b7..beca186 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -2388,7 +2388,7 @@ static void hsw_compute_wm_parameters(struct 
  drm_device *dev,
  pipe = intel_plane-pipe;
  p = params[pipe];
 
  -   p-sprite_enabled = intel_plane-wm.enable;
  +   p-sprite_enabled = intel_plane-wm.enabled;
  p-spr_bytes_per_pixel = intel_plane-wm.bytes_per_pixel;
  p-spr_horiz_pixels = intel_plane-wm.horiz_pixels;
 
  @@ -2616,7 +2616,7 @@ static void haswell_update_wm(struct drm_device *dev)
 
   static void haswell_update_sprite_wm(struct drm_device *dev, int pipe,
   uint32_t sprite_width, int pixel_size,
  -bool enable)
  +bool enabled, bool scaled)
   {
  struct drm_plane *plane;
 
  @@ -2624,7 +2624,8 @@ static void haswell_update_sprite_wm(struct 
  drm_device *dev, int pipe,
  struct intel_plane *intel_plane = to_intel_plane(plane);
 
  if (intel_plane-pipe == pipe) {
  -   intel_plane-wm.enable = enable;
  +   intel_plane-wm.enabled = enabled;
  +   

Re: [Intel-gfx] [PATCH 02/35] drm/i915: Pass the actual sprite width to watermarks functions

2013-07-30 Thread Paulo Zanoni
2013/7/5  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Don't subtract one from the sprite width before watermark calculations.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

Looks like the bikeshed I just gave about the scaled variable would
cause the need to rebase this patch. So just forget it :)

 ---
  drivers/gpu/drm/i915/intel_pm.c |  2 +-
  drivers/gpu/drm/i915/intel_sprite.c | 18 +-
  2 files changed, 10 insertions(+), 10 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index beca186..db548a1 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2626,7 +2626,7 @@ static void haswell_update_sprite_wm(struct drm_device 
 *dev, int pipe,
 if (intel_plane-pipe == pipe) {
 intel_plane-wm.enabled = enabled;
 intel_plane-wm.scaled = scaled;
 -   intel_plane-wm.horiz_pixels = sprite_width + 1;
 +   intel_plane-wm.horiz_pixels = sprite_width;
 intel_plane-wm.bytes_per_pixel = pixel_size;
 break;
 }
 diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
 b/drivers/gpu/drm/i915/intel_sprite.c
 index 5a1f3fd..d7fca56 100644
 --- a/drivers/gpu/drm/i915/intel_sprite.c
 +++ b/drivers/gpu/drm/i915/intel_sprite.c
 @@ -108,15 +108,15 @@ vlv_update_plane(struct drm_plane *dplane, struct 
 drm_framebuffer *fb,

 sprctl |= SP_ENABLE;

 +   intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
 +  src_w != crtc_w || src_h != crtc_h);
 +
 /* Sizes are 0 based */
 src_w--;
 src_h--;
 crtc_w--;
 crtc_h--;

 -   intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
 -  src_w != crtc_w || src_h != crtc_h);
 -
 I915_WRITE(SPSTRIDE(pipe, plane), fb-pitches[0]);
 I915_WRITE(SPPOS(pipe, plane), (crtc_y  16) | crtc_x);

 @@ -263,15 +263,15 @@ ivb_update_plane(struct drm_plane *plane, struct 
 drm_framebuffer *fb,
 if (IS_HASWELL(dev))
 sprctl |= SPRITE_PIPE_CSC_ENABLE;

 +   intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
 +  src_w != crtc_w || src_h != crtc_h);
 +
 /* Sizes are 0 based */
 src_w--;
 src_h--;
 crtc_w--;
 crtc_h--;

 -   intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
 -  src_w != crtc_w || src_h != crtc_h);
 -
 /*
  * IVB workaround: must disable low power watermarks for at least
  * one frame before enabling scaling.  LP watermarks can be re-enabled
 @@ -451,15 +451,15 @@ ilk_update_plane(struct drm_plane *plane, struct 
 drm_framebuffer *fb,
 dvscntr |= DVS_TRICKLE_FEED_DISABLE; /* must disable */
 dvscntr |= DVS_ENABLE;

 +   intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
 +  src_w != crtc_w || src_h != crtc_h);
 +
 /* Sizes are 0 based */
 src_w--;
 src_h--;
 crtc_w--;
 crtc_h--;

 -   intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
 -  src_w != crtc_w || src_h != crtc_h);
 -
 dvsscale = 0;
 if (IS_GEN5(dev) || crtc_w != src_w || crtc_h != src_h)
 dvsscale = DVS_SCALE_ENABLE | (src_w  16) | src_h;
 --
 1.8.1.5

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



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


Re: [Intel-gfx] [PATCH 01/35] drm/i915: Add scaled paramater to update_sprite_watermarks()

2013-07-30 Thread Paulo Zanoni
2013/7/30 Ville Syrjälä ville.syrj...@linux.intel.com:
 On Tue, Jul 30, 2013 at 03:26:12PM -0300, Paulo Zanoni wrote:
 2013/7/5  ville.syrj...@linux.intel.com:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
 
  Fro calculating watermarks we want to know whether sprites are

 Fro

  scaled. Pass that information to update_sprite_watermarks() so that
  eventually we may do some watermark pre-computing.

 On this patch you're also renaming some variables from enable to
 enabled, but not all of them. You should probably either rename them
 all, or none. Example: intel_update_sprite_watermarks definition at
 intel_drv.h says bool enabled, but the implementation inside
 intel_pm.c says bool enable, but there are also other examples.

 The idea was to use enabled consistently, but it seems I messed up.
 I can fix that up.


 Besides the styling detail the patch looks correct, so if Daniel/Ville
 consider my suggestion is just a bikeshed, Reviewed-by: Paulo Zanoni
 paulo.r.zan...@intel.com.

 And another bikeshed would be to create variables called scaled
 inside the update_plane funcs :)

 You mean bool scaled = crtc_w != src_w || crtc_h != src_h; or so?

Yes, but I just realized that will trigger a rebase of many other
patches of your series... So you may skip it if you want. Maybe do it
in the end, or not at all.


 Yeah I suppose could make the code a bit easier to parse.


 
  Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
  ---
   drivers/gpu/drm/i915/i915_drv.h |  2 +-
   drivers/gpu/drm/i915/intel_drv.h|  7 ---
   drivers/gpu/drm/i915/intel_pm.c | 13 +++--
   drivers/gpu/drm/i915/intel_sprite.c | 11 +++
   4 files changed, 19 insertions(+), 14 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_drv.h 
  b/drivers/gpu/drm/i915/i915_drv.h
  index fd0f589..99eb980 100644
  --- a/drivers/gpu/drm/i915/i915_drv.h
  +++ b/drivers/gpu/drm/i915/i915_drv.h
  @@ -361,7 +361,7 @@ struct drm_i915_display_funcs {
  void (*update_wm)(struct drm_device *dev);
  void (*update_sprite_wm)(struct drm_device *dev, int pipe,
   uint32_t sprite_width, int pixel_size,
  -bool enable);
  +bool enable, bool scaled);
  void (*modeset_global_resources)(struct drm_device *dev);
  /* Returns the active state of the crtc, and if the crtc is active,
   * fills out the pipe-config with the hw state. */
  diff --git a/drivers/gpu/drm/i915/intel_drv.h 
  b/drivers/gpu/drm/i915/intel_drv.h
  index 5dfc1a0..3371ecc 100644
  --- a/drivers/gpu/drm/i915/intel_drv.h
  +++ b/drivers/gpu/drm/i915/intel_drv.h
  @@ -353,7 +353,8 @@ struct intel_plane {
   * for the watermark calculations. Currently only Haswell uses 
  this.
   */
  struct {
  -   bool enable;
  +   bool enabled;
  +   bool scaled;
  uint8_t bytes_per_pixel;
  uint32_t horiz_pixels;
  } wm;
  @@ -772,8 +773,8 @@ extern void intel_ddi_init(struct drm_device *dev, 
  enum port port);
   /* For use by IVB LP watermark workaround in intel_sprite.c */
   extern void intel_update_watermarks(struct drm_device *dev);
   extern void intel_update_sprite_watermarks(struct drm_device *dev, int 
  pipe,
  -  uint32_t sprite_width,
  -  int pixel_size, bool enable);
  +  uint32_t sprite_width, int 
  pixel_size,
  +  bool enabled, bool scaled);
 
   extern unsigned long intel_gen4_compute_page_offset(int *x, int *y,
  unsigned int 
  tiling_mode,
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index 7cfd3b7..beca186 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -2388,7 +2388,7 @@ static void hsw_compute_wm_parameters(struct 
  drm_device *dev,
  pipe = intel_plane-pipe;
  p = params[pipe];
 
  -   p-sprite_enabled = intel_plane-wm.enable;
  +   p-sprite_enabled = intel_plane-wm.enabled;
  p-spr_bytes_per_pixel = intel_plane-wm.bytes_per_pixel;
  p-spr_horiz_pixels = intel_plane-wm.horiz_pixels;
 
  @@ -2616,7 +2616,7 @@ static void haswell_update_wm(struct drm_device *dev)
 
   static void haswell_update_sprite_wm(struct drm_device *dev, int pipe,
   uint32_t sprite_width, int pixel_size,
  -bool enable)
  +bool enabled, bool scaled)
   {
  struct drm_plane *plane;
 
  @@ -2624,7 +2624,8 @@ static void haswell_update_sprite_wm(struct 
  drm_device *dev, int pipe,
  struct intel_plane *intel_plane = 

Re: [Intel-gfx] [PATCH 03/35] drm/i915: Calculate the sprite WM based on the source width instead of the destination width

2013-07-30 Thread Paulo Zanoni
2013/7/5  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Using the destination width in the sprite WM calculations isn't correct.
 We should be using the source width.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com

Seems correct according to the IVB WM calculator. It is also worth
mentioning that this doesn't affect HSW watermarks since HSW doesn't
support sprite scaling.

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

 ---
  drivers/gpu/drm/i915/intel_sprite.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
 b/drivers/gpu/drm/i915/intel_sprite.c
 index d7fca56..7c6cce7 100644
 --- a/drivers/gpu/drm/i915/intel_sprite.c
 +++ b/drivers/gpu/drm/i915/intel_sprite.c
 @@ -108,7 +108,7 @@ vlv_update_plane(struct drm_plane *dplane, struct 
 drm_framebuffer *fb,

 sprctl |= SP_ENABLE;

 -   intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
 +   intel_update_sprite_watermarks(dev, pipe, src_w, pixel_size, true,
src_w != crtc_w || src_h != crtc_h);

 /* Sizes are 0 based */
 @@ -263,7 +263,7 @@ ivb_update_plane(struct drm_plane *plane, struct 
 drm_framebuffer *fb,
 if (IS_HASWELL(dev))
 sprctl |= SPRITE_PIPE_CSC_ENABLE;

 -   intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
 +   intel_update_sprite_watermarks(dev, pipe, src_w, pixel_size, true,
src_w != crtc_w || src_h != crtc_h);

 /* Sizes are 0 based */
 @@ -451,7 +451,7 @@ ilk_update_plane(struct drm_plane *plane, struct 
 drm_framebuffer *fb,
 dvscntr |= DVS_TRICKLE_FEED_DISABLE; /* must disable */
 dvscntr |= DVS_ENABLE;

 -   intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true,
 +   intel_update_sprite_watermarks(dev, pipe, src_w, pixel_size, true,
src_w != crtc_w || src_h != crtc_h);

 /* Sizes are 0 based */
 --
 1.8.1.5

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



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


Re: [Intel-gfx] [PATCH 04/35] drm/i915: Rename hsw_wm_get_pixel_rate to ilk_pipe_pixel_rate

2013-07-30 Thread Paulo Zanoni
2013/7/5  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 hsw_wm_get_pixel_rate() isn't specific to HSW. In fact it should be made
 to handle all gens, but for now it depends on the PCH panel fitter
 state, so give it an ilk_ prefix.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com

I couldn't find the ILK docs to check this, but SNB/IVB seems correct.

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

 ---
  drivers/gpu/drm/i915/intel_pm.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index db548a1..b8fde13 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2080,8 +2080,8 @@ static void ivybridge_update_wm(struct drm_device *dev)
cursor_wm);
  }

 -static uint32_t hsw_wm_get_pixel_rate(struct drm_device *dev,
 - struct drm_crtc *crtc)
 +static uint32_t ilk_pipe_pixel_rate(struct drm_device *dev,
 +   struct drm_crtc *crtc)
  {
 struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 uint32_t pixel_rate, pfit_size;
 @@ -2373,7 +2373,7 @@ static void hsw_compute_wm_parameters(struct drm_device 
 *dev,
 pipes_active++;

 p-pipe_htotal = intel_crtc-config.adjusted_mode.htotal;
 -   p-pixel_rate = hsw_wm_get_pixel_rate(dev, crtc);
 +   p-pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
 p-pri_bytes_per_pixel = crtc-fb-bits_per_pixel / 8;
 p-cur_bytes_per_pixel = 4;
 p-pri_horiz_pixels =
 --
 1.8.1.5

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



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


[Intel-gfx] [PATCH] drm/i915: Use Write-Through cacheing for the display plane on Iris

2013-07-30 Thread Chris Wilson
Haswell GT3e has the unique feature of supporting Write-Through cacheing
of objects within the eLLC/LLC. The purpose of this is to enable the display
plane to remain coherent whilst objects lie resident in the eLLC/LLC - so
that we, in theory, get the best of both worlds, perfect display and fast
access.

However, we still need to be careful as the CPU does not see the WT when
accessing the cache. In particular, this means that we need to flush the
cache lines after writing to an object through the CPU, and on
transitioning from a cached state to WT.

v2: Actually do the clflush on transition to WT, nagging by Ville.
v3: Flush the CPU cache after writes into WT objects.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Cc: Kenneth Graunke kenn...@whitecape.org
---
 drivers/gpu/drm/i915/i915_dma.c |  3 +++
 drivers/gpu/drm/i915/i915_drv.h |  4 +++-
 drivers/gpu/drm/i915/i915_gem.c | 38 -
 drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++-
 include/uapi/drm/i915_drm.h |  1 +
 5 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 8da0b3d..75989fc 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -976,6 +976,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
case I915_PARAM_HAS_LLC:
value = HAS_LLC(dev);
break;
+   case I915_PARAM_HAS_WT:
+   value = HAS_WT(dev);
+   break;
case I915_PARAM_HAS_ALIASING_PPGTT:
value = dev_priv-mm.aliasing_ppgtt ? 1 : 0;
break;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 34d2b9d..d27a82a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -452,6 +452,7 @@ enum i915_cache_level {
I915_CACHE_NONE = 0,
I915_CACHE_LLC,
I915_CACHE_LLC_MLC, /* gen6+, in docs at least! */
+   I915_CACHE_WT,
 };
 
 typedef uint32_t gen6_gtt_pte_t;
@@ -1344,7 +1345,7 @@ struct drm_i915_gem_object {
unsigned int pending_fenced_gpu_access:1;
unsigned int fenced_gpu_access:1;
 
-   unsigned int cache_level:2;
+   unsigned int cache_level:3;
 
unsigned int has_aliasing_ppgtt_mapping:1;
unsigned int has_global_gtt_mapping:1;
@@ -1547,6 +1548,7 @@ struct drm_i915_file_private {
 #define HAS_BLT(dev)(INTEL_INFO(dev)-has_blt_ring)
 #define HAS_VEBOX(dev)  (INTEL_INFO(dev)-has_vebox_ring)
 #define HAS_LLC(dev)(INTEL_INFO(dev)-has_llc)
+#define HAS_WT(dev)(IS_HASWELL(dev)  ((struct drm_i915_private 
*)(dev)-dev_private)-ellc_size)
 #define I915_NEED_GFX_HWS(dev) (INTEL_INFO(dev)-need_gfx_hws)
 
 #define HAS_HW_CONTEXTS(dev)   (INTEL_INFO(dev)-gen = 5)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 99362f7..1c5bcc7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3286,11 +3286,13 @@ i915_gem_clflush_object(struct drm_i915_gem_object *obj)
 * snooping behaviour occurs naturally as the result of our domain
 * tracking.
 */
-   if (obj-cache_level != I915_CACHE_NONE)
+   switch (obj-cache_level) {
+   case I915_CACHE_LLC:
+   case I915_CACHE_LLC_MLC:
return;
+   }
 
trace_i915_gem_object_clflush(obj);
-
drm_clflush_sg(obj-pages);
 }
 
@@ -3447,27 +3449,27 @@ int i915_gem_object_set_cache_level(struct 
drm_i915_gem_object *obj,
i915_gem_obj_ggtt_set_color(obj, cache_level);
}
 
-   if (cache_level == I915_CACHE_NONE) {
-   u32 old_read_domains, old_write_domain;
-
+   if (cache_level == I915_CACHE_NONE ||
+   cache_level == I915_CACHE_WT) {
/* If we're coming from LLC cached, then we haven't
 * actually been tracking whether the data is in the
 * CPU cache or not, since we only allow one bit set
 * in obj-write_domain and have been skipping the clflushes.
-* Just set it to the CPU cache for now.
+* Do them now.
 */
-   WARN_ON(obj-base.write_domain  ~I915_GEM_DOMAIN_CPU);
-   WARN_ON(obj-base.read_domains  ~I915_GEM_DOMAIN_CPU);
+   if (obj-pages  !obj-stolen) {
+   u32 old_write_domain;
 
-   old_read_domains = obj-base.read_domains;
-   old_write_domain = obj-base.write_domain;
+   trace_i915_gem_object_clflush(obj);
+   drm_clflush_sg(obj-pages);
 
-   obj-base.read_domains = I915_GEM_DOMAIN_CPU;
-   obj-base.write_domain = I915_GEM_DOMAIN_CPU;
+   old_write_domain = obj-base.write_domain;
+   

Re: [Intel-gfx] [PATCH 05/35] drm/i915: Rename most wm compute functions to ilk_ prefix

2013-07-30 Thread Paulo Zanoni
2013/7/5  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 These functions are appropriate for everything since ILK.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com

Again, I don't have the ILK docs, so this is just reviewed for SNB/IVB:

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com (SNB/IVB only)

 ---
  drivers/gpu/drm/i915/intel_pm.c | 40 
  1 file changed, 20 insertions(+), 20 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index b8fde13..6b820c4 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2111,7 +2111,7 @@ static uint32_t ilk_pipe_pixel_rate(struct drm_device 
 *dev,
 return pixel_rate;
  }

 -static uint32_t hsw_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
 +static uint32_t ilk_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
uint32_t latency)
  {
 uint64_t ret;
 @@ -2122,7 +2122,7 @@ static uint32_t hsw_wm_method1(uint32_t pixel_rate, 
 uint8_t bytes_per_pixel,
 return ret;
  }

 -static uint32_t hsw_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
 +static uint32_t ilk_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
uint32_t horiz_pixels, uint8_t bytes_per_pixel,
uint32_t latency)
  {
 @@ -2134,7 +2134,7 @@ static uint32_t hsw_wm_method2(uint32_t pixel_rate, 
 uint32_t pipe_htotal,
 return ret;
  }

 -static uint32_t hsw_wm_fbc(uint32_t pri_val, uint32_t horiz_pixels,
 +static uint32_t ilk_wm_fbc(uint32_t pri_val, uint32_t horiz_pixels,
uint8_t bytes_per_pixel)
  {
 return DIV_ROUND_UP(pri_val * 64, horiz_pixels * bytes_per_pixel) + 2;
 @@ -2183,7 +2183,7 @@ enum hsw_data_buf_partitioning {
  };

  /* For both WM_PIPE and WM_LP. */
 -static uint32_t hsw_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
 +static uint32_t ilk_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
uint32_t mem_value,
bool is_lp)
  {
 @@ -2193,14 +2193,14 @@ static uint32_t hsw_compute_pri_wm(struct 
 hsw_pipe_wm_parameters *params,
 if (!params-active)
 return 0;

 -   method1 = hsw_wm_method1(params-pixel_rate,
 +   method1 = ilk_wm_method1(params-pixel_rate,
  params-pri_bytes_per_pixel,
  mem_value);

 if (!is_lp)
 return method1;

 -   method2 = hsw_wm_method2(params-pixel_rate,
 +   method2 = ilk_wm_method2(params-pixel_rate,
  params-pipe_htotal,
  params-pri_horiz_pixels,
  params-pri_bytes_per_pixel,
 @@ -2210,7 +2210,7 @@ static uint32_t hsw_compute_pri_wm(struct 
 hsw_pipe_wm_parameters *params,
  }

  /* For both WM_PIPE and WM_LP. */
 -static uint32_t hsw_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
 +static uint32_t ilk_compute_spr_wm(struct hsw_pipe_wm_parameters *params,
uint32_t mem_value)
  {
 uint32_t method1, method2;
 @@ -2218,10 +2218,10 @@ static uint32_t hsw_compute_spr_wm(struct 
 hsw_pipe_wm_parameters *params,
 if (!params-active || !params-sprite_enabled)
 return 0;

 -   method1 = hsw_wm_method1(params-pixel_rate,
 +   method1 = ilk_wm_method1(params-pixel_rate,
  params-spr_bytes_per_pixel,
  mem_value);
 -   method2 = hsw_wm_method2(params-pixel_rate,
 +   method2 = ilk_wm_method2(params-pixel_rate,
  params-pipe_htotal,
  params-spr_horiz_pixels,
  params-spr_bytes_per_pixel,
 @@ -2230,13 +2230,13 @@ static uint32_t hsw_compute_spr_wm(struct 
 hsw_pipe_wm_parameters *params,
  }

  /* For both WM_PIPE and WM_LP. */
 -static uint32_t hsw_compute_cur_wm(struct hsw_pipe_wm_parameters *params,
 +static uint32_t ilk_compute_cur_wm(struct hsw_pipe_wm_parameters *params,
uint32_t mem_value)
  {
 if (!params-active)
 return 0;

 -   return hsw_wm_method2(params-pixel_rate,
 +   return ilk_wm_method2(params-pixel_rate,
   params-pipe_htotal,
   params-cur_horiz_pixels,
   params-cur_bytes_per_pixel,
 @@ -2244,14 +2244,14 @@ static uint32_t hsw_compute_cur_wm(struct 
 hsw_pipe_wm_parameters *params,
  }

  /* Only for WM_LP. */
 -static uint32_t hsw_compute_fbc_wm(struct hsw_pipe_wm_parameters *params,
 +static uint32_t ilk_compute_fbc_wm(struct hsw_pipe_wm_parameters *params,
uint32_t pri_val,
  

Re: [Intel-gfx] [PATCH 06/35] drm/i915: Pass the watermark level to primary WM compute functions

2013-07-30 Thread Paulo Zanoni
2013/7/5  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Passing the level insted of is_lp seems easier. The end result is the
 same though.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_pm.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 6b820c4..f178e26 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2185,7 +2185,7 @@ enum hsw_data_buf_partitioning {
  /* For both WM_PIPE and WM_LP. */
  static uint32_t ilk_compute_pri_wm(struct hsw_pipe_wm_parameters *params,
uint32_t mem_value,
 -  bool is_lp)
 +  int level)
  {
 uint32_t method1, method2;

 @@ -2197,7 +2197,7 @@ static uint32_t ilk_compute_pri_wm(struct 
 hsw_pipe_wm_parameters *params,
  params-pri_bytes_per_pixel,
  mem_value);

 -   if (!is_lp)
 +   if (level == 0)
 return method1;

 method2 = ilk_wm_method2(params-pixel_rate,
 @@ -2266,7 +2266,7 @@ static bool hsw_compute_lp_wm(uint32_t mem_value, 
 struct hsw_wm_maximums *max,
 for (pipe = PIPE_A; pipe = PIPE_C; pipe++) {
 struct hsw_pipe_wm_parameters *p = params[pipe];

 -   pri_val[pipe] = ilk_compute_pri_wm(p, mem_value, true);
 +   pri_val[pipe] = ilk_compute_pri_wm(p, mem_value, 1);

But now even when you're processing levels 2/3/4 you're telling the
function that you're doing level 1, which is not true. We don't know
what kind of code changes we'll be doing in January 2023, so we may
use that incorrect level variable to do something wrong. IMHO,
either we keep the current code or we pass the actual level.


 spr_val[pipe] = ilk_compute_spr_wm(p, mem_value);
 cur_val[pipe] = ilk_compute_cur_wm(p, mem_value);
 fbc_val[pipe] = ilk_compute_fbc_wm(p, pri_val[pipe], 
 mem_value);
 @@ -2296,7 +2296,7 @@ static uint32_t hsw_compute_wm_pipe(struct 
 drm_i915_private *dev_priv,
  {
 uint32_t pri_val, cur_val, spr_val;

 -   pri_val = ilk_compute_pri_wm(params, mem_value, false);
 +   pri_val = ilk_compute_pri_wm(params, mem_value, 0);
 spr_val = ilk_compute_spr_wm(params, mem_value);
 cur_val = ilk_compute_cur_wm(params, mem_value);

 --
 1.8.1.5

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



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


Re: [Intel-gfx] [PATCH 07/35] drm/i915: Don't pass mem_value to ilk_compute_fbc_wm

2013-07-30 Thread Paulo Zanoni
2013/7/5  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 The FBC watermark doesn't depend on the latency value, so no point in
 passing it in.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com

/me hides from the git blame

It does depend, but indirectly, through pri_val :P

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com

 ---
  drivers/gpu/drm/i915/intel_pm.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index f178e26..981416c 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2245,8 +2245,7 @@ static uint32_t ilk_compute_cur_wm(struct 
 hsw_pipe_wm_parameters *params,

  /* Only for WM_LP. */
  static uint32_t ilk_compute_fbc_wm(struct hsw_pipe_wm_parameters *params,
 -  uint32_t pri_val,
 -  uint32_t mem_value)
 +  uint32_t pri_val)
  {
 if (!params-active)
 return 0;
 @@ -2269,7 +2268,7 @@ static bool hsw_compute_lp_wm(uint32_t mem_value, 
 struct hsw_wm_maximums *max,
 pri_val[pipe] = ilk_compute_pri_wm(p, mem_value, 1);
 spr_val[pipe] = ilk_compute_spr_wm(p, mem_value);
 cur_val[pipe] = ilk_compute_cur_wm(p, mem_value);
 -   fbc_val[pipe] = ilk_compute_fbc_wm(p, pri_val[pipe], 
 mem_value);
 +   fbc_val[pipe] = ilk_compute_fbc_wm(p, pri_val[pipe]);
 }

 result-pri_val = max3(pri_val[0], pri_val[1], pri_val[2]);
 --
 1.8.1.5

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



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


Re: [Intel-gfx] [PATCH 08/35] drm/i915: Change the watermark latency type to uint16_t

2013-07-30 Thread Paulo Zanoni
2013/7/5  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 The latency values fit in uint16_t, so let's save a few bytes.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com


 ---
  drivers/gpu/drm/i915/intel_pm.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 981416c..2239cdb 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2338,7 +2338,7 @@ hsw_compute_linetime_wm(struct drm_device *dev, struct 
 drm_crtc *crtc)

  static void hsw_compute_wm_parameters(struct drm_device *dev,
   struct hsw_pipe_wm_parameters *params,
 - uint32_t *wm,
 + uint16_t *wm,
   struct hsw_wm_maximums *lp_max_1_2,
   struct hsw_wm_maximums *lp_max_5_6)
  {
 @@ -2411,7 +2411,7 @@ static void hsw_compute_wm_parameters(struct drm_device 
 *dev,

  static void hsw_compute_wm_results(struct drm_device *dev,
struct hsw_pipe_wm_parameters *params,
 -  uint32_t *wm,
 +  uint16_t *wm,
struct hsw_wm_maximums *lp_maximums,
struct hsw_wm_values *results)
  {
 @@ -2593,7 +2593,7 @@ static void haswell_update_wm(struct drm_device *dev)
 struct hsw_wm_maximums lp_max_1_2, lp_max_5_6;
 struct hsw_pipe_wm_parameters params[3];
 struct hsw_wm_values results_1_2, results_5_6, *best_results;
 -   uint32_t wm[5];
 +   uint16_t wm[5];
 enum hsw_data_buf_partitioning partitioning;

 hsw_compute_wm_parameters(dev, params, wm, lp_max_1_2, lp_max_5_6);
 --
 1.8.1.5

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



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


Re: [Intel-gfx] [PATCH 09/35] drm/i915: Split out reading of HSW watermark latency values

2013-07-30 Thread Paulo Zanoni
2013/7/5  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Move parsing of MCH_SSKPD to a separate function, we'll add other
 platforms there later.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_pm.c | 34 --
  1 file changed, 20 insertions(+), 14 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 2239cdb..c266e47 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2336,28 +2336,33 @@ hsw_compute_linetime_wm(struct drm_device *dev, 
 struct drm_crtc *crtc)
PIPE_WM_LINETIME_TIME(linetime);
  }

 +static void intel_read_wm_latency(struct drm_device *dev, uint16_t wm[5])
 +{
 +   struct drm_i915_private *dev_priv = dev-dev_private;
 +
 +   if (IS_HASWELL(dev)) {
 +   uint64_t sskpd = I915_READ64(MCH_SSKPD);
 +
 +   wm[0] = (sskpd  56)  0xFF;
 +   if (wm[0] == 0)
 +   wm[0] = sskpd  0xF;
 +   wm[1] = ((sskpd  4)  0xFF) * 5;
 +   wm[2] = ((sskpd  12)  0xFF) * 5;
 +   wm[3] = ((sskpd  20)  0x1FF) * 5;
 +   wm[4] = ((sskpd  32)  0x1FF) * 5;
 +   }
 +}
 +
  static void hsw_compute_wm_parameters(struct drm_device *dev,
   struct hsw_pipe_wm_parameters *params,
 - uint16_t *wm,
   struct hsw_wm_maximums *lp_max_1_2,
   struct hsw_wm_maximums *lp_max_5_6)
  {
 -   struct drm_i915_private *dev_priv = dev-dev_private;
 struct drm_crtc *crtc;
 struct drm_plane *plane;
 -   uint64_t sskpd = I915_READ64(MCH_SSKPD);
 enum pipe pipe;
 int pipes_active = 0, sprites_enabled = 0;

 -   if ((sskpd  56)  0xFF)
 -   wm[0] = (sskpd  56)  0xFF;
 -   else
 -   wm[0] = sskpd  0xF;
 -   wm[1] = ((sskpd  4)  0xFF) * 5;
 -   wm[2] = ((sskpd  12)  0xFF) * 5;
 -   wm[3] = ((sskpd  20)  0x1FF) * 5;
 -   wm[4] = ((sskpd  32)  0x1FF) * 5;
 -
 list_for_each_entry(crtc, dev-mode_config.crtc_list, head) {
 struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 struct hsw_pipe_wm_parameters *p;
 @@ -2593,10 +2598,11 @@ static void haswell_update_wm(struct drm_device *dev)
 struct hsw_wm_maximums lp_max_1_2, lp_max_5_6;
 struct hsw_pipe_wm_parameters params[3];
 struct hsw_wm_values results_1_2, results_5_6, *best_results;
 -   uint16_t wm[5];
 +   uint16_t wm[5] = {};

Maybe we could make the wm array be part of struct
hsw_pipe_wm_parameters? Then we could move the call to
intel_read_wm_latency back to hsw_compute_wm_parameters. But that
would be material for a separate patch.

Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com


 enum hsw_data_buf_partitioning partitioning;

 -   hsw_compute_wm_parameters(dev, params, wm, lp_max_1_2, lp_max_5_6);
 +   intel_read_wm_latency(dev, wm);
 +   hsw_compute_wm_parameters(dev, params, lp_max_1_2, lp_max_5_6);

 hsw_compute_wm_results(dev, params, wm, lp_max_1_2, results_1_2);
 if (lp_max_1_2.pri != lp_max_5_6.pri) {
 --
 1.8.1.5

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



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


Re: [Intel-gfx] [PATCH 15/35] drm/i915: Print the watermark latencies during init

2013-07-30 Thread Paulo Zanoni
2013/7/5  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Seeing the watermark latency values in dmesg might help sometimes.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_pm.c | 25 +
  1 file changed, 25 insertions(+)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 37919df..5687957 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2392,6 +2392,24 @@ static void intel_fixup_cur_wm_latency(struct 
 drm_device *dev, uint16_t wm[5])
 wm[3] *= 2;
  }

 +static void intel_print_wm_latency(struct drm_device *dev, const uint16_t 
 wm[5])
 +{
 +   int level;
 +
 +   for (level = 0; level = 4; level++) {
 +   unsigned int latency = wm[level];
 +
 +   if (latency == 0)
 +   continue;

One of the cases that should be interesting to print is exactly when a
latency we expect to be non-zero is zero. Maybe you should do a simple
switch statement to get max_level depending on Gen number and print
everything from level 0 to max_level? Then if latency == 0 we could
even promote the message to a DRM_ERROR?

 +
 +   if (level  0)
 +   latency *= 5;
 +
 +   DRM_DEBUG_KMS( WM%d latency %u (%u.%u usec)\n,
 + level, wm[level], latency / 10, latency % 10);
 +   }
 +}
 +
  static void intel_setup_wm_latency(struct drm_device *dev)
  {
 struct drm_i915_private *dev_priv = dev-dev_private;
 @@ -2405,6 +2423,13 @@ static void intel_setup_wm_latency(struct drm_device 
 *dev)

 intel_fixup_spr_wm_latency(dev, dev_priv-wm.spr_latency);
 intel_fixup_cur_wm_latency(dev, dev_priv-wm.cur_latency);
 +
 +   DRM_DEBUG_KMS(Primary watermark latencies:\n);
 +   intel_print_wm_latency(dev, dev_priv-wm.pri_latency);
 +   DRM_DEBUG_KMS(Sprite watermark latencies:\n);
 +   intel_print_wm_latency(dev, dev_priv-wm.spr_latency);
 +   DRM_DEBUG_KMS(Cursor watermark latencies:\n);
 +   intel_print_wm_latency(dev, dev_priv-wm.cur_latency);
  }

  static void hsw_compute_wm_parameters(struct drm_device *dev,
 --
 1.8.1.5

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



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


Re: [Intel-gfx] [PATCH 16/35] drm/i915: Disable specific watermark levels when latency is zero

2013-07-30 Thread Paulo Zanoni
2013/7/5  ville.syrj...@linux.intel.com:
 From: Ville Syrjälä ville.syrj...@linux.intel.com

 Return UINT_MAX for the calculated WM level if the latency is zero.
 This will lead to marking the WM level as disabled.

 I'm not sure if latency==0 should mean that we want to disable the
 level. But that's the implication I got from the fact that we don't
 even enable the watermark code of the SSKDP register is 0.

 Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
 ---
  drivers/gpu/drm/i915/intel_pm.c | 6 ++
  1 file changed, 6 insertions(+)

 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 5687957..a919445 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -2116,6 +2116,9 @@ static uint32_t ilk_wm_method1(uint32_t pixel_rate, 
 uint8_t bytes_per_pixel,
  {
 uint64_t ret;

 +   if (latency == 0)
 +   return UINT_MAX;

IMHO we should scream loud.

 +
 ret = (uint64_t) pixel_rate * bytes_per_pixel * latency;
 ret = DIV_ROUND_UP_ULL(ret, 64 * 1) + 2;

 @@ -2128,6 +2131,9 @@ static uint32_t ilk_wm_method2(uint32_t pixel_rate, 
 uint32_t pipe_htotal,
  {
 uint32_t ret;

 +   if (latency == 0)
 +   return UINT_MAX;
 +
 ret = (latency * pixel_rate) / (pipe_htotal * 1);
 ret = (ret + 1) * horiz_pixels * bytes_per_pixel;
 ret = DIV_ROUND_UP(ret, 64) + 2;
 --
 1.8.1.5

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



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


[Intel-gfx] [PATCH] drm/i915: fix missed hunk after GT access breakage

2013-07-30 Thread Ben Widawsky
Upon some code refactoring, a hunk was missed. This was fixed for
next, but missed the current trees, and hasn't yet been merged by Dave
Airlie. It is fixed in:
commit 907b28c56ea40629aa6595ddfa414ec2fc7da41c
Author: Chris Wilson ch...@chris-wilson.co.uk
Date:   Fri Jul 19 20:36:52 2013 +0100

drm/i915: Colocate all GT access routines in the same file

It is introduced by:
commit 181d1b9e31c668259d3798c521672afb8edd355c
Author: Daniel Vetter daniel.vet...@ffwll.ch
Date:   Sun Jul 21 13:16:24 2013 +0200

drm/i915: fix up gt init sequence fallout

Reported-by: Dave Jones da...@redhat.com
Cc: stable sta...@vger.kernel.org
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
 drivers/gpu/drm/i915/i915_dma.c | 1 +
 drivers/gpu/drm/i915/i915_drv.h | 1 +
 drivers/gpu/drm/i915/intel_pm.c | 6 ++
 3 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 66c6380..f466980 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1594,6 +1594,7 @@ int i915_driver_load(struct drm_device *dev, unsigned 
long flags)
intel_detect_pch(dev);
 
intel_irq_init(dev);
+   intel_pm_init(dev);
intel_gt_sanitize(dev);
intel_gt_init(dev);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d2ee334..1929bff 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1582,6 +1582,7 @@ void i915_hangcheck_elapsed(unsigned long data);
 void i915_handle_error(struct drm_device *dev, bool wedged);
 
 extern void intel_irq_init(struct drm_device *dev);
+extern void intel_pm_init(struct drm_device *dev);
 extern void intel_hpd_init(struct drm_device *dev);
 extern void intel_gt_init(struct drm_device *dev);
 extern void intel_gt_sanitize(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 51a2a60..f895d15 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5536,6 +5536,12 @@ void intel_gt_init(struct drm_device *dev)
dev_priv-gt.force_wake_get = __gen6_gt_force_wake_get;
dev_priv-gt.force_wake_put = __gen6_gt_force_wake_put;
}
+}
+
+void intel_pm_init(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = dev-dev_private;
+
INIT_DELAYED_WORK(dev_priv-rps.delayed_resume_work,
  intel_gen6_powersave_work);
 }
-- 
1.8.3.4

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


Re: [Intel-gfx] [Update][PATCH 0/3] Fix backlight issues on some Windows 8 systems

2013-07-30 Thread Rafael J. Wysocki
On Thursday, July 18, 2013 02:16:09 AM Rafael J. Wysocki wrote:
 On Sunday, June 09, 2013 07:01:36 PM Matthew Garrett wrote:
  Windows 8 introduced new policy for backlight control by pushing it out to
  graphics drivers. This appears to have coincided with a range of vendors
  adding Windows 8 checks to their backlight control code which trigger either
  awkward behaviour (Lenovo) or complete brokenness (some Dells). The simplest
  thing to do would be to just disable ACPI backlight control entirely if the
  firmware indicates Windows 8 support, but it's entirely possible that
  individual graphics drivers might still make use of the ACPI functionality 
  in
  preference to native control.
  
  The first two patches in this series are picked from other patchesets aimed 
  at
  solving similar problems. The last simply unregisters ACPI backlight control
  on Windows 8 systems when using an Intel GPU. Similar code could be added to
  other drivers, but I'm reluctant to do so without further investigation as
  to the behaviour of the vendor drivers under Windows.
 
 Well, after some more time spent on that, we now have a series of 3 patches
 (different from the $subject one) that we think may be used to address this
 issue.  As far as I can say, it has been tested by multiple people whose
 systems have those problems and they generally saw improvement.
 
 It is not my ideal approach, but it seems to be the least intrusive and/or
 with the least amount of possible side effects that we can do right now
 as a general measure (alternatively, we could create a possibly long
 blacklist table of affected systems with different workarounds for them,
 but let's just say that is not overwhelmingly attractive).
 
 [1/3] Make ACPICA export things that we need for checking OSI(Win8).
 
 [2/3] Make acpi_video_device_find_cap() call acpi_video_init_brightness() even
   if it is not going to register the backlight interface (needed for
   Thinkpads).
 
 [3/3] Avoid using ACPI backlight if i915 is in use and the firmware believes
   we are Windows 8.
 
 Many thanks to everyone involved!

So this didn't work, as we had to revert [3/3], but I think we should try to
make some progress with this nevertheless.  A way forward I'm seeing now could
be to
(1) Split the ACPI video driver so that it is possible to register the
backlight control separately from the event interface.
(2) Add a command line option to i915 to make it use the native backlight
control (without registering the ACPI one) if set.  Make unset the
default initially.
(3) Fix i915 backlight control issues for all systems known to have them
(that may take a while) and flip the defailt for that option to set when we
think we're ready.
(4) If there still are problem reports, flip the default back to unset and
repeat (3).

If this converges, everyone will be using the native backlight control by
default and the original problem will go away automatically.

Thoughts?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [Update][PATCH 0/3] Fix backlight issues on some Windows 8 systems

2013-07-30 Thread Matthew Garrett
On Wed, 2013-07-31 at 02:01 +0200, Rafael J. Wysocki wrote:

 (3) Fix i915 backlight control issues for all systems known to have them
 (that may take a while) and flip the defailt for that option to set when 
 we
 think we're ready.

Unfortunately I don't have any systems that reproduce problems here, so
I think Intel are going to have to take the lead on this one.

-- 
Matthew Garrett | mj...@srcf.ucam.org
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 1/3] drm/gma500: Remove useless define

2013-07-30 Thread Stéphane Marchesin
Signed-off-by: Stéphane Marchesin marc...@chromium.org
---
 drivers/gpu/drm/gma500/cdv_intel_display.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/gma500/cdv_intel_display.c 
b/drivers/gpu/drm/gma500/cdv_intel_display.c
index 82430ad..d691a3a 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_display.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_display.c
@@ -52,8 +52,6 @@ struct cdv_intel_clock_t {
int p;
 };
 
-#define INTEL_P2_NUM 2
-
 struct cdv_intel_limit_t {
struct cdv_intel_range_t dot, vco, n, m, m1, m2, p, p1;
struct cdv_intel_p2_t p2;
-- 
1.8.3

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


[Intel-gfx] [PATCH 2/3] drm/i915: Remove useless define

2013-07-30 Thread Stéphane Marchesin
Signed-off-by: Stéphane Marchesin marc...@chromium.org
---
 drivers/gpu/drm/i915/intel_display.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 5fb3058..37b33c9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -54,7 +54,6 @@ typedef struct {
int p2_slow, p2_fast;
 } intel_p2_t;
 
-#define INTEL_P2_NUM 2
 typedef struct intel_limit intel_limit_t;
 struct intel_limit {
intel_range_t   dot, vco, n, m, m1, m2, p, p1;
-- 
1.8.3

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


[Intel-gfx] [PATCH 3/3] drm: Remove drm_mode_validate_clocks

2013-07-30 Thread Stéphane Marchesin
Signed-off-by: Stéphane Marchesin marc...@chromium.org
---
 drivers/gpu/drm/drm_modes.c | 37 -
 include/drm/drm_crtc.h  |  3 ---
 2 files changed, 40 deletions(-)

diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
index a6729bf..504a602 100644
--- a/drivers/gpu/drm/drm_modes.c
+++ b/drivers/gpu/drm/drm_modes.c
@@ -923,43 +923,6 @@ void drm_mode_validate_size(struct drm_device *dev,
 EXPORT_SYMBOL(drm_mode_validate_size);
 
 /**
- * drm_mode_validate_clocks - validate modes against clock limits
- * @dev: DRM device
- * @mode_list: list of modes to check
- * @min: minimum clock rate array
- * @max: maximum clock rate array
- * @n_ranges: number of clock ranges (size of arrays)
- *
- * LOCKING:
- * Caller must hold a lock protecting @mode_list.
- *
- * Some code may need to check a mode list against the clock limits of the
- * device in question.  This function walks the mode list, testing to make
- * sure each mode falls within a given range (defined by @min and @max
- * arrays) and sets @mode-status as needed.
- */
-void drm_mode_validate_clocks(struct drm_device *dev,
- struct list_head *mode_list,
- int *min, int *max, int n_ranges)
-{
-   struct drm_display_mode *mode;
-   int i;
-
-   list_for_each_entry(mode, mode_list, head) {
-   bool good = false;
-   for (i = 0; i  n_ranges; i++) {
-   if (mode-clock = min[i]  mode-clock = max[i]) {
-   good = true;
-   break;
-   }
-   }
-   if (!good)
-   mode-status = MODE_CLOCK_RANGE;
-   }
-}
-EXPORT_SYMBOL(drm_mode_validate_clocks);
-
-/**
  * drm_mode_prune_invalid - remove invalid modes from mode list
  * @dev: DRM device
  * @mode_list: list of modes to check
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index fa12a2f..32e0820 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -930,9 +930,6 @@ extern void drm_mode_list_concat(struct list_head *head,
 extern void drm_mode_validate_size(struct drm_device *dev,
   struct list_head *mode_list,
   int maxX, int maxY, int maxPitch);
-extern void drm_mode_validate_clocks(struct drm_device *dev,
-struct list_head *mode_list,
-int *min, int *max, int n_ranges);
 extern void drm_mode_prune_invalid(struct drm_device *dev,
   struct list_head *mode_list, bool verbose);
 extern void drm_mode_sort(struct list_head *mode_list);
-- 
1.8.3

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