Re: [Intel-gfx] [PATCH] drm/i915: Remove wait for for punit to updates freq.
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5889 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -1 280/280 279/280 ILK 308/308 308/308 SNB 328/328 328/328 IVB 379/379 379/379 BYT 294/294 294/294 HSW -1 387/387 386/387 BDW -1 316/316 315/316 -Detailed- Platform Testdrm-intel-nightly Series Applied *PNV igt_gem_userptr_blits_minor-unsync-interruptible PASS(2) DMESG_WARN(2) *HSW igt_gem_storedw_loop_bsd PASS(2) DMESG_WARN(1)PASS(1) *BDW igt_gem_gtt_hog PASS(4) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] checkpatch spell checking (was: Re: [PATCH] drm/i915: Reudce CHV DPLL min vco frequency to 4.8 GHz)
On Wed, 04 Mar 2015, Ville Syrjälä wrote: > On Wed, Mar 04, 2015 at 08:11:38PM +0530, Purushothaman, Vijay A wrote: >> Minor nitpick: typo in patch title > > Dang. I already fixed a typo there before sending this out, but turns > out I only managed to cchange it into a different typo :( Maybe I need > to invest in a spell checker... These days checkpatch.pl does some of this for you (see scripts/spelling.txt). However your "reudce" isn't there, and, more importantly, AFAICT checkpatch.pl does not look at the patch subject anyway. Joe, Andy, hint, hint. ;) BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Beignet] [PATCH] drm/i915: Export total subslice and EU counts
There is one minor conflict when apply the KMD patch to latest
drm-intel-nightly branch. It should be easy to fix.
Another issue is that IMO, we should bump libdrm's version number
when increase these new APIs. Then in Beignet, we can check the
libdrm version at build time and determine whether we will use
these new interfaces. Thus, we can avoid breaking beignet on
those systems which have previous libdrm/kernel installed.
The other parts of the whole patchset,
including patches for KMD/libdrm/Intel gpu tools and Beignet,
all look good to me.
And I just tested it on BDW and SKL platforms, it works fine.
Thanks,
Zhigang Gong.
On Mon, Mar 02, 2015 at 03:37:32PM -0800, jeff.mc...@intel.com wrote:
> From: Jeff McGee
>
> Setup new I915_GETPARAM ioctl entries for subslice total and
> EU total. Userspace drivers need these values when constructing
> GPGPU commands. This kernel query method is intended to replace
> the PCI ID-based tables that userspace drivers currently maintain.
> The kernel driver can employ fuse register reads as needed to
> ensure the most accurate determination of GT config attributes.
> This first became important with Cherryview in which the config
> could differ between devices with the same PCI ID.
>
> The kernel detection of these values is device-specific and not
> included in this patch. Because zero is not a valid value for any of
> these parameters, a value of zero is interpreted as unknown for the
> device. Userspace drivers should continue to maintain ID-based tables
> for older devices not supported by the new query method.
>
> For: VIZ-4636
> Signed-off-by: Jeff McGee
> ---
> drivers/gpu/drm/i915/i915_dma.c | 10 ++
> include/uapi/drm/i915_drm.h | 2 ++
> 2 files changed, 12 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 053e178..9350ea2 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -150,6 +150,16 @@ static int i915_getparam(struct drm_device *dev, void
> *data,
> case I915_PARAM_MMAP_VERSION:
> value = 1;
> break;
> + case I915_PARAM_SUBSLICE_TOTAL:
> + value = INTEL_INFO(dev)->subslice_total;
> + if (!value)
> + return -ENODEV;
> + break;
> + case I915_PARAM_EU_TOTAL:
> + value = INTEL_INFO(dev)->eu_total;
> + if (!value)
> + return -ENODEV;
> + break;
> default:
> DRM_DEBUG("Unknown parameter %d\n", param->param);
> return -EINVAL;
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 6eed16b..8672efc 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -347,6 +347,8 @@ typedef struct drm_i915_irq_wait {
> #define I915_PARAM_HAS_COHERENT_PHYS_GTT 29
> #define I915_PARAM_MMAP_VERSION 30
> #define I915_PARAM_HAS_BSD2 31
> +#define I915_PARAM_SUBSLICE_TOTAL 32
> +#define I915_PARAM_EU_TOTAL 33
>
> typedef struct drm_i915_getparam {
> int param;
> --
> 2.3.0
>
> ___
> Beignet mailing list
> beig...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/skl: Read sink supported rates from edp panel
v2: Using DP_SUPPORTED_LINK_RATES macro for supported_rates array (Satheesh).
v3: Reading dpcd's supported link rates tables based upon edp version in the
same patch.
v4: Move version check under is_edp (Satheesh)
v5: Using le16 for rates, some naming, and removing nested if block (Ville)
v6: Correctly using DP_MAX_SUPPORTED_RATES and removing DP_SUPPORTED_LINK_RATES
(Ville)
v7: Incorrectly removed DP_SUPPORTED_LINK_RATES in v6, re-adding it
v8: Checking return value of intel_dp_dpcd_read_wake() (Ville)
Reviewed-by: Ville Syrjälä
Signed-off-by: Sonika Jindal
---
drivers/gpu/drm/i915/intel_dp.c | 38 ++
drivers/gpu/drm/i915/intel_drv.h |1 +
2 files changed, 39 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d1141d3..0ae8454 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1117,6 +1117,33 @@ hsw_dp_set_ddi_pll_sel(struct intel_crtc_state
*pipe_config, int link_bw)
}
}
+static int
+intel_read_sink_rates(struct intel_dp *intel_dp, uint32_t *sink_rates)
+{
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ int i = 0;
+ uint16_t val;
+
+ if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
+ /*
+* Receiver supports only main-link rate selection by
+* link rate table method, so read link rates from
+* supported_link_rates
+*/
+ for (i = 0; i < DP_MAX_SUPPORTED_RATES; ++i) {
+ val = le16_to_cpu(intel_dp->supported_rates[i]);
+ if (val == 0)
+ break;
+
+ sink_rates[i] = val * 200;
+ }
+
+ if (i <= 0)
+ DRM_ERROR("No rates in SUPPORTED_LINK_RATES");
+ }
+ return i;
+}
+
static void
intel_dp_set_clock(struct intel_encoder *encoder,
struct intel_crtc_state *pipe_config, int link_bw)
@@ -3578,6 +3605,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = dig_port->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
+ uint8_t rev;
if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd,
sizeof(intel_dp->dpcd)) < 0)
@@ -3609,6 +3637,16 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
} else
intel_dp->use_tps3 = false;
+ /* Intermediate frequency support */
+ if (is_edp(intel_dp) &&
+ (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] &
DP_DPCD_DISPLAY_CONTROL_CAPABLE) &&
+ (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1)
== 1) &&
+ (rev >= 0x03)) { /* eDp v1.4 or higher */
+ intel_dp_dpcd_read_wake(&intel_dp->aux,
+ DP_SUPPORTED_LINK_RATES,
+ intel_dp->supported_rates,
+ sizeof(intel_dp->supported_rates));
+ }
if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
DP_DWN_STRM_PORT_PRESENT))
return true; /* native DP sink */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1fb1529..1f41a83 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -626,6 +626,7 @@ struct intel_dp {
uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
+ __le16 supported_rates[DP_MAX_SUPPORTED_RATES];
struct drm_dp_aux aux;
uint8_t train_set[4];
int panel_power_up_delay;
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/skl: Add support for edp 1.4 intermediate frequencies
eDp 1.4 supports custom frequencies.
Skylake supports following intermediate frequencies : 3.24 GHz, 2.16 GHz and
4.32 GHz along with usual LBR, HBR and HBR2 frequencies.
Read sink supported frequencies and get common frequencies from sink and
source and use these for link training.
v2: Rebased, removed calculation of min_clock since for edp it is taken as
max_clock (as per comment).
v3: Keeping single array for link rates (Satheesh)
v4: Setting LINK_BW_SET to 0 when setting LINK_RATE_SET (Satheesh)
v5: Some minor nits (Ville)
v6: Keeping separate arrays for source and sink rates (Ville)
v7: Remove redundant setting of DP_LINK_BW_SET to 0 (Ville)
Reviewed-by: Ville Syrjälä
Signed-off-by: Sonika Jindal
---
drivers/gpu/drm/i915/intel_ddi.c |9
drivers/gpu/drm/i915/intel_dp.c | 109 +++---
drivers/gpu/drm/i915/intel_drv.h |1 +
3 files changed, 111 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 985d531..437d285 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -786,9 +786,18 @@ static void skl_ddi_clock_get(struct intel_encoder
*encoder,
case DPLL_CRTL1_LINK_RATE_810:
link_clock = 81000;
break;
+ case DPLL_CRTL1_LINK_RATE_1080:
+ link_clock = 108000;
+ break;
case DPLL_CRTL1_LINK_RATE_1350:
link_clock = 135000;
break;
+ case DPLL_CRTL1_LINK_RATE_1620:
+ link_clock = 162000;
+ break;
+ case DPLL_CRTL1_LINK_RATE_2160:
+ link_clock = 216000;
+ break;
case DPLL_CRTL1_LINK_RATE_2700:
link_clock = 27;
break;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0ae8454..2e451de 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -84,6 +84,11 @@ static const struct dp_link_dpll chv_dpll[] = {
{ DP_LINK_BW_5_4, /* m2_int = 27, m2_fraction = 0 */
{ .p1 = 2, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } }
};
+/* Skylake supports following rates */
+static const uint32_t gen9_rates[] = { 162000, 216000, 27, 324000,
+ 432000, 54 };
+
+static const uint32_t default_rates[] = { 162000, 27, 54 };
/**
* is_edp - is the given port attached to an eDP panel (either CPU or PCH)
@@ -1144,6 +1149,25 @@ intel_read_sink_rates(struct intel_dp *intel_dp,
uint32_t *sink_rates)
return i;
}
+static int
+intel_read_source_rates(struct intel_dp *intel_dp, uint32_t *source_rates)
+{
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ int i;
+ int max_default_rate;
+
+ if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
+ for (i = 0; i < ARRAY_SIZE(gen9_rates); ++i)
+ source_rates[i] = gen9_rates[i];
+ } else {
+ /* Index of the max_link_bw supported + 1 */
+ max_default_rate = (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
+ for (i = 0; i < max_default_rate; ++i)
+ source_rates[i] = default_rates[i];
+ }
+ return i;
+}
+
static void
intel_dp_set_clock(struct intel_encoder *encoder,
struct intel_crtc_state *pipe_config, int link_bw)
@@ -1177,6 +1201,45 @@ intel_dp_set_clock(struct intel_encoder *encoder,
}
}
+static int intel_supported_rates(const uint32_t *source_rates, int source_len,
+const uint32_t *sink_rates, int sink_len, uint32_t *supported_rates)
+{
+ int i = 0, j = 0, k = 0;
+
+ /* For panels with edp version less than 1.4 */
+ if (sink_len == 0) {
+ for (i = 0; i < source_len; ++i)
+ supported_rates[i] = source_rates[i];
+ return source_len;
+ }
+
+ /* For edp1.4 panels, find the common rates between source and sink */
+ while (i < source_len && j < sink_len) {
+ if (source_rates[i] == sink_rates[j]) {
+ supported_rates[k] = source_rates[i];
+ ++k;
+ ++i;
+ ++j;
+ } else if (source_rates[i] < sink_rates[j]) {
+ ++i;
+ } else {
+ ++j;
+ }
+ }
+ return k;
+}
+
+static int rate_to_index(uint32_t find, const uint32_t *rates)
+{
+ int i = 0;
+
+ for (i = 0; i < DP_MAX_SUPPORTED_RATES; ++i)
+ if (find == rates[i])
+ break;
+
+ return i;
+}
+
bool
intel_dp_compute_config(struct intel_encoder *encoder,
struct int
[Intel-gfx] [PATCH] drm/i915: Update PM interrupts before updating the freq
From: Deepak S We update the GT PM interrupts mask at the end of set rps. We observed even though we are requesting a RPn or RP0, there is a chance to get a DOWN or UP interrupts before interrupts mask. These extra interrupts are simply wasting cpu cycles. In this patch we mask the interrupts for given freq before requesting new frequency. Signed-off-by: Deepak S --- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2e1ed07..bbfe4f0 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3879,12 +3879,12 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val) if (WARN_ONCE(IS_CHERRYVIEW(dev) && (val & 1), "Odd GPU freq value\n")) val &= ~1; + + I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val)); if (val != dev_priv->rps.cur_freq) vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val); - I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val)); - dev_priv->rps.cur_freq = val; trace_intel_gpu_freq_change(intel_gpu_freq(dev_priv, val)); } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Remove wait for for punit to updates freq.
From: Deepak S
When GPU is idle on VLV, Request freq to punit should be good enough to
get the voltage back to VNN. Also, make sure gfx clock force applies
before requesting the freq fot vlv.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75244
suggested-by: Jesse Barnes
Signed-off-by: Deepak S
---
drivers/gpu/drm/i915/intel_pm.c | 20
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e710b43..2e1ed07 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3894,7 +3894,7 @@ static void valleyview_set_rps(struct drm_device *dev, u8
val)
* * If Gfx is Idle, then
* 1. Mask Turbo interrupts
* 2. Bring up Gfx clock
- * 3. Change the freq to Rpn and wait till P-Unit updates freq
+ * 3. Request the freq to Rpn.
* 4. Clear the Force GFX CLK ON bit so that Gfx can down
* 5. Unmask Turbo interrupts
*/
@@ -3902,8 +3902,8 @@ static void vlv_set_rps_idle(struct drm_i915_private
*dev_priv)
{
struct drm_device *dev = dev_priv->dev;
- /* CHV and latest VLV don't need to force the gfx clock */
- if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
+ /* CHV don't need to force the gfx clock */
+ if (IS_CHERRYVIEW(dev)) {
valleyview_set_rps(dev_priv->dev,
dev_priv->rps.min_freq_softlimit);
return;
}
@@ -3920,20 +3920,8 @@ static void vlv_set_rps_idle(struct drm_i915_private
*dev_priv)
gen6_sanitize_rps_pm_mask(dev_priv, ~0));
vlv_force_gfx_clock(dev_priv, true);
-
- dev_priv->rps.cur_freq = dev_priv->rps.min_freq_softlimit;
-
- vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ,
- dev_priv->rps.min_freq_softlimit);
-
- if (wait_for(((vlv_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS))
- & GENFREQSTATUS) == 0, 100))
- DRM_ERROR("timed out waiting for Punit\n");
-
+ valleyview_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
vlv_force_gfx_clock(dev_priv, false);
-
- I915_WRITE(GEN6_PMINTRMSK,
- gen6_rps_pm_mask(dev_priv, dev_priv->rps.cur_freq));
}
void gen6_rps_idle(struct drm_i915_private *dev_priv)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Don't clobber plane state on internal disables
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5887 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 280/280 280/280 ILK 308/308 308/308 SNB -1 328/328 327/328 IVB -1 379/379 378/379 BYT 294/294 294/294 HSW 387/387 387/387 BDW -1 316/316 315/316 -Detailed- Platform Testdrm-intel-nightly Series Applied SNB igt_kms_rotation_crc_sprite-rotation DMESG_WARN(1)PASS(4) DMESG_WARN(2) *IVB igt_kms_rotation_crc_sprite-rotation PASS(2) DMESG_WARN(2) *BDW igt_gem_gtt_hog PASS(3) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Preventing zero GPU virtual address allocation
Hi Daniel, OpenCL language support NULL pointer, using zero as the NULL pointer is the obvious way. That is zero will be treated as invalid address. Then it requires drm won't allocate zero to drm buffer. And David in CC list has help us make a patch, please see attached. The logic is only for ppgtt, and he said zero offset is used under ggtt. My question is what is offset zero used under ggtt? Will it make sure zero is not allocatable to drm buffer object? Ruiling nozerooffset2.patch Description: nozerooffset2.patch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915: PSR VLV: Add single frame update.
Reviewed by: Dhinakaran Pandiyan
Tested by: Dhinakaran Pandiyan
The screen update lag that was earlier seen on BSW is fixed by this patch.
From: Vivi, Rodrigo
Sent: Friday, February 27, 2015 5:26 PM
To: intel-gfx@lists.freedesktop.org
Cc: Vivi, Rodrigo; Pandiyan, Dhinakaran
Subject: [PATCH 4/7] drm/i915: PSR VLV: Add single frame update.
According to spec: "In PSR HW or SW mode, SW set this bit before writing
registers for a flip. It will be self-clear when it gets to the PSR
active state."
Some versions of spec mention that this is needed when in
"Persistent mode" but define it as same as "SW mode". Since this
fix the page flip case let's assume this is exactly what we need.
Cc: Dhinakaran Pandiyan
Signed-off-by: Rodrigo Vivi
---
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_frontbuffer.c | 2 ++
drivers/gpu/drm/i915/intel_psr.c | 42
3 files changed, 45 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1fb1529..55ece8f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1182,6 +1182,7 @@ void intel_psr_invalidate(struct drm_device *dev,
void intel_psr_flush(struct drm_device *dev,
unsigned frontbuffer_bits);
void intel_psr_init(struct drm_device *dev);
+void intel_psr_single_frame_update(struct drm_device *dev);
/* intel_runtime_pm.c */
int intel_power_domains_init(struct drm_i915_private *);
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c
b/drivers/gpu/drm/i915/intel_frontbuffer.c
index 73cb6e0..2094c06 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -254,6 +254,8 @@ void intel_frontbuffer_flip_prepare(struct drm_device *dev,
/* Remove stale busy bits due to the old buffer. */
dev_priv->fb_tracking.busy_bits &= ~frontbuffer_bits;
mutex_unlock(&dev_priv->fb_tracking.lock);
+
+ intel_psr_single_frame_update(dev);
}
/**
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index d2ff87d..c1ca923 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -553,6 +553,48 @@ static void intel_psr_exit(struct drm_device *dev)
}
/**
+ * intel_psr_single_frame_update - Single Frame Update
+ * @dev: DRM device
+ *
+ * Some platforms support a single frame update feature that is used to
+ * send and update only one frame on Remote Frame Buffer.
+ * So far it is only implemented for Valleyview and Cherryview because
+ * hardware requires this to be done before a page flip.
+ */
+void intel_psr_single_frame_update(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_crtc *crtc;
+ enum pipe pipe;
+ u32 val;
+
+ /*
+* Single frame update is already supported on BDW+ but it requires
+* many W/A and it isn't really needed.
+*/
+ if (!IS_VALLEYVIEW(dev))
+ return;
+
+ mutex_lock(&dev_priv->psr.lock);
+ if (!dev_priv->psr.enabled) {
+ mutex_unlock(&dev_priv->psr.lock);
+ return;
+ }
+
+ crtc = dp_to_dig_port(dev_priv->psr.enabled)->base.base.crtc;
+ pipe = to_intel_crtc(crtc)->pipe;
+ val = I915_READ(VLV_PSRCTL(pipe));
+
+ /*
+* We need to set this bit before writing registers for a flip.
+* This bit will be self-clear when it gets to the PSR active state.
+*/
+ I915_WRITE(VLV_PSRCTL(pipe), val | VLV_EDP_PSR_SINGLE_FRAME_UPDATE);
+
+ mutex_unlock(&dev_priv->psr.lock);
+}
+
+/**
* intel_psr_invalidate - Invalidade PSR
* @dev: DRM device
* @frontbuffer_bits: frontbuffer plane tracking bits
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Make WAIT_IOCTL negative timeouts be indefinite again
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5886 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -2 280/280 278/280 ILK 308/308 308/308 SNB -1 328/328 327/328 IVB 379/379 379/379 BYT 294/294 294/294 HSW 387/387 387/387 BDW -1 316/316 315/316 -Detailed- Platform Testdrm-intel-nightly Series Applied *PNV igt_gen3_render_mixed_blits FAIL(1)PASS(2) NRUN(1)PASS(1) *PNV igt_gen3_render_tiledx_blits FAIL(1)PASS(4) CRASH(1)PASS(1) SNB igt_kms_pipe_crc_basic_read-crc-pipe-A-frame-sequence DMESG_WARN(1)PASS(4) DMESG_WARN(1)PASS(1) *BDW igt_gem_gtt_hog PASS(3) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Make sure we invalidate frontbuffer on fbcon.
On Wed, Mar 4, 2015 at 6:30 AM, Daniel Vetter wrote: > On Tue, Mar 03, 2015 at 08:03:13PM +, Vivi, Rodrigo wrote: >> On Tue, 2015-03-03 at 09:28 +0100, Daniel Vetter wrote: >> > On Mon, Mar 02, 2015 at 06:35:26PM +, Vivi, Rodrigo wrote: >> > > On Mon, 2015-03-02 at 18:59 +0100, Daniel Vetter wrote: >> > > > On Fri, Feb 27, 2015 at 08:26:05PM -0500, Rodrigo Vivi wrote: >> > > > > There are some cases like suspend/resume or dpms off/on sequences >> > > > > that can flush frontbuffer bits. In these cases features that relies >> > > > > on frontbuffer tracking can start working and user can stop getting >> > > > > screen updates on fbcon having impression the system is frozen. >> > > > > >> > > > > So, let's make sure on fbcon write operation we also invalidate >> > > > > frontbuffer bits so we will be on the safest side with fbcon. >> > > > >> > > > This is just a bandaid since you can always just directly access the >> > > > fbdev framebuffer. We really need to figure out why we have frontbuffer >> > > > bit flushes after we've invalidated them for fbcon and catch them all. >> > > >> > > yeah, an ugly bandaid... Just to make PSR a bit more reliable without >> > > breaking fbcon environment when it gets enabled by default. >> > > >> > > The issue is that on the logs I see: >> > > >> > > 1.fbdev_blank dpms off >> > > 2. disable planes >> > > 3. flush frontbuffer bits >> > > --- blank stage --- >> > > 4. fbdev_blank dpms on >> > >> > so fbdev_blank returns _before_ the below enable_planes/frontbuf_flush? >> > Can you please attach full backtraces for steps 5&6? >> >> [ 156.665517] [drm:intel_fbdev_set_par] PSR FBDEV modeset >> [ 759.232969] [drm:intel_fbdev_blank] PSR FBDEV blank normal >> [ 759.232987] [drm:intel_crtc_disable_planes] PSR FBDEV crtc disable >> planes flush fb bits >> [ 897.313095] [drm:intel_fbdev_blank] PSR FBDEV unblank >> [ 897.313112] [drm:intel_crtc_control] PSR FBDEV crtc enable planes >> [ 897.527818] [drm:haswell_crtc_enable] PSR FBDEV crtc enable planes >> [ 897.542925] [drm:intel_crtc_enable_planes] PSR FBDEV crtc enable >> planes flush fb bits > > I didn't mean the drm.debug log but the full backtrace for every time we > flush psr fb bits. I want to know who's the top-level function which > eventualy leads to the fb flush. I.e. something like > > WARN_ON(frontbuffer_bits); > > in intel_psr_flush (after we've filtered out any already set bits and > other stuff that doesn't apply ofc). I'm not sure if I understood your request... This [ 897.542925] [drm:intel_crtc_enable_planes] PSR FBDEV crtc enable planes flush fb bits is the point where frontbuffer bits will be flushed calling psr_flush and psr gets back to life after the fbdev unblank. > > Cheers, Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add debugfs entry for DRRS
On Tue, Mar 3, 2015 at 7:23 AM, Ramalingam C wrote:
> From: Vandana Kannan
>
> Adding a debugfs entry to determine if DRRS is supported or not
>
> V2: [By Ram]: Following details about the active crtc will be filled
> in seq-file of the debugfs
> 1. Encoder output type
> 2. DRRS Support on this CRTC
> 3. DRRS current state
> 4. Current Vrefresh
> Format is as follows:
> CRTC 1: Output: eDP, DRRS Supported: Yes (Seamless), DRRS_State:
> DRRS_HIGH_RR, Vrefresh: 60
> CRTC 2: Output: HDMI, DRRS Supported : No, VBT DRRS_type: Seamless
> CRTC 1: Output: eDP, DRRS Supported: Yes (Seamless), DRRS_State:
> DRRS_LOW_RR, Vrefresh: 40
> CRTC 2: Output: HDMI, DRRS Supported : No, VBT DRRS_type: Seamless
>
> V3: [By Ram]: Readability is improved.
> Another error case is covered [Daniel]
>
> V4: [By Ram]: Current status of the Idleness DRRS along with
> the Front buffer bits are added to the debugfs. [Rodrigo]
>
> V5: [By Ram]: Rephrased to make it easy to understand.
> And format is modified. [Rodrigo]
>
> V6: [By Ram]: Modeset mutex are acquired for each crtc along with
> renaming the Idleness detection states [Daniel]
>
> Signed-off-by: Vandana Kannan
> Signed-off-by: Ramalingam C
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 141
> +++
> 1 file changed, 141 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 94b3984..90e56ca 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2870,6 +2870,146 @@ static int i915_ddb_info(struct seq_file *m, void
> *unused)
> return 0;
> }
>
> +static void drrs_status_per_crtc(struct seq_file *m,
> + struct drm_device *dev, struct intel_crtc *intel_crtc)
> +{
> + struct intel_encoder *intel_encoder;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct i915_drrs *drrs = &dev_priv->drrs;
> + int vrefresh = 0;
> + u32 work_status;
> +
> + for_each_encoder_on_crtc(dev, &intel_crtc->base, intel_encoder) {
> + /* Encoder connected on this CRTC */
> + switch (intel_encoder->type) {
> + case INTEL_OUTPUT_EDP:
> + seq_puts(m, "eDP:\n");
> + break;
> + case INTEL_OUTPUT_DSI:
> + seq_puts(m, "DSI:\n");
> + break;
> + case INTEL_OUTPUT_HDMI:
> + seq_puts(m, "HDMI:\n");
> + break;
> + case INTEL_OUTPUT_DISPLAYPORT:
> + seq_puts(m, "DP:\n");
> + break;
> + default:
> + seq_printf(m, "Other encoder (id=%d).\n",
> + intel_encoder->type);
> + return;
> + }
> + }
> +
> + if (dev_priv->vbt.drrs_type == STATIC_DRRS_SUPPORT)
> + seq_puts(m, "\tVBT: DRRS_type: Static");
> + else if (dev_priv->vbt.drrs_type == SEAMLESS_DRRS_SUPPORT)
> + seq_puts(m, "\tVBT: DRRS_type: Seamless");
> + else if (dev_priv->vbt.drrs_type == DRRS_NOT_SUPPORTED)
> + seq_puts(m, "\tVBT: DRRS_type: None");
> + else
> + seq_puts(m, "\tVBT: DRRS_type: FIXME: Unrecognized Value");
> +
> + seq_puts(m, "\n\n");
> +
> + /*
> +* Idleness DRRS detection states:
> +* Enabled : Idleness detection is active. When system is
> +* Idle for the defined duration DRRS_LOW_RR
> +* will be set. Or Idleness is already detected
> +* and DRRS_LOW_RR is applied.
> +* Suspended : Due to frontbuffer's busy state, Idleness
> +* detection is suspended.
> +* Disabled : Idleness detection is disabled until a call is
> +* made to enable. No encoder pointer will be
> +* available.
> +*/
> + if (intel_crtc->config->has_drrs) {
> + struct intel_panel *panel;
> +
> + mutex_lock(&drrs->mutex);
> + /* DRRS Supported */
> + seq_puts(m, "\tDRRS Supported: Yes\n");
> +
> + /* disable_drrs() will make drrs->dp NULL */
> + if (!drrs->dp) {
> + seq_puts(m, "Idleness DRRS: Disabled");
> + mutex_unlock(&drrs->mutex);
> + return;
> + }
> +
> + panel = &drrs->dp->attached_connector->panel;
> + seq_printf(m, "\t\tBusy_frontbuffer_bits: 0x%X",
> + drrs->busy_frontbuffer_bits);
> +
> + seq_puts(m, "\n\t\t");
> + if (
Re: [Intel-gfx] [PATCH] drm/i915/skl: power on DP AUX well when getting CRTC power domains
On 03/04/2015 02:40 PM, Jesse Barnes wrote:
> I need this on my machine or eDP doesn't come up.
>
> Signed-off-by: Jesse Barnes
> ---
> drivers/gpu/drm/i915/intel_display.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 480dd79..741a454 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4868,8 +4868,10 @@ static unsigned long get_crtc_power_domains(struct
> drm_crtc *crtc)
> intel_crtc->config->pch_pfit.force_thru)
> mask |= BIT(POWER_DOMAIN_PIPE_PANEL_FITTER(pipe));
>
> - for_each_encoder_on_crtc(dev, crtc, intel_encoder)
> + for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
> + mask |= BIT(intel_display_aux_power_domain(intel_encoder));
> mask |= BIT(intel_display_port_power_domain(intel_encoder));
> + }
>
> return mask;
> }
>
Oops, ignore this one, against an old tree.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fixing mutex deadlock window at eDP DRRS
Looks enough for me...
Reviewed-by: Rodrigo Vivi
On Mon, Mar 2, 2015 at 10:41 PM, Ramalingam C wrote:
> In invalidate and flush functions of eDP DRRS, if deferred downclock
> work starts execution at a time window between acquiring the drrs
> mutex and cancellation of the deferred work
> (intel_edp_drrs_downclock_work), then deferred work will find
> drrs mutex locked and wait for the same.
>
> Meanwhile the function that acquired mutex drrs invalidate/flush will
> wait for the completion of the deferred work before releasing the mutex.
> Thats a deadlock.
>
> To avoid such deadlock scenario, this change cancels the deferred work
> before acquiring the mutex at invalidate and flush functions.
>
> Signed-off-by: Ramalingam C
> ---
> drivers/gpu/drm/i915/intel_dp.c |7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d1141d3..0a57763 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4966,12 +4966,13 @@ void intel_edp_drrs_invalidate(struct drm_device *dev,
> if (!dev_priv->drrs.dp)
> return;
>
> + cancel_delayed_work_sync(&dev_priv->drrs.work);
> +
> mutex_lock(&dev_priv->drrs.mutex);
> crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc;
> pipe = to_intel_crtc(crtc)->pipe;
>
> if (dev_priv->drrs.refresh_rate_type == DRRS_LOW_RR) {
> - cancel_delayed_work_sync(&dev_priv->drrs.work);
> intel_dp_set_drrs_state(dev_priv->dev,
> dev_priv->drrs.dp->attached_connector->panel.
> fixed_mode->vrefresh);
> @@ -5004,13 +5005,13 @@ void intel_edp_drrs_flush(struct drm_device *dev,
> if (!dev_priv->drrs.dp)
> return;
>
> + cancel_delayed_work_sync(&dev_priv->drrs.work);
> +
> mutex_lock(&dev_priv->drrs.mutex);
> crtc = dp_to_dig_port(dev_priv->drrs.dp)->base.base.crtc;
> pipe = to_intel_crtc(crtc)->pipe;
> dev_priv->drrs.busy_frontbuffer_bits &= ~frontbuffer_bits;
>
> - cancel_delayed_work_sync(&dev_priv->drrs.work);
> -
> if (dev_priv->drrs.refresh_rate_type != DRRS_LOW_RR &&
> !dev_priv->drrs.busy_frontbuffer_bits)
> schedule_delayed_work(&dev_priv->drrs.work,
> --
> 1.7.9.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/skl: power on DP AUX well when getting CRTC power domains
I need this on my machine or eDP doesn't come up.
Signed-off-by: Jesse Barnes
---
drivers/gpu/drm/i915/intel_display.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 480dd79..741a454 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4868,8 +4868,10 @@ static unsigned long get_crtc_power_domains(struct
drm_crtc *crtc)
intel_crtc->config->pch_pfit.force_thru)
mask |= BIT(POWER_DOMAIN_PIPE_PANEL_FITTER(pipe));
- for_each_encoder_on_crtc(dev, crtc, intel_encoder)
+ for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
+ mask |= BIT(intel_display_aux_power_domain(intel_encoder));
mask |= BIT(intel_display_port_power_domain(intel_encoder));
+ }
return mask;
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/7] drm/i915: add frontbuffer tracking to FBC
On Wed, 2015-03-04 at 15:03 -0300, Paulo Zanoni wrote:
> 2015-03-03 21:57 GMT-03:00 Rodrigo Vivi :
> > On Fri, Feb 13, 2015 at 11:23 AM, Paulo Zanoni wrote:
> >> From: Paulo Zanoni
> >>
> >> Kill the blt/render tracking we currently have and use the frontbuffer
> >> tracking infrastructure.
> >>
> >> Don't enable things by default yet.
> >>
> >> v2: (Rodrigo) Fix small conflict on rebase and typo at subject.
> >> v3: (Paulo) Rebase on RENDER_CS change.
> >> v4: (Paulo) Rebase.
> >> v5: (Paulo) Simplify: flushes don't have origin (Daniel).
> >> Also rebase due to patch order changes.
> >>
> >> Signed-off-by: Rodrigo Vivi
> >> Signed-off-by: Paulo Zanoni
> >> ---
> >> drivers/gpu/drm/i915/i915_drv.h | 10 +---
> >> drivers/gpu/drm/i915/intel_drv.h | 6 ++-
> >> drivers/gpu/drm/i915/intel_fbc.c | 91
> >> +++-
> >> drivers/gpu/drm/i915/intel_frontbuffer.c | 14 +
> >> drivers/gpu/drm/i915/intel_ringbuffer.c | 41 +-
> >> drivers/gpu/drm/i915/intel_ringbuffer.h | 1 -
> >> 6 files changed, 65 insertions(+), 98 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >> b/drivers/gpu/drm/i915/i915_drv.h
> >> index 30aaa8e..7680ca0 100644
> >> --- a/drivers/gpu/drm/i915/i915_drv.h
> >> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >> @@ -783,6 +783,8 @@ struct i915_fbc {
> >> unsigned long uncompressed_size;
> >> unsigned threshold;
> >> unsigned int fb_id;
> >> + unsigned int possible_framebuffer_bits;
> >> + unsigned int busy_bits;
> >
> > I'm not sure if I understood why you keep 2 variables here?
>
> After Daniel's last suggestion, the possible_framebuffer_bits could
> probably be removed and left as a local variable at intel_fbc_validate
> now: it's just a matter of coding style. Some platforms support FBC on
> more than just pipe A, so this variable is used to simplify checks
> related to this.
>
> The busy_bits was also part of Daniel's last request: we need to store
> which bits triggered intel_fbc_invalidate calls, and then check them
> when intel_fbc_flush is called.
Got it.
Thanks for explaining it.
>
> > Is it because we keep enabling/disabling fbc with updates all over the code?
> > In this case it is ok we just need to kill it when we kill update_fbc...
>
> I don't understand what you mean here. Please clarify.
I had missunderstood the reason of possible_frontbuffer bits, sorry.
But about killing update_fbc I believe that after frontbuffer rework is
in place we don't need to use that update_fbc function that disable and
enable fbc on the cases we knew it could fail. With frontbuffer bits we
could let it always enabled and kicking with frontbuffer bits.
>
> >
> >> struct intel_crtc *crtc;
> >> int y;
> >>
> >> @@ -795,14 +797,6 @@ struct i915_fbc {
> >> * possible. */
> >> bool enabled;
> >>
> >> - /* On gen8 some rings cannont perform fbc clean operation so for
> >> now
> >> -* we are doing this on SW with mmio.
> >> -* This variable works in the opposite information direction
> >> -* of ring->fbc_dirty telling software on frontbuffer tracking
> >> -* to perform the cache clean on sw side.
> >> -*/
> >> - bool need_sw_cache_clean;
> >> -
> >> struct intel_fbc_work {
> >> struct delayed_work work;
> >> struct drm_crtc *crtc;
> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> >> b/drivers/gpu/drm/i915/intel_drv.h
> >> index 05d0a43f..571174f 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -1091,7 +1091,11 @@ bool intel_fbc_enabled(struct drm_device *dev);
> >> void intel_fbc_update(struct drm_device *dev);
> >> void intel_fbc_init(struct drm_i915_private *dev_priv);
> >> void intel_fbc_disable(struct drm_device *dev);
> >> -void bdw_fbc_sw_flush(struct drm_device *dev, u32 value);
> >> +void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
> >> + unsigned int frontbuffer_bits,
> >> + enum fb_op_origin origin);
> >> +void intel_fbc_flush(struct drm_i915_private *dev_priv,
> >> +unsigned int frontbuffer_bits);
> >>
> >> /* intel_hdmi.c */
> >> void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port
> >> port);
> >> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
> >> b/drivers/gpu/drm/i915/intel_fbc.c
> >> index 618f7bd..9fcf446 100644
> >> --- a/drivers/gpu/drm/i915/intel_fbc.c
> >> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> >> @@ -174,29 +174,10 @@ static bool g4x_fbc_enabled(struct drm_device *dev)
> >> return I915_READ(DPFC_CONTROL) & DPFC_CTL_EN;
> >> }
> >>
> >> -static void snb_fbc_blit_update(struct drm_device *dev)
> >> +static void intel_fbc_nuke(struct drm_i915_private *dev_priv)
> >> {
> >> - struct drm_i915_private *dev_priv = dev->dev_private;
> >> - u32 b
Re: [Intel-gfx] [PATCH] drm/i915: Setup all page directories for gen8
On Tue, Mar 03, 2015 at 05:03:29PM +0200, Mika Kuoppala wrote:
> If the mappable size is less than what the full range
> of pdps can address, we end up setting pdps for only the
> mappable area.
>
> The logical context however needs valid pdp entries.
> Prior to commit 06fda602dbca ("drm/i915: Create page table allocators")
> we just have been writing pdp entries with dma address of zero instead
> of valid pdps. This is supposedly bad even if those pdps are not
> addressed.
>
> As commit 06fda602dbca ("drm/i915: Create page table allocators")
> introduced more dynamic structure for pdps, we ended up oopsing
> when we populated the lrc context. Analyzing this oops revealed
> the fact that we have not been writing valid pdps with bsw, as
> it is doing the ppgtt init with 2gb limit.
>
> We should do the right thing and setup the non addressable part
> pdps/pde/pte to scratch page through the minimal structure by
> having just pdp with pde entries pointing to same page with
> pte entries pointing to scratch page.
>
> But instead of going through that trouble, setup all the pdps
> through individual pd pages and pt entries, even for non
> addressable parts. This way we populate the lrc with valid
> pdps and gives us a base for dynamic page allocation to
> introduce code that truncates the page table structure.
>
> The regression of oopsing in init was introduced by
> commit 06fda602dbca ("drm/i915: Create page table allocators")
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89350
> Tested-by: Valtteri Rantala
> Cc: Michel Thierry
> Cc: Ben Widawsky
> Cc: Ville Syrjälä
> Signed-off-by: Mika Kuoppala
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index bd95776..848a821 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -709,7 +709,7 @@ static int gen8_ppgtt_setup_page_tables(struct
> i915_hw_ppgtt *ppgtt,
> */
> static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt, uint64_t size)
> {
> - const int max_pdp = DIV_ROUND_UP(size, 1 << 30);
> + const int max_pdp = GEN8_LEGACY_PDPES;
> const int min_pt_pages = GEN8_PDES_PER_PAGE * max_pdp;
> int i, j, ret;
>
FWIW, I think I solved this later in my original series:
http://lists.freedesktop.org/archives/intel-gfx/2014-August/051162.html
--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Setup all page directories for gen8
On Wed, Mar 04, 2015 at 02:55:17PM +0200, Mika Kuoppala wrote:
> If the requested size is less than what the full range
> of pdps can address, we end up setting pdps for only the
> requested area.
>
> The logical context however needs all pdp entries to be valid.
> Prior to commit 06fda602dbca ("drm/i915: Create page table allocators")
> we have been writing pdp entries with dma address of zero instead
> of valid pdps. This is supposedly bad even if those pdps are not
> addressed.
>
> As commit 06fda602dbca ("drm/i915: Create page table allocators")
> introduced more dynamic structure for pdps, we ended up oopsing
> when we populated the lrc context. Analyzing this oops revealed
> the fact that we have not been writing valid pdps with bsw, as
> it is doing the ppgtt init with 2GB limit in some cases.
>
> We should do the right thing and setup the non addressable part
> pdps/pde/pte to scratch page through the minimal structure by
> having just pdp with pde entries pointing to same page with
> pte entries pointing to scratch page.
>
> But instead of going through that trouble, setup all the pdps
> through individual pd pages and pt entries, even for non
> addressable parts. And let the clear range point them to scratch
> page. This way we populate the lrc with valid pdps and wait
> for dynamic page allocation work to land, and do the heavy lifting
> for truncating page table tree according to usage.
>
> The regression of oopsing in init was introduced by
> commit 06fda602dbca ("drm/i915: Create page table allocators")
>
> v2: Clear the range for the unused part also (Ville)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89350
> Cc: Michel Thierry
> Cc: Ben Widawsky
> Cc: Ville Syrjälä
> Tested-by: Valtteri Rantala
> Signed-off-by: Mika Kuoppala
Seems sane enough assuming we're willing to pay the cost.
Reviewed-by: Ville Syrjälä
And then we could also throw another patch on top to actually increase the
PPGTT size to 4GiB since there's no extra cost anymore :) Well, assuming
everything else is 4GiB safe. But I guess it ought to be if BDW machines
have already been running with 4GiB GGTT/PPGTT.
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 21 +++--
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index bd95776..1aa2a2c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -716,15 +716,19 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
> uint64_t size)
> if (size % (1<<30))
> DRM_INFO("Pages will be wasted unless GTT size (%llu) is
> divisible by 1GB\n", size);
>
> - /* 1. Do all our allocations for page directories and page tables. */
> - ret = gen8_ppgtt_alloc(ppgtt, max_pdp);
> + /* 1. Do all our allocations for page directories and page tables.
> + * We allocate more than was asked so that we can point the unused parts
> + * to valid entries that point to scratch page. Dynamic page tables
> + * will fix this eventually.
> + */
> + ret = gen8_ppgtt_alloc(ppgtt, GEN8_LEGACY_PDPES);
> if (ret)
> return ret;
>
> /*
>* 2. Create DMA mappings for the page directories and page tables.
>*/
> - for (i = 0; i < max_pdp; i++) {
> + for (i = 0; i < GEN8_LEGACY_PDPES; i++) {
> ret = gen8_ppgtt_setup_page_directories(ppgtt, i);
> if (ret)
> goto bail;
> @@ -744,7 +748,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
> uint64_t size)
>* plugged in correctly. So we do that now/here. For aliasing PPGTT, we
>* will never need to touch the PDEs again.
>*/
> - for (i = 0; i < max_pdp; i++) {
> + for (i = 0; i < GEN8_LEGACY_PDPES; i++) {
> struct i915_page_directory_entry *pd =
> ppgtt->pdp.page_directory[i];
> gen8_ppgtt_pde_t *pd_vaddr;
> pd_vaddr = kmap_atomic(ppgtt->pdp.page_directory[i]->page);
> @@ -764,9 +768,14 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
> uint64_t size)
> ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
> ppgtt->base.cleanup = gen8_ppgtt_cleanup;
> ppgtt->base.start = 0;
> - ppgtt->base.total = ppgtt->num_pd_entries * GEN8_PTES_PER_PAGE *
> PAGE_SIZE;
>
> - ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->base.total, true);
> + /* This is the area that we advertise as usable for the caller */
> + ppgtt->base.total = max_pdp * GEN8_PDES_PER_PAGE * GEN8_PTES_PER_PAGE *
> PAGE_SIZE;
> +
> + /* Set all ptes to a valid scratch page. Also above requested space */
> + ppgtt->base.clear_range(&ppgtt->base, 0,
> + ppgtt->num_pd_pages * GEN8_PTES_PER_PAGE *
> PAGE_SIZE,
> + true);
>
> DRM_DEBUG_DRIVER("Allocated %d pages for page director
[Intel-gfx] [PATCH i-g-t] tests/kms_plane: Ensure planes recover from DPMS
i915 was using the main atomic 'disable plane' to turn off sprite planes
during a CRTC disable. This was problematic because it modified the
plane state, preventing us from recovering the original state later.
One such case was that during a DPMS OFF followed by a DPMS ON, any
sprite planes would not be restored properly.
Let's add a test that toggles DPMS off and on and ensures that the CRC
remains the same (i.e., planes are successfully restored unchanged).
Signed-off-by: Matt Roper
---
tests/kms_plane.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/tests/kms_plane.c b/tests/kms_plane.c
index 8a08f20..b66ab1d 100644
--- a/tests/kms_plane.c
+++ b/tests/kms_plane.c
@@ -145,6 +145,7 @@ create_fb_for_mode__position(data_t *data, drmModeModeInfo
*mode,
enum {
TEST_POSITION_FULLY_COVERED = 1 << 0,
+ TEST_DPMS = 1 << 1,
};
static void
@@ -158,7 +159,7 @@ test_plane_position_with_output(data_t *data,
igt_plane_t *primary, *sprite;
struct igt_fb primary_fb, sprite_fb;
drmModeModeInfo *mode;
- igt_crc_t crc;
+ igt_crc_t crc, crc2;
igt_info("Testing connector %s using pipe %s plane %d\n",
igt_output_name(output), kmstest_pipe_name(pipe), plane);
@@ -194,11 +195,24 @@ test_plane_position_with_output(data_t *data,
igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
+ if (flags & TEST_DPMS) {
+ kmstest_set_connector_dpms(data->drm_fd,
+ output->config.connector,
+ DRM_MODE_DPMS_OFF);
+ kmstest_set_connector_dpms(data->drm_fd,
+ output->config.connector,
+ DRM_MODE_DPMS_ON);
+ }
+
+ igt_pipe_crc_collect_crc(data->pipe_crc, &crc2);
+
if (flags & TEST_POSITION_FULLY_COVERED)
igt_assert(igt_crc_equal(&test.reference_crc, &crc));
else
igt_assert(!igt_crc_equal(&test.reference_crc, &crc));
+ igt_assert(igt_crc_equal(&crc, &crc2));
+
igt_plane_set_fb(primary, NULL);
igt_plane_set_fb(sprite, NULL);
@@ -358,6 +372,11 @@ run_tests_for_pipe_plane(data_t *data, enum pipe pipe,
enum igt_plane plane)
kmstest_pipe_name(pipe), plane)
test_plane_position(data, pipe, plane, 0);
+ igt_subtest_f("plane-position-hole-dpms-pipe-%s-plane-%d",
+ kmstest_pipe_name(pipe), plane)
+ test_plane_position(data, pipe, plane,
+ TEST_DPMS);
+
igt_subtest_f("plane-panning-top-left-pipe-%s-plane-%d",
kmstest_pipe_name(pipe), plane)
test_plane_panning(data, pipe, plane, TEST_PANNING_TOP_LEFT);
--
1.8.5.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Don't clobber plane state on internal disables
We need to disable all sprite planes when disabling the CRTC. We had
been using the top-level atomic 'disable' entrypoint to accomplish this,
which was wrong. Not only can this lead to various locking issues, it
also modifies the actual plane state, making it impossible to restore
the plane properly later. For example, a DPMS off followed by a DPMS on
will result in any sprite planes in use not being restored properly.
The proper solution here is to call directly into our 'commit plane'
hook with a copy of the plane's current state that has 'visible' set to
false. Committing this dummy state will turn off the plane, but will
not touch the actual plane->state pointer, allowing us to properly
restore the plane state later.
Signed-off-by: Matt Roper
---
This was a pretty big problem so I suspect this is probably tied to some of our
existing bugzilla's but I'm not sure exactly which ones. It seems that we lack
an i-g-t test for plane status across DPMS, so I'll send along a new i-g-t test
for that shortly.
drivers/gpu/drm/i915/intel_display.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 92cb2ff..6277701 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4201,6 +4201,24 @@ static void intel_enable_sprite_planes(struct drm_crtc
*crtc)
}
}
+/*
+ * Disable a plane internally without actually modifying the plane's state.
+ * This will allow us to easily restore the plane later by just reprogramming
+ * its state.
+ */
+static void disable_plane_internal(struct drm_plane *plane)
+{
+ struct intel_plane *intel_plane = to_intel_plane(plane);
+ struct drm_plane_state *state =
+ plane->funcs->atomic_duplicate_state(plane);
+ struct intel_plane_state *intel_state = to_intel_plane_state(state);
+
+ intel_state->visible = false;
+ intel_plane->commit_plane(plane, intel_state);
+
+ intel_plane_destroy_state(plane, state);
+}
+
static void intel_disable_sprite_planes(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
@@ -4210,8 +4228,8 @@ static void intel_disable_sprite_planes(struct drm_crtc
*crtc)
drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
intel_plane = to_intel_plane(plane);
- if (intel_plane->pipe == pipe)
- plane->funcs->disable_plane(plane);
+ if (plane->fb && intel_plane->pipe == pipe)
+ disable_plane_internal(plane);
}
}
--
1.8.5.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [4.0-rc2] WARNING at intel_check_page_flip
On Wed, Mar 04, 2015 at 06:42:53PM +0100, Thomas Meyer wrote: > Hi, > > my kernel log is full with those messages: > > [ 262.685467] [ cut here ] > [ 262.685481] WARNING: CPU: 0 PID: 50 at > drivers/gpu/drm/i915/intel_display.c:9719 intel_check_page_flip+0x9a/0xe0() > [ 262.685484] WARN_ON(!in_irq()) > [ 262.685486] Modules linked in: > [ 262.685489] vfat fat bluetooth fuse ipt_MASQUERADE nf_nat_masquerade_ipv4 > ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ip6table_nat > nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle > ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat > nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack > iptable_mangle iptable_security iptable_raw kvm_intel iwldvm kvm mac80211 > acer_wmi sparse_keymap snd_hda_codec_hdmi snd_hda_codec_realtek acerhdf > snd_hda_codec_generic pcspkr joydev snd_hda_intel snd_hda_controller > snd_hda_codec iwlwifi snd_hwdep snd_seq snd_seq_device cfg80211 snd_pcm > rfkill snd_timer snd soundcore wmi acpi_cpufreq sch_fq_codel ipv6 usb_storage > atl1c > [ 262.685547] CPU: 0 PID: 50 Comm: irq/26-i915 Tainted: GW > 4.0.0-rc2-23552-ga6c5170 #7 > [ 262.685549] Hardware name: Acer Aspire 1810T/JM11-MS, BIOS v1.3310 > 03/25/2010 > [ 262.685552] 57822dd5 8800b4563c38 > 816c0383 > [ 262.685556] 8800b4563c90 8800b4563c78 > 81053955 > [ 262.685559] 8800b4563c78 880233d01000 880233d51800 > 88023560 > [ 262.685564] Call Trace: > [ 262.685572] [] dump_stack+0x4c/0x6e > [ 262.685578] [] warn_slowpath_common+0x85/0xc0 > [ 262.685582] [] warn_slowpath_fmt+0x50/0x70 > [ 262.685587] [] intel_check_page_flip+0x9a/0xe0 > [ 262.685592] [] i915_handle_vblank+0x50/0xb0 > [ 262.685597] [] ? gen2_write32+0x2f/0xc0 > [ 262.685601] [] i965_irq_handler+0x2b6/0x390 > [ 262.685606] [] ? irq_thread_fn+0x40/0x40 > [ 262.685610] [] irq_forced_thread_fn+0x28/0x60 > [ 262.685614] [] irq_thread+0x137/0x160 > [ 262.685618] [] ? wake_threads_waitq+0x30/0x30 > [ 262.685622] [] ? irq_thread_check_affinity+0x90/0x90 > [ 262.685626] [] kthread+0xd3/0xf0 > [ 262.685631] [] ? kthread_create_on_node+0x1b0/0x1b0 > [ 262.685635] [] ret_from_fork+0x7c/0xb0 > [ 262.685639] [] ? kthread_create_on_node+0x1b0/0x1b0 > [ 262.685642] ---[ end trace 7601ad6d6a76fe2f ]--- > > $ cat /proc/cmdline > BOOT_IMAGE=/vmlinuz-4.0.0-rc2-23552-ga6c5170 ... threadirqs LANG=de_DE.UTF-8 ^^ There's your problem. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Setup all page directories for gen8
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5884 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -6 278/278 272/278 ILK -1 308/308 307/308 SNB -1 284/284 283/284 IVB 380/380 380/380 BYT 294/294 294/294 HSW 387/387 387/387 BDW -1 316/316 315/316 -Detailed- Platform Testdrm-intel-nightly Series Applied *PNV igt_gem_fence_thrash_bo-write-verify-none NRUN(1)PASS(7) FAIL(1)PASS(1) *PNV igt_gem_fence_thrash_bo-write-verify-x PASS(8) FAIL(1)PASS(1) *PNV igt_gem_fence_thrash_bo-write-verify-y NO_RESULT(1)PASS(7) FAIL(1)PASS(1) *PNV igt_gem_userptr_blits_minor-normal-sync PASS(5) DMESG_WARN(1)PASS(1) PNV igt_gen3_render_mixed_blits FAIL(10)PASS(9) FAIL(2) PNV igt_gem_fence_thrash_bo-write-verify-threaded-none FAIL(2)CRASH(5)PASS(7) CRASH(2) ILK igt_gem_unfence_active_buffers DMESG_WARN(1)PASS(3) DMESG_WARN(1)PASS(1) *SNB igt_gem_exec_params_sol-reset-not-gen7 PASS(2) DMESG_WARN(1)PASS(1) *BDW igt_gem_gtt_hog PASS(20) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ 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: Don't require primary->fb in intel_crtc_active()
On Wed, Mar 04, 2015 at 05:13:07PM +0100, Daniel Vetter wrote: > On Wed, Mar 04, 2015 at 11:45:42AM +0200, Ville Syrjälä wrote: > > On Tue, Mar 03, 2015 at 06:15:12PM -0800, Matt Roper wrote: > > > Universal planes allow us to have an active CRTC without a primary plane > > > framebuffer bound. Drop the test for primary->fb from > > > intel_crtc_active() since we can clearly have active CRTC's without a > > > framebuffer, and this check is now interfering with watermark > > > calculations when we try to re-enable the primary plane. > > > > > > Note that commit > > > > > > commit 0fda65680e92545caea5be7805a7f0a617fb6c20 > > > Author: Tvrtko Ursulin > > > Date: Fri Feb 27 15:12:35 2015 + > > > > > > drm/i915/skl: Update watermarks for Y tiling > > > > > > adds a test for primary plane enable/disable to trigger a watermark > > > update (previously we ignored updates to primary planes, which wasn't > > > really correct, but we got lucky since we always pretended the primary > > > plane was on). Tvrtko's patch tries to update watermarks when we > > > re-enable the primary plane, but that watermark computation gets aborted > > > early because intel_crtc_active() always returns false when the primary > > > plane is disabled; this leads to watermark underruns (at least on > > > platforms with ILK-style watermark code). > > > > I think this will make a bunch of the old platforms blow up. Well, I > > think they might already blow up since they now look at crtc->state->fb > > based on the result of intel_crtc_active(). So I believe more fixes are > > needed all over the wm code at least. > > > > In fact looking at the code, I think most (or all?) of the call sites in > > the in the ilk+ wm code should just be using crtc->active. So for some > > extra safety it might make sense to do leave intel_crtc_active() alone > > for now, or in fact we should maybe change it to also look at > > primary->state->fb instead. That should more or less keep the old > > platforms in the same state as they were before, while letting new > > platforms handle each plane properly. > > intel_crtc->active shouldn't be used anywhere in state computation code. > The only thing you're allowed to look at is crtc_state->enable in the > atomic world. We'll want crtc->active in a bunch of places in the ilk+ wm code since it's actually intersted in the current state, not the future state. The way it should work (after getting my remaining ilk+ wm patches reworked+merged): 1. Compute new watermarks for this specific pipe using the future plane/crtc state. We don't consider at all how many pipes will be active or are active currently, as those are not variables in our actual watermark equations. 2. Fail the operation if LP0 comes out invalid. LP0 limits do not depend on the number of active pipes, so a failure to meet the LP0 limits is always fatal. LP1+ limits we can ignore at this stage since LP1+ are optional. 3. Merge the watermarks computed at step 1 with the current watermarks on the pipe and store the result as the current watermarks on this pipe. These are what I called "intermediate watermarks" and are needed due to lack of double buffered watermark registers. 4. Merge the current watermarks from all currently active pipes, applying whatever extra limits are imposed by the number of active pipes (eg. disallow LP1+ on ilk/snb/ivb with multiple pipes) and write the result to the hardware registers 5. Commit the plane/crtc state 6. Wait until the hardware plane/crtc state has changed (ie. until the next vblank) 7. Store the watermarks computed at step 1 as the current watermarks of the pipe, replacing the "intermediate watermarks" from step 3 8. So step 1 is pretty much the only place where we should consider the future state as opposed to the current state of the hardware. -- 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: Make WAIT_IOCTL negative timeouts be indefinite again
This fixes a regression from
commit 5ed0bdf21a85d78e04f89f15ccf227562177cbd9
Author: Thomas Gleixner
Date: Wed Jul 16 21:05:06 2014 +
drm: i915: Use nsec based interfaces
that made a negative timeout return immediately rather than the
previously defined behaviour of waiting indefinitely.
Signed-off-by: Chris Wilson
Cc: Daniel Vetter
Cc: Ben Widawsky
Cc: Kristian Høgsberg
Cc: sta...@vger.kernel.org
---
drivers/gpu/drm/i915/i915_gem.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0cb858269be9..9d0df4d85693 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2957,9 +2957,9 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *file)
req = obj->last_read_req;
/* Do this after OLR check to make sure we make forward progress polling
-* on this IOCTL with a timeout <=0 (like busy ioctl)
+* on this IOCTL with a timeout == 0 (like busy ioctl)
*/
- if (args->timeout_ns <= 0) {
+ if (args->timeout_ns == 0) {
ret = -ETIME;
goto out;
}
@@ -2969,7 +2969,8 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *file)
i915_gem_request_reference(req);
mutex_unlock(&dev->struct_mutex);
- ret = __i915_wait_request(req, reset_counter, true, &args->timeout_ns,
+ ret = __i915_wait_request(req, reset_counter, true,
+ args->timeout_ns>0 ? &args->timeout_ns : NULL,
file->driver_priv);
mutex_lock(&dev->struct_mutex);
i915_gem_request_unreference(req);
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/7] drm/i915: add frontbuffer tracking to FBC
2015-03-03 21:57 GMT-03:00 Rodrigo Vivi :
> On Fri, Feb 13, 2015 at 11:23 AM, Paulo Zanoni wrote:
>> From: Paulo Zanoni
>>
>> Kill the blt/render tracking we currently have and use the frontbuffer
>> tracking infrastructure.
>>
>> Don't enable things by default yet.
>>
>> v2: (Rodrigo) Fix small conflict on rebase and typo at subject.
>> v3: (Paulo) Rebase on RENDER_CS change.
>> v4: (Paulo) Rebase.
>> v5: (Paulo) Simplify: flushes don't have origin (Daniel).
>> Also rebase due to patch order changes.
>>
>> Signed-off-by: Rodrigo Vivi
>> Signed-off-by: Paulo Zanoni
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 10 +---
>> drivers/gpu/drm/i915/intel_drv.h | 6 ++-
>> drivers/gpu/drm/i915/intel_fbc.c | 91
>> +++-
>> drivers/gpu/drm/i915/intel_frontbuffer.c | 14 +
>> drivers/gpu/drm/i915/intel_ringbuffer.c | 41 +-
>> drivers/gpu/drm/i915/intel_ringbuffer.h | 1 -
>> 6 files changed, 65 insertions(+), 98 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 30aaa8e..7680ca0 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -783,6 +783,8 @@ struct i915_fbc {
>> unsigned long uncompressed_size;
>> unsigned threshold;
>> unsigned int fb_id;
>> + unsigned int possible_framebuffer_bits;
>> + unsigned int busy_bits;
>
> I'm not sure if I understood why you keep 2 variables here?
After Daniel's last suggestion, the possible_framebuffer_bits could
probably be removed and left as a local variable at intel_fbc_validate
now: it's just a matter of coding style. Some platforms support FBC on
more than just pipe A, so this variable is used to simplify checks
related to this.
The busy_bits was also part of Daniel's last request: we need to store
which bits triggered intel_fbc_invalidate calls, and then check them
when intel_fbc_flush is called.
> Is it because we keep enabling/disabling fbc with updates all over the code?
> In this case it is ok we just need to kill it when we kill update_fbc...
I don't understand what you mean here. Please clarify.
>
>> struct intel_crtc *crtc;
>> int y;
>>
>> @@ -795,14 +797,6 @@ struct i915_fbc {
>> * possible. */
>> bool enabled;
>>
>> - /* On gen8 some rings cannont perform fbc clean operation so for now
>> -* we are doing this on SW with mmio.
>> -* This variable works in the opposite information direction
>> -* of ring->fbc_dirty telling software on frontbuffer tracking
>> -* to perform the cache clean on sw side.
>> -*/
>> - bool need_sw_cache_clean;
>> -
>> struct intel_fbc_work {
>> struct delayed_work work;
>> struct drm_crtc *crtc;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index 05d0a43f..571174f 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1091,7 +1091,11 @@ bool intel_fbc_enabled(struct drm_device *dev);
>> void intel_fbc_update(struct drm_device *dev);
>> void intel_fbc_init(struct drm_i915_private *dev_priv);
>> void intel_fbc_disable(struct drm_device *dev);
>> -void bdw_fbc_sw_flush(struct drm_device *dev, u32 value);
>> +void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
>> + unsigned int frontbuffer_bits,
>> + enum fb_op_origin origin);
>> +void intel_fbc_flush(struct drm_i915_private *dev_priv,
>> +unsigned int frontbuffer_bits);
>>
>> /* intel_hdmi.c */
>> void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port);
>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
>> b/drivers/gpu/drm/i915/intel_fbc.c
>> index 618f7bd..9fcf446 100644
>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>> @@ -174,29 +174,10 @@ static bool g4x_fbc_enabled(struct drm_device *dev)
>> return I915_READ(DPFC_CONTROL) & DPFC_CTL_EN;
>> }
>>
>> -static void snb_fbc_blit_update(struct drm_device *dev)
>> +static void intel_fbc_nuke(struct drm_i915_private *dev_priv)
>> {
>> - struct drm_i915_private *dev_priv = dev->dev_private;
>> - u32 blt_ecoskpd;
>> -
>> - /* Make sure blitter notifies FBC of writes */
>> -
>> - /* Blitter is part of Media powerwell on VLV. No impact of
>> -* his param in other platforms for now */
>> - intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
>> -
>> - blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD);
>> - blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY <<
>> - GEN6_BLITTER_LOCK_SHIFT;
>> - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
>> - blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY;
>> - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd);
>> - blt_ecoskpd &= ~(GEN6_BLITTER_FBC_NOTIFY <<
>> -
[Intel-gfx] [4.0-rc2] WARNING at intel_check_page_flip
Hi, my kernel log is full with those messages: [ 262.685467] [ cut here ] [ 262.685481] WARNING: CPU: 0 PID: 50 at drivers/gpu/drm/i915/intel_display.c:9719 intel_check_page_flip+0x9a/0xe0() [ 262.685484] WARN_ON(!in_irq()) [ 262.685486] Modules linked in: [ 262.685489] vfat fat bluetooth fuse ipt_MASQUERADE nf_nat_masquerade_ipv4 ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle iptable_security iptable_raw kvm_intel iwldvm kvm mac80211 acer_wmi sparse_keymap snd_hda_codec_hdmi snd_hda_codec_realtek acerhdf snd_hda_codec_generic pcspkr joydev snd_hda_intel snd_hda_controller snd_hda_codec iwlwifi snd_hwdep snd_seq snd_seq_device cfg80211 snd_pcm rfkill snd_timer snd soundcore wmi acpi_cpufreq sch_fq_codel ipv6 usb_storage atl1c [ 262.685547] CPU: 0 PID: 50 Comm: irq/26-i915 Tainted: GW 4.0.0-rc2-23552-ga6c5170 #7 [ 262.685549] Hardware name: Acer Aspire 1810T/JM11-MS, BIOS v1.3310 03/25/2010 [ 262.685552] 57822dd5 8800b4563c38 816c0383 [ 262.685556] 8800b4563c90 8800b4563c78 81053955 [ 262.685559] 8800b4563c78 880233d01000 880233d51800 88023560 [ 262.685564] Call Trace: [ 262.685572] [] dump_stack+0x4c/0x6e [ 262.685578] [] warn_slowpath_common+0x85/0xc0 [ 262.685582] [] warn_slowpath_fmt+0x50/0x70 [ 262.685587] [] intel_check_page_flip+0x9a/0xe0 [ 262.685592] [] i915_handle_vblank+0x50/0xb0 [ 262.685597] [] ? gen2_write32+0x2f/0xc0 [ 262.685601] [] i965_irq_handler+0x2b6/0x390 [ 262.685606] [] ? irq_thread_fn+0x40/0x40 [ 262.685610] [] irq_forced_thread_fn+0x28/0x60 [ 262.685614] [] irq_thread+0x137/0x160 [ 262.685618] [] ? wake_threads_waitq+0x30/0x30 [ 262.685622] [] ? irq_thread_check_affinity+0x90/0x90 [ 262.685626] [] kthread+0xd3/0xf0 [ 262.685631] [] ? kthread_create_on_node+0x1b0/0x1b0 [ 262.685635] [] ret_from_fork+0x7c/0xb0 [ 262.685639] [] ? kthread_create_on_node+0x1b0/0x1b0 [ 262.685642] ---[ end trace 7601ad6d6a76fe2f ]--- $ cat /proc/cmdline BOOT_IMAGE=/vmlinuz-4.0.0-rc2-23552-ga6c5170 root=UUID=8f9db3b8-8984-442c-bb88-e7de6f4f7c2d ro quiet rhgb threadirqs LANG=de_DE.UTF-8 any ideas? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/23] drm/i915: Copy the staged connector config to the legacy atomic state
On Wed, Mar 4, 2015 at 5:58 PM, Conselvan De Oliveira, Ander wrote: >> > + I meant this empty line between 2 closing braces ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Don't require primary->fb in intel_crtc_active()
On Wed, Mar 04, 2015 at 05:26:36PM +, Tvrtko Ursulin wrote: > > On 03/04/2015 02:15 AM, Matt Roper wrote: > >Universal planes allow us to have an active CRTC without a primary plane > >framebuffer bound. Drop the test for primary->fb from > >intel_crtc_active() since we can clearly have active CRTC's without a > >framebuffer, and this check is now interfering with watermark > >calculations when we try to re-enable the primary plane. > > > >Note that commit > > > > commit 0fda65680e92545caea5be7805a7f0a617fb6c20 > > Author: Tvrtko Ursulin > > Date: Fri Feb 27 15:12:35 2015 + > > > > drm/i915/skl: Update watermarks for Y tiling > > > >adds a test for primary plane enable/disable to trigger a watermark > >update (previously we ignored updates to primary planes, which wasn't > >really correct, but we got lucky since we always pretended the primary > >plane was on). Tvrtko's patch tries to update watermarks when we > >re-enable the primary plane, but that watermark computation gets aborted > >early because intel_crtc_active() always returns false when the primary > >plane is disabled; this leads to watermark underruns (at least on > >platforms with ILK-style watermark code). > > > >Cc: Tvrtko Ursulin > >Signed-off-by: Matt Roper > >--- > > drivers/gpu/drm/i915/intel_display.c | 5 + > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/intel_display.c > >b/drivers/gpu/drm/i915/intel_display.c > >index 589addf..92cb2ff 100644 > >--- a/drivers/gpu/drm/i915/intel_display.c > >+++ b/drivers/gpu/drm/i915/intel_display.c > >@@ -893,11 +893,8 @@ bool intel_crtc_active(struct drm_crtc *crtc) > > * > > * We can ditch the adjusted_mode.crtc_clock check as soon > > * as Haswell has gained clock readout/fastboot support. > >- * > >- * We can ditch the crtc->primary->fb check as soon as we can > >- * properly reconstruct framebuffers. > > */ > >-return intel_crtc->active && crtc->primary->fb && > >+return intel_crtc->active && > > intel_crtc->config->base.adjusted_mode.crtc_clock; > > Struggling to paint the whole picture here.. > > Why it was correct to replace primary->fb with primary->state->fb > elsewhere, but not here? > > Does the commit message mean there can be an active crtc with an > active plane, but crtc->primary->fb == NULL so the wm recompute > incorrectly configures for disabled plane? Back when this code was first written, we didn't have universal planes yet and the fb pointer was directly in the CRTC. So at that time, (crtc->fb == NULL) meant that the entire CRTC was off. When I added universal plane support, I used Coccinelle to globally do a s/crtc->fb/crtc->primary->fb/, so that change didn't really have any special thought going into it. However at that point we started allowing you to have an explicitly disabled primary plane with an active CRTC (something that had never been possible before with the legacy API's). In this case, crtc->primary->fb = NULL, but the CRTC is still running. So only considering a CRTC to be active if its primary plane has a framebuffer isn't really correct in the universal plane and atomic world. Whether we use primary->fb or primary->state->fb doesn't really matter in this case. My understanding is that the reason we also checked fb and mode in addition to intel_crtc->active had to do with quickboot logic and the driver's ability to inherit state setup by the boot firmware. I think Jesse or Ville might be able to explain that part better and describe any quickboot problems that this change might cause. This patch is still sort of a bandaid; ultimately our watermark code should all be state-based and we'll be looking at crtc->state->active and plane->state->FOO and such to figure out how/when to update our watermarks. But the watermark rework isn't done yet and we're also sort of in a weird limbo stage where planes are pretty much converted to atomic, but CRTC's are still being worked on. Matt > > Regards, > > Tvrtko -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 ___ 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: Don't assume primary & cursor are always on for wm calculation
On 03/04/2015 02:15 AM, Matt Roper wrote:
Current ILK-style watermark code assumes the primary plane and cursor
plane are always enabled. This assumption, along with the combination
of two independent commits that got merged at the same time, results in
a NULL dereference. The offending commits are:
commit fd2d61341bf39d1054256c07d6eddd624ebc4241
Author: Matt Roper
Date: Fri Feb 27 10:12:01 2015 -0800
drm/i915: Use plane->state->fb in watermark code (v2)
and
commit 0fda65680e92545caea5be7805a7f0a617fb6c20
Author: Tvrtko Ursulin
Date: Fri Feb 27 15:12:35 2015 +
drm/i915/skl: Update watermarks for Y tiling
The first commit causes us to use the FB from plane->state->fb rather
than the legacy plane->fb, which is updated a bit later in the process.
The second commit includes a change that now triggers watermark
reprogramming on primary plane enable/disable where we didn't have one
before (which wasn't really correct, but we had been getting lucky
because we always calculated as if the primary plane was on).
Together, these two commits cause the watermark calculation to
(properly) see plane->state->fb = NULL when we're in the process of
disabling the primary plane. However the existing watermark code
assumes there's always a primary fb and tries to dereference it to find
out pixel format / bpp information.
The fix is to make ILK-style watermark calculation actually check the
true status of primary & cursor planes and adjust our watermark logic
accordingly.
Cc: Tvrtko Ursulin
Cc: Michael Leuchtenburg
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89388
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/intel_pm.c | 22 +-
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e710b43..93fd15f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1924,13 +1924,25 @@ static void ilk_compute_wm_parameters(struct drm_crtc
*crtc,
p->active = true;
p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
- p->pri.bytes_per_pixel = crtc->primary->state->fb->bits_per_pixel / 8;
- p->cur.bytes_per_pixel = 4;
+
+ if (crtc->primary->state->fb) {
+ p->pri.enabled = true;
+ p->pri.bytes_per_pixel =
+ crtc->primary->state->fb->bits_per_pixel / 8;
+ } else {
+ p->pri.enabled = false;
+ p->pri.bytes_per_pixel = 0;
+ }
+
+ if (crtc->cursor->state->fb) {
+ p->cur.enabled = true;
+ p->cur.bytes_per_pixel = 4;
+ } else {
+ p->cur.enabled = false;
+ p->cur.bytes_per_pixel = 0;
+ }
p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
- /* TODO: for now, assume primary and cursor planes are always enabled.
*/
- p->pri.enabled = true;
- p->cur.enabled = true;
drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
struct intel_plane *intel_plane = to_intel_plane(plane);
Hm there are other place which dereference crtc->primary->state->fb,
like i9xx_update_wm and skl_compute_wm_pipe_parameters - they won't
suffer the same problem?
Regards,
Tvrtko
___
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: Don't require primary->fb in intel_crtc_active()
On 03/04/2015 02:15 AM, Matt Roper wrote: Universal planes allow us to have an active CRTC without a primary plane framebuffer bound. Drop the test for primary->fb from intel_crtc_active() since we can clearly have active CRTC's without a framebuffer, and this check is now interfering with watermark calculations when we try to re-enable the primary plane. Note that commit commit 0fda65680e92545caea5be7805a7f0a617fb6c20 Author: Tvrtko Ursulin Date: Fri Feb 27 15:12:35 2015 + drm/i915/skl: Update watermarks for Y tiling adds a test for primary plane enable/disable to trigger a watermark update (previously we ignored updates to primary planes, which wasn't really correct, but we got lucky since we always pretended the primary plane was on). Tvrtko's patch tries to update watermarks when we re-enable the primary plane, but that watermark computation gets aborted early because intel_crtc_active() always returns false when the primary plane is disabled; this leads to watermark underruns (at least on platforms with ILK-style watermark code). Cc: Tvrtko Ursulin Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/intel_display.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 589addf..92cb2ff 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -893,11 +893,8 @@ bool intel_crtc_active(struct drm_crtc *crtc) * * We can ditch the adjusted_mode.crtc_clock check as soon * as Haswell has gained clock readout/fastboot support. -* -* We can ditch the crtc->primary->fb check as soon as we can -* properly reconstruct framebuffers. */ - return intel_crtc->active && crtc->primary->fb && + return intel_crtc->active && intel_crtc->config->base.adjusted_mode.crtc_clock; Struggling to paint the whole picture here.. Why it was correct to replace primary->fb with primary->state->fb elsewhere, but not here? Does the commit message mean there can be an active crtc with an active plane, but crtc->primary->fb == NULL so the wm recompute incorrectly configures for disabled plane? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 21/23] drm/i915: Convert intel_pipe_will_have_type() to using atomic state
On Wed, 2015-03-04 at 17:03 +0100, Daniel Vetter wrote:
> On Tue, Mar 03, 2015 at 03:22:15PM +0200, Ander Conselvan de Oliveira wrote:
> > Pass a crtc_state to it and find whether the pipe has an encoder of a
> > given type by looking at the drm_atomic_state the crtc_state points to.
> >
> > Note that is possible to reach i9xx_get_refclk() without a proper atomic
> > state, since in the function vlv_force_pll_on() a minimally initialized
> > crtc_state is allocated in the stack. With the current code, it is not
> > possible to end up in a call to intel_pipe_will_have_type() with that
> > bogus atomic state. To avoid future problems, a comment is added to warn
> > people changing that code.
> >
> > Signed-off-by: Ander Conselvan de Oliveira
> >
> > ---
> > drivers/gpu/drm/i915/i915_drv.h | 2 +-
> > drivers/gpu/drm/i915/intel_display.c | 134
> > +--
> > 2 files changed, 83 insertions(+), 53 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 0c6ba2d..aab4421 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -541,7 +541,7 @@ struct drm_i915_display_funcs {
> > * Returns true on success, false on failure.
> > */
> > bool (*find_dpll)(const struct intel_limit *limit,
> > - struct intel_crtc *crtc,
> > + struct intel_crtc_state *crtc_state,
> > int target, int refclk,
> > struct dpll *match_clock,
> > struct dpll *best_clock);
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 518903e..f3652f9 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -431,25 +431,37 @@ bool intel_pipe_has_type(struct intel_crtc *crtc,
> > enum intel_output_type type)
> > * intel_pipe_has_type() but looking at encoder->new_crtc instead of
> > * encoder->crtc.
> > */
> > -static bool intel_pipe_will_have_type(struct intel_crtc *crtc, int type)
> > +static bool intel_pipe_will_have_type(const struct intel_crtc_state
> > *crtc_state,
> > + int type)
> > {
> > - struct drm_device *dev = crtc->base.dev;
> > + struct drm_atomic_state *state = crtc_state->base.state;
> > + struct drm_connector_state *connector_state;
> > struct intel_encoder *encoder;
> > + int i;
> > +
> > + for (i = 0; i < state->num_connector; i++) {
> > + if (!state->connectors[i])
> > + continue;
> > +
> > + connector_state = state->connector_states[i];
> > + if (connector_state->crtc != crtc_state->base.crtc)
> > + continue;
> >
> > - for_each_intel_encoder(dev, encoder)
> > - if (encoder->new_crtc == crtc && encoder->type == type)
> > + encoder = to_intel_encoder(connector_state->best_encoder);
> > + if (encoder->type == type)
> > return true;
> > + }
> >
> > return false;
> > }
>
> The tricky bit here is that we must have all the connectors added to the
> drm_atomic_sate for the given crtc. Otherwise there might be no connector
> at all and we'd return a bogus answer. drm_atomic_add_affected_connectors
> is the function which does this for you, and I think we need to call it
> somewhere in the top-level compute_config code. And I haven't spotted that
> call anywhere in your series.
I did add it in patch 12, but now I realize it won't be called for the
disable case, since the call is in intel_modeset_pipe_config(). I'll
move that to intel_modeset_compute_config().
Ander
> Otherwise the implementation of the new will_have_type looks correct.
> -Daniel
>
> >
> > -static const intel_limit_t *intel_ironlake_limit(struct intel_crtc *crtc,
> > - int refclk)
> > +static const intel_limit_t *
> > +intel_ironlake_limit(struct intel_crtc_state *crtc_state, int refclk)
> > {
> > - struct drm_device *dev = crtc->base.dev;
> > + struct drm_device *dev = crtc_state->base.crtc->dev;
> > const intel_limit_t *limit;
> >
> > - if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> > + if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> > if (intel_is_dual_link_lvds(dev)) {
> > if (refclk == 10)
> > limit = &intel_limits_ironlake_dual_lvds_100m;
> > @@ -467,20 +479,21 @@ static const intel_limit_t
> > *intel_ironlake_limit(struct intel_crtc *crtc,
> > return limit;
> > }
> >
> > -static const intel_limit_t *intel_g4x_limit(struct intel_crtc *crtc)
> > +static const intel_limit_t *
> > +intel_g4x_limit(struct intel_crtc_state *crtc_state)
> > {
> > - struct drm_device *dev = crtc->base.dev;
> > + struct drm_device *dev = crtc_state->base.crtc->dev;
> > const inte
Re: [Intel-gfx] [PATCH 21/23] drm/i915: Convert intel_pipe_will_have_type() to using atomic state
On Wed, Mar 4, 2015 at 5:51 PM, Conselvan De Oliveira, Ander wrote: >> The tricky bit here is that we must have all the connectors added to the >> drm_atomic_sate for the given crtc. Otherwise there might be no connector >> at all and we'd return a bogus answer. drm_atomic_add_affected_connectors >> is the function which does this for you, and I think we need to call it >> somewhere in the top-level compute_config code. And I haven't spotted that >> call anywhere in your series. > > I did add it in patch 12, but now I realize it won't be called for the > disable case, since the call is in intel_modeset_pipe_config(). I'll > move that to intel_modeset_compute_config(). Indeed. I've used my mail client to search for it, no idea why that didn't show up ... For safety maybe we should have a WARN_ON if there are no connectors for a given crtc? Normally there's no cloning going on hence should catch all such bugs. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/23] drm/i915: Copy the staged connector config to the legacy atomic state
On Wed, 2015-03-04 at 16:46 +0100, Daniel Vetter wrote:
> On Tue, Mar 03, 2015 at 03:22:05PM +0200, Ander Conselvan de Oliveira wrote:
> > With this in place, we can start converting pieces of the modeset code
> > to look at the connector atomic state instead of the staged config.
> >
> > Signed-off-by: Ander Conselvan de Oliveira
> >
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 23 ---
> > 1 file changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 108d3d2..4e90cb4 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11607,9 +11607,11 @@ intel_set_config_compute_mode_changes(struct
> > drm_mode_set *set,
> > static int
> > intel_modeset_stage_output_state(struct drm_device *dev,
> > struct drm_mode_set *set,
> > -struct intel_set_config *config)
> > +struct intel_set_config *config,
> > +struct drm_atomic_state *state)
> > {
> > struct intel_connector *connector;
> > + struct drm_connector_state *connector_state;
> > struct intel_encoder *encoder;
> > struct intel_crtc *crtc;
> > int ro;
> > @@ -11673,6 +11675,14 @@ intel_modeset_stage_output_state(struct drm_device
> > *dev,
> > }
> > connector->new_encoder->new_crtc = to_intel_crtc(new_crtc);
> >
> > + connector_state =
> > + drm_atomic_get_connector_state(state, &connector->base);
> > + if (IS_ERR(connector_state))
> > + return PTR_ERR(connector_state);
> > +
> > + connector_state->crtc = new_crtc;
> > + connector_state->best_encoder = &connector->new_encoder->base;
> > +
> > DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [CRTC:%d]\n",
> > connector->base.base.id,
> > connector->base.name,
> > @@ -11705,9 +11715,16 @@ intel_modeset_stage_output_state(struct drm_device
> > *dev,
> > }
> > /* Now we've also updated encoder->new_crtc for all encoders. */
> > for_each_intel_connector(dev, connector) {
> > - if (connector->new_encoder)
> > + connector_state =
> > + drm_atomic_get_connector_state(state, &connector->base);
> > +
> > + if (connector->new_encoder) {
> > if (connector->new_encoder != connector->encoder)
> > connector->encoder = connector->new_encoder;
> > + } else {
> > + connector_state->crtc = NULL;
> > + }
> > +
>
> Unecessary line. Of course you've put that in there to check that I
> actually read your patches ;-)
It actually is needed, since the other hunk we only update the
connector's crtc when it has an enabled connector. It makes sense for
the staged config, since the new_crtc field is in the encoder, which
would be NULL in that case. The loop just above this one sets new_crtc
to NULL for these encoders, but for the connectors it was just more
convenient to set it here.
Ander
>
> Cheers, Daniel
>
> > }
> > for_each_intel_crtc(dev, crtc) {
> > crtc->new_enabled = false;
> > @@ -11816,7 +11833,7 @@ static int intel_crtc_set_config(struct
> > drm_mode_set *set)
> >
> > state->acquire_ctx = dev->mode_config.acquire_ctx;
> >
> > - ret = intel_modeset_stage_output_state(dev, set, config);
> > + ret = intel_modeset_stage_output_state(dev, set, config, state);
> > if (ret)
> > goto fail;
> >
> > --
> > 2.1.0
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
-
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/8] drm: Pass in new and old plane state to prepare_fb and cleanup_fb
On Tue, Mar 3, 2015 at 9:22 AM, Tvrtko Ursulin
wrote:
> From: Tvrtko Ursulin
>
> Use cases like rotation require these hooks to have some context so they
> know how to prepare and cleanup the frame buffer correctly.
>
> For i915 specifically, object backing pages need to be mapped differently
> for different rotation modes and the driver needs to know which mapping to
> instantiate and which to tear down when transitioning between them.
>
> v2: Made passed in states const. (Daniel Vetter)
>
> Signed-off-by: Tvrtko Ursulin
> Cc: Daniel Vetter
> Cc: dri-de...@lists.freedesktop.org
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 13 -
> drivers/gpu/drm/drm_plane_helper.c| 5 +++--
> drivers/gpu/drm/i915/intel_display.c | 6 --
> drivers/gpu/drm/i915/intel_drv.h | 6 --
> drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 6 --
jfyi, mdp5 would need similar fixup..
and with my bikeshedding hat on, maybe swap the order of plane_state
and fb.. somehow that feels like a more natural order (but that is
just bikeshedding, feel free to ignore that part)
with mdp5 (and with or without bikeshed),
Reviewed-by: Rob Clark
BR,
-R
> drivers/gpu/drm/tegra/dc.c| 6 --
> include/drm/drm_plane_helper.h| 6 --
> 7 files changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 3ce57f4..a745881 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1116,6 +1116,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device
> *dev,
> for (i = 0; i < nplanes; i++) {
> struct drm_plane_helper_funcs *funcs;
> struct drm_plane *plane = state->planes[i];
> + struct drm_plane_state *plane_state = state->plane_states[i];
> struct drm_framebuffer *fb;
>
> if (!plane)
> @@ -1123,10 +1124,10 @@ int drm_atomic_helper_prepare_planes(struct
> drm_device *dev,
>
> funcs = plane->helper_private;
>
> - fb = state->plane_states[i]->fb;
> + fb = plane_state->fb;
>
> if (fb && funcs->prepare_fb) {
> - ret = funcs->prepare_fb(plane, fb);
> + ret = funcs->prepare_fb(plane, fb, plane_state);
> if (ret)
> goto fail;
> }
> @@ -1138,6 +1139,7 @@ fail:
> for (i--; i >= 0; i--) {
> struct drm_plane_helper_funcs *funcs;
> struct drm_plane *plane = state->planes[i];
> + struct drm_plane_state *plane_state = state->plane_states[i];
> struct drm_framebuffer *fb;
>
> if (!plane)
> @@ -1148,7 +1150,7 @@ fail:
> fb = state->plane_states[i]->fb;
>
> if (fb && funcs->cleanup_fb)
> - funcs->cleanup_fb(plane, fb);
> + funcs->cleanup_fb(plane, fb, plane_state);
>
> }
>
> @@ -1254,6 +1256,7 @@ void drm_atomic_helper_cleanup_planes(struct drm_device
> *dev,
> for (i = 0; i < nplanes; i++) {
> struct drm_plane_helper_funcs *funcs;
> struct drm_plane *plane = old_state->planes[i];
> + struct drm_plane_state *plane_state =
> old_state->plane_states[i];
> struct drm_framebuffer *old_fb;
>
> if (!plane)
> @@ -1261,10 +1264,10 @@ void drm_atomic_helper_cleanup_planes(struct
> drm_device *dev,
>
> funcs = plane->helper_private;
>
> - old_fb = old_state->plane_states[i]->fb;
> + old_fb = plane_state->fb;
>
> if (old_fb && funcs->cleanup_fb)
> - funcs->cleanup_fb(plane, old_fb);
> + funcs->cleanup_fb(plane, old_fb, plane_state);
> }
> }
> EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
> diff --git a/drivers/gpu/drm/drm_plane_helper.c
> b/drivers/gpu/drm/drm_plane_helper.c
> index 5ba5792..813a066 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -437,7 +437,8 @@ int drm_plane_helper_commit(struct drm_plane *plane,
>
> if (plane_funcs->prepare_fb && plane_state->fb &&
> plane_state->fb != old_fb) {
> - ret = plane_funcs->prepare_fb(plane, plane_state->fb);
> + ret = plane_funcs->prepare_fb(plane, plane_state->fb,
> + plane_state);
> if (ret)
> goto out;
> }
> @@ -487,7 +488,7 @@ int drm_plane_helper_commit(struct drm_plane *plane,
> }
>
> if (plane_funcs->cleanup_fb && old_fb)
> - plane_funcs->cleanup_fb(plane, old_fb);
> + plane_funcs->cleanup_fb(plane, old_fb, plane_state);
> out:
> if (pla
Re: [Intel-gfx] [PATCH] drm/mm: Support 4 GiB and larger ranges
On Fri, Jan 23, 2015 at 3:05 AM, Thierry Reding
wrote:
> From: Thierry Reding
>
> The current implementation is limited by the number of addresses that
> fit into an unsigned long. This causes problems on 32-bit Tegra where
> unsigned long is 32-bit but drm_mm is used to manage an IOVA space of
> 4 GiB. Given the 32-bit limitation, the range is limited to 4 GiB - 1
> (or 4 GiB - 4 KiB for page granularity).
>
> This commit changes the start and size of the range to be an unsigned
> 64-bit integer, thus allowing much larger ranges to be supported.
>
> Signed-off-by: Thierry Reding
Did this land yet? I think we need this for for the full fix for the
case when device address space does not match CPU address space.
E.g., radeons have 40 or 48 bit internal address spaces. On radeon's
with 4GB or more vram, the gart aperture ends up above that in the
GPU's address space which ends up getting truncated on 32 bit. See
the discussion here:
http://lists.freedesktop.org/archives/dri-devel/2015-March/078614.html
Alex
> ---
> drivers/gpu/drm/drm_mm.c | 152
> ---
> include/drm/drm_mm.h | 52
> 2 files changed, 105 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 04a209e2b66d..7fc6f8bd4821 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -91,29 +91,29 @@
> */
>
> static struct drm_mm_node *drm_mm_search_free_generic(const struct drm_mm
> *mm,
> - unsigned long size,
> + u64 size,
> unsigned alignment,
> unsigned long color,
> enum drm_mm_search_flags
> flags);
> static struct drm_mm_node *drm_mm_search_free_in_range_generic(const struct
> drm_mm *mm,
> - unsigned long size,
> + u64 size,
> unsigned alignment,
> unsigned long color,
> - unsigned long start,
> - unsigned long end,
> + u64 start,
> + u64 end,
> enum drm_mm_search_flags
> flags);
>
> static void drm_mm_insert_helper(struct drm_mm_node *hole_node,
> struct drm_mm_node *node,
> -unsigned long size, unsigned alignment,
> +u64 size, unsigned alignment,
> unsigned long color,
> enum drm_mm_allocator_flags flags)
> {
> struct drm_mm *mm = hole_node->mm;
> - unsigned long hole_start = drm_mm_hole_node_start(hole_node);
> - unsigned long hole_end = drm_mm_hole_node_end(hole_node);
> - unsigned long adj_start = hole_start;
> - unsigned long adj_end = hole_end;
> + u64 hole_start = drm_mm_hole_node_start(hole_node);
> + u64 hole_end = drm_mm_hole_node_end(hole_node);
> + u64 adj_start = hole_start;
> + u64 adj_end = hole_end;
>
> BUG_ON(node->allocated);
>
> @@ -124,12 +124,15 @@ static void drm_mm_insert_helper(struct drm_mm_node
> *hole_node,
> adj_start = adj_end - size;
>
> if (alignment) {
> - unsigned tmp = adj_start % alignment;
> - if (tmp) {
> + u64 tmp = adj_start;
> + unsigned rem;
> +
> + rem = do_div(tmp, alignment);
> + if (rem) {
> if (flags & DRM_MM_CREATE_TOP)
> - adj_start -= tmp;
> + adj_start -= rem;
> else
> - adj_start += alignment - tmp;
> + adj_start += alignment - rem;
> }
> }
>
> @@ -176,9 +179,9 @@ static void drm_mm_insert_helper(struct drm_mm_node
> *hole_node,
> int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node)
> {
> struct drm_mm_node *hole;
> - unsigned long end = node->start + node->size;
> - unsigned long hole_start;
> - unsigned long hole_end;
> + u64 end = node->start + node->size;
> + u64 hole_start;
> + u64 hole_end;
>
> BUG_ON(node == NULL);
>
> @@ -227,7 +230,7 @@ EXPORT_SYMBOL(drm_mm_reserve_node);
> * 0 on success, -ENOSPC if there's no suitable hole.
> */
> int drm_mm_insert_node_generic(struct drm_mm *mm, struct drm_mm_node *node,
> - unsigned long size, unsigned alignment,
> +
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Don't assume primary & cursor are always on for wm calculation
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5883 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -8 278/278 270/278 ILK 308/308 308/308 SNB -1 284/284 283/284 IVB 380/380 380/380 BYT 294/294 294/294 HSW 387/387 387/387 BDW -1 316/316 315/316 -Detailed- Platform Testdrm-intel-nightly Series Applied PNV igt_gem_userptr_blits_coherency-sync NO_RESULT(1)CRASH(8)NRUN(1)PASS(8) CRASH(1)PASS(1) PNV igt_gem_userptr_blits_coherency-unsync FAIL(1)NO_RESULT(1)CRASH(6)PASS(7) CRASH(1)PASS(1) *PNV igt_gem_userptr_blits_forbidden-operations PASS(2) NRUN(1)PASS(1) *PNV igt_gem_userptr_blits_forked-access PASS(2) NRUN(1)PASS(1) *PNV igt_gem_userptr_blits_forked-sync-interruptible PASS(2) NRUN(1)PASS(1) PNV igt_gen3_render_linear_blits FAIL(6)NRUN(1)DMESG_WARN(1)PASS(9) FAIL(2) PNV igt_gen3_render_mixed_blits FAIL(10)PASS(9) FAIL(2) PNV igt_gem_fence_thrash_bo-write-verify-threaded-none FAIL(2)CRASH(5)PASS(7) CRASH(2) *SNB igt_gem_flink_bad-flink PASS(3) DMESG_WARN(1)PASS(1) *BDW igt_gem_gtt_hog PASS(19) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add I915_PARAM_REVISION
On Wed, Mar 04, 2015 at 02:41:16PM +, Neil Roberts wrote:
> Adds a parameter which can be used with DRM_I915_GETPARAM to query the
> GPU revision. The intention is to use this in Mesa to implement the
> WaDisableSIMD16On3SrcInstr workaround on Skylake but only for
> revision 2.
>
> Signed-off-by: Neil Roberts
Just listened to Damien explain this all and pulled it right into dinq.
Cheers, Daniel
> ---
> The corresponding Mesa patches are here:
>
> http://lists.freedesktop.org/archives/mesa-dev/2015-March/078534.html
>
> and the libdrm patch is here:
>
> http://lists.freedesktop.org/archives/dri-devel/2015-March/078645.html
>
> drivers/gpu/drm/i915/i915_dma.c | 3 +++
> include/uapi/drm/i915_drm.h | 1 +
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 0312879..36dd1df 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -68,6 +68,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
> case I915_PARAM_CHIPSET_ID:
> value = dev->pdev->device;
> break;
> + case I915_PARAM_REVISION:
> + value = dev->pdev->revision;
> + break;
> case I915_PARAM_HAS_GEM:
> value = 1;
> break;
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 6eed16b..b768f3b 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -347,6 +347,7 @@ typedef struct drm_i915_irq_wait {
> #define I915_PARAM_HAS_COHERENT_PHYS_GTT 29
> #define I915_PARAM_MMAP_VERSION 30
> #define I915_PARAM_HAS_BSD2 31
> +#define I915_PARAM_REVISION 32
>
> typedef struct drm_i915_getparam {
> int param;
> --
> 1.9.3
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add module param to test the load detect code
On 03/04/2015 12:56 AM, Chris Wilson wrote: > On Tue, Mar 03, 2015 at 04:27:47PM -0800, Jesse Barnes wrote: >> My "rant du jour" still is. mlock() is a good solution for some things, >> but for the simple task of testing kernel swap out code, just running >> that code is the most straightforward thing to do, rather than trying to >> contort into it from userspace. > > Once upon a time, there was a minor request for MADV_POPULATE and > MADV_INVALIDATE (basically wrappers around get_pages() and put_pages()) > which would give you the fuzzy feeling of testing those paths without > actually testing the shrinker. But no use cases. Yeah, that sounds cool, but otoh adding stable ABI just for debug is a little scary... Jesse ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Don't require primary->fb in intel_crtc_active()
On Tue, Mar 03, 2015 at 06:15:12PM -0800, Matt Roper wrote: > Universal planes allow us to have an active CRTC without a primary plane > framebuffer bound. Drop the test for primary->fb from > intel_crtc_active() since we can clearly have active CRTC's without a > framebuffer, and this check is now interfering with watermark > calculations when we try to re-enable the primary plane. > > Note that commit > > commit 0fda65680e92545caea5be7805a7f0a617fb6c20 > Author: Tvrtko Ursulin > Date: Fri Feb 27 15:12:35 2015 + > > drm/i915/skl: Update watermarks for Y tiling > > adds a test for primary plane enable/disable to trigger a watermark > update (previously we ignored updates to primary planes, which wasn't > really correct, but we got lucky since we always pretended the primary > plane was on). Tvrtko's patch tries to update watermarks when we > re-enable the primary plane, but that watermark computation gets aborted > early because intel_crtc_active() always returns false when the primary > plane is disabled; this leads to watermark underruns (at least on > platforms with ILK-style watermark code). That pretty much means the igt test coverage isn't exhaustive enough and doesn't properly check that the right stuff shows up on the screens with CRCs. Tvrtko can you please add relevant tests for this scenario? Thanks, Daniel > > Cc: Tvrtko Ursulin > Signed-off-by: Matt Roper > --- > drivers/gpu/drm/i915/intel_display.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 589addf..92cb2ff 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -893,11 +893,8 @@ bool intel_crtc_active(struct drm_crtc *crtc) >* >* We can ditch the adjusted_mode.crtc_clock check as soon >* as Haswell has gained clock readout/fastboot support. > - * > - * We can ditch the crtc->primary->fb check as soon as we can > - * properly reconstruct framebuffers. >*/ > - return intel_crtc->active && crtc->primary->fb && > + return intel_crtc->active && > intel_crtc->config->base.adjusted_mode.crtc_clock; > } > > -- > 1.8.5.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add module param to test the load detect code
On Tue, Mar 03, 2015 at 04:27:47PM -0800, Jesse Barnes wrote: > On 03/03/2015 04:06 PM, Daniel Vetter wrote: > > On Tue, Mar 3, 2015 at 10:15 PM, Chris Wilson > > wrote: > >> On Tue, Mar 03, 2015 at 11:23:53AM -0800, Jesse Barnes wrote: > >>> On 03/03/2015 09:03 AM, Daniel Vetter wrote: > This is useful for writing igts to make sure we don't break this, > without being forced to own a one of these dinosaurs. > > Suggested-by: Jesse Barnes > Cc: Matt Roper > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_drv.h| 1 + > drivers/gpu/drm/i915/i915_params.c | 8 +++- > drivers/gpu/drm/i915/intel_crt.c | 6 -- > 3 files changed, 12 insertions(+), 3 deletions(-) > >>> > >>> See below for comments. > >>> > >>> I think there's probably even more room for testing like this. E.g. the > >>> tiled swapping test could be done this way rather than trying to force > >>> swapping. Some of the races we try to induce could probably also be > >>> done this way with code in the kernel to trigger the case we're worried > >>> about... > >> > >> I am not wholly convinced. The primary purpose of the test suite is > >> prevent bugs of tomorrow, not to chase bugs of yesterday. If we focus too > >> much on bugs we have fixed, I worry we won't serendipitously detect bugs > >> early. Yes, bugs cluster and a mistake once made is likely to be made > >> again (so regression testing is vital) but I think we cross a line if > >> igt only exercises code written for conformance testing. > > > > Also swapping tests are a solved problem really, we've had an "mlock > > most of mem" todo since years and Thomas Wood is implementing it. Well > > it's committed for gem_tiled_swapping already > > > > commit 42b02c284ed24871528df8f1b3eaad7fe1554fd9 > > Author: Thomas Wood > > Date: Mon Dec 8 11:12:51 2014 + > > > > lib: add a function to lock memory into RAM > > > > what's missing is rolling this out for other tests. > > > > Now I love igt bashing as much as the next person, but maybe check > > occasionally whether your rant-du-jour is still relevant ... > > My "rant du jour" still is. mlock() is a good solution for some things, > but for the simple task of testing kernel swap out code, just running > that code is the most straightforward thing to do, rather than trying to > contort into it from userspace. swap-out/in isn't really simple. We could extract the bit 17 swizzling logic into a separate bit with everything else mock-ups, but that's just testing our sw implementation against our sw model. Not that useful imo, and like Chris I'm not convinced of the value of lots of such special-case code. On top of that bit17 swizzling is but a facet of swap-in/out support. Active tracking, page dirtying and all that also need to work correctly. I don't see how you can test that in a simple fashion. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Don't require primary->fb in intel_crtc_active()
On Wed, Mar 04, 2015 at 11:45:42AM +0200, Ville Syrjälä wrote: > On Tue, Mar 03, 2015 at 06:15:12PM -0800, Matt Roper wrote: > > Universal planes allow us to have an active CRTC without a primary plane > > framebuffer bound. Drop the test for primary->fb from > > intel_crtc_active() since we can clearly have active CRTC's without a > > framebuffer, and this check is now interfering with watermark > > calculations when we try to re-enable the primary plane. > > > > Note that commit > > > > commit 0fda65680e92545caea5be7805a7f0a617fb6c20 > > Author: Tvrtko Ursulin > > Date: Fri Feb 27 15:12:35 2015 + > > > > drm/i915/skl: Update watermarks for Y tiling > > > > adds a test for primary plane enable/disable to trigger a watermark > > update (previously we ignored updates to primary planes, which wasn't > > really correct, but we got lucky since we always pretended the primary > > plane was on). Tvrtko's patch tries to update watermarks when we > > re-enable the primary plane, but that watermark computation gets aborted > > early because intel_crtc_active() always returns false when the primary > > plane is disabled; this leads to watermark underruns (at least on > > platforms with ILK-style watermark code). > > I think this will make a bunch of the old platforms blow up. Well, I > think they might already blow up since they now look at crtc->state->fb > based on the result of intel_crtc_active(). So I believe more fixes are > needed all over the wm code at least. > > In fact looking at the code, I think most (or all?) of the call sites in > the in the ilk+ wm code should just be using crtc->active. So for some > extra safety it might make sense to do leave intel_crtc_active() alone > for now, or in fact we should maybe change it to also look at > primary->state->fb instead. That should more or less keep the old > platforms in the same state as they were before, while letting new > platforms handle each plane properly. intel_crtc->active shouldn't be used anywhere in state computation code. The only thing you're allowed to look at is crtc_state->enable in the atomic world. Otherwise resource allocation isn't properly done when the pipe is in dpms off state and then stuff blows up when userspace tries to dpms on. Which is a big no-no, at least for proper backwards compat. -Daniel > > > > > Cc: Tvrtko Ursulin > > Signed-off-by: Matt Roper > > --- > > drivers/gpu/drm/i915/intel_display.c | 5 + > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 589addf..92cb2ff 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -893,11 +893,8 @@ bool intel_crtc_active(struct drm_crtc *crtc) > > * > > * We can ditch the adjusted_mode.crtc_clock check as soon > > * as Haswell has gained clock readout/fastboot support. > > -* > > -* We can ditch the crtc->primary->fb check as soon as we can > > -* properly reconstruct framebuffers. > > */ > > - return intel_crtc->active && crtc->primary->fb && > > + return intel_crtc->active && > > intel_crtc->config->base.adjusted_mode.crtc_clock; > > } > > > > -- > > 1.8.5.1 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 21/23] drm/i915: Convert intel_pipe_will_have_type() to using atomic state
On Tue, Mar 03, 2015 at 03:22:15PM +0200, Ander Conselvan de Oliveira wrote:
> Pass a crtc_state to it and find whether the pipe has an encoder of a
> given type by looking at the drm_atomic_state the crtc_state points to.
>
> Note that is possible to reach i9xx_get_refclk() without a proper atomic
> state, since in the function vlv_force_pll_on() a minimally initialized
> crtc_state is allocated in the stack. With the current code, it is not
> possible to end up in a call to intel_pipe_will_have_type() with that
> bogus atomic state. To avoid future problems, a comment is added to warn
> people changing that code.
>
> Signed-off-by: Ander Conselvan de Oliveira
>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 134
> +--
> 2 files changed, 83 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 0c6ba2d..aab4421 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -541,7 +541,7 @@ struct drm_i915_display_funcs {
>* Returns true on success, false on failure.
>*/
> bool (*find_dpll)(const struct intel_limit *limit,
> - struct intel_crtc *crtc,
> + struct intel_crtc_state *crtc_state,
> int target, int refclk,
> struct dpll *match_clock,
> struct dpll *best_clock);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 518903e..f3652f9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -431,25 +431,37 @@ bool intel_pipe_has_type(struct intel_crtc *crtc, enum
> intel_output_type type)
> * intel_pipe_has_type() but looking at encoder->new_crtc instead of
> * encoder->crtc.
> */
> -static bool intel_pipe_will_have_type(struct intel_crtc *crtc, int type)
> +static bool intel_pipe_will_have_type(const struct intel_crtc_state
> *crtc_state,
> + int type)
> {
> - struct drm_device *dev = crtc->base.dev;
> + struct drm_atomic_state *state = crtc_state->base.state;
> + struct drm_connector_state *connector_state;
> struct intel_encoder *encoder;
> + int i;
> +
> + for (i = 0; i < state->num_connector; i++) {
> + if (!state->connectors[i])
> + continue;
> +
> + connector_state = state->connector_states[i];
> + if (connector_state->crtc != crtc_state->base.crtc)
> + continue;
>
> - for_each_intel_encoder(dev, encoder)
> - if (encoder->new_crtc == crtc && encoder->type == type)
> + encoder = to_intel_encoder(connector_state->best_encoder);
> + if (encoder->type == type)
> return true;
> + }
>
> return false;
> }
The tricky bit here is that we must have all the connectors added to the
drm_atomic_sate for the given crtc. Otherwise there might be no connector
at all and we'd return a bogus answer. drm_atomic_add_affected_connectors
is the function which does this for you, and I think we need to call it
somewhere in the top-level compute_config code. And I haven't spotted that
call anywhere in your series.
Otherwise the implementation of the new will_have_type looks correct.
-Daniel
>
> -static const intel_limit_t *intel_ironlake_limit(struct intel_crtc *crtc,
> - int refclk)
> +static const intel_limit_t *
> +intel_ironlake_limit(struct intel_crtc_state *crtc_state, int refclk)
> {
> - struct drm_device *dev = crtc->base.dev;
> + struct drm_device *dev = crtc_state->base.crtc->dev;
> const intel_limit_t *limit;
>
> - if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> + if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> if (intel_is_dual_link_lvds(dev)) {
> if (refclk == 10)
> limit = &intel_limits_ironlake_dual_lvds_100m;
> @@ -467,20 +479,21 @@ static const intel_limit_t *intel_ironlake_limit(struct
> intel_crtc *crtc,
> return limit;
> }
>
> -static const intel_limit_t *intel_g4x_limit(struct intel_crtc *crtc)
> +static const intel_limit_t *
> +intel_g4x_limit(struct intel_crtc_state *crtc_state)
> {
> - struct drm_device *dev = crtc->base.dev;
> + struct drm_device *dev = crtc_state->base.crtc->dev;
> const intel_limit_t *limit;
>
> - if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
> + if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> if (intel_is_dual_link_lvds(dev))
> limit = &intel_limits_g4x_dual_channel_lvds;
> else
> limit = &intel_limits_g4x_single_channel_lvds;
> - } else if
Re: [Intel-gfx] [PATCH 20/23] drm/i915: Use atomic state in pipe_has_enabled_pch()
On Tue, Mar 03, 2015 at 03:22:14PM +0200, Ander Conselvan de Oliveira wrote:
> This function is called indirectly by intel_crtc_compute_config(),
> which needs to be converted to work only with an atomic state.
>
> ---
>
> I'm not sure what are the implications of ignoring intel_crtc->active in
> pipe_has_enabled_pch(). If we allow a config because the third pipe is
> enabled but not active, wouldn't we run into trouble when we tried to
> activate the crtc?
Once we have more of atomic ready and switched to the helpers (or
something equivalent) for enabling/disabling display pipes we need to
remove our usage of intel_crtc->active. Maybe keep it as a debug check.
The new rules are:
- crtc_state->enable controls resource allocation, i.e. this is the bit we
need to look at for checking shared resources and stuff like that.
- crtc_state->active (and also intel_crtc->active) only control/track the
physical active state of the pipe (controlled by dpms or the new atomic
active property). Changing crtc_state->active may never fail.
I think your change actually fixes a bug with the fdi config check code:
If the other pipe is in dpms off we might steal the fdi lanes and then the
subsequent dpms on could fail.
Perhaps split out the removal of intel_crtc->active from the
has_enabled_pch as a bugfix? Would at least help with review.
It should be possible to reproduce this on an ivb with the following
sequence:
1. Enable big mode (uses more than 2 lanes) on pipe B.
2. dpms off pipe B
3. Enable big mode (uses more than 2 lanes) on pipe C.
4. dpms on pipe B should go boom now on one of the FDI checks.
Maybe we need another modeset on pipe C to hit them though or something
like that, we don't have full checks in the dpms patch.
Thanks, Daniel
>
> Signed-off-by: Ander Conselvan de Oliveira
>
> ---
> drivers/gpu/drm/i915/intel_display.c | 22 +++---
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 64751b6..518903e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3150,10 +3150,9 @@ static void intel_fdi_normal_train(struct drm_crtc
> *crtc)
> FDI_FE_ERRC_ENABLE);
> }
>
> -static bool pipe_has_enabled_pch(struct intel_crtc *crtc)
> +static bool pipe_has_enabled_pch(struct intel_crtc_state *crtc_state)
> {
> - return crtc->base.state->enable && crtc->active &&
> - crtc->config->has_pch_encoder;
> + return crtc_state->base.enable && crtc_state->has_pch_encoder;
> }
>
> static void ivb_modeset_global_resources(struct drm_atomic_state *state)
> @@ -3164,15 +3163,21 @@ static void ivb_modeset_global_resources(struct
> drm_atomic_state *state)
> to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
> struct intel_crtc *pipe_C_crtc =
> to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]);
> + struct intel_crtc_state *pipe_B_crtc_state, *pipe_C_crtc_state;
> uint32_t temp;
>
> + pipe_B_crtc_state = intel_atomic_get_crtc_state(state, pipe_B_crtc);
> + pipe_C_crtc_state = intel_atomic_get_crtc_state(state, pipe_C_crtc);
> + if (WARN_ON(IS_ERR(pipe_B_crtc_state) || IS_ERR(pipe_C_crtc_state)))
> + return;
> +
> /*
>* When everything is off disable fdi C so that we could enable fdi B
>* with all lanes. Note that we don't care about enabled pipes without
>* an enabled pch encoder.
>*/
> - if (!pipe_has_enabled_pch(pipe_B_crtc) &&
> - !pipe_has_enabled_pch(pipe_C_crtc)) {
> + if (!pipe_has_enabled_pch(pipe_B_crtc_state) &&
> + !pipe_has_enabled_pch(pipe_C_crtc_state)) {
> WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE);
> WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE);
>
> @@ -5528,6 +5533,9 @@ static bool ironlake_check_fdi_lanes(struct drm_device
> *dev, enum pipe pipe,
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *pipe_B_crtc =
> to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]);
> + struct intel_crtc_state *pipe_B_crtc_state =
> + intel_atomic_get_crtc_state(pipe_config->base.state,
> + pipe_B_crtc);
>
> DRM_DEBUG_KMS("checking fdi config on pipe %c, lanes %i\n",
> pipe_name(pipe), pipe_config->fdi_lanes);
> @@ -5563,8 +5571,8 @@ static bool ironlake_check_fdi_lanes(struct drm_device
> *dev, enum pipe pipe,
> }
> return true;
> case PIPE_C:
> - if (!pipe_has_enabled_pch(pipe_B_crtc) ||
> - pipe_B_crtc->config->fdi_lanes <= 2) {
> + if (!pipe_has_enabled_pch(pipe_B_crtc_state) ||
> + pipe_B_crtc_state->fdi_lanes <= 2) {
> if (pipe_config->fdi_lanes > 2) {
>
Re: [Intel-gfx] [PATCH 11/23] drm/i915: Copy the staged connector config to the legacy atomic state
On Tue, Mar 03, 2015 at 03:22:05PM +0200, Ander Conselvan de Oliveira wrote:
> With this in place, we can start converting pieces of the modeset code
> to look at the connector atomic state instead of the staged config.
>
> Signed-off-by: Ander Conselvan de Oliveira
>
> ---
> drivers/gpu/drm/i915/intel_display.c | 23 ---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 108d3d2..4e90cb4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11607,9 +11607,11 @@ intel_set_config_compute_mode_changes(struct
> drm_mode_set *set,
> static int
> intel_modeset_stage_output_state(struct drm_device *dev,
>struct drm_mode_set *set,
> - struct intel_set_config *config)
> + struct intel_set_config *config,
> + struct drm_atomic_state *state)
> {
> struct intel_connector *connector;
> + struct drm_connector_state *connector_state;
> struct intel_encoder *encoder;
> struct intel_crtc *crtc;
> int ro;
> @@ -11673,6 +11675,14 @@ intel_modeset_stage_output_state(struct drm_device
> *dev,
> }
> connector->new_encoder->new_crtc = to_intel_crtc(new_crtc);
>
> + connector_state =
> + drm_atomic_get_connector_state(state, &connector->base);
> + if (IS_ERR(connector_state))
> + return PTR_ERR(connector_state);
> +
> + connector_state->crtc = new_crtc;
> + connector_state->best_encoder = &connector->new_encoder->base;
> +
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [CRTC:%d]\n",
> connector->base.base.id,
> connector->base.name,
> @@ -11705,9 +11715,16 @@ intel_modeset_stage_output_state(struct drm_device
> *dev,
> }
> /* Now we've also updated encoder->new_crtc for all encoders. */
> for_each_intel_connector(dev, connector) {
> - if (connector->new_encoder)
> + connector_state =
> + drm_atomic_get_connector_state(state, &connector->base);
> +
> + if (connector->new_encoder) {
> if (connector->new_encoder != connector->encoder)
> connector->encoder = connector->new_encoder;
> + } else {
> + connector_state->crtc = NULL;
> + }
> +
Unecessary line. Of course you've put that in there to check that I
actually read your patches ;-)
Cheers, Daniel
> }
> for_each_intel_crtc(dev, crtc) {
> crtc->new_enabled = false;
> @@ -11816,7 +11833,7 @@ static int intel_crtc_set_config(struct drm_mode_set
> *set)
>
> state->acquire_ctx = dev->mode_config.acquire_ctx;
>
> - ret = intel_modeset_stage_output_state(dev, set, config);
> + ret = intel_modeset_stage_output_state(dev, set, config, state);
> if (ret)
> goto fail;
>
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/23] drm/i915: Add an optional atomic state argument to intel_set_mode()
On Tue, Mar 03, 2015 at 03:22:00PM +0200, Ander Conselvan de Oliveira wrote:
> In the set config modeset path, the atomic state is updated when
> changing the staged config in intel_modeset_stage_output_config(). The
> load detect code also causes a modeset, but it changes the staged config
> before calling intel_set_mode(). A follow up patch will change that
> function to also update a drm_atomic_state, and it will need to be able
> to pass that to intel_set_mode().
>
> Signed-off-by: Ander Conselvan de Oliveira
>
Hm given this patch and the nesting in the previous one maybe we should
just add drm_atomic_state scaffolding to all callers of intel_set_mode?
Long term we need to switch over the restore_mode function over to atomic
anyway (I have ideas for generic helpers already). And the load detect
code needs to switch to atomic too anyway.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 29 +
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 97d4df5..3c3b5b2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -83,7 +83,8 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> struct intel_crtc_state *pipe_config);
>
> static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode
> *mode,
> - int x, int y, struct drm_framebuffer *old_fb);
> + int x, int y, struct drm_framebuffer *old_fb,
> + struct drm_atomic_state *state);
> static int intel_framebuffer_init(struct drm_device *dev,
> struct intel_framebuffer *ifb,
> struct drm_mode_fb_cmd2 *mode_cmd,
> @@ -8854,7 +8855,7 @@ retry:
> goto fail;
> }
>
> - if (intel_set_mode(crtc, mode, 0, 0, fb)) {
> + if (intel_set_mode(crtc, mode, 0, 0, fb, NULL)) {
> DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
> if (old->release_fb)
> old->release_fb->funcs->destroy(old->release_fb);
> @@ -8898,7 +8899,7 @@ void intel_release_load_detect_pipe(struct
> drm_connector *connector,
> intel_encoder->new_crtc = NULL;
> intel_crtc->new_enabled = false;
> intel_crtc->new_config = NULL;
> - intel_set_mode(crtc, NULL, 0, 0, NULL);
> + intel_set_mode(crtc, NULL, 0, 0, NULL, NULL);
>
> if (old->release_fb) {
> drm_framebuffer_unregister_private(old->release_fb);
> @@ -11316,19 +11317,21 @@ static int intel_set_mode_pipes(struct drm_crtc
> *crtc,
>
> static int intel_set_mode(struct drm_crtc *crtc,
> struct drm_display_mode *mode,
> - int x, int y, struct drm_framebuffer *fb)
> + int x, int y, struct drm_framebuffer *fb,
> + struct drm_atomic_state *state)
> {
> struct drm_device *dev = crtc->dev;
> - struct drm_atomic_state *state;
> struct intel_crtc_state *pipe_config;
> unsigned modeset_pipes, prepare_pipes, disable_pipes;
> int ret = 0;
>
> - state = drm_atomic_state_alloc(dev);
> - if (!state)
> - return -ENOMEM;
> + if (!state) {
> + state = drm_atomic_state_alloc(dev);
> + if (!state)
> + return -ENOMEM;
>
> - state->acquire_ctx = dev->mode_config.acquire_ctx;
> + state->acquire_ctx = dev->mode_config.acquire_ctx;
> + }
>
> pipe_config = intel_modeset_compute_config(crtc, mode, fb, state,
> &modeset_pipes,
> @@ -11353,7 +11356,8 @@ out:
>
> void intel_crtc_restore_mode(struct drm_crtc *crtc)
> {
> - intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->primary->fb);
> + intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->primary->fb,
> +NULL);
> }
>
> #undef for_each_intel_crtc_masked
> @@ -11813,7 +11817,8 @@ fail:
> /* Try to restore the config */
> if (config->mode_changed &&
> intel_set_mode(save_set.crtc, save_set.mode,
> -save_set.x, save_set.y, save_set.fb))
> +save_set.x, save_set.y, save_set.fb,
> +NULL))
> DRM_ERROR("failed to restore config after modeset
> failure\n");
> }
>
> @@ -13857,7 +13862,7 @@ void intel_modeset_setup_hw_state(struct drm_device
> *dev,
> dev_priv->pipe_to_crtc_mapping[pipe];
>
> intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y,
> -crtc->primary->fb);
> +crtc-
Re: [Intel-gfx] [PATCH 05/23] drm/i915: Allocate a drm_atomic_state for the legacy modeset code
On Tue, Mar 03, 2015 at 03:21:59PM +0200, Ander Conselvan de Oliveira wrote:
> For the atomic conversion, the mode set paths need to be changed to rely
> on an atomic state instead of using the staged config. By using an
> atomic state for the legacy code, we will be able to convert the code
> base in small chunks.
>
> Signed-off-by: Ander Conselvan de Oliveira
>
Two small comments below.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 118
> +++
> 1 file changed, 91 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 798de7b..97d4df5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -37,6 +37,7 @@
> #include
> #include "i915_drv.h"
> #include "i915_trace.h"
> +#include
> #include
> #include
> #include
> @@ -10290,10 +10291,22 @@ static bool check_digital_port_conflicts(struct
> drm_device *dev)
> return true;
> }
>
> -static struct intel_crtc_state *
> +static void
> +clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
> +{
> + struct drm_crtc_state tmp_state;
> +
> + /* Clear only the intel specific part of the crtc state */
> + tmp_state = crtc_state->base;
> + memset(crtc_state, 0, sizeof *crtc_state);
> + crtc_state->base = tmp_state;
> +}
I guess this is to clear out state which we want to recompute, and our
compute_config code assumes that it's always kzalloc'ed a new config?
I think this should be part of the crtc duplicate_state callback to make
sure we're doing this consistently.
> +
> +static int
> intel_modeset_pipe_config(struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> - struct drm_display_mode *mode)
> + struct drm_display_mode *mode,
> + struct drm_atomic_state *state)
> {
> struct drm_device *dev = crtc->dev;
> struct intel_encoder *encoder;
> @@ -10303,17 +10316,19 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>
> if (!check_encoder_cloning(to_intel_crtc(crtc))) {
> DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
> - return ERR_PTR(-EINVAL);
> + return -EINVAL;
> }
>
> if (!check_digital_port_conflicts(dev)) {
> DRM_DEBUG_KMS("rejecting conflicting digital port
> configuration\n");
> - return ERR_PTR(-EINVAL);
> + return -EINVAL;
> }
>
> - pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
> - if (!pipe_config)
> - return ERR_PTR(-ENOMEM);
> + pipe_config = intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));
> + if (IS_ERR(pipe_config))
> + return PTR_ERR(pipe_config);
> +
> + clear_intel_crtc_state(pipe_config);
>
> pipe_config->base.crtc = crtc;
> drm_mode_copy(&pipe_config->base.adjusted_mode, mode);
> @@ -10408,10 +10423,9 @@ encoder_retry:
> DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n",
> plane_bpp, pipe_config->pipe_bpp, pipe_config->dither);
>
> - return pipe_config;
> + return 0;
> fail:
> - kfree(pipe_config);
> - return ERR_PTR(ret);
> + return ret;
> }
>
> /* Computes which crtcs are affected and sets the relevant bits in the mask.
> For
> @@ -11089,17 +11103,19 @@ static struct intel_crtc_state *
> intel_modeset_compute_config(struct drm_crtc *crtc,
>struct drm_display_mode *mode,
>struct drm_framebuffer *fb,
> + struct drm_atomic_state *state,
>unsigned *modeset_pipes,
>unsigned *prepare_pipes,
>unsigned *disable_pipes)
> {
> struct intel_crtc_state *pipe_config = NULL;
> + int ret = 0;
>
> intel_modeset_affected_pipes(crtc, modeset_pipes,
>prepare_pipes, disable_pipes);
>
> if ((*modeset_pipes) == 0)
> - goto out;
> + return NULL;
>
> /*
>* Note this needs changes when we start tracking multiple modes
> @@ -11107,14 +11123,17 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
>* (i.e. one pipe_config for each crtc) rather than just the one
>* for this crtc.
>*/
> - pipe_config = intel_modeset_pipe_config(crtc, fb, mode);
> - if (IS_ERR(pipe_config)) {
> - goto out;
> - }
> + ret = intel_modeset_pipe_config(crtc, fb, mode, state);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + pipe_config = intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));
> + if (IS_ERR(pipe_config))
> + return pipe_config;
> +
> intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
> "[modeset]");
>
> -out:
>
Re: [Intel-gfx] [PATCH 04/23] drm/i915: Add intel_atomic_get_crtc_state() helper function
On Tue, Mar 03, 2015 at 03:21:58PM +0200, Ander Conselvan de Oliveira wrote:
> The pattern of getting the crtc state with drm_atomic_get_crtc_state()
> and then converting it to intel_crtc_state will repeat quite often in
> the following patches, so add a helper function to save some typing.
>
> Signed-off-by: Ander Conselvan de Oliveira
>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 632df1c..c1959e0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -35,6 +35,7 @@
> #include
> #include
> #include
> +#include
>
> #define DIV_ROUND_CLOSEST_ULL(ll, d) \
> ({ unsigned long long _tmp = (ll)+(d)/2; do_div(_tmp, d); _tmp; })
> @@ -564,6 +565,7 @@ struct cxsr_latency {
> };
>
> #define to_intel_crtc(x) container_of(x, struct intel_crtc, base)
> +#define to_intel_crtc_state(x) container_of(x, struct intel_crtc_state, base)
> #define to_intel_connector(x) container_of(x, struct intel_connector, base)
> #define to_intel_encoder(x) container_of(x, struct intel_encoder, base)
> #define to_intel_framebuffer(x) container_of(x, struct intel_framebuffer,
> base)
> @@ -1273,6 +1275,14 @@ int intel_connector_atomic_get_property(struct
> drm_connector *connector,
> struct drm_crtc_state *intel_crtc_duplicate_state(struct drm_crtc *crtc);
> void intel_crtc_destroy_state(struct drm_crtc *crtc,
> struct drm_crtc_state *state);
> +static inline struct intel_crtc_state *
> +intel_atomic_get_crtc_state(struct drm_atomic_state *state,
> + struct intel_crtc *crtc)
> +{
> + struct drm_crtc_state *crtc_state;
> + crtc_state = drm_atomic_get_crtc_state(state, &crtc->base);
> + return to_intel_crtc_state(crtc_state);
You need to check for error pointers before upcasting. Just to be robust
against someone putting the base structure not as the first thing.
I've merged all previous patches thus far, thanks.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/12] drm/i915: Program PFI credits for VLV
On Wed, Mar 04, 2015 at 07:55:21PM +0530, Purushothaman, Vijay A wrote:
> On 2/10/2015 6:58 PM, ville.syrj...@linux.intel.com wrote:
> > From: Vidya Srinivas
> >
> > PFI credit programming is required when CD clock (related to data flow from
> > display pipeline to end display) is greater than CZ clock (related to data
> > flow from memory to display plane). This programming should be done when all
> > planes are OFF to avoid intermittent hangs while accessing memory even from
> > different Gfx units (not just display).
> >
> > If cdclk/czclk >=1, PFI credits could be set as any number. To get better
> > performance, larger PFI credit can be assigned to PND. Otherwise if
> > cdclk/czclk<1, the default PFI credit of 8 should be set.
> >
> > v2:
> > - Change log to lower log level instead of DRM_ERROR
> > - Change function name to valleyview_program_pfi_credits
> > - Move program PFI credits to modeset_init instead of intel_set_mode
> > - Change magic numbers to logical constants
> >
> > [vsyrjala v3:
> > - only program in response to cdclk update
> > - program the credits also when cdclk > - add CHV bits]
> >
> > Signed-off-by: Vidya Srinivas
> > Signed-off-by: Gajanan Bhat
> > Signed-off-by: Vandana Kannan
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 8
> > drivers/gpu/drm/i915/intel_display.c | 33
> > +
> > 2 files changed, 41 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index aacf90b..a0a7688 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2061,6 +2061,14 @@ enum skl_disp_power_wells {
> > #define CDCLK_FREQ_SHIFT4
> > #define CDCLK_FREQ_MASK (0x1f << CDCLK_FREQ_SHIFT)
> > #define CZCLK_FREQ_MASK 0xf
> > +
> > +#define GCI_CONTROL(VLV_DISPLAY_BASE + 0x650C)
> > +#define PFI_CREDIT_63(9 << 28) /* chv only */
> > +#define PFI_CREDIT_31(8 << 28) /* chv only */
> > +#define PFI_CREDIT(x)(((x) - 8) << 28) /* 8-15 */
> > +#define PFI_CREDIT_RESEND(1 << 27)
> > +#define VGA_FAST_MODE_DISABLE(1 << 14)
> > +
> > #define GMBUSFREQ_VLV (VLV_DISPLAY_BASE + 0x6510)
> >
> > /*
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 3fe9598..9dcab4b 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4987,6 +4987,37 @@ static void valleyview_modeset_global_pipes(struct
> > drm_device *dev,
> > *prepare_pipes |= (1 << intel_crtc->pipe);
> > }
> >
> > +static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
> > +{
> > + unsigned int credits;
> > +
> > + if (DIV_ROUND_CLOSEST(dev_priv->vlv_cdclk_freq, 1000) >=
> > dev_priv->rps.cz_freq) {
> > + /* CHV suggested value is 31 or 63 */
> > + if (IS_CHERRYVIEW(dev_priv))
> > + credits = PFI_CREDIT_31;
> > + else
> > + credits = PFI_CREDIT(15);
> > + } else {
> > + credits = PFI_CREDIT(8);
> The default value should be 4 credits for CHV and 0 for VLV.
>
> > + }
> > +
> > + /*
> > +* WA - write default credits before re-programming
> > +* FIXME: should we also set the resend bit here?
> > +*/
> > + I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE |
> > + PFI_CREDIT(8));
> Default credit should be 4 credits for CHV. PFI_CREDIT(12). Document
> update is pending. But this is the latest recommendation to windows team.
BTW do you have any specific information about this workaround? I asked
Vandana about it when she first submitted the patch and she said she'd
get back to me on that, but I don't think I ever got an actual answer.
The FIXMEs in particular would need to be answered.
Also if we still need this workaround on CHV, I don't know if we should
set the credits to 8 or 12 here. Any ideas?
>
> Thanks,
> Vijay
>
> > +
> > + I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE |
> > + credits | PFI_CREDIT_RESEND);
> > +
> > + /*
> > +* FIXME is this guaranteed to clear
> > +* immediately or should we poll for it?
> > +*/
> > + WARN_ON(I915_READ(GCI_CONTROL) & PFI_CREDIT_RESEND);
> > +}
> > +
> > static void valleyview_modeset_global_resources(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -5010,6 +5041,8 @@ static void
> > valleyview_modeset_global_resources(struct drm_device *dev)
> > else
> > valleyview_set_cdclk(dev, req_cdclk);
> >
> > + vlv_program_pfi_credits(dev_priv);
> > +
> > intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
> > }
> > }
>
> __
Re: [Intel-gfx] [PATCH 01/23] drm/i915: Set crtc backpointer when duplicating crtc state
On Tue, Mar 03, 2015 at 03:21:55PM +0200, Ander Conselvan de Oliveira wrote:
> In the path were there is no state to duplicate, the allocated crtc
> state wouldn't have the crtc backpointer initialized.
>
> Signed-off-by: Ander Conselvan de Oliveira
>
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 12 +---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 011b896..3903b90 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -214,12 +214,18 @@ struct drm_crtc_state *
> intel_crtc_duplicate_state(struct drm_crtc *crtc)
> {
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_crtc_state *crtc_state;
>
> if (WARN_ON(!intel_crtc->config))
> - return kzalloc(sizeof(*intel_crtc->config), GFP_KERNEL);
> + crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
> + else
> + crtc_state = kmemdup(intel_crtc->config,
> + sizeof(*intel_crtc->config), GFP_KERNEL);
>
> - return kmemdup(intel_crtc->config, sizeof(*intel_crtc->config),
> -GFP_KERNEL);
> + if (crtc_state)
> + crtc_state->base.crtc = crtc;
> +
> + return &crtc_state->base;
I think we should eventually extract another set of helpers to share the
core initialization code common with all drivers and the atomic helpers.
We've already screwed up this a few times when additions to the helper
state tracking haven't propagated to all drivers.
Last time I've tried this I got stuck on a good name for this function
though.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2] tests/gem_render_tiled_blits: split into subtests
On Tue, Mar 03, 2015 at 10:43:43AM +, tim.g...@intel.com wrote:
> From: Tim Gore
>
> The gem_render_tiled_blits test tends to get oom killed
> on low memory (< 4GB) Android systems. This is because the
> test tries to allocate (sysinfo.totalram * 9 / 10) in
> buffer objects and the remaining 10% of memory is not
> always enough for the Android system.
> A similar issue with gem_render_linear_blits was resolved
> by creating several subtests. A "basic" subtest that uses
> minimal memory buffers to test the basic operation, and
> two stress tests which are skipped if there is insufficient
> memory. The first stress test uses more memory than the
> graphics apperture and the second uses enough to ensure
> that swap space is used (if present).
> This patch makes the same changes to gem_render_tiled_blits.
>
> v2: Following comments from Daniel Vetter:
> a) Use igt_main macro instead of "open coding", and
> b) cull some leading spaces
>
> Signed-off-by: Tim Gore
Both patches applied, thanks.
-Daniel
> ---
> tests/gem_render_tiled_blits.c | 72
> ++
> 1 file changed, 45 insertions(+), 27 deletions(-)
>
> diff --git a/tests/gem_render_tiled_blits.c b/tests/gem_render_tiled_blits.c
> index dc22529..f669270 100644
> --- a/tests/gem_render_tiled_blits.c
> +++ b/tests/gem_render_tiled_blits.c
> @@ -85,30 +85,25 @@ check_bo(struct intel_batchbuffer *batch, struct igt_buf
> *buf, uint32_t val)
> }
> for (i = 0; i < WIDTH*HEIGHT; i++) {
> igt_assert_f(ptr[i] == val,
> - "Expected 0x%08x, found 0x%08x "
> - "at offset 0x%08x\n",
> - val, ptr[i], i * 4);
> + "Expected 0x%08x, found 0x%08x "
> + "at offset 0x%08x\n",
> + val, ptr[i], i * 4);
> val++;
> }
> if (ptr != data)
> dri_bo_unmap(linear);
> }
>
> -int main(int argc, char **argv)
> +static void run_test (int fd, int count)
> {
> drm_intel_bufmgr *bufmgr;
> struct intel_batchbuffer *batch;
> uint32_t *start_val;
> struct igt_buf *buf;
> uint32_t start = 0;
> - int i, j, fd, count;
> + int i, j;
> uint32_t devid;
>
> - igt_simple_init(argc, argv);
> -
> - igt_skip_on_simulation();
> -
> - fd = drm_open_any();
> devid = intel_get_drm_devid(fd);
>
> render_copy = igt_get_render_copyfunc(devid);
> @@ -124,23 +119,6 @@ int main(int argc, char **argv)
> drm_intel_bufmgr_gem_set_vma_cache_size(bufmgr, 32);
> batch = intel_batchbuffer_alloc(bufmgr, devid);
>
> - count = 0;
> - if (argc > 1)
> - count = atoi(argv[1]);
> - if (count == 0)
> - count = 3 * gem_aperture_size(fd) / SIZE / 2;
> - else if (count < 2) {
> - igt_warn("count must be >= 2\n");
> - return 1;
> - }
> -
> - if (count > intel_get_total_ram_mb() * 9 / 10) {
> - count = intel_get_total_ram_mb() * 9 / 10;
> - igt_info("not enough RAM to run test, reducing buffer count\n");
> - }
> -
> - igt_info("Using %d 1MiB buffers\n", count);
> -
> linear = drm_intel_bo_alloc(bufmgr, "linear", WIDTH*HEIGHT*4, 0);
> if (snoop) {
> gem_set_caching(fd, linear->handle, 1);
> @@ -211,5 +189,45 @@ int main(int argc, char **argv)
> for (i = 0; i < count; i++)
> check_bo(batch, &buf[i], start_val[i]);
>
> + /* release resources */
> + drm_intel_bo_unreference(linear);
> + for (i = 0; i < count; i++) {
> + drm_intel_bo_unreference(buf[i].bo);
> + }
> + intel_batchbuffer_free(batch);
> + drm_intel_bufmgr_destroy(bufmgr);
> +}
> +
> +
> +igt_main
> +{
> + int fd = 0;
> + int count = 0;
> +
> + igt_fixture {
> + fd = drm_open_any();
> + }
> +
> + igt_subtest("basic") {
> + run_test(fd, 2);
> + }
> +
> + /* the rest of the tests are too long for simulation */
> + igt_skip_on_simulation();
> +
> + igt_subtest("apperture-thrash") {
> + count = 3 * gem_aperture_size(fd) / SIZE / 2;
> + intel_require_memory(count, SIZE, CHECK_RAM);
> + run_test(fd, count);
> + }
> +
> + igt_subtest("swap-thrash") {
> + uint64_t swap_mb = intel_get_total_swap_mb();
> + igt_require(swap_mb > 0);
> + count = ((intel_get_avail_ram_mb() + (swap_mb / 2)) *
> 1024*1024) / SIZE;
> + intel_require_memory(count, SIZE, CHECK_RAM | CHECK_SWAP);
> + run_test(fd, count);
> + }
> +
> igt_exit();
> }
> --
> 2.3.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.f
Re: [Intel-gfx] [PATCH 00/23] Remove depencies on staged config for atomic transition
On Tue, Mar 03, 2015 at 03:21:54PM +0200, Ander Conselvan de Oliveira wrote: > This patch series starts to remove dependencies from the modeset code to > enable the transition to atomic. That is achieved by using an atomic > state struct for the legacy modeset, and changing related functiond to > depend on it. > > I wasn't able to test all of the changes, so I'm very interested on > PRTS results for this. In particular, I expect support for 3 pipes and > the load detect pipe to be problematic. Matt has promised to create an igt using my load-detect test code. With that you can test load-detect on any modern-ish platform that still supports vga (i.e. up to hsw). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Reudce CHV DPLL min vco frequency to 4.8 GHz
On Wed, Mar 04, 2015 at 05:10:02PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 04, 2015 at 08:11:38PM +0530, Purushothaman, Vijay A wrote:
> > On 2/27/2015 12:31 AM, ville.syrj...@linux.intel.com wrote:
> > > From: Ville Syrjälä
> > >
> > > The current minimum vco frequency leaves us with a gap in our supported
> > > frequencies at 233-243 MHz. Your typical 2560x1440@60 display wants a
> > > pixel clock of 241.5 MHz, which is just withing that gap. Reduce the
> > > allowed vco min frequency to 4.8GHz to reduce the gap to 233-240 MHz,
> > > and thus allow such displays to work.
> > >
> > > 4.8 GHz is actually the documented (at least in some docs) limit of the
> > > PLL, and we just picked 4.86 GHz originally because that was the lowest
> > > value produced by the PLL spreadsheet, which obviously didn't consider
> > > 2560x1440 displays.
> > >
> > > Signed-off-by: Ville Syrjälä
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index 102b12d..d437a21 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -390,7 +390,7 @@ static const intel_limit_t intel_limits_chv = {
> > >* them would make no difference.
> > >*/
> > > .dot = { .min = 25000 * 5, .max = 54 * 5},
> > > - .vco = { .min = 486, .max = 670 },
> > > + .vco = { .min = 480, .max = 670 },
> > > .n = { .min = 1, .max = 1 },
> > > .m1 = { .min = 2, .max = 2 },
> > > .m2 = { .min = 24 << 22, .max = 175 << 22 },
> >
> > Minor nitpick: typo in patch title
>
> Dang. I already fixed a typo there before sending this out, but turns
> out I only managed to cchange it into a different typo :( Maybe I need
> to invest in a spell checker...
Fixed and applied, thanks.
-Daniel
>
> >
> > Reviewed-by: Vijay Purushothaman
> >
> > Thanks,
> > Vijay
> > ___
> > 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
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Reudce CHV DPLL min vco frequency to 4.8 GHz
On Wed, Mar 04, 2015 at 08:11:38PM +0530, Purushothaman, Vijay A wrote:
> On 2/27/2015 12:31 AM, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > The current minimum vco frequency leaves us with a gap in our supported
> > frequencies at 233-243 MHz. Your typical 2560x1440@60 display wants a
> > pixel clock of 241.5 MHz, which is just withing that gap. Reduce the
> > allowed vco min frequency to 4.8GHz to reduce the gap to 233-240 MHz,
> > and thus allow such displays to work.
> >
> > 4.8 GHz is actually the documented (at least in some docs) limit of the
> > PLL, and we just picked 4.86 GHz originally because that was the lowest
> > value produced by the PLL spreadsheet, which obviously didn't consider
> > 2560x1440 displays.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 102b12d..d437a21 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -390,7 +390,7 @@ static const intel_limit_t intel_limits_chv = {
> > * them would make no difference.
> > */
> > .dot = { .min = 25000 * 5, .max = 54 * 5},
> > - .vco = { .min = 486, .max = 670 },
> > + .vco = { .min = 480, .max = 670 },
> > .n = { .min = 1, .max = 1 },
> > .m1 = { .min = 2, .max = 2 },
> > .m2 = { .min = 24 << 22, .max = 175 << 22 },
>
> Minor nitpick: typo in patch title
Dang. I already fixed a typo there before sending this out, but turns
out I only managed to cchange it into a different typo :( Maybe I need
to invest in a spell checker...
>
> Reviewed-by: Vijay Purushothaman
>
> Thanks,
> Vijay
> ___
> 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 12/12] drm/i915: Enable the maxfifo PM5 mode when appropriate on CHV
On Wed, Mar 04, 2015 at 07:58:50PM +0530, Purushothaman, Vijay A wrote:
> On 2/10/2015 6:58 PM, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > CHV has a new knob in Punit to select between some memory power savings
> > modes PM2 and PM5. We can allow the deeper PM5 when maxfifo mode is
> > enabled, so let's do so in the hopes for moar power savings.
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 3 +++
> > drivers/gpu/drm/i915/intel_pm.c | 13 -
> > 2 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index a0a7688..2196e57 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -552,6 +552,9 @@
> > #define DSPFREQSTAT_MASK(0x3 <<
> > DSPFREQSTAT_SHIFT)
> > #define DSPFREQGUAR_SHIFT 14
> > #define DSPFREQGUAR_MASK(0x3 <<
> > DSPFREQGUAR_SHIFT)
> > +#define DSP_MAXFIFO_PM5_STATUS (1 << 22) /* chv */
> > +#define DSP_AUTO_CDCLK_GATE_DISABLE (1 << 7) /* chv */
> > +#define DSP_MAXFIFO_PM5_ENABLE (1 << 6) /* chv */
> > #define _DP_SSC(val, pipe) ((val) << (2 * (pipe)))
> > #define DP_SSC_MASK(pipe) _DP_SSC(0x3, (pipe))
> > #define DP_SSC_PWR_ON(pipe) _DP_SSC(0x0, (pipe))
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index e6cbc24..4e11552 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -240,7 +240,18 @@ void intel_set_memory_cxsr(struct drm_i915_private
> > *dev_priv, bool enable)
> > struct drm_device *dev = dev_priv->dev;
> > u32 val;
> >
> > - if (IS_VALLEYVIEW(dev)) {
> > + if (IS_CHERRYVIEW(dev)) {
> > + I915_WRITE(FW_BLC_SELF_VLV, enable ? FW_CSPWRDWNEN : 0);
> > +
> > + mutex_lock(&dev_priv->rps.hw_lock);
> > + val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> > + if (enable)
> > + val |= DSP_MAXFIFO_PM5_ENABLE;
> > + else
> > + val &= ~DSP_MAXFIFO_PM5_ENABLE;
> > + vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
> > + mutex_unlock(&dev_priv->rps.hw_lock);
> > + } else if (IS_VALLEYVIEW(dev)) {
> > I915_WRITE(FW_BLC_SELF_VLV, enable ? FW_CSPWRDWNEN : 0);
> > } else if (IS_G4X(dev) || IS_CRESTLINE(dev)) {
> > I915_WRITE(FW_BLC_SELF, enable ? FW_BLC_SELF_EN : 0);
> Since you are enabling MaxFIFO for multi plane in one of the previous
> patches, i guess PM5 will also be enabled when more than one plane is
> active in this flow.
> Let's enable MaxFIFO and PM5 when only one plane is active for now. This
> is the only validated scenario by SV.
Yep that's good enough for me.
>
> With this addressed,
> Reviewed-by: Vijay Purushothaman
>
> Thanks,
> Vijay
>
>
> ___
> 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 11/12] drm/i915: Program PFI credits for VLV
On Wed, Mar 04, 2015 at 07:55:21PM +0530, Purushothaman, Vijay A wrote:
> On 2/10/2015 6:58 PM, ville.syrj...@linux.intel.com wrote:
> > From: Vidya Srinivas
> >
> > PFI credit programming is required when CD clock (related to data flow from
> > display pipeline to end display) is greater than CZ clock (related to data
> > flow from memory to display plane). This programming should be done when all
> > planes are OFF to avoid intermittent hangs while accessing memory even from
> > different Gfx units (not just display).
> >
> > If cdclk/czclk >=1, PFI credits could be set as any number. To get better
> > performance, larger PFI credit can be assigned to PND. Otherwise if
> > cdclk/czclk<1, the default PFI credit of 8 should be set.
> >
> > v2:
> > - Change log to lower log level instead of DRM_ERROR
> > - Change function name to valleyview_program_pfi_credits
> > - Move program PFI credits to modeset_init instead of intel_set_mode
> > - Change magic numbers to logical constants
> >
> > [vsyrjala v3:
> > - only program in response to cdclk update
> > - program the credits also when cdclk > - add CHV bits]
> >
> > Signed-off-by: Vidya Srinivas
> > Signed-off-by: Gajanan Bhat
> > Signed-off-by: Vandana Kannan
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 8
> > drivers/gpu/drm/i915/intel_display.c | 33
> > +
> > 2 files changed, 41 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index aacf90b..a0a7688 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -2061,6 +2061,14 @@ enum skl_disp_power_wells {
> > #define CDCLK_FREQ_SHIFT4
> > #define CDCLK_FREQ_MASK (0x1f << CDCLK_FREQ_SHIFT)
> > #define CZCLK_FREQ_MASK 0xf
> > +
> > +#define GCI_CONTROL(VLV_DISPLAY_BASE + 0x650C)
> > +#define PFI_CREDIT_63(9 << 28) /* chv only */
> > +#define PFI_CREDIT_31(8 << 28) /* chv only */
> > +#define PFI_CREDIT(x)(((x) - 8) << 28) /* 8-15 */
> > +#define PFI_CREDIT_RESEND(1 << 27)
> > +#define VGA_FAST_MODE_DISABLE(1 << 14)
> > +
> > #define GMBUSFREQ_VLV (VLV_DISPLAY_BASE + 0x6510)
> >
> > /*
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 3fe9598..9dcab4b 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4987,6 +4987,37 @@ static void valleyview_modeset_global_pipes(struct
> > drm_device *dev,
> > *prepare_pipes |= (1 << intel_crtc->pipe);
> > }
> >
> > +static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
> > +{
> > + unsigned int credits;
> > +
> > + if (DIV_ROUND_CLOSEST(dev_priv->vlv_cdclk_freq, 1000) >=
> > dev_priv->rps.cz_freq) {
> > + /* CHV suggested value is 31 or 63 */
> > + if (IS_CHERRYVIEW(dev_priv))
> > + credits = PFI_CREDIT_31;
> > + else
> > + credits = PFI_CREDIT(15);
> > + } else {
> > + credits = PFI_CREDIT(8);
> The default value should be 4 credits for CHV and 0 for VLV.
8 is the minimum. So I think on CHV we actually want PFI_CREDIT(12).
The reason the CHV value is wrong here is because I wrote the patch
based on the spec, and sadly the new recommended value isn't in the
spec.
>
> > + }
> > +
> > + /*
> > +* WA - write default credits before re-programming
> > +* FIXME: should we also set the resend bit here?
> > +*/
> > + I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE |
> > + PFI_CREDIT(8));
> Default credit should be 4 credits for CHV. PFI_CREDIT(12). Document
> update is pending. But this is the latest recommendation to windows team.
Right. So exactly as I thought too. I'll respin the patch with the new
value.
>
> Thanks,
> Vijay
>
> > +
> > + I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE |
> > + credits | PFI_CREDIT_RESEND);
> > +
> > + /*
> > +* FIXME is this guaranteed to clear
> > +* immediately or should we poll for it?
> > +*/
> > + WARN_ON(I915_READ(GCI_CONTROL) & PFI_CREDIT_RESEND);
> > +}
> > +
> > static void valleyview_modeset_global_resources(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -5010,6 +5041,8 @@ static void
> > valleyview_modeset_global_resources(struct drm_device *dev)
> > else
> > valleyview_set_cdclk(dev, req_cdclk);
> >
> > + vlv_program_pfi_credits(dev_priv);
> > +
> > intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
> > }
> > }
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org
Re: [Intel-gfx] [PATCH 3/5] drm/i915/skl: Support secondary (rotated) frame buffer mapping
On 03/04/2015 02:43 PM, Daniel Vetter wrote:
On Tue, Mar 03, 2015 at 09:59:31AM +, Tvrtko Ursulin wrote:
On 03/02/2015 06:21 PM, Daniel Vetter wrote:
On Mon, Mar 02, 2015 at 02:43:50PM +, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin
90/270 rotated scanout needs a rotated GTT view of the framebuffer.
This is put in a separate VMA with a dedicated ggtt_view and wired suchs that
it is created when a framebuffer is pinned to a 90/270 rotated plane.
Rotation is only possible with Yb/Yf buffers and error is propagated to
user space in case of a mismatch.
Special rotated page view is constructed at the VMA creation time by
borrowing the DMA addresses from obj->pages.
v2:
* Do not bother with pages for rotated sg list, just populate the DMA
addresses. (Daniel Vetter)
* Checkpatch cleanup.
v3:
* Rebased on top of new plane handling (create rotated mapping when
setting the rotation property).
* Unpin rotated VMA on unpinning from display plane.
* Simplify rotation check using bitwise AND. (Chris Wilson)
v4:
* Fix unpinning of optional rotated mapping so it is really considered
to be optional.
v5:
* Rebased for fb modifier changes.
* Rebased for atomic commit.
* Only pin needed view for display. (Ville Syrjälä, Daniel Vetter)
For: VIZ-4726
Signed-off-by: Tvrtko Ursulin
Reviewed-by: Michel Thierry (v4)
Bunch of nitpicks below. Also I think it'd be good to split this patch
into the rote refactoring work to add view parameters all over the place
and the actual implementation.
Ok will split it. Rest below...
---
drivers/gpu/drm/i915/i915_drv.h | 33 +-
drivers/gpu/drm/i915/i915_gem.c | 27 +++--
drivers/gpu/drm/i915/i915_gem_gtt.c | 14 ++-
drivers/gpu/drm/i915/i915_gem_gtt.h | 12 +++
drivers/gpu/drm/i915/intel_display.c | 204 +++
drivers/gpu/drm/i915/intel_drv.h | 4 +
drivers/gpu/drm/i915/intel_fbdev.c | 2 +-
drivers/gpu/drm/i915/intel_overlay.c | 3 +-
8 files changed, 263 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e07a1cb..79d3f2c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2743,8 +2743,10 @@ i915_gem_object_set_to_cpu_domain(struct
drm_i915_gem_object *obj, bool write);
int __must_check
i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
u32 alignment,
-struct intel_engine_cs *pipelined);
-void i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object *obj);
+struct intel_engine_cs *pipelined,
+const struct i915_ggtt_view *view);
+void i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object *obj,
+ const struct i915_ggtt_view
*view);
int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
int align);
int i915_gem_open(struct drm_device *dev, struct drm_file *file);
@@ -2813,7 +2815,13 @@ i915_gem_obj_lookup_or_create_vma(struct
drm_i915_gem_object *obj,
&i915_ggtt_view_normal);
}
-struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj);
+struct i915_vma *i915_gem_obj_to_ggtt_view(struct drm_i915_gem_object *obj,
+ enum i915_ggtt_view_type view);
+static inline
+struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj)
+{
+ return i915_gem_obj_to_ggtt_view(obj, I915_GGTT_VIEW_NORMAL);
+}
static inline bool i915_gem_obj_is_pinned(struct drm_i915_gem_object *obj) {
struct i915_vma *vma;
list_for_each_entry(vma, &obj->vma_list, vma_link)
@@ -2867,13 +2875,30 @@ i915_gem_obj_ggtt_pin(struct drm_i915_gem_object *obj,
alignment, flags | PIN_GLOBAL);
}
+static inline int __must_check
+i915_gem_obj_ggtt_pin_view(struct drm_i915_gem_object *obj,
+ uint32_t alignment,
+ unsigned flags,
+ const struct i915_ggtt_view *ggtt_view)
+{
+ return i915_gem_object_pin_view(obj, i915_obj_to_ggtt(obj),
+ alignment, flags | PIN_GLOBAL,
+ ggtt_view);
+}
+
static inline int
i915_gem_object_ggtt_unbind(struct drm_i915_gem_object *obj)
{
return i915_vma_unbind(i915_gem_obj_to_ggtt(obj));
}
-void i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj);
+void i915_gem_object_ggtt_unpin_view(struct drm_i915_gem_object *obj,
+enum i915_ggtt_view_type view);
+static inline void
+i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj)
+{
+ i915_gem_object_ggtt_unpin_view(obj, I915_GGTT_VIEW_NO
Re: [Intel-gfx] [PATCH] drm/i915: Reudce CHV DPLL min vco frequency to 4.8 GHz
On 2/27/2015 12:31 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
The current minimum vco frequency leaves us with a gap in our supported
frequencies at 233-243 MHz. Your typical 2560x1440@60 display wants a
pixel clock of 241.5 MHz, which is just withing that gap. Reduce the
allowed vco min frequency to 4.8GHz to reduce the gap to 233-240 MHz,
and thus allow such displays to work.
4.8 GHz is actually the documented (at least in some docs) limit of the
PLL, and we just picked 4.86 GHz originally because that was the lowest
value produced by the PLL spreadsheet, which obviously didn't consider
2560x1440 displays.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_display.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 102b12d..d437a21 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -390,7 +390,7 @@ static const intel_limit_t intel_limits_chv = {
* them would make no difference.
*/
.dot = { .min = 25000 * 5, .max = 54 * 5},
- .vco = { .min = 486, .max = 670 },
+ .vco = { .min = 480, .max = 670 },
.n = { .min = 1, .max = 1 },
.m1 = { .min = 2, .max = 2 },
.m2 = { .min = 24 << 22, .max = 175 << 22 },
Minor nitpick: typo in patch title
Reviewed-by: Vijay Purushothaman
Thanks,
Vijay
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/12] drm/i915: Support maxfifo with two planes on CHV
On Wed, Mar 04, 2015 at 07:34:50PM +0530, Purushothaman, Vijay A wrote:
> On 2/10/2015 6:58 PM, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > CHV supposedly does maxfifo mode even with two enabled
> > (primary/sprite) planes. Lets try to support that by halving the FIFO
> > size for the calculations and picking the smallest calculcated
> > watermark from the enabled planes.
> Where is this mentioned in the spec? My understanding is MaxFIFO can be
> enabled when only one plane is active (either ARGB or YUV). I do
> remember some TR task that was talking about enabling MaxFIFO for two
> planes. But i didn't get any confirmation that this can be enabled.
It was mentioned in the display cluster HAS (also maybe some version of
the PUNIT HAS), but I wasn't sure it's actually there. IIRC the answer
I got in one DDR DVFS meeting was that yes it does exit. But I've not
actually confirmed whether it seems to work on real hardware. The
annoying thing is that there doesn't seem to be any kind of status bit
to tell me if it has entered maxfifo or not, so it would need to be
deduced from some other facts.
Anyway, we can just skip this patch as it's still somewhat unclear if
this can be used or not.
> We
> have not enabled this for CHT in any other OS (Windows / Android).
>
> Other features like PM2 / PM5 / DVFS can be enabled for multi plane
> scenarios
As far as I know we can't even control PM2. So the only thing we can
influence is whether it will enter PM2 or PM5 when things are
sufficiently idle. So we must always tolerate the PM2 latency, and
if we can tolerate the extra PM5 latency we can then choose PM5 over
PM2.
Previously I thought the something would check if the display is in
maxfifo or not before entering PM2/PM5, but now I can't see anything to
back that up. So I suppose we must always verify that even the
non-maxfifo FIFO sizes can tolerate the latency, whether or not we have
enable maxfifo or not (since we can't force maxfifo entry, and instead
the hardware just decides that by itself).
>
> Thanks,
> Vijay
> >
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/intel_pm.c | 22 +-
> > 1 file changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index d29c02c..e6cbc24 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -907,6 +907,8 @@ static bool vlv_compute_sr_wm(struct drm_device *dev,
> > int num_planes = 0;
> > int fifo_size = 0;
> > struct intel_plane *plane;
> > + /* CHV supports max fifo with two planes (1:1 split) */
> > + int max_planes = IS_CHERRYVIEW(dev) ? 2 : 1;
> >
> > wm->sr.cursor = wm->sr.plane = 0;
> >
> > @@ -920,23 +922,33 @@ static bool vlv_compute_sr_wm(struct drm_device *dev,
> > fifo_size = INTEL_INFO(dev_priv)->num_pipes * 512 - 1;
> > }
> >
> > - if (fifo_size == 0 || num_planes > 1)
> > + if (fifo_size == 0 || num_planes > max_planes)
> > return false;
> >
> > + if (num_planes)
> > + fifo_size /= num_planes;
> > +
> > wm->sr.cursor = vlv_compute_wm(to_intel_crtc(crtc),
> >to_intel_plane(crtc->cursor), 0x3f);
> >
> > list_for_each_entry(plane, &dev->mode_config.plane_list, base.head) {
> > + uint16_t sr_wm;
> > +
> > if (plane->base.type == DRM_PLANE_TYPE_CURSOR)
> > continue;
> >
> > if (plane->pipe != pipe)
> > continue;
> >
> > - wm->sr.plane = vlv_compute_wm(to_intel_crtc(crtc),
> > - plane, fifo_size);
> > - if (wm->sr.plane != 0)
> > - break;
> > + sr_wm = vlv_compute_wm(to_intel_crtc(crtc),
> > + plane, fifo_size);
> > + if (sr_wm == 0)
> > + continue;
> > +
> > + if (wm->sr.plane == 0)
> > + wm->sr.plane = sr_wm;
> > + else
> > + wm->sr.plane = min(wm->sr.plane, sr_wm);
> > }
> >
> > return true;
>
> ___
> 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] demos/intel_sprite_on : Sprite Stress Testing
On Tue, Mar 03, 2015 at 02:22:53PM +0530, meghanelogal wrote:
> From: meghanelogal
>
> Adding the Sprite Stress Test Feature
>
> Signed-off-by: meghanelogal
I really, really prefer if all automated and stress tests would be a part
of igt testsuite and that we'd abandon intel_sprite_on as a demo.
The igt library has a lot of support for combining automated modes with
manual testing and additional options, see e.g. the pm_rpm or
kms_psr_sink_crc testcases for what's possible.
If you're using intel_sprite_on for development testing or validation
please evaluate what's missing in proper igt testcoverage and work towards
converting over as soon as possible.
Thanks, Daniel
> ---
> demos/intel_sprite_on.c | 653
> +--
> 1 file changed, 347 insertions(+), 306 deletions(-)
>
> diff --git a/demos/intel_sprite_on.c b/demos/intel_sprite_on.c
> index 23fc56c..ad6b163 100644
> --- a/demos/intel_sprite_on.c
> +++ b/demos/intel_sprite_on.c
> @@ -49,6 +49,8 @@
>
> #include "ioctl_wrappers.h"
>
> +
> +static int sprite_iteration_count = 1;
> /*
> * Mode setting with the kernel interfaces is a bit of a chore.
> * First you have to find the connector in question and make sure the
> @@ -476,7 +478,7 @@ static int prepare_sprite_surfaces(int fd, int
> sprite_width, int sprite_height,
> }
>
> static void ricochet(int tiled, int sprite_w, int sprite_h,
> - int out_w, int out_h, int dump_info)
> + int out_w, int out_h, int dump_info, int delay_time)
> {
> int ret;
> int gfx_fd;
> @@ -499,7 +501,7 @@ static void ricochet(int tiled, int sprite_w, int
> sprite_h,
> prim_fb_id;
> struct drm_intel_sprite_colorkeyset;
> struct connectorcurr_connector;
> - drmModeRes *gfx_resources;
> + drmModeRes *gfx_resources = NULL;
> struct termios orig_term,
> curr_term;
> int c_index;
> @@ -518,259 +520,264 @@ static void ricochet(int tiled, int sprite_w, int
> sprite_h,
> charkey;
> int sprite_plane_count = 0;
> int i;
> + int sprite_loop = 0;
> + int disable_timer = 0;
> + drmModePlaneRes *plane_resources;
> + drmModePlane *ovr;
> // Open up I915 graphics device
> gfx_fd = drmOpen("i915", NULL);
> if (gfx_fd < 0) {
> printf("Failed to load i915 driver: %s\n", strerror(errno));
> return;
> }
> -
> - // Obtain pointer to struct containing graphics resources
> - gfx_resources = drmModeGetResources(gfx_fd);
> - if (!gfx_resources) {
> - printf("drmModeGetResources failed: %s\n", strerror(errno));
> - return;
> - }
> -
> - if (dump_info != 0) {
> - dump_connectors(gfx_fd, gfx_resources);
> - dump_crtcs(gfx_fd, gfx_resources);
> - dump_planes(gfx_fd, gfx_resources);
> - }
> -
> - // Save previous terminal settings
> - if (tcgetattr( 0, &orig_term) != 0) {
> - printf("tcgetattr failure: %s\n",
> - strerror(errno));
> - return;
> - }
> -
> - // Set up input to return characters immediately
> - curr_term = orig_term;
> - curr_term.c_lflag &= ~(ICANON | ECHO | ECHONL);
> - curr_term.c_cc[VMIN] = 0; // No minimum number of characters
> - curr_term.c_cc[VTIME] = 0 ; // Return immediately, even if
> - // nothing has been entered.
> - if (tcsetattr( 0, TCSANOW, &curr_term) != 0) {
> - printf("tcgetattr failure: %s\n", strerror(errno));
> - return;
> - }
> -
> - // Cycle through all connectors and display the flying sprite
> - // where there are displays attached and the hardware will support it.
> - for (c_index = 0; c_index < gfx_resources->count_connectors; c_index++)
> {
> - curr_connector.id = gfx_resources->connectors[c_index];
> -
> - // Find the native (preferred) display mode
> - connector_find_preferred_mode(gfx_fd, gfx_resources,
> &curr_connector);
> - if (curr_connector.mode_valid == 0) {
> - printf("No valid preferred mode detected\n");
> - goto out;
> - }
> -
> - // Determine if sprite hardware is available on pipe
> - // associated with this connector.
> - sprite_plane_count = connector_find_plane(gfx_fd,
> &curr_connector,
> - &sprite_plane_id);
> - if (!sprite_plane_count) {
> - printf("Failed to find
Re: [Intel-gfx] [PATCH 3/5] drm/i915/skl: Support secondary (rotated) frame buffer mapping
On Tue, Mar 03, 2015 at 09:59:31AM +, Tvrtko Ursulin wrote:
>
> On 03/02/2015 06:21 PM, Daniel Vetter wrote:
> >On Mon, Mar 02, 2015 at 02:43:50PM +, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin
> >>
> >>90/270 rotated scanout needs a rotated GTT view of the framebuffer.
> >>
> >>This is put in a separate VMA with a dedicated ggtt_view and wired suchs
> >>that
> >>it is created when a framebuffer is pinned to a 90/270 rotated plane.
> >>
> >>Rotation is only possible with Yb/Yf buffers and error is propagated to
> >>user space in case of a mismatch.
> >>
> >>Special rotated page view is constructed at the VMA creation time by
> >>borrowing the DMA addresses from obj->pages.
> >>
> >>v2:
> >> * Do not bother with pages for rotated sg list, just populate the DMA
> >> addresses. (Daniel Vetter)
> >> * Checkpatch cleanup.
> >>
> >>v3:
> >> * Rebased on top of new plane handling (create rotated mapping when
> >> setting the rotation property).
> >> * Unpin rotated VMA on unpinning from display plane.
> >> * Simplify rotation check using bitwise AND. (Chris Wilson)
> >>
> >>v4:
> >> * Fix unpinning of optional rotated mapping so it is really considered
> >> to be optional.
> >>
> >>v5:
> >>* Rebased for fb modifier changes.
> >>* Rebased for atomic commit.
> >>* Only pin needed view for display. (Ville Syrjälä, Daniel Vetter)
> >>
> >>For: VIZ-4726
> >>Signed-off-by: Tvrtko Ursulin
> >>Reviewed-by: Michel Thierry (v4)
> >
> >Bunch of nitpicks below. Also I think it'd be good to split this patch
> >into the rote refactoring work to add view parameters all over the place
> >and the actual implementation.
>
> Ok will split it. Rest below...
>
> >>---
> >> drivers/gpu/drm/i915/i915_drv.h | 33 +-
> >> drivers/gpu/drm/i915/i915_gem.c | 27 +++--
> >> drivers/gpu/drm/i915/i915_gem_gtt.c | 14 ++-
> >> drivers/gpu/drm/i915/i915_gem_gtt.h | 12 +++
> >> drivers/gpu/drm/i915/intel_display.c | 204
> >> +++
> >> drivers/gpu/drm/i915/intel_drv.h | 4 +
> >> drivers/gpu/drm/i915/intel_fbdev.c | 2 +-
> >> drivers/gpu/drm/i915/intel_overlay.c | 3 +-
> >> 8 files changed, 263 insertions(+), 36 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >>b/drivers/gpu/drm/i915/i915_drv.h
> >>index e07a1cb..79d3f2c 100644
> >>--- a/drivers/gpu/drm/i915/i915_drv.h
> >>+++ b/drivers/gpu/drm/i915/i915_drv.h
> >>@@ -2743,8 +2743,10 @@ i915_gem_object_set_to_cpu_domain(struct
> >>drm_i915_gem_object *obj, bool write);
> >> int __must_check
> >> i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> >> u32 alignment,
> >>-struct intel_engine_cs *pipelined);
> >>-void i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object
> >>*obj);
> >>+struct intel_engine_cs *pipelined,
> >>+const struct i915_ggtt_view *view);
> >>+void i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object
> >>*obj,
> >>+ const struct i915_ggtt_view
> >>*view);
> >> int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
> >>int align);
> >> int i915_gem_open(struct drm_device *dev, struct drm_file *file);
> >>@@ -2813,7 +2815,13 @@ i915_gem_obj_lookup_or_create_vma(struct
> >>drm_i915_gem_object *obj,
> >>&i915_ggtt_view_normal);
> >> }
> >>
> >>-struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj);
> >>+struct i915_vma *i915_gem_obj_to_ggtt_view(struct drm_i915_gem_object *obj,
> >>+ enum i915_ggtt_view_type view);
> >>+static inline
> >>+struct i915_vma *i915_gem_obj_to_ggtt(struct drm_i915_gem_object *obj)
> >>+{
> >>+ return i915_gem_obj_to_ggtt_view(obj, I915_GGTT_VIEW_NORMAL);
> >>+}
> >> static inline bool i915_gem_obj_is_pinned(struct drm_i915_gem_object
> >> *obj) {
> >>struct i915_vma *vma;
> >>list_for_each_entry(vma, &obj->vma_list, vma_link)
> >>@@ -2867,13 +2875,30 @@ i915_gem_obj_ggtt_pin(struct drm_i915_gem_object
> >>*obj,
> >> alignment, flags | PIN_GLOBAL);
> >> }
> >>
> >>+static inline int __must_check
> >>+i915_gem_obj_ggtt_pin_view(struct drm_i915_gem_object *obj,
> >>+ uint32_t alignment,
> >>+ unsigned flags,
> >>+ const struct i915_ggtt_view *ggtt_view)
> >>+{
> >>+ return i915_gem_object_pin_view(obj, i915_obj_to_ggtt(obj),
> >>+ alignment, flags | PIN_GLOBAL,
> >>+ ggtt_view);
> >>+}
> >>+
> >> static inline int
> >> i915_gem_object_ggtt_unbind(struct drm_i915_gem_object *obj)
> >> {
> >>return i915_vma_unbind(i915_gem_obj_to_ggtt(obj));
> >> }
> >>
> >>-void
[Intel-gfx] [PATCH] drm/i915: Add I915_PARAM_REVISION
Adds a parameter which can be used with DRM_I915_GETPARAM to query the
GPU revision. The intention is to use this in Mesa to implement the
WaDisableSIMD16On3SrcInstr workaround on Skylake but only for
revision 2.
Signed-off-by: Neil Roberts
---
The corresponding Mesa patches are here:
http://lists.freedesktop.org/archives/mesa-dev/2015-March/078534.html
and the libdrm patch is here:
http://lists.freedesktop.org/archives/dri-devel/2015-March/078645.html
drivers/gpu/drm/i915/i915_dma.c | 3 +++
include/uapi/drm/i915_drm.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 0312879..36dd1df 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -68,6 +68,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
case I915_PARAM_CHIPSET_ID:
value = dev->pdev->device;
break;
+ case I915_PARAM_REVISION:
+ value = dev->pdev->revision;
+ break;
case I915_PARAM_HAS_GEM:
value = 1;
break;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 6eed16b..b768f3b 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -347,6 +347,7 @@ typedef struct drm_i915_irq_wait {
#define I915_PARAM_HAS_COHERENT_PHYS_GTT 29
#define I915_PARAM_MMAP_VERSION 30
#define I915_PARAM_HAS_BSD2 31
+#define I915_PARAM_REVISION 32
typedef struct drm_i915_getparam {
int param;
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add module param to test the load detect code
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5880 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -2 278/278 276/278 ILK 308/308 308/308 SNB 284/284 284/284 IVB 380/380 380/380 BYT 294/294 294/294 HSW 387/387 387/387 BDW -1 316/316 315/316 -Detailed- Platform Testdrm-intel-nightly Series Applied PNV igt_gem_userptr_blits_coherency-sync NO_RESULT(1)CRASH(8)NRUN(1)PASS(8) CRASH(1)PASS(1) PNV igt_gem_userptr_blits_coherency-unsync FAIL(1)NO_RESULT(1)CRASH(6)PASS(7) CRASH(1)PASS(1) *BDW igt_gem_gtt_hog PASS(19) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Make sure we invalidate frontbuffer on fbcon.
On Tue, Mar 03, 2015 at 08:03:13PM +, Vivi, Rodrigo wrote: > On Tue, 2015-03-03 at 09:28 +0100, Daniel Vetter wrote: > > On Mon, Mar 02, 2015 at 06:35:26PM +, Vivi, Rodrigo wrote: > > > On Mon, 2015-03-02 at 18:59 +0100, Daniel Vetter wrote: > > > > On Fri, Feb 27, 2015 at 08:26:05PM -0500, Rodrigo Vivi wrote: > > > > > There are some cases like suspend/resume or dpms off/on sequences > > > > > that can flush frontbuffer bits. In these cases features that relies > > > > > on frontbuffer tracking can start working and user can stop getting > > > > > screen updates on fbcon having impression the system is frozen. > > > > > > > > > > So, let's make sure on fbcon write operation we also invalidate > > > > > frontbuffer bits so we will be on the safest side with fbcon. > > > > > > > > This is just a bandaid since you can always just directly access the > > > > fbdev framebuffer. We really need to figure out why we have frontbuffer > > > > bit flushes after we've invalidated them for fbcon and catch them all. > > > > > > yeah, an ugly bandaid... Just to make PSR a bit more reliable without > > > breaking fbcon environment when it gets enabled by default. > > > > > > The issue is that on the logs I see: > > > > > > 1.fbdev_blank dpms off > > > 2. disable planes > > > 3. flush frontbuffer bits > > > --- blank stage --- > > > 4. fbdev_blank dpms on > > > > so fbdev_blank returns _before_ the below enable_planes/frontbuf_flush? > > Can you please attach full backtraces for steps 5&6? > > [ 156.665517] [drm:intel_fbdev_set_par] PSR FBDEV modeset > [ 759.232969] [drm:intel_fbdev_blank] PSR FBDEV blank normal > [ 759.232987] [drm:intel_crtc_disable_planes] PSR FBDEV crtc disable > planes flush fb bits > [ 897.313095] [drm:intel_fbdev_blank] PSR FBDEV unblank > [ 897.313112] [drm:intel_crtc_control] PSR FBDEV crtc enable planes > [ 897.527818] [drm:haswell_crtc_enable] PSR FBDEV crtc enable planes > [ 897.542925] [drm:intel_crtc_enable_planes] PSR FBDEV crtc enable > planes flush fb bits I didn't mean the drm.debug log but the full backtrace for every time we flush psr fb bits. I want to know who's the top-level function which eventualy leads to the fb flush. I.e. something like WARN_ON(frontbuffer_bits); in intel_psr_flush (after we've filtered out any already set bits and other stuff that doesn't apply ofc). Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/12] drm/i915: Enable the maxfifo PM5 mode when appropriate on CHV
On 2/10/2015 6:58 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
CHV has a new knob in Punit to select between some memory power savings
modes PM2 and PM5. We can allow the deeper PM5 when maxfifo mode is
enabled, so let's do so in the hopes for moar power savings.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_reg.h | 3 +++
drivers/gpu/drm/i915/intel_pm.c | 13 -
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a0a7688..2196e57 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -552,6 +552,9 @@
#define DSPFREQSTAT_MASK(0x3 << DSPFREQSTAT_SHIFT)
#define DSPFREQGUAR_SHIFT 14
#define DSPFREQGUAR_MASK(0x3 << DSPFREQGUAR_SHIFT)
+#define DSP_MAXFIFO_PM5_STATUS (1 << 22) /* chv */
+#define DSP_AUTO_CDCLK_GATE_DISABLE (1 << 7) /* chv */
+#define DSP_MAXFIFO_PM5_ENABLE (1 << 6) /* chv */
#define _DP_SSC(val, pipe) ((val) << (2 * (pipe)))
#define DP_SSC_MASK(pipe) _DP_SSC(0x3, (pipe))
#define DP_SSC_PWR_ON(pipe) _DP_SSC(0x0, (pipe))
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e6cbc24..4e11552 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -240,7 +240,18 @@ void intel_set_memory_cxsr(struct drm_i915_private
*dev_priv, bool enable)
struct drm_device *dev = dev_priv->dev;
u32 val;
- if (IS_VALLEYVIEW(dev)) {
+ if (IS_CHERRYVIEW(dev)) {
+ I915_WRITE(FW_BLC_SELF_VLV, enable ? FW_CSPWRDWNEN : 0);
+
+ mutex_lock(&dev_priv->rps.hw_lock);
+ val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
+ if (enable)
+ val |= DSP_MAXFIFO_PM5_ENABLE;
+ else
+ val &= ~DSP_MAXFIFO_PM5_ENABLE;
+ vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
+ mutex_unlock(&dev_priv->rps.hw_lock);
+ } else if (IS_VALLEYVIEW(dev)) {
I915_WRITE(FW_BLC_SELF_VLV, enable ? FW_CSPWRDWNEN : 0);
} else if (IS_G4X(dev) || IS_CRESTLINE(dev)) {
I915_WRITE(FW_BLC_SELF, enable ? FW_BLC_SELF_EN : 0);
Since you are enabling MaxFIFO for multi plane in one of the previous
patches, i guess PM5 will also be enabled when more than one plane is
active in this flow.
Let's enable MaxFIFO and PM5 when only one plane is active for now. This
is the only validated scenario by SV.
With this addressed,
Reviewed-by: Vijay Purushothaman
Thanks,
Vijay
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/12] drm/i915: Program PFI credits for VLV
On 2/10/2015 6:58 PM, ville.syrj...@linux.intel.com wrote:
From: Vidya Srinivas
PFI credit programming is required when CD clock (related to data flow from
display pipeline to end display) is greater than CZ clock (related to data
flow from memory to display plane). This programming should be done when all
planes are OFF to avoid intermittent hangs while accessing memory even from
different Gfx units (not just display).
If cdclk/czclk >=1, PFI credits could be set as any number. To get better
performance, larger PFI credit can be assigned to PND. Otherwise if
cdclk/czclk<1, the default PFI credit of 8 should be set.
v2:
- Change log to lower log level instead of DRM_ERROR
- Change function name to valleyview_program_pfi_credits
- Move program PFI credits to modeset_init instead of intel_set_mode
- Change magic numbers to logical constants
[vsyrjala v3:
- only program in response to cdclk update
- program the credits also when cdclk
Signed-off-by: Gajanan Bhat
Signed-off-by: Vandana Kannan
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_reg.h | 8
drivers/gpu/drm/i915/intel_display.c | 33 +
2 files changed, 41 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index aacf90b..a0a7688 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2061,6 +2061,14 @@ enum skl_disp_power_wells {
#define CDCLK_FREQ_SHIFT4
#define CDCLK_FREQ_MASK (0x1f << CDCLK_FREQ_SHIFT)
#define CZCLK_FREQ_MASK 0xf
+
+#define GCI_CONTROL(VLV_DISPLAY_BASE + 0x650C)
+#define PFI_CREDIT_63(9 << 28) /* chv only */
+#define PFI_CREDIT_31(8 << 28) /* chv only */
+#define PFI_CREDIT(x)(((x) - 8) << 28) /* 8-15 */
+#define PFI_CREDIT_RESEND(1 << 27)
+#define VGA_FAST_MODE_DISABLE(1 << 14)
+
#define GMBUSFREQ_VLV (VLV_DISPLAY_BASE + 0x6510)
/*
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 3fe9598..9dcab4b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4987,6 +4987,37 @@ static void valleyview_modeset_global_pipes(struct
drm_device *dev,
*prepare_pipes |= (1 << intel_crtc->pipe);
}
+static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
+{
+ unsigned int credits;
+
+ if (DIV_ROUND_CLOSEST(dev_priv->vlv_cdclk_freq, 1000) >=
dev_priv->rps.cz_freq) {
+ /* CHV suggested value is 31 or 63 */
+ if (IS_CHERRYVIEW(dev_priv))
+ credits = PFI_CREDIT_31;
+ else
+ credits = PFI_CREDIT(15);
+ } else {
+ credits = PFI_CREDIT(8);
The default value should be 4 credits for CHV and 0 for VLV.
+ }
+
+ /*
+* WA - write default credits before re-programming
+* FIXME: should we also set the resend bit here?
+*/
+ I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE |
+ PFI_CREDIT(8));
Default credit should be 4 credits for CHV. PFI_CREDIT(12). Document
update is pending. But this is the latest recommendation to windows team.
Thanks,
Vijay
+
+ I915_WRITE(GCI_CONTROL, VGA_FAST_MODE_DISABLE |
+ credits | PFI_CREDIT_RESEND);
+
+ /*
+* FIXME is this guaranteed to clear
+* immediately or should we poll for it?
+*/
+ WARN_ON(I915_READ(GCI_CONTROL) & PFI_CREDIT_RESEND);
+}
+
static void valleyview_modeset_global_resources(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -5010,6 +5041,8 @@ static void valleyview_modeset_global_resources(struct
drm_device *dev)
else
valleyview_set_cdclk(dev, req_cdclk);
+ vlv_program_pfi_credits(dev_priv);
+
intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
}
}
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] acpi/video: Load the module even if ACPI is disabled
On Wednesday, March 04, 2015 09:55:55 AM Aaron Lu wrote: > On Sun, Mar 01, 2015 at 10:41:37AM +, Chris Wilson wrote: > > i915.ko depends upon the acpi/video.ko module and so refuses to load if > > ACPI is disabled at runtime if for example the BIOS is broken beyond > > repair. acpi/video provides an optional service for i915.ko and so we > > should just allow the modules to load, but do no nothing in order to let > > the machines boot correctly. > > > > Reported-by: Bill Augur > > Signed-off-by: Chris Wilson > > Cc: Daniel Vetter > > Cc: Jani Nikula > > Cc: Zhang Rui > > Cc: "Rafael J. Wysocki" > > Cc: Len Brown > > Cc: linux-a...@vger.kernel.org > > Cc: linux-ker...@vger.kernel.org > > Cc: sta...@vger.kernel.org > > For both patches: > Acked-by: Aaron Lu Both applied, thanks! -- 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] [PATCH 10/12] drm/i915: Support maxfifo with two planes on CHV
On 2/10/2015 6:58 PM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä
CHV supposedly does maxfifo mode even with two enabled
(primary/sprite) planes. Lets try to support that by halving the FIFO
size for the calculations and picking the smallest calculcated
watermark from the enabled planes.
Where is this mentioned in the spec? My understanding is MaxFIFO can be
enabled when only one plane is active (either ARGB or YUV). I do
remember some TR task that was talking about enabling MaxFIFO for two
planes. But i didn't get any confirmation that this can be enabled. We
have not enabled this for CHT in any other OS (Windows / Android).
Other features like PM2 / PM5 / DVFS can be enabled for multi plane
scenarios
Thanks,
Vijay
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_pm.c | 22 +-
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d29c02c..e6cbc24 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -907,6 +907,8 @@ static bool vlv_compute_sr_wm(struct drm_device *dev,
int num_planes = 0;
int fifo_size = 0;
struct intel_plane *plane;
+ /* CHV supports max fifo with two planes (1:1 split) */
+ int max_planes = IS_CHERRYVIEW(dev) ? 2 : 1;
wm->sr.cursor = wm->sr.plane = 0;
@@ -920,23 +922,33 @@ static bool vlv_compute_sr_wm(struct drm_device *dev,
fifo_size = INTEL_INFO(dev_priv)->num_pipes * 512 - 1;
}
- if (fifo_size == 0 || num_planes > 1)
+ if (fifo_size == 0 || num_planes > max_planes)
return false;
+ if (num_planes)
+ fifo_size /= num_planes;
+
wm->sr.cursor = vlv_compute_wm(to_intel_crtc(crtc),
to_intel_plane(crtc->cursor), 0x3f);
list_for_each_entry(plane, &dev->mode_config.plane_list, base.head) {
+ uint16_t sr_wm;
+
if (plane->base.type == DRM_PLANE_TYPE_CURSOR)
continue;
if (plane->pipe != pipe)
continue;
- wm->sr.plane = vlv_compute_wm(to_intel_crtc(crtc),
- plane, fifo_size);
- if (wm->sr.plane != 0)
- break;
+ sr_wm = vlv_compute_wm(to_intel_crtc(crtc),
+ plane, fifo_size);
+ if (sr_wm == 0)
+ continue;
+
+ if (wm->sr.plane == 0)
+ wm->sr.plane = sr_wm;
+ else
+ wm->sr.plane = min(wm->sr.plane, sr_wm);
}
return true;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/12] drm/i915/bdw: Add 4 level switching infrastructure
On Tue, Mar 03, 2015 at 06:31:03PM +0530, akash goel wrote:
> On Fri, Feb 20, 2015 at 11:16 PM, Michel Thierry
> wrote:
> > +static void gen8_map_page_directory(struct
> > i915_page_directory_pointer_entry *pdp,
> > + struct i915_page_directory_entry *pd,
> > + int index,
> > + struct drm_device *dev)
> > +{
> > + gen8_ppgtt_pdpe_t *page_directorypo;
> > + gen8_ppgtt_pdpe_t pdpe;
> > +
> > + /* We do not need to clflush because no platform requiring flush
> > +* supports 64b pagetables. */
>
> Would be more appropriate to place this comment, either after the ‘if’
> condition or
> at the end of the function (where clflush would have been placed, had
> LLC not been there for platforms supporting 64 bit).
> And same comment can be probably added, at the end of
> gen8_map_page_directory_pointer function also.
>
> > + if (!USES_FULL_48BIT_PPGTT(dev))
> > + return;
Ok this on a lot of levels:
- a function calle map_something, which doesn't actually return a map.
Must be renamed asap to something that makes sense, in the kernel
everything call map_foo actually maps foo somewhere and returns where.
- The comment is fairly useless since it doesn't mention which platforms
flushing is required on. Either we need to split functions up more into
4G and 48bit variants if this difference is due to the pagetable layout.
Or we need to replace it with appropriate platform checks.
Or maybe this if is just dead code and should be remove entirely?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/12] drm/i915/bdw: implement alloc/free for 4lvl
On Tue, Mar 03, 2015 at 06:25:27PM +0530, akash goel wrote: > On Fri, Feb 20, 2015 at 11:15 PM, Michel Thierry > > + pdp = ppgtt->pml4.pdps[i]; > > + if (!pdp->daddr) > > + pci_unmap_page(hwdev, pdp->daddr, PAGE_SIZE, > > + PCI_DMA_BIDIRECTIONAL); > > + > > For consistency & cleanup, the call to pci_unmap_page can be replaced > with i915_dma_unmap_single. > Same can be done inside the gen8_ppgtt_unmap_pages_3lvl function also. Everything but the dma api interfaces (dma_unmap_page) is deprecated. A follow-up patch to go through all the i915 code and do these replacements would be nice. After all this landed ofc. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Setup all page directories for gen8
If the requested size is less than what the full range
of pdps can address, we end up setting pdps for only the
requested area.
The logical context however needs all pdp entries to be valid.
Prior to commit 06fda602dbca ("drm/i915: Create page table allocators")
we have been writing pdp entries with dma address of zero instead
of valid pdps. This is supposedly bad even if those pdps are not
addressed.
As commit 06fda602dbca ("drm/i915: Create page table allocators")
introduced more dynamic structure for pdps, we ended up oopsing
when we populated the lrc context. Analyzing this oops revealed
the fact that we have not been writing valid pdps with bsw, as
it is doing the ppgtt init with 2GB limit in some cases.
We should do the right thing and setup the non addressable part
pdps/pde/pte to scratch page through the minimal structure by
having just pdp with pde entries pointing to same page with
pte entries pointing to scratch page.
But instead of going through that trouble, setup all the pdps
through individual pd pages and pt entries, even for non
addressable parts. And let the clear range point them to scratch
page. This way we populate the lrc with valid pdps and wait
for dynamic page allocation work to land, and do the heavy lifting
for truncating page table tree according to usage.
The regression of oopsing in init was introduced by
commit 06fda602dbca ("drm/i915: Create page table allocators")
v2: Clear the range for the unused part also (Ville)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89350
Cc: Michel Thierry
Cc: Ben Widawsky
Cc: Ville Syrjälä
Tested-by: Valtteri Rantala
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index bd95776..1aa2a2c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -716,15 +716,19 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
uint64_t size)
if (size % (1<<30))
DRM_INFO("Pages will be wasted unless GTT size (%llu) is
divisible by 1GB\n", size);
- /* 1. Do all our allocations for page directories and page tables. */
- ret = gen8_ppgtt_alloc(ppgtt, max_pdp);
+ /* 1. Do all our allocations for page directories and page tables.
+* We allocate more than was asked so that we can point the unused parts
+* to valid entries that point to scratch page. Dynamic page tables
+* will fix this eventually.
+*/
+ ret = gen8_ppgtt_alloc(ppgtt, GEN8_LEGACY_PDPES);
if (ret)
return ret;
/*
* 2. Create DMA mappings for the page directories and page tables.
*/
- for (i = 0; i < max_pdp; i++) {
+ for (i = 0; i < GEN8_LEGACY_PDPES; i++) {
ret = gen8_ppgtt_setup_page_directories(ppgtt, i);
if (ret)
goto bail;
@@ -744,7 +748,7 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
uint64_t size)
* plugged in correctly. So we do that now/here. For aliasing PPGTT, we
* will never need to touch the PDEs again.
*/
- for (i = 0; i < max_pdp; i++) {
+ for (i = 0; i < GEN8_LEGACY_PDPES; i++) {
struct i915_page_directory_entry *pd =
ppgtt->pdp.page_directory[i];
gen8_ppgtt_pde_t *pd_vaddr;
pd_vaddr = kmap_atomic(ppgtt->pdp.page_directory[i]->page);
@@ -764,9 +768,14 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt,
uint64_t size)
ppgtt->base.insert_entries = gen8_ppgtt_insert_entries;
ppgtt->base.cleanup = gen8_ppgtt_cleanup;
ppgtt->base.start = 0;
- ppgtt->base.total = ppgtt->num_pd_entries * GEN8_PTES_PER_PAGE *
PAGE_SIZE;
- ppgtt->base.clear_range(&ppgtt->base, 0, ppgtt->base.total, true);
+ /* This is the area that we advertise as usable for the caller */
+ ppgtt->base.total = max_pdp * GEN8_PDES_PER_PAGE * GEN8_PTES_PER_PAGE *
PAGE_SIZE;
+
+ /* Set all ptes to a valid scratch page. Also above requested space */
+ ppgtt->base.clear_range(&ppgtt->base, 0,
+ ppgtt->num_pd_pages * GEN8_PTES_PER_PAGE *
PAGE_SIZE,
+ true);
DRM_DEBUG_DRIVER("Allocated %d pages for page directories (%d
wasted)\n",
ppgtt->num_pd_pages, ppgtt->num_pd_pages - max_pdp);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix modeset state confusion in the load detect code
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5879 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -6 278/278 272/278 ILK 308/308 308/308 SNB -1 284/284 283/284 IVB -1 380/380 379/380 BYT 294/294 294/294 HSW 387/387 387/387 BDW -1 316/316 315/316 -Detailed- Platform Testdrm-intel-nightly Series Applied PNV igt_gem_userptr_blits_coherency-sync NO_RESULT(1)CRASH(8)NRUN(1)PASS(8) CRASH(1)NRUN(1) PNV igt_gem_userptr_blits_coherency-unsync FAIL(1)NO_RESULT(1)CRASH(6)PASS(7) CRASH(1)PASS(1) PNV igt_gem_userptr_blits_minor-unsync-interruptible DMESG_WARN(1)PASS(5) DMESG_WARN(1)PASS(1) PNV igt_gen3_render_linear_blits FAIL(6)NRUN(1)DMESG_WARN(1)PASS(9) FAIL(2) PNV igt_gen3_render_mixed_blits FAIL(9)PASS(9) FAIL(2) PNV igt_gem_fence_thrash_bo-write-verify-threaded-none FAIL(2)CRASH(4)PASS(7) CRASH(2) *SNB igt_gem_fence_thrash_bo-write-verify-y PASS(5) DMESG_WARN(1)PASS(1) *IVB igt_gem_storedw_batches_loop_secure-dispatch PASS(3) DMESG_WARN(1)PASS(1) *BDW igt_gem_gtt_hog PASS(19) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915/skl: Program PLL for edp1.4 intermediate frequencies
On Sat, Feb 21, 2015 at 11:12:13AM +0530, Sonika Jindal wrote:
> v2: Making the link_clock half in switch inline with the DPLL_CTRL1_* macros
> (Ville)
>
> Signed-off-by: Sonika Jindal
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/intel_dp.c | 28 ++--
> 1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index cf7a0f5..62bc6c1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1079,7 +1079,7 @@ intel_dp_connector_unregister(struct intel_connector
> *intel_connector)
> }
>
> static void
> -skl_edp_set_pll_config(struct intel_crtc_state *pipe_config, int link_bw)
> +skl_edp_set_pll_config(struct intel_crtc_state *pipe_config, int link_clock)
> {
> u32 ctrl1;
>
> @@ -1088,19 +1088,35 @@ skl_edp_set_pll_config(struct intel_crtc_state
> *pipe_config, int link_bw)
> pipe_config->dpll_hw_state.cfgcr2 = 0;
>
> ctrl1 = DPLL_CTRL1_OVERRIDE(SKL_DPLL0);
> - switch (link_bw) {
> - case DP_LINK_BW_1_62:
> + switch (link_clock / 2) {
> + case 81000:
> ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_810,
> SKL_DPLL0);
> break;
> - case DP_LINK_BW_2_7:
> + case 135000:
> ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_1350,
> SKL_DPLL0);
> break;
> - case DP_LINK_BW_5_4:
> + case 27:
> ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_2700,
> SKL_DPLL0);
> break;
> + case 162000:
> + ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_1620,
> + SKL_DPLL0);
> + break;
> + /* TBD: For DP link rates 2.16 GHz and 4.32 GHz, VCO is 8640 which
> + results in CDCLK change. Need to handle the change of CDCLK by
> + disabling pipes and re-enabling them */
> + case 108000:
> + ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_1080,
> + SKL_DPLL0);
> + break;
> + case 216000:
> + ctrl1 |= DPLL_CRTL1_LINK_RATE(DPLL_CRTL1_LINK_RATE_2160,
> + SKL_DPLL0);
> + break;
> +
> }
> pipe_config->dpll_hw_state.ctrl1 = ctrl1;
> }
> @@ -1395,7 +1411,7 @@ found:
> }
>
> if (IS_SKYLAKE(dev) && is_edp(intel_dp))
> - skl_edp_set_pll_config(pipe_config, intel_dp->link_bw);
> + skl_edp_set_pll_config(pipe_config, supported_rates[clock]);
> else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> hsw_dp_set_ddi_pll_sel(pipe_config, intel_dp->link_bw);
> else
> --
> 1.7.10.4
--
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 3/4] drm/i915/skl: Add support for edp 1.4 intermediate frequencies
On Sat, Feb 21, 2015 at 11:12:12AM +0530, Sonika Jindal wrote:
> eDp 1.4 supports custom frequencies.
> Skylake supports following intermediate frequencies : 3.24 GHz, 2.16 GHz and
> 4.32 GHz along with usual LBR, HBR and HBR2 frequencies.
> Read sink supported frequencies and get common frequencies from sink and
> source and use these for link training.
>
> v2: Rebased, removed calculation of min_clock since for edp it is taken as
> max_clock (as per comment).
> v3: Keeping single array for link rates (Satheesh)
> v4: Setting LINK_BW_SET to 0 when setting LINK_RATE_SET (Satheesh)
> v5: Some minor nits (Ville)
> v6: Keeping separate arrays for source and sink rates (Ville)
>
> Signed-off-by: Sonika Jindal
> ---
> drivers/gpu/drm/i915/intel_ddi.c |9 +++
> drivers/gpu/drm/i915/intel_dp.c | 113
> +++---
> drivers/gpu/drm/i915/intel_drv.h |1 +
> 3 files changed, 115 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 4d8c38d..db00db8 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -803,9 +803,18 @@ static void skl_ddi_clock_get(struct intel_encoder
> *encoder,
> case DPLL_CRTL1_LINK_RATE_810:
> link_clock = 81000;
> break;
> + case DPLL_CRTL1_LINK_RATE_1080:
> + link_clock = 108000;
> + break;
> case DPLL_CRTL1_LINK_RATE_1350:
> link_clock = 135000;
> break;
> + case DPLL_CRTL1_LINK_RATE_1620:
> + link_clock = 162000;
> + break;
> + case DPLL_CRTL1_LINK_RATE_2160:
> + link_clock = 216000;
> + break;
> case DPLL_CRTL1_LINK_RATE_2700:
> link_clock = 27;
> break;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 72deac6..cf7a0f5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -83,6 +83,11 @@ static const struct dp_link_dpll chv_dpll[] = {
> { DP_LINK_BW_5_4, /* m2_int = 27, m2_fraction = 0 */
> { .p1 = 2, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } }
> };
> +/* Skylake supports following rates */
> +static const uint32_t gen9_rates[] = { 162000, 216000, 27, 324000,
> + 432000, 54 };
> +
> +static const uint32_t default_rates[] = { 162000, 27, 54 };
>
> /**
> * is_edp - is the given port attached to an eDP panel (either CPU or PCH)
> @@ -1143,6 +1148,25 @@ intel_read_sink_rates(struct intel_dp *intel_dp,
> uint32_t *sink_rates)
> return i;
> }
>
> +static int
> +intel_read_source_rates(struct intel_dp *intel_dp, uint32_t *source_rates)
> +{
> + struct drm_device *dev = intel_dp_to_dev(intel_dp);
> + int i;
> + int max_default_rate;
> +
> + if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
> + for (i = 0; i < ARRAY_SIZE(gen9_rates); ++i)
> + source_rates[i] = gen9_rates[i];
> + } else {
> + /* Index of the max_link_bw supported + 1 */
> + max_default_rate = (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
> + for (i = 0; i < max_default_rate; ++i)
> + source_rates[i] = default_rates[i];
> + }
> + return i;
> +}
> +
> static void
> intel_dp_set_clock(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config, int link_bw)
> @@ -1176,6 +1200,45 @@ intel_dp_set_clock(struct intel_encoder *encoder,
> }
> }
>
> +static int intel_supported_rates(const uint32_t *source_rates, int
> source_len,
> +const uint32_t *sink_rates, int sink_len, uint32_t *supported_rates)
> +{
> + int i = 0, j = 0, k = 0;
> +
> + /* For panels with edp version less than 1.4 */
> + if (sink_len == 0) {
> + for (i = 0; i < source_len; ++i)
> + supported_rates[i] = source_rates[i];
> + return source_len;
> + }
> +
> + /* For edp1.4 panels, find the common rates between source and sink */
> + while (i < source_len && j < sink_len) {
> + if (source_rates[i] == sink_rates[j]) {
> + supported_rates[k] = source_rates[i];
> + ++k;
> + ++i;
> + ++j;
> + } else if (source_rates[i] < sink_rates[j]) {
> + ++i;
> + } else {
> + ++j;
> + }
> + }
> + return k;
> +}
> +
> +static int rate_to_index(uint32_t find, const uint32_t *rates)
> +{
> + int i = 0;
> +
> + for (i = 0; i < DP_MAX_SUPPORTED_RATES; ++i)
> + if (find == rates[i])
> + break;
> +
> +
Re: [Intel-gfx] [PATCH 5/7] drm/i915: also do frontbuffer tracking on pwrites
On Tue, Mar 03, 2015 at 04:47:33PM -0800, Rodrigo Vivi wrote:
> Reviewed-by: Rodrigo Vivi
>
> On Fri, Feb 13, 2015 at 11:23 AM, Paulo Zanoni wrote:
> > From: Paulo Zanoni
> >
> > We need this for FBC, and possibly for PSR too.
> >
> > v2: Don't only flush: invalidate too (Daniel).
> >
> > Signed-off-by: Paulo Zanoni
Merged up to this patch, thanks.
-Daniel
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 23 ++-
> > 1 file changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 8b1cda6..ca979b1 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -351,7 +351,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj,
> > struct drm_device *dev = obj->base.dev;
> > void *vaddr = obj->phys_handle->vaddr + args->offset;
> > char __user *user_data = to_user_ptr(args->data_ptr);
> > - int ret;
> > + int ret = 0;
> >
> > /* We manually control the domain here and pretend that it
> > * remains coherent i.e. in the GTT domain, like shmem_pwrite.
> > @@ -360,6 +360,7 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj,
> > if (ret)
> > return ret;
> >
> > + intel_fb_obj_invalidate(obj, NULL, ORIGIN_CPU);
> > if (__copy_from_user_inatomic_nocache(vaddr, user_data,
> > args->size)) {
> > unsigned long unwritten;
> >
> > @@ -370,13 +371,18 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj,
> > mutex_unlock(&dev->struct_mutex);
> > unwritten = copy_from_user(vaddr, user_data, args->size);
> > mutex_lock(&dev->struct_mutex);
> > - if (unwritten)
> > - return -EFAULT;
> > + if (unwritten) {
> > + ret = -EFAULT;
> > + goto out;
> > + }
> > }
> >
> > drm_clflush_virt_range(vaddr, args->size);
> > i915_gem_chipset_flush(dev);
> > - return 0;
> > +
> > +out:
> > + intel_fb_obj_flush(obj, false);
> > + return ret;
> > }
> >
> > void *i915_gem_object_alloc(struct drm_device *dev)
> > @@ -810,6 +816,8 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev,
> >
> > offset = i915_gem_obj_ggtt_offset(obj) + args->offset;
> >
> > + intel_fb_obj_invalidate(obj, NULL, ORIGIN_GTT);
> > +
> > while (remain > 0) {
> > /* Operation in this page
> > *
> > @@ -830,7 +838,7 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev,
> > if (fast_user_write(dev_priv->gtt.mappable, page_base,
> > page_offset, user_data, page_length)) {
> > ret = -EFAULT;
> > - goto out_unpin;
> > + goto out_flush;
> > }
> >
> > remain -= page_length;
> > @@ -838,6 +846,8 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev,
> > offset += page_length;
> > }
> >
> > +out_flush:
> > + intel_fb_obj_flush(obj, false);
> > out_unpin:
> > i915_gem_object_ggtt_unpin(obj);
> > out:
> > @@ -952,6 +962,8 @@ i915_gem_shmem_pwrite(struct drm_device *dev,
> > if (ret)
> > return ret;
> >
> > + intel_fb_obj_invalidate(obj, NULL, ORIGIN_CPU);
> > +
> > i915_gem_object_pin_pages(obj);
> >
> > offset = args->offset;
> > @@ -1030,6 +1042,7 @@ out:
> > if (needs_clflush_after)
> > i915_gem_chipset_flush(dev);
> >
> > + intel_fb_obj_flush(obj, false);
> > return ret;
> > }
> >
> > --
> > 2.1.4
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Setup all page directories for gen8
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5878 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -9 278/278 269/278 ILK 308/308 308/308 SNB 284/284 284/284 IVB 380/380 380/380 BYT 294/294 294/294 HSW 387/387 387/387 BDW -1 316/316 315/316 -Detailed- Platform Testdrm-intel-nightly Series Applied *PNV igt_gem_fence_thrash_bo-write-verify-none NRUN(1)PASS(6) FAIL(1)PASS(1) *PNV igt_gem_fence_thrash_bo-write-verify-x PASS(7) FAIL(1)PASS(1) *PNV igt_gem_fence_thrash_bo-write-verify-y NO_RESULT(1)PASS(6) FAIL(1)PASS(1) PNV igt_gem_userptr_blits_coherency-sync NO_RESULT(1)CRASH(7)NRUN(1)PASS(8) CRASH(2) PNV igt_gem_userptr_blits_coherency-unsync NO_RESULT(1)CRASH(6)PASS(7) CRASH(2) *PNV igt_gem_userptr_blits_forked-unsync-swapping-multifd-mempressure-interruptible PASS(2) NRUN(1)PASS(1) *PNV igt_gem_userptr_blits_input-checking PASS(2) NRUN(1)PASS(1) PNV igt_gem_userptr_blits_minor-unsync-interruptible DMESG_WARN(1)PASS(5) DMESG_WARN(1)PASS(1) PNV igt_gem_fence_thrash_bo-write-verify-threaded-none FAIL(2)CRASH(4)PASS(6) CRASH(1)PASS(1) *BDW igt_gem_gtt_hog PASS(19) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: DP link training optimization
Generalization to cover DP case
Signed-off-by: Mika Kahola
---
drivers/gpu/drm/i915/intel_dp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9497eb6..abf8c7d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3566,7 +3566,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
if (channel_eq) {
DRM_DEBUG_KMS("Channel EQ done. DP Training successful\n");
- intel_dp->link_trained = is_edp(intel_dp);
+ intel_dp->link_trained = true;
}
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915/skl: Read sink supported rates from edp panel
On Sat, Feb 21, 2015 at 11:12:11AM +0530, Sonika Jindal wrote:
> v2: Using DP_SUPPORTED_LINK_RATES macro for supported_rates array (Satheesh).
> v3: Reading dpcd's supported link rates tables based upon edp version in the
> same patch.
> v4: Move version check under is_edp (Satheesh)
> v5: Using le16 for rates, some naming, and removing nested if block (Ville)
> v6: Correctly using DP_MAX_SUPPORTED_RATES and removing
> DP_SUPPORTED_LINK_RATES
> (Ville)
> v7: Incorrectly removed DP_SUPPORTED_LINK_RATES in v6, re-adding it
>
> Signed-off-by: Sonika Jindal
> ---
> drivers/gpu/drm/i915/intel_dp.c | 38
> ++
> drivers/gpu/drm/i915/intel_drv.h |1 +
> 2 files changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 42ac99f..72deac6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1116,6 +1116,33 @@ hsw_dp_set_ddi_pll_sel(struct intel_crtc_state
> *pipe_config, int link_bw)
> }
> }
>
> +static int
> +intel_read_sink_rates(struct intel_dp *intel_dp, uint32_t *sink_rates)
> +{
> + struct drm_device *dev = intel_dp_to_dev(intel_dp);
> + int i = 0;
> + uint16_t val;
> +
> + if (INTEL_INFO(dev)->gen >= 9 && intel_dp->supported_rates[0]) {
> + /*
> + * Receiver supports only main-link rate selection by
> + * link rate table method, so read link rates from
> + * supported_link_rates
> + */
> + for (i = 0; i < DP_MAX_SUPPORTED_RATES; ++i) {
> + val = le16_to_cpu(intel_dp->supported_rates[i]);
> + if (val == 0)
> + break;
> +
> + sink_rates[i] = val * 200;
> + }
> +
> + if (i <= 0)
> + DRM_ERROR("No rates in SUPPORTED_LINK_RATES");
> + }
> + return i;
> +}
> +
> static void
> intel_dp_set_clock(struct intel_encoder *encoder,
> struct intel_crtc_state *pipe_config, int link_bw)
> @@ -3592,6 +3619,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> struct drm_device *dev = dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + uint8_t rev;
>
> if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd,
> sizeof(intel_dp->dpcd)) < 0)
> @@ -3623,6 +3651,16 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
> } else
> intel_dp->use_tps3 = false;
>
> + /* Intermediate frequency support */
> + if (is_edp(intel_dp) &&
> + (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] &
> DP_DPCD_DISPLAY_CONTROL_CAPABLE) &&
> + intel_dp_dpcd_read_wake(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) &&
intel_dp_dpcd_read_wake() == 1
would be the right thing to check I think.
With that fixed:
Reviewed-by: Ville Syrjälä
> + (rev >= 0x03)) { /* eDp v1.4 or higher */
> + intel_dp_dpcd_read_wake(&intel_dp->aux,
> + DP_SUPPORTED_LINK_RATES,
> + intel_dp->supported_rates,
> + sizeof(intel_dp->supported_rates));
> + }
> if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> DP_DWN_STRM_PORT_PRESENT))
> return true; /* native DP sink */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 6832eb8..24e5411 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -602,6 +602,7 @@ struct intel_dp {
> uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
> uint8_t psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
> uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> + __le16 supported_rates[DP_MAX_SUPPORTED_RATES];
> struct drm_dp_aux aux;
> uint8_t train_set[4];
> int panel_power_up_delay;
> --
> 1.7.10.4
--
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: Don't require primary->fb in intel_crtc_active()
On Tue, Mar 03, 2015 at 06:15:12PM -0800, Matt Roper wrote: > Universal planes allow us to have an active CRTC without a primary plane > framebuffer bound. Drop the test for primary->fb from > intel_crtc_active() since we can clearly have active CRTC's without a > framebuffer, and this check is now interfering with watermark > calculations when we try to re-enable the primary plane. > > Note that commit > > commit 0fda65680e92545caea5be7805a7f0a617fb6c20 > Author: Tvrtko Ursulin > Date: Fri Feb 27 15:12:35 2015 + > > drm/i915/skl: Update watermarks for Y tiling > > adds a test for primary plane enable/disable to trigger a watermark > update (previously we ignored updates to primary planes, which wasn't > really correct, but we got lucky since we always pretended the primary > plane was on). Tvrtko's patch tries to update watermarks when we > re-enable the primary plane, but that watermark computation gets aborted > early because intel_crtc_active() always returns false when the primary > plane is disabled; this leads to watermark underruns (at least on > platforms with ILK-style watermark code). I think this will make a bunch of the old platforms blow up. Well, I think they might already blow up since they now look at crtc->state->fb based on the result of intel_crtc_active(). So I believe more fixes are needed all over the wm code at least. In fact looking at the code, I think most (or all?) of the call sites in the in the ilk+ wm code should just be using crtc->active. So for some extra safety it might make sense to do leave intel_crtc_active() alone for now, or in fact we should maybe change it to also look at primary->state->fb instead. That should more or less keep the old platforms in the same state as they were before, while letting new platforms handle each plane properly. > > Cc: Tvrtko Ursulin > Signed-off-by: Matt Roper > --- > drivers/gpu/drm/i915/intel_display.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 589addf..92cb2ff 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -893,11 +893,8 @@ bool intel_crtc_active(struct drm_crtc *crtc) >* >* We can ditch the adjusted_mode.crtc_clock check as soon >* as Haswell has gained clock readout/fastboot support. > - * > - * We can ditch the crtc->primary->fb check as soon as we can > - * properly reconstruct framebuffers. >*/ > - return intel_crtc->active && crtc->primary->fb && > + return intel_crtc->active && > intel_crtc->config->base.adjusted_mode.crtc_clock; > } > > -- > 1.8.5.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/8] drm/i915/skl: Take 90/270 rotation into account in watermark calculations
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5877 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -2 278/278 276/278 ILK 308/308 308/308 SNB -1 284/284 283/284 IVB -1 380/380 379/380 BYT 294/294 294/294 HSW 387/387 387/387 BDW -1 316/316 315/316 -Detailed- Platform Testdrm-intel-nightly Series Applied PNV igt_gen3_render_linear_blits FAIL(5)NRUN(1)DMESG_WARN(1)PASS(9) FAIL(2) PNV igt_gen3_render_mixed_blits FAIL(8)PASS(9) FAIL(2) *SNB igt_gem_fence_thrash_bo-write-verify-y PASS(5) DMESG_WARN(1)PASS(1) *IVB igt_gem_storedw_batches_loop_secure-dispatch PASS(3) DMESG_WARN(1)PASS(1) *BDW igt_gem_gtt_hog PASS(19) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add module param to test the load detect code
On Tue, Mar 03, 2015 at 04:27:47PM -0800, Jesse Barnes wrote: > My "rant du jour" still is. mlock() is a good solution for some things, > but for the simple task of testing kernel swap out code, just running > that code is the most straightforward thing to do, rather than trying to > contort into it from userspace. Once upon a time, there was a minor request for MADV_POPULATE and MADV_INVALIDATE (basically wrappers around get_pages() and put_pages()) which would give you the fuzzy feeling of testing those paths without actually testing the shrinker. But no use cases. -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/2] drm/i915: Don't assume primary & cursor are always on for wm calculation
On Wed, 04 Mar 2015, Matt Roper wrote:
> Current ILK-style watermark code assumes the primary plane and cursor
> plane are always enabled. This assumption, along with the combination
> of two independent commits that got merged at the same time, results in
> a NULL dereference. The offending commits are:
>
> commit fd2d61341bf39d1054256c07d6eddd624ebc4241
> Author: Matt Roper
> Date: Fri Feb 27 10:12:01 2015 -0800
>
> drm/i915: Use plane->state->fb in watermark code (v2)
>
> and
>
> commit 0fda65680e92545caea5be7805a7f0a617fb6c20
> Author: Tvrtko Ursulin
> Date: Fri Feb 27 15:12:35 2015 +
>
> drm/i915/skl: Update watermarks for Y tiling
>
> The first commit causes us to use the FB from plane->state->fb rather
> than the legacy plane->fb, which is updated a bit later in the process.
>
> The second commit includes a change that now triggers watermark
> reprogramming on primary plane enable/disable where we didn't have one
> before (which wasn't really correct, but we had been getting lucky
> because we always calculated as if the primary plane was on).
>
> Together, these two commits cause the watermark calculation to
> (properly) see plane->state->fb = NULL when we're in the process of
> disabling the primary plane. However the existing watermark code
> assumes there's always a primary fb and tries to dereference it to find
> out pixel format / bpp information.
>
> The fix is to make ILK-style watermark calculation actually check the
> true status of primary & cursor planes and adjust our watermark logic
> accordingly.
>
> Cc: Tvrtko Ursulin
> Cc: Michael Leuchtenburg
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89388
> Signed-off-by: Matt Roper
Tested-by: lu hua
> ---
> drivers/gpu/drm/i915/intel_pm.c | 22 +-
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e710b43..93fd15f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1924,13 +1924,25 @@ static void ilk_compute_wm_parameters(struct drm_crtc
> *crtc,
> p->active = true;
> p->pipe_htotal = intel_crtc->config->base.adjusted_mode.crtc_htotal;
> p->pixel_rate = ilk_pipe_pixel_rate(dev, crtc);
> - p->pri.bytes_per_pixel = crtc->primary->state->fb->bits_per_pixel / 8;
> - p->cur.bytes_per_pixel = 4;
> +
> + if (crtc->primary->state->fb) {
> + p->pri.enabled = true;
> + p->pri.bytes_per_pixel =
> + crtc->primary->state->fb->bits_per_pixel / 8;
> + } else {
> + p->pri.enabled = false;
> + p->pri.bytes_per_pixel = 0;
> + }
> +
> + if (crtc->cursor->state->fb) {
> + p->cur.enabled = true;
> + p->cur.bytes_per_pixel = 4;
> + } else {
> + p->cur.enabled = false;
> + p->cur.bytes_per_pixel = 0;
> + }
> p->pri.horiz_pixels = intel_crtc->config->pipe_src_w;
> p->cur.horiz_pixels = intel_crtc->base.cursor->state->crtc_w;
> - /* TODO: for now, assume primary and cursor planes are always enabled.
> */
> - p->pri.enabled = true;
> - p->cur.enabled = true;
>
> drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
> struct intel_plane *intel_plane = to_intel_plane(plane);
> --
> 1.8.5.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: gen4: work around hang during hibernation
On Mon, 02 Mar 2015, Bjørn Mork wrote: > Jani Nikula writes: > >> On Mon, 02 Mar 2015, Imre Deak wrote: >>> Bjørn reported that his machine hang during hibernation and eventually >>> bisected the problem to the following commit: >>> >>> commit da2bc1b9db3351addd293e5b82757efe1f77ed1d >>> Author: Imre Deak >>> Date: Thu Oct 23 19:23:26 2014 +0300 >>> >>> drm/i915: add poweroff_late handler >>> >>> The problem seems to be that after the kernel puts the device into D3 >>> the BIOS still tries to access it, or otherwise assumes that it's in D0. >>> This is clearly bogus, since ACPI mandates that devices are put into D3 >>> by the OSPM if they are not wake-up sources. In the future we want to >>> unify more of the driver's runtime and system suspend paths, for example >>> by skipping all the system suspend/hibernation hooks if the device is >>> runtime suspended already. Accordingly for all other platforms the goal >>> is still to properly power down the device during hibernation. >>> >>> v2: >>> - Another GEN4 Lenovo laptop had the same issue, while platforms from >>> other vendors (including mobile and desktop, GEN4 and non-GEN4) seem >>> to work fine. Based on this apply the workaround on all GEN4 Lenovo >>> platforms. >>> - add code comment about failing platforms (Ville) >>> >>> Reference: >>> http://lists.freedesktop.org/archives/intel-gfx/2015-February/060633.html >>> Reported-and-bisected-by: Bjørn Mork >>> Signed-off-by: Imre Deak >> >> Bjørn, I would really appreciate your Tested-by on this patch before I >> queue it for v4.0 and cc: stable for v3.19. > > No problem. This version still works fine for me. Feel free to add > > Tested-by: Bjørn Mork Pushed to drm-intel-fixes with Daniel's IRC ack. Thanks for the patch and testing. BR, Jani. > > > > Bjørn -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
