[Intel-gfx] [RFC] drm: Add utility function to check for edp1.4
From: Sonika Jindal sonika.jin...@intel.com
v2: Reading DP_EDP_REV, only when DISPLAY_CONTROL_CAPABLE field is set
(Satheesh)
v3: Moving the utility function to drm_dp_helper (Daniel)
Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
drivers/gpu/drm/drm_dp_helper.c | 15 +++
include/drm/drm_dp_helper.h |2 ++
2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 08e33b8..a54a760 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -768,3 +768,18 @@ void drm_dp_aux_unregister(struct drm_dp_aux *aux)
i2c_del_adapter(aux-ddc);
}
EXPORT_SYMBOL(drm_dp_aux_unregister);
+
+bool drm_dp_is_edp_v1_4(struct drm_dp_aux *aux, const u8
dpcd[DP_RECEIVER_CAP_SIZE])
+{
+ uint8_t reg;
+
+ if (dpcd[DP_EDP_CONFIGURATION_CAP]
+DP_DPCD_DISPLAY_CONTROL_CAPABLE) {
+
+ if (drm_dp_dpcd_read(aux, DP_EDP_REV, reg, 1))
+ if (reg == 0x03)
+ return true;
+ }
+ return false;
+}
+EXPORT_SYMBOL(drm_dp_is_edp_v1_4);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 8edeed0..b017e1e 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -102,6 +102,8 @@
#define DP_EDP_CONFIGURATION_CAP0x00d /* XXX 1.2? */
#define DP_TRAINING_AUX_RD_INTERVAL 0x00e /* XXX 1.2? */
+#define DP_DPCD_DISPLAY_CONTROL_CAPABLE (1 3)
+#define DP_EDP_REV 0x700
/* Multiple stream transport */
#define DP_FAUX_CAP0x020 /* 1.2 */
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/chv: Use 16 and 32 for low and high drain latency precision.
On Fri, Oct 17, 2014 at 08:05:08AM -0700, Rodrigo Vivi wrote:
Current chv spec teels we can only use either 16 or 32 bits as precision.
Although in the past VLV went from 16/32 to 32/64 and spec might not be
updated,
these precision values brings stability and fixes some issues Wayne was
facing.
Lies, damned lies, specs!
Well in this case I guess the spec might be correct since it helps
Wayne.
Cc: Wayne Boyer wayne.bo...@intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 13 +++--
drivers/gpu/drm/i915/intel_pm.c | 40 +---
2 files changed, 32 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6db369a..dcd5650 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4054,17 +4054,18 @@ enum punit_power_well {
#define DSPFW_PLANEA_WM1_HI_MASK (10)
/* drain latency register values*/
+#define DRAIN_LATENCY_PRECISION_16 16
#define DRAIN_LATENCY_PRECISION_32 32
#define DRAIN_LATENCY_PRECISION_64 64
While you're poking at this stuff, could I trouble you to remove these
defines and just use the values directly? Could be a separate patch if
you prefer.
#define VLV_DDL(pipe)(VLV_DISPLAY_BASE + 0x70050 + 4
* (pipe))
-#define DDL_CURSOR_PRECISION_64 (131)
-#define DDL_CURSOR_PRECISION_32 (031)
+#define DDL_CURSOR_PRECISION_HIGH(131)
+#define DDL_CURSOR_PRECISION_LOW (031)
#define DDL_CURSOR_SHIFT 24
-#define DDL_SPRITE_PRECISION_64(sprite) (1(15+8*(sprite)))
-#define DDL_SPRITE_PRECISION_32(sprite) (0(15+8*(sprite)))
+#define DDL_SPRITE_PRECISION_HIGH(sprite)(1(15+8*(sprite)))
+#define DDL_SPRITE_PRECISION_LOW(sprite) (0(15+8*(sprite)))
#define DDL_SPRITE_SHIFT(sprite) (8+8*(sprite))
-#define DDL_PLANE_PRECISION_64 (17)
-#define DDL_PLANE_PRECISION_32 (07)
+#define DDL_PLANE_PRECISION_HIGH (17)
+#define DDL_PLANE_PRECISION_LOW (07)
#define DDL_PLANE_SHIFT 0
#define DRAIN_LATENCY_MASK 0x7f
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index daa99e7..50c3512 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1345,6 +1345,7 @@ static bool vlv_compute_drain_latency(struct drm_crtc
*crtc,
int *prec_mult,
int *drain_latency)
{
+ struct drm_device *dev = crtc-dev;
int entries;
int clock = to_intel_crtc(crtc)-config.adjusted_mode.crtc_clock;
@@ -1355,8 +1356,12 @@ static bool vlv_compute_drain_latency(struct drm_crtc
*crtc,
return false;
entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
- *prec_mult = (entries 128) ? DRAIN_LATENCY_PRECISION_64 :
-DRAIN_LATENCY_PRECISION_32;
+ if (IS_CHERRYVIEW(dev))
+ *prec_mult = (entries 128) ? DRAIN_LATENCY_PRECISION_32 :
+DRAIN_LATENCY_PRECISION_16;
+ else
+ *prec_mult = (entries 128) ? DRAIN_LATENCY_PRECISION_64 :
+DRAIN_LATENCY_PRECISION_32;
*drain_latency = (64 * (*prec_mult) * 4) / entries;
if (*drain_latency DRAIN_LATENCY_MASK)
@@ -1375,15 +1380,18 @@ static bool vlv_compute_drain_latency(struct drm_crtc
*crtc,
static void vlv_update_drain_latency(struct drm_crtc *crtc)
{
- struct drm_i915_private *dev_priv = crtc-dev-dev_private;
+ struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pixel_size;
int drain_latency;
enum pipe pipe = intel_crtc-pipe;
int plane_prec, prec_mult, plane_dl;
+ int high_precision = IS_CHERRYVIEW(dev) ? DRAIN_LATENCY_PRECISION_32 :
+ DRAIN_LATENCY_PRECISION_64;
Maybe const just help the reader remember that it's a constant.
- plane_dl = I915_READ(VLV_DDL(pipe)) ~(DDL_PLANE_PRECISION_64 |
-DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_64 |
+ plane_dl = I915_READ(VLV_DDL(pipe)) ~(DDL_PLANE_PRECISION_HIGH |
+DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_HIGH |
(DRAIN_LATENCY_MASK DDL_CURSOR_SHIFT));
if (!intel_crtc_active(crtc)) {
@@ -1394,9 +1402,9 @@ static void vlv_update_drain_latency(struct drm_crtc
*crtc)
/* Primary plane Drain Latency */
pixel_size = crtc-primary-fb-bits_per_pixel / 8; /* BPP */
if (vlv_compute_drain_latency(crtc, pixel_size, prec_mult,
drain_latency)) {
-
Re: [Intel-gfx] [PATCH v2 0/8] Add enlightenments for vGPU
Hi Daniel, Thanks a lot for your reply. Indeed, I sent two v2 patches, because the format of the first v2 patchset is incorrect - I forgot to add the what changed part in those messages. :) Yu On 10/22/2014 12:16 AM, Daniel Vetter wrote: On Fri, Oct 17, 2014 at 01:37:11PM +0800, Yu Zhang wrote: Intel GVT-g (previously known as XenGT), is a complete GPU virtualization solution with mediated pass-through for 4th generation Intel Core processors - Haswell platform. This technology presents a virtual full-fledged GPU to each Virtual Machine (VM). VMs can directly access performance-critical resources, without intervention from the hypervisor in most cases, while privileged operations from VMs are trap-and-emulated at minimal cost. For detail, please refer to https://01.org/xen/blogs/wangbo85/2014/intel-gvt-gxengt-pubic-release This patch set includes necessary code changes when i915 driver runs inside a VM. Though ideally we can run an unmodified i915 driver in VM, adding such enlightenments can greatly reduce the virtualization complexity in orders of magnitude. Code changes for the host side, which includes the actual Intel GVT-g implementation, were sent out in another patchset. The primary change introduced here is to implement so-called address space ballooning technique. XenGT partitions global graphics memory among multiple VMs, so each VM can directly access a portion of the memory w/o hypervisor's intervention, e.g. filling textures and queuing commands. However w/ the partitioning an unmodified i915 driver would assume a smaller graphics memory starting from address ZERO, so requires XenGT core module (vgt) to translate the graphics address between 'guest view' and 'host view', for all registers and command opcodes which contain a graphics memory address. To reduce the complexity, XenGT introduces address space ballooning, by telling the exact partitioning knowledge to each guest i915 driver, which then reserves and prevents non-allocated portions from allocation. Then vgt module only needs to scan and validate graphics addresses w/o complexity of translation. Note: The partitioning of global graphics memory may break some applications, with large objects in the aperture, because current userspace assumes half of the aperture usable. That would need separate fix either in user space (e.g. remove assumption in mesa) or in kernel (w/ some faulting mechanism). The partitioning knowledge is conveyed through a reserved MMIO range, called PVINFO, which will be architecturally reserved in future hardware generations. Another information carried through PVINFO is about the number of fence registers. As a global resource XenGT also partitions them among VMs. Other changes are trivial as optimizations, to either reduce the trap overhead or disable power management features which don't make sense in a virtualized environment. Yu Zhang (8): drm/i915: Introduce a PV INFO page structure for Intel GVT-g. drm/i915: Adds graphic address space ballooning logic drm/i915: Partition the fence registers for vgpu in i915 driver drm/i915: Disable framebuffer compression for i915 driver in VM drm/i915: Add the display switch logic for vgpu in i915 driver drm/i915: Disable power management for i915 driver in VM drm/i915: Create vgpu specific write MMIO to reduce traps drm/i915: Support alias ppgtt in VM if ppgtt is enabled drivers/gpu/drm/i915/i915_dma.c | 11 +++ drivers/gpu/drm/i915/i915_drv.h | 12 +++ drivers/gpu/drm/i915/i915_gem.c | 5 + drivers/gpu/drm/i915/i915_gem_gtt.c | 186 +++- drivers/gpu/drm/i915/i915_vgt_if.h | 93 ++ drivers/gpu/drm/i915/intel_pm.c | 8 ++ drivers/gpu/drm/i915/intel_uncore.c | 22 + 7 files changed, 334 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_vgt_if.h I seem to have two v2 versions of this patch series. Anything changed or why the resend? I didn't see any comment on the older version ... -Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PULL] drm-intel-next
On Wed, Oct 22, 2014 at 09:09:43AM +1000, Dave Airlie wrote: On 21 October 2014 23:38, Daniel Vetter daniel.vet...@ffwll.ch wrote: Hi Dave, drm-intel-next-2014-10-03: - first batch of skl stage 1 enabling - fixes from Rodrigo to the PSR, fbc and sink crc code - kerneldoc for the frontbuffer tracking code, runtime pm code and the basic interrupt enable/disable functions - smaller stuff all over drm-intel-next-2014-09-19: - bunch more i830M fixes from Ville - full ppgtt now again enabled by default - more ppgtt fixes from Michel Thierry and Chris Wilson - plane config work from Gustavo Padovan - spinlock clarifications - piles of smaller improvements all over, as usual Chris made some noises about PPGTT being broken somewhere on irc last week, Chris, did you figure that out? Nope. All full-ppgtt platforms (ivb/byt/hsw) suffer from spontaneously loosing track of the right page directories and end up executing garbage. It correlates with load, so frequently makes igt (and a few tests in particular) die, along with piglit, webgl conformance, benchmarking and even eventually light loads on composited desktops. I've made the pd uncached, added copious flushing, forced switch_mm every time, added noops, cacheline alignment, srm, forced the execution of batches to be synchronous, and yet IPEHR != *ACTHD. The register and command state looks valid. The gpu resets ok and runs fine until the error strikes again. [So I need to test whether switch_mm(aliasing_ppgtt) on every batch fails as well, and whether i915.enable_rc6=0 masks it. There is the worrying bits in bspec that talk of non-RCS as being part of the render context state, but only the RCS pd registers are shown in the context diagrams. I guess I should inspect the context state and see if I can spot the other registers. If context restore (and with rc6 that could happen at any time) switched the pd on the other rings, that would be a nice snafu.] I would suggest that full-ppgtt be disabled unless someone else has had better luck finding a hsd or figuring out the missing magic. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/8] Add enlightenments for vGPU
On 10/22/2014 12:51 AM, Daniel Vetter wrote: On Tue, Oct 21, 2014 at 06:16:26PM +0200, Daniel Vetter wrote: On Fri, Oct 17, 2014 at 01:37:11PM +0800, Yu Zhang wrote: Intel GVT-g (previously known as XenGT), is a complete GPU virtualization solution with mediated pass-through for 4th generation Intel Core processors - Haswell platform. This technology presents a virtual full-fledged GPU to each Virtual Machine (VM). VMs can directly access performance-critical resources, without intervention from the hypervisor in most cases, while privileged operations from VMs are trap-and-emulated at minimal cost. For detail, please refer to https://01.org/xen/blogs/wangbo85/2014/intel-gvt-gxengt-pubic-release This patch set includes necessary code changes when i915 driver runs inside a VM. Though ideally we can run an unmodified i915 driver in VM, adding such enlightenments can greatly reduce the virtualization complexity in orders of magnitude. Code changes for the host side, which includes the actual Intel GVT-g implementation, were sent out in another patchset. The primary change introduced here is to implement so-called address space ballooning technique. XenGT partitions global graphics memory among multiple VMs, so each VM can directly access a portion of the memory w/o hypervisor's intervention, e.g. filling textures and queuing commands. However w/ the partitioning an unmodified i915 driver would assume a smaller graphics memory starting from address ZERO, so requires XenGT core module (vgt) to translate the graphics address between 'guest view' and 'host view', for all registers and command opcodes which contain a graphics memory address. To reduce the complexity, XenGT introduces address space ballooning, by telling the exact partitioning knowledge to each guest i915 driver, which then reserves and prevents non-allocated portions from allocation. Then vgt module only needs to scan and validate graphics addresses w/o complexity of translation. Note: The partitioning of global graphics memory may break some applications, with large objects in the aperture, because current userspace assumes half of the aperture usable. That would need separate fix either in user space (e.g. remove assumption in mesa) or in kernel (w/ some faulting mechanism). The partitioning knowledge is conveyed through a reserved MMIO range, called PVINFO, which will be architecturally reserved in future hardware generations. Another information carried through PVINFO is about the number of fence registers. As a global resource XenGT also partitions them among VMs. Other changes are trivial as optimizations, to either reduce the trap overhead or disable power management features which don't make sense in a virtualized environment. Yu Zhang (8): drm/i915: Introduce a PV INFO page structure for Intel GVT-g. drm/i915: Adds graphic address space ballooning logic drm/i915: Partition the fence registers for vgpu in i915 driver drm/i915: Disable framebuffer compression for i915 driver in VM drm/i915: Add the display switch logic for vgpu in i915 driver drm/i915: Disable power management for i915 driver in VM drm/i915: Create vgpu specific write MMIO to reduce traps drm/i915: Support alias ppgtt in VM if ppgtt is enabled drivers/gpu/drm/i915/i915_dma.c | 11 +++ drivers/gpu/drm/i915/i915_drv.h | 12 +++ drivers/gpu/drm/i915/i915_gem.c | 5 + drivers/gpu/drm/i915/i915_gem_gtt.c | 186 +++- drivers/gpu/drm/i915/i915_vgt_if.h | 93 ++ drivers/gpu/drm/i915/intel_pm.c | 8 ++ drivers/gpu/drm/i915/intel_uncore.c | 22 + 7 files changed, 334 insertions(+), 3 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_vgt_if.h I seem to have two v2 versions of this patch series. Anything changed or why the resend? I didn't see any comment on the older version ... Well, looked through it anyway. On a high level this looks good for the vgt integration for guests. I think we need some polish though still, specifically for documentation. - Please extract all the various intel_vgt_* functions spread all over the tree into a new i915_vgt_if.c (or intel_vgt.c, but then the header should be changed, too I think). - Please add kerneldoc to all the functions which are non-static and so used by the driver outside of your kernel module. - Please add a DOC: section detailing some of the high-level design considerations of vGT and also put that into the new file. I think in the future we should also keep the guest-host abi revisions in there (i.e. the stuff in PV_INFO). Sure. Thanks! - Please pull all the new documentation together and integrate it into the i915 section of the drm docbook. A good place is probably a new subsection Paravirtualized Guest Support (vGPU) under the driver core section. How about subsection name Intel GVT-g Guest Support(vGPU)? :) There's quite a few i915 driver subsystems which are
Re: [Intel-gfx] [RFC PATCH v2 1/4] drm/i915: add i915_ved.c to setup bridge for VED
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Tuesday, October 21, 2014 6:30 PM
To: Cheng, Yao
Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; Kelley,
Sean V; Vetter, Daniel; Abel, Michael J; Jiang, Fei; Rao, Ram R
Subject: Re: [Intel-gfx] [RFC PATCH v2 1/4] drm/i915: add i915_ved.c to setup
bridge for VED
On Tue, Oct 21, 2014 at 02:36:41PM +0800, Yao Cheng wrote:
Setup minimum required resources during i915_driver_load:
1. Create a platform device to share MMIO/IRQ resources 2. Make the
platform device child of i915 device for runtime PM.
3. Create IRQ chip to forward the VED irqs.
VED driver (a standalone drm driver) probes the VED device and creates
a new dri card on install.
Currently only supports VED on valleyview.
Kerneldoc is updated for i915_ved.c.
Signed-off-by: Yao Cheng yao.ch...@intel.com
---
Documentation/DocBook/drm.tmpl | 5 +
drivers/gpu/drm/i915/Makefile | 3 +
drivers/gpu/drm/i915/i915_dma.c | 11 ++
drivers/gpu/drm/i915/i915_drv.h | 9 ++
drivers/gpu/drm/i915/i915_irq.c | 2 +
drivers/gpu/drm/i915/i915_reg.h | 5 +
drivers/gpu/drm/i915/i915_ved.c | 264
7 files changed, 299 insertions(+)
create mode 100644 drivers/gpu/drm/i915/i915_ved.c
diff --git a/Documentation/DocBook/drm.tmpl
b/Documentation/DocBook/drm.tmpl index d7cfc98..f1787b4 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -3806,6 +3806,11 @@ int num_ioctls;/synopsis
!Fdrivers/gpu/drm/i915/i915_irq.c intel_runtime_pm_disable_interrupts
!Fdrivers/gpu/drm/i915/i915_irq.c intel_runtime_pm_enable_interrupts
/sect2
+ sect2
+titleVED video core integration/title
+!Pdrivers/gpu/drm/i915/i915_ved.c VED video core integration
+!Idrivers/gpu/drm/i915/i915_ved.c
+ /sect2
/sect1
sect1
titleDisplay Hardware Handling/title diff --git
a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index
3a6bce0..a4b9252 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -80,6 +80,9 @@ i915-y += dvo_ch7017.o \ i915-y += i915_dma.o \
i915_ums.o
+# VED for VLV
+i915-y += i915_ved.o
+
obj-$(CONFIG_DRM_I915) += i915.o
CFLAGS_i915_trace_points.o := -I$(src) diff --git
a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 85d14e1..47714e1 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1791,6 +1791,13 @@ int i915_driver_load(struct drm_device *dev,
unsigned long flags)
if (IS_GEN5(dev))
intel_gpu_ips_init(dev_priv);
+ if (IS_VALLEYVIEW(dev)) {
These must be (IS_VLV !IS_CHV), or maybe define some HAS_VED()
macro to hide that.
Accepted. Will add HAS_VED() to hide this.
+ ret = vlv_setup_ved(dev);
+ if (ret 0) {
+ DRM_ERROR(failed to setup VED bridge: %d\n, ret);
+ }
+ }
+
intel_runtime_pm_enable(dev_priv);
return 0;
@@ -1833,6 +1840,10 @@ int i915_driver_unload(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev-dev_private;
int ret;
+ if (IS_VALLEYVIEW(dev)) {
+ vlv_teardown_ved(dev);
+ }
+
ret = i915_gem_suspend(dev);
if (ret) {
DRM_ERROR(failed to idle hardware: %d\n, ret); diff --git
a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 821ba26..952df34 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1709,6 +1709,10 @@ struct drm_i915_private {
uint32_t bios_vgacntr;
+ /* used for setup VED bridge */
+ struct platform_device *ved_platdev;
+ int ved_irq;
+
Could be neater to wrap this in a struct:
struct {
struct platform_device *platdev;
int irq;
} ved;
Ok, thanks for the suggestion.
/* Old dri1 support infrastructure, beware the dragons ya fools
entering
* here! */
struct i915_dri1_state dri1;
@@ -2785,6 +2789,11 @@ void i915_restore_display_reg(struct
drm_device
*dev); void i915_setup_sysfs(struct drm_device *dev_priv); void
i915_teardown_sysfs(struct drm_device *dev_priv);
+/* i915_ved.c */
+int vlv_setup_ved(struct drm_device *dev); void
+vlv_teardown_ved(struct drm_device *dev); void
+vlv_ved_irq_handler(struct drm_device *dev);
+
/* intel_i2c.c */
extern int intel_setup_gmbus(struct drm_device *dev); extern void
intel_teardown_gmbus(struct drm_device *dev); diff --git
a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 737b239..68c2977 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2177,6 +2177,8 @@ static irqreturn_t valleyview_irq_handler(int irq,
void *arg)
Re: [Intel-gfx] [RFC PATCH v2 1/4] drm/i915: add i915_ved.c to setup bridge for VED
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
Vetter
Sent: Tuesday, October 21, 2014 8:09 PM
To: Cheng, Yao
Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; Kelley,
Sean V; Vetter, Daniel; Abel, Michael J; Jiang, Fei; Rao, Ram R
Subject: Re: [Intel-gfx] [RFC PATCH v2 1/4] drm/i915: add i915_ved.c to setup
bridge for VED
On Tue, Oct 21, 2014 at 02:36:41PM +0800, Yao Cheng wrote:
Setup minimum required resources during i915_driver_load:
1. Create a platform device to share MMIO/IRQ resources 2. Make the
platform device child of i915 device for runtime PM.
3. Create IRQ chip to forward the VED irqs.
VED driver (a standalone drm driver) probes the VED device and creates
a new dri card on install.
Currently only supports VED on valleyview.
Kerneldoc is updated for i915_ved.c.
Signed-off-by: Yao Cheng yao.ch...@intel.com
Please resend with a patch changelog to account for my review comments.
And Ville's. Plus cc us both. And if there's anything you didn't address, you
must reply to the review and we need to further discuss this.
Daniel, I see, thanks for the instruction.
Do you mean resending the [RFC PATCH v2] with changelog and cc list?
Or adding changelog/cc when sending [RFC PATCH v3]?
Thanks, Daniel
---
Documentation/DocBook/drm.tmpl | 5 +
drivers/gpu/drm/i915/Makefile | 3 +
drivers/gpu/drm/i915/i915_dma.c | 11 ++
drivers/gpu/drm/i915/i915_drv.h | 9 ++
drivers/gpu/drm/i915/i915_irq.c | 2 +
drivers/gpu/drm/i915/i915_reg.h | 5 +
drivers/gpu/drm/i915/i915_ved.c | 264
7 files changed, 299 insertions(+)
create mode 100644 drivers/gpu/drm/i915/i915_ved.c
diff --git a/Documentation/DocBook/drm.tmpl
b/Documentation/DocBook/drm.tmpl index d7cfc98..f1787b4 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -3806,6 +3806,11 @@ int num_ioctls;/synopsis
!Fdrivers/gpu/drm/i915/i915_irq.c intel_runtime_pm_disable_interrupts
!Fdrivers/gpu/drm/i915/i915_irq.c intel_runtime_pm_enable_interrupts
/sect2
+ sect2
+titleVED video core integration/title
+!Pdrivers/gpu/drm/i915/i915_ved.c VED video core integration
+!Idrivers/gpu/drm/i915/i915_ved.c
+ /sect2
/sect1
sect1
titleDisplay Hardware Handling/title diff --git
a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index
3a6bce0..a4b9252 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -80,6 +80,9 @@ i915-y += dvo_ch7017.o \ i915-y += i915_dma.o \
i915_ums.o
+# VED for VLV
+i915-y += i915_ved.o
+
obj-$(CONFIG_DRM_I915) += i915.o
CFLAGS_i915_trace_points.o := -I$(src) diff --git
a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 85d14e1..47714e1 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1791,6 +1791,13 @@ int i915_driver_load(struct drm_device *dev,
unsigned long flags)
if (IS_GEN5(dev))
intel_gpu_ips_init(dev_priv);
+ if (IS_VALLEYVIEW(dev)) {
+ ret = vlv_setup_ved(dev);
+ if (ret 0) {
+ DRM_ERROR(failed to setup VED bridge: %d\n, ret);
+ }
+ }
+
intel_runtime_pm_enable(dev_priv);
return 0;
@@ -1833,6 +1840,10 @@ int i915_driver_unload(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev-dev_private;
int ret;
+ if (IS_VALLEYVIEW(dev)) {
+ vlv_teardown_ved(dev);
+ }
+
ret = i915_gem_suspend(dev);
if (ret) {
DRM_ERROR(failed to idle hardware: %d\n, ret); diff --git
a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 821ba26..952df34 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1709,6 +1709,10 @@ struct drm_i915_private {
uint32_t bios_vgacntr;
+ /* used for setup VED bridge */
+ struct platform_device *ved_platdev;
+ int ved_irq;
+
/* Old dri1 support infrastructure, beware the dragons ya fools
entering
* here! */
struct i915_dri1_state dri1;
@@ -2785,6 +2789,11 @@ void i915_restore_display_reg(struct
drm_device
*dev); void i915_setup_sysfs(struct drm_device *dev_priv); void
i915_teardown_sysfs(struct drm_device *dev_priv);
+/* i915_ved.c */
+int vlv_setup_ved(struct drm_device *dev); void
+vlv_teardown_ved(struct drm_device *dev); void
+vlv_ved_irq_handler(struct drm_device *dev);
+
/* intel_i2c.c */
extern int intel_setup_gmbus(struct drm_device *dev); extern void
intel_teardown_gmbus(struct drm_device *dev); diff --git
a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 737b239..68c2977 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Change order of operations for VLV/CHV to not train DP link before PHYs are ready
On Fri, Oct 17, 2014 at 11:41:13AM -0700, Todd Previte wrote:
Reorder the function calls in chv/vlv_pre_enable_dp() such that link training
is not initiated before the PHYs come up out of reset. Also check the status
of vlv_wait_port_ready() and only attempt to train if the PHYs are actually
running.
The specification lists the wait for the PHYs as one of the final steps in
enabling the Displayport hardware for use. While the PHYs are in reset, no
communication is possible across the link. Attempting to train the link while
the PHYs are in reset will result in link training failure with one or more
WARN() in the logs. Moving the intel_enable_dp() function after
vlv_wait_port_ready() and only when the PHYs are ready helps ensure reliable
operation of the Displayport link.
To comply with the specification, the call to enable_port() has been moved of
enable_dp() and placed before the wait functions for the PHYs and prior to
the call to enable_dp().
This is going to conflict with my PPS series. I have a similar patch in
there, except it doesn't skip the link training. I'm not sure we should
bother doing that since the wait_port_ready() problem should never ever
happen as long as we do things correctly, which we should do after my
series lands.
Signed-off-by: Todd Previte tprev...@gmail.com
---
drivers/gpu/drm/i915/intel_display.c | 8 ++--
drivers/gpu/drm/i915/intel_dp.c | 16
drivers/gpu/drm/i915/intel_drv.h | 2 +-
3 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index c51d950..4b280c1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1723,7 +1723,7 @@ static void chv_disable_pll(struct drm_i915_private
*dev_priv, enum pipe pipe)
mutex_unlock(dev_priv-dpio_lock);
}
-void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
+int vlv_wait_port_ready(struct drm_i915_private *dev_priv,
struct intel_digital_port *dport)
{
u32 port_mask;
@@ -1746,9 +1746,13 @@ void vlv_wait_port_ready(struct drm_i915_private
*dev_priv,
BUG();
}
- if (wait_for((I915_READ(dpll_reg) port_mask) == 0, 1000))
+ if (wait_for((I915_READ(dpll_reg) port_mask) == 0, 1000)) {
WARN(1, timed out waiting for port %c ready: 0x%08x\n,
port_name(dport-port), I915_READ(dpll_reg));
+ return -EIO;
+ }
+
+ return 0;
}
static void intel_prepare_shared_dpll(struct intel_crtc *crtc)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a8352c4..c1ce738 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2532,7 +2532,7 @@ static void intel_dp_enable_port(struct intel_dp
*intel_dp)
POSTING_READ(intel_dp-output_reg);
}
-static void intel_enable_dp(struct intel_encoder *encoder)
+static bool intel_enable_dp(struct intel_encoder *encoder)
{
struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base);
struct drm_device *dev = encoder-base.dev;
@@ -2544,7 +2544,6 @@ static void intel_enable_dp(struct intel_encoder
*encoder)
if (WARN_ON(dp_reg DP_PORT_EN))
return false;
- intel_dp_enable_port(intel_dp);
intel_edp_panel_vdd_on(intel_dp);
intel_edp_panel_on(intel_dp);
intel_edp_panel_vdd_off(intel_dp, true);
@@ -2576,6 +2575,7 @@ static void g4x_enable_dp(struct intel_encoder *encoder)
{
struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base);
+ intel_dp_enable_port(intel_dp);
intel_enable_dp(encoder);
intel_edp_backlight_on(intel_dp);
}
@@ -2705,9 +2705,9 @@ static void vlv_pre_enable_dp(struct intel_encoder
*encoder)
pps_unlock(intel_dp);
}
- intel_enable_dp(encoder);
-
- vlv_wait_port_ready(dev_priv, dport);
+ intel_dp_enable_port(intel_dp);
+ if (vlv_wait_port_ready(dev_priv, dport) == 0)
+ intel_enable_dp(encoder);
}
static void vlv_dp_pre_pll_enable(struct intel_encoder *encoder)
@@ -2805,9 +2805,9 @@ static void chv_pre_enable_dp(struct intel_encoder
*encoder)
pps_unlock(intel_dp);
}
- intel_enable_dp(encoder);
-
- vlv_wait_port_ready(dev_priv, dport);
+ intel_dp_enable_port(intel_dp);
+ if (vlv_wait_port_ready(dev_priv, dport) == 0)
+ intel_enable_dp(encoder);
}
static void chv_dp_pre_pll_enable(struct intel_encoder *encoder)
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index dc80444..2ff2c8c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -876,7 +876,7 @@ intel_wait_for_vblank(struct drm_device *dev, int pipe)
drm_wait_one_vblank(dev, pipe);
}
int ironlake_get_lanes_required(int
Re: [Intel-gfx] [PATCH] drm/i915: add missing forcewake put on i915_wa_registers()
On Tue, Oct 21, 2014 at 07:40:35PM +0200, Daniel Vetter wrote: On Tue, Oct 21, 2014 at 02:58:08PM -0200, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Otherwise, a simple cat to the debugfs file can make the machine use much more power than needed, and prevent it from runtime suspending. Related commit: commit 8452e1d173a16d9812422a2272c4ab0f0ba81057 Author: Mika Kuoppala mika.kuopp...@linux.intel.com Date: Tue Oct 7 17:21:26 2014 +0300 drm/i915: Build workaround list in ring initialization Cc: Mika Kuoppala mika.kuopp...@linux.intel.com Cc: Arun Siluvery arun.siluv...@linux.intel.com Testcase: igt/pm_rpm/debugfs-read Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com tbh I'm not even sure we want to do the manual forcewake get here - I915_READ will do it for us, and this is a debug interface. So no one should care about perf. Mika, is that right? If so I'd like to merge the inverse patch which drops the fw_get. Don't we still need the idle msg disable+poll CSPWRFSM trick here on gen8? That also needs forcewake around it. -Daniel --- drivers/gpu/drm/i915/i915_debugfs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 9600285..36a4baa 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2671,6 +2671,7 @@ static int i915_wa_registers(struct seq_file *m, void *unused) addr, value, mask, read, ok ? OK : FAIL); } + gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL); intel_runtime_pm_put(dev_priv); mutex_unlock(dev-struct_mutex); -- 2.1.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 -- 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] [RFC PATCH 7/8] drm/i915: vgt irq mediation - via a tasklet based mechanism
On 10/01/2014 12:26 AM, Tian, Kevin wrote: From virtualization p.o.v, the ideal case is to run host i915 irq handler in the interrupt context, which meets all the assumption from original code. Using tasklet or other manner still has some restriction. This is a major open we'd like to hear more from you guys. Is it possible to have i915 driver to request two irq numbers: irq1 for real device and irq2 is purely faked one. vgt handler registers on irq1 and i915 hanlder registers on irq2, and then we can use self IPI to trigger irq2 when injection is required. But I'm not sure whether this is an existing feature in kernel, or need some core enhancement in irq sub-system... Hi Kevin, Daniel, I'm so excited to know that, there is an existing feature in kernel: irq_work. Basicly it allows us to run the host i915 ISR in hardirq context prefectly, what's needed is to select CONFIG_IRQ_WORK in Kconfig :) I'll use that in the v2 patches. Thanks Kevin -- Thanks, Jike ___ 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: Ignore long hpds on eDP ports
On Tue, Oct 21, 2014 at 06:00:05PM +0200, Daniel Vetter wrote:
On Fri, Oct 17, 2014 at 09:08:28AM -0700, Todd Previte wrote:
On 10/17/2014 1:43 AM, Ville Syrjälä wrote:
On Thu, Oct 16, 2014 at 12:38:55PM -0700, Todd Previte wrote:
On 10/16/2014 10:46 AM, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Turning vdd on/off can generate a long hpd pulse on eDP ports. In order
to handle hpd we would need to turn on vdd to perform aux transfers.
This would lead to an endless cycle of
vdd off - long hpd - vdd on - detect - vdd off - ...
So ignore long hpd pulses on eDP ports. eDP panels should be physically
tied to the machine anyway so they should not actually disappear and
thus don't need long hpd handling. Short hpds are still needed for link
re-train and whatnot so we can't just turn off the hpd interrupt
entirely for eDP ports. Perhaps we could turn it off whenever the panel
is disabled, but just ignoring the long hpd seems sufficient.
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_dp.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index f07f02c..4455009 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4611,6 +4611,18 @@ intel_dp_hpd_pulse(struct intel_digital_port
*intel_dig_port, bool long_hpd)
if (intel_dig_port-base.type != INTEL_OUTPUT_EDP)
intel_dig_port-base.type = INTEL_OUTPUT_DISPLAYPORT;
+if (long_hpd intel_dig_port-base.type == INTEL_OUTPUT_EDP) {
+/*
+ * vdd off can generate a long pulse on eDP which
+ * would require vdd on to handle it, and thus we
+ * would end up in an endless cycle of
+ * vdd off - long hpd - vdd on - detect - vdd off
- ...
+ */
+DRM_DEBUG_KMS(ignoring long hpd on eDP port %c\n,
+ port_name(intel_dig_port-port));
+return false;
+}
+
DRM_DEBUG_KMS(got hpd irq on port %c - %s\n,
port_name(intel_dig_port-port),
long_hpd ? long : short);
I'm not sure that ignoring a long pulse is the best way to handle it.
eDP does not appear to differentiate between short and long pulses per
the specification (not to mention that HPD support for eDP is optional
in the first place). It seems to me that it would probably be better to
handle them as a normal (short) HPD pulse and just do the regular link
maintenance stuff. As I said, the spec doesn't differentiate between the
long and short pulses for eDP so it's a safer bet to handle them as a
short pulse than to ignore them entirely.
The spec does talk a lot about IRQ_HPD (which is the short pulse) and
the power sequencing diagrams explicitly show that HPD should be asserted
after the T3 delay and deasserted immediately on VDD off, so those would
be the long pulses. So based on that my patch seems to make sense.
It seems HPD is optional for the source only, in the sink it's madatory.
But that doesn't really matter anyway because if either end doesn't have
it it won't work. The spec does go on to say that we should periodically
poll the sink status if HPD_IRQ isn't available. We don't do that
currently, but it does sound like a sane idea in case the link drops out.
Yeah I saw some of the info on IRQ_HPD but didn't see the long pulse stuff.
In any case, my concern was with missing a valid HPD event. In light of the
above, that doesn't look like it's an issue, so I'm good with this patch.
It looks like HPD support in an eDP sink device is optional as well, at
least according to the table on pg.34 of the eDP 1.4 spec. As you pointed
out though, unless both source and sink devices support HPD, it doesn't
really matter. I saw the bit about polling in there, too. It might be worth
implementing a polling mechanism as a backup, but I don't know how useful it
would be. I don't recall running across a sink device that doesn't support
HPD, but that's not to say they don't exist.
Reviewed-by: Todd Previte tprev...@gmail.com
Aside: We don't handle hpd for port A anyway, so for most panels this
doesn't matter all that much. Or we'd have piles more bug reports I think.
Yeah, but we should. Then we could actually enable PSR main-link off
mode and the fallback to manual retrain would work if/when the automagic
training fails.
Anyway, Cc: sta...@vger.kernel.org and one for Jani.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] [RFC PATCH v2 1/4] drm/i915: add i915_ved.c to setup bridge for VED
On Wed, Oct 22, 2014 at 07:09:11AM +, Cheng, Yao wrote:
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Tuesday, October 21, 2014 6:30 PM
To: Cheng, Yao
Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org;
Kelley,
Sean V; Vetter, Daniel; Abel, Michael J; Jiang, Fei; Rao, Ram R
Subject: Re: [Intel-gfx] [RFC PATCH v2 1/4] drm/i915: add i915_ved.c to
setup
bridge for VED
On Tue, Oct 21, 2014 at 02:36:41PM +0800, Yao Cheng wrote:
snip
/* Call regardless, as some status bits might not be
* signalled in iir */
valleyview_pipestat_irq_handler(dev, iir); diff --git
a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2ed02c3..95421ef 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1284,6 +1284,11 @@ enum punit_power_well { #define
VLV_DISPLAY_BASE 0x18 #define VLV_MIPI_BASE
VLV_DISPLAY_BASE
+/* forwarding VED irq and sharing MMIO to the VED driver */
+#define VLV_VED_BLOCK_INTERRUPT (1 23)
This define should be alongside the other IxR bits.
Do you mean like this:
Rename it to VLV_VED_IRQ and put alongside VLV_IIR?
No, I think the name is fine as is, but it should be where the other old
IxR bits are defined. So between I915_PM_INTERRUPT and I915_ISP_INTERRUPT
looks like right spot to me.
I'm not sure why those defines are where they are. We should probably
move the lot to sit next to the IxR register defines themselves, but
that's a separate issue.
snip
+ if (!rsc) {
+ DRM_ERROR(Failed to allocate resource for VED platform
device\n);
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ WARN_ON(dev_priv-ved_irq 0);
+ rsc[0].start= rsc[0].end = dev_priv-ved_irq;
+ rsc[0].flags= IORESOURCE_IRQ;
+ rsc[0].name = ipvr-ved-vlv-irq;
+
+ /* MMIO/REG for child's use */
+ rsc[1].start= pci_resource_start(dev-pdev, 0);
+ rsc[1].end = pci_resource_start(dev-pdev, 0) + 2*1024*1024; /*
gen7 */
+ rsc[1].flags= IORESOURCE_MEM;
+ rsc[1].name = ipvr-ved-vlv-mmio;
+
+ rsc[2].start= VLV_VED_BASE;
+ rsc[2].end = VLV_VED_BASE + VLV_VED_SIZE;
+ rsc[2].flags= IORESOURCE_REG;
+ rsc[2].name = ipvr-ved-vlv-reg;
I don't see why you need both MEM and REG resources. Just the MEM itself
should suffice:
rsc[1].start= pci_resource_start(dev-pdev, 0) + VLV_VED_BASE;
rsc[1].end = pci_resource_start(dev-pdev, 0) + VLV_VED_BASE +
VLV_VED_SIZE;
rsc[1].flags= IORESOURCE_MEM;
rsc[1].name = ipvr-ved-vlv-mmio;
When I write the original code on k3.10 I always got ioremap conflict by
binding only one MMIO resource.
I just tested this on k3.17 and no conflict. :) thanks for pointing out this
and I will update the code.
BTW, it's interesting, is there any update on the ioremap code from 3.10 to
3.17?
Not sure. But the UC vs. WC could be one explanation for the conflict.
Also in the ved driver you're mapping it with ioremap_wc() which isn't
generally safe to do with registers. I'm not sure the kernel would even
allow
it since i915 has already mapped it as UC.
Thanks, I'll change it to UC.
--
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] [PULL] drm-intel-next
On 22 October 2014 17:05, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Oct 22, 2014 at 09:09:43AM +1000, Dave Airlie wrote: On 21 October 2014 23:38, Daniel Vetter daniel.vet...@ffwll.ch wrote: Hi Dave, drm-intel-next-2014-10-03: - first batch of skl stage 1 enabling - fixes from Rodrigo to the PSR, fbc and sink crc code - kerneldoc for the frontbuffer tracking code, runtime pm code and the basic interrupt enable/disable functions - smaller stuff all over drm-intel-next-2014-09-19: - bunch more i830M fixes from Ville - full ppgtt now again enabled by default - more ppgtt fixes from Michel Thierry and Chris Wilson - plane config work from Gustavo Padovan - spinlock clarifications - piles of smaller improvements all over, as usual Chris made some noises about PPGTT being broken somewhere on irc last week, Chris, did you figure that out? Nope. All full-ppgtt platforms (ivb/byt/hsw) suffer from spontaneously loosing track of the right page directories and end up executing garbage. It correlates with load, so frequently makes igt (and a few tests in particular) die, along with piglit, webgl conformance, benchmarking and even eventually light loads on composited desktops. I've made the pd uncached, added copious flushing, forced switch_mm every time, added noops, cacheline alignment, srm, forced the execution of batches to be synchronous, and yet IPEHR != *ACTHD. The register and command state looks valid. The gpu resets ok and runs fine until the error strikes again. [So I need to test whether switch_mm(aliasing_ppgtt) on every batch fails as well, and whether i915.enable_rc6=0 masks it. There is the worrying bits in bspec that talk of non-RCS as being part of the render context state, but only the RCS pd registers are shown in the context diagrams. I guess I should inspect the context state and see if I can spot the other registers. If context restore (and with rc6 that could happen at any time) switched the pd on the other rings, that would be a nice snafu.] I would suggest that full-ppgtt be disabled unless someone else has had better luck finding a hsd or figuring out the missing magic. -Chris Thanks Chris for the report, Daniel, fill in the swear words where you like, but yeah don't think I want to pull this in this state. Either pull the enable ppgtt patch or revert it on top, Thanks, Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/mode: document path property and function to set it.
On Wed, Oct 22, 2014 at 12:11:24PM +1000, Dave Airlie wrote:
From: Dave Airlie airl...@redhat.com
These two didn't get documented properly, do so.
Pointed out by Daniel.
Signed-off-by: Dave Airlie airl...@redhat.com
---
Documentation/DocBook/drm.tmpl | 9 -
drivers/gpu/drm/drm_crtc.c | 10 ++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index bacefc5..0a5cbbb 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2509,7 +2509,7 @@ void intel_crt_init(struct drm_device *dev)
/tr
tr
td rowspan=21 valign=top DRM/td
- td rowspan=2 valign=top Generic/td
+ td rowspan=3 valign=top Generic/td
td valign=top “EDID”/td
td valign=top BLOB | IMMUTABLE/td
td valign=top 0/td
@@ -2524,6 +2524,13 @@ void intel_crt_init(struct drm_device *dev)
td valign=top Contains DPMS operation mode value./td
/tr
tr
+ td valign=top “PATH”/td
+ td valign=top BLOB | IMMUTABLE/td
+ td valign=top 0/td
+ td valign=top Connector/td
+ td valign=top Contains topology path to a connector./td
+ /tr
+ tr
td rowspan=1 valign=top Plane/td
td valign=top “type”/td
td valign=top ENUM | IMMUTABLE/td
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 90e7730..363301c 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3980,6 +3980,16 @@ done:
return ret;
}
+/**
+ * drm_mode_connector_set_path_property - set tile property on connector
+ * @connector: connector to set property on.
+ * @path: path to use for property.
+ *
+ * This creates a property to expose to userspace to specify a
+ * connector path. This is mainly used for DisplayPort MST where
+ * connectors have a topology and we want to allow userspace to give
+ * them more meaningful names.
+ */
The
* Returns:
* Zero on success, error code on failure.
boilerplate is missing, with this is
Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
int drm_mode_connector_set_path_property(struct drm_connector *connector,
char *path)
{
--
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 2/6] drm: add tile_group support. (v2)
On Wed, Oct 22, 2014 at 12:32:03PM +1000, Dave Airlie wrote:
From: Dave Airlie airl...@redhat.com
A tile group is an identifier shared by a single monitor,
DisplayID topology has 8 bytes we can use for this, just
use those for now until something else comes up in the
future. We assign these to an idr and use the idr to
tell userspace what connectors are in the same tile group.
DisplayID v1.3 says the serial number must be unique for
displays from the same manufacturer.
v2:
destroy idr (dvdhrm)
add docbook (danvet)
airlied:- not sure how to make docbook add fns to tile group section.
Either you have to extract them into a new file or you have to list them
all explicitly. The kerneldoc nano howto has the various options you can
use. Thus far we haven't documented drm-internal functions though, only
those exported to drivers or helpers as guidelines to driver writers. Not
stopping you ofc ;-) But imo just documenting the tile prop registration
function is good enough.
wrt the patch I'm not 100% sure the kref_get_unless_zero is perfectly
race-free, but that depends upon how we solve the hotplugging of
properties and stuff I think.
Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Dave Airlie airl...@redhat.com
---
Documentation/DocBook/drm.tmpl | 4 ++
drivers/gpu/drm/drm_crtc.c | 99
++
include/drm/drm_crtc.h | 16 +++
3 files changed, 119 insertions(+)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 0a5cbbb..5ea6289 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2374,6 +2374,10 @@ void intel_crt_init(struct drm_device *dev)
title id=drm-kms-planehelpersPlane Helper Reference/title
!Edrivers/gpu/drm/drm_plane_helper.c Plane Helpers
/sect2
+sect2
+ titleTile group/title
+!Pdrivers/gpu/drm/drm_crtc.c Tile group
+/sect2
/sect1
!-- Internals: kms properties --
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 363301c..7f45fdc 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5047,6 +5047,7 @@ void drm_mode_config_init(struct drm_device *dev)
INIT_LIST_HEAD(dev-mode_config.property_blob_list);
INIT_LIST_HEAD(dev-mode_config.plane_list);
idr_init(dev-mode_config.crtc_idr);
+ idr_init(dev-mode_config.tile_idr);
drm_modeset_lock_all(dev);
drm_mode_create_standard_connector_properties(dev);
@@ -5134,6 +5135,7 @@ void drm_mode_config_cleanup(struct drm_device *dev)
crtc-funcs-destroy(crtc);
}
+ idr_destroy(dev-mode_config.tile_idr);
idr_destroy(dev-mode_config.crtc_idr);
drm_modeset_lock_fini(dev-mode_config.connection_mutex);
}
@@ -5156,3 +5158,100 @@ struct drm_property
*drm_mode_create_rotation_property(struct drm_device *dev,
supported_rotations);
}
EXPORT_SYMBOL(drm_mode_create_rotation_property);
+
+/**
+ * DOC: Tile group
+ *
+ * Tile groups are used to represent tiled monitors with a unique
+ * integer identifier. Tiled monitors using DisplayID v1.3 have
+ * a unique 8-byte handle, we store this in a tile group, so we
+ * have a common identifier for all tiles in a monitor group.
+ */
+static void drm_tile_group_free(struct kref *kref)
+{
+ struct drm_tile_group *tg = container_of(kref, struct drm_tile_group,
refcount);
+ struct drm_device *dev = tg-dev;
+ mutex_lock(dev-mode_config.idr_mutex);
+ idr_remove(dev-mode_config.tile_idr, tg-id);
+ mutex_lock(dev-mode_config.idr_mutex);
+ kfree(tg);
+}
+
+/**
+ * drm_mode_put_tile_group - drop a reference to a tile group.
+ * @dev: DRM device
+ * @tg: tile group to drop reference to.
+ *
+ * drop reference to tile group and free if 0.
+ */
+void drm_mode_put_tile_group(struct drm_device *dev,
+ struct drm_tile_group *tg)
+{
+ kref_put(tg-refcount, drm_tile_group_free);
+}
+
+/**
+ * drm_mode_get_tile_group - get a reference to an existing tile group
+ * @dev: DRM device
+ * @topology: 8-bytes unique per monitor.
+ *
+ * Use the unique bytes to get a reference to an existing tile group.
+ *
+ * RETURNS:
+ * tile group or NULL if not found.
+ */
+struct drm_tile_group *drm_mode_get_tile_group(struct drm_device *dev,
+char topology[8])
+{
+ struct drm_tile_group *tg;
+ int id;
+ mutex_lock(dev-mode_config.idr_mutex);
+ idr_for_each_entry(dev-mode_config.tile_idr, tg, id) {
+ if (!memcmp(tg-group_data, topology, 8)) {
+ if (!kref_get_unless_zero(tg-refcount))
+ tg = NULL;
+ mutex_unlock(dev-mode_config.idr_mutex);
+ return tg;
+ }
+ }
+
Re: [Intel-gfx] [PATCH 1/8] drm/i915: Convert shared dpll reference count to a crtc mask
On Tue, Oct 21, 2014 at 04:02:02PM +0300, Ander Conselvan de Oliveira wrote:
This will be used in a follow up patch to properly release shared DPLLs
without relying on the shared_dpll field in pipe_config.
Signed-off-by: Ander Conselvan de Oliveira
ander.conselvan.de.olive...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 4 +--
drivers/gpu/drm/i915/i915_drv.h | 4 +--
drivers/gpu/drm/i915/intel_display.c | 63
+++-
3 files changed, 44 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index da4036d..71885d8 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2627,8 +2627,8 @@ static int i915_shared_dplls_info(struct seq_file *m,
void *unused)
struct intel_shared_dpll *pll = dev_priv-shared_dplls[i];
seq_printf(m, DPLL%i: %s, id: %i\n, i, pll-name, pll-id);
- seq_printf(m, refcount: %i, active: %i, on: %s\n,
pll-refcount,
-pll-active, yesno(pll-on));
+ seq_printf(m, crtc_mask: 0x%08x, active: %d, on: %s\n,
+pll-crtc_mask, pll-active, yesno(pll-on));
seq_printf(m, tracked hardware state:\n);
seq_printf(m, dpll:0x%08x\n, pll-hw_state.dpll);
seq_printf(m, dpll_md: 0x%08x\n, pll-hw_state.dpll_md);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9f3d689..0db3e1c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -227,8 +227,8 @@ struct intel_dpll_hw_state {
};
struct intel_shared_dpll {
- int refcount; /* count of number of CRTCs sharing this PLL */
- int active; /* count of number of active CRTCs (i.e. DPMS on) */
+ unsigned crtc_mask; /* mask of CRTCs sharing this PLL */
+ int active; /* count of number of active CRTCs (i.e. DPMS on) */
^^
Spurious whitespace change.
bool on; /* is the PLL actually active? Disabled during modeset */
const char *name;
/* should match the index in the dev_priv-shared_dplls array */
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 7d9beaa..51c7cf8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1775,7 +1775,7 @@ static void intel_prepare_shared_dpll(struct intel_crtc
*crtc)
if (WARN_ON(pll == NULL))
return;
- WARN_ON(!pll-refcount);
+ WARN_ON(!pll-crtc_mask);
if (pll-active == 0) {
DRM_DEBUG_DRIVER(setting up %s\n, pll-name);
WARN_ON(pll-on);
@@ -1802,7 +1802,7 @@ static void intel_enable_shared_dpll(struct intel_crtc
*crtc)
if (WARN_ON(pll == NULL))
return;
- if (WARN_ON(pll-refcount == 0))
+ if (WARN_ON(pll-crtc_mask == 0))
return;
DRM_DEBUG_KMS(enable %s (active %d, on? %d) for crtc %d\n,
@@ -1834,7 +1834,7 @@ static void intel_disable_shared_dpll(struct intel_crtc
*crtc)
if (WARN_ON(pll == NULL))
return;
- if (WARN_ON(pll-refcount == 0))
+ if (WARN_ON(pll-crtc_mask == 0))
return;
DRM_DEBUG_KMS(disable %s (active %d, on? %d) for crtc %d\n,
@@ -3834,12 +3834,13 @@ void intel_put_shared_dpll(struct intel_crtc *crtc)
if (pll == NULL)
return;
- if (pll-refcount == 0) {
- WARN(1, bad %s refcount\n, pll-name);
+ if (!(pll-crtc_mask (1 crtc-pipe))) {
+ WARN(1, bad %s crtc mask\n, pll-name);
return;
}
- if (--pll-refcount == 0) {
+ pll-crtc_mask = ~(1 crtc-pipe);
+ if (pll-crtc_mask == 0) {
WARN_ON(pll-on);
WARN_ON(pll-active);
}
@@ -3867,7 +3868,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct
intel_crtc *crtc)
DRM_DEBUG_KMS(CRTC:%d using pre-allocated %s\n,
crtc-base.base.id, pll-name);
- WARN_ON(pll-refcount);
+ WARN_ON(pll-crtc_mask);
goto found;
}
@@ -3876,14 +3877,15 @@ struct intel_shared_dpll
*intel_get_shared_dpll(struct intel_crtc *crtc)
pll = dev_priv-shared_dplls[i];
/* Only want to check enabled timings first */
- if (pll-refcount == 0)
+ if (pll-crtc_mask == 0)
continue;
if (memcmp(crtc-config.dpll_hw_state, pll-hw_state,
sizeof(pll-hw_state)) == 0) {
- DRM_DEBUG_KMS(CRTC:%d sharing existing %s (refcount
%d, ative %d)\n,
- crtc-base.base.id,
- pll-name, pll-refcount, pll-active);
+ DRM_DEBUG_KMS(CRTC:%d sharing
Re: [Intel-gfx] [PATCH 3/8] drm/i915: Add infrastructure for choosing DPLLs before disabling crtcs
On Tue, Oct 21, 2014 at 04:02:04PM +0300, Ander Conselvan de Oliveira wrote:
It is possible for a mode set to fail if there aren't shared DPLLS that
match the new configuration requirement or other errors in clock
computation. If that step is executed after disabling crtcs, in the
failure case the hardware configuration is changed and needs to be
restored. Doing those things early will allow the mode set to fail
before actually touching the hardware.
Follow up patches will convert different platforms to use the new
infrastructure.
Signed-off-by: Ander Conselvan de Oliveira
ander.conselvan.de.olive...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/intel_ddi.c | 2 +
drivers/gpu/drm/i915/intel_display.c | 125
+++
3 files changed, 102 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d7412d9..5b2464f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -233,6 +233,8 @@ struct intel_shared_dpll_config {
struct intel_shared_dpll {
struct intel_shared_dpll_config config;
+ struct intel_shared_dpll_config *new_config;
+
int active; /* count of number of active CRTCs (i.e. DPMS on) */
bool on; /* is the PLL actually active? Disabled during modeset */
const char *name;
@@ -480,6 +482,7 @@ struct drm_i915_display_funcs {
struct intel_crtc_config *);
void (*get_plane_config)(struct intel_crtc *,
struct intel_plane_config *);
+ int (*crtc_compute_clock)(struct intel_crtc *crtc);
int (*crtc_mode_set)(struct intel_crtc *crtc,
int x, int y,
struct drm_framebuffer *old_fb);
diff --git a/drivers/gpu/drm/i915/intel_ddi.c
b/drivers/gpu/drm/i915/intel_ddi.c
index 5915a07..7b8c4b8 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1375,6 +1375,8 @@ static void hsw_shared_dplls_init(struct
drm_i915_private *dev_priv)
dev_priv-num_shared_dpll = 2;
for (i = 0; i dev_priv-num_shared_dpll; i++) {
+ dev_priv-shared_dplls[i].new_config =
+ dev_priv-shared_dplls[i].config;
dev_priv-shared_dplls[i].id = i;
dev_priv-shared_dplls[i].name = hsw_ddi_pll_names[i];
dev_priv-shared_dplls[i].disable = hsw_ddi_pll_disable;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index cdaf8ef..f2f7e8f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3851,15 +3851,9 @@ void intel_put_shared_dpll(struct intel_crtc *crtc)
struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
{
struct drm_i915_private *dev_priv = crtc-base.dev-dev_private;
- struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
+ struct intel_shared_dpll *pll;
enum intel_dpll_id i;
- if (pll) {
- DRM_DEBUG_KMS(CRTC:%d dropping existing %s\n,
- crtc-base.base.id, pll-name);
- intel_put_shared_dpll(crtc);
- }
-
if (HAS_PCH_IBX(dev_priv-dev)) {
/* Ironlake PCH has a fixed PLL-PCH pipe mapping. */
i = (enum intel_dpll_id) crtc-pipe;
@@ -3868,7 +3862,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct
intel_crtc *crtc)
DRM_DEBUG_KMS(CRTC:%d using pre-allocated %s\n,
crtc-base.base.id, pll-name);
- WARN_ON(pll-config.crtc_mask);
+ WARN_ON(pll-new_config-crtc_mask);
goto found;
}
@@ -3877,17 +3871,16 @@ struct intel_shared_dpll
*intel_get_shared_dpll(struct intel_crtc *crtc)
pll = dev_priv-shared_dplls[i];
/* Only want to check enabled timings first */
- if (pll-config.crtc_mask == 0)
+ if (pll-new_config-crtc_mask == 0)
continue;
- if (memcmp(crtc-config.dpll_hw_state,
-pll-config.hw_state,
-sizeof(pll-config.hw_state)) == 0) {
- DRM_DEBUG_KMS(CRTC:%d sharing existing %s
- (crtc_mask 0x%08x, active %d)\n,
+ if (memcmp(crtc-new_config-dpll_hw_state,
+pll-new_config-hw_state,
+sizeof(pll-new_config-hw_state)) == 0) {
+ DRM_DEBUG_KMS(CRTC:%d sharing existing %s (crtc mask
0x%08x, ative %d)\n,
crtc-base.base.id, pll-name,
- pll-config.crtc_mask, pll-active);
-
+ pll-new_config-crtc_mask,
+
Re: [Intel-gfx] [PATCH 2/6] drm: add tile_group support.
On Wed, Oct 22, 2014 at 12:23:30PM +1000, Dave Airlie wrote: And kerneldoc for the non-exported functions please, preferrably with some overview DOC: section to pull it all together. I'm posting v2, advice on kerneldoc required, so the functions end up in the right place, I don't really wnat to add a new C file for this. Argh sorry that was a boilerplate reply that escated ;-) I've sent the same one to pretty much all the recent i915 patch series where this is the new rule. I don't think drm internal docs make a lot of sense as long as all the driver stuff is nicely documented. Some barrier to entry for core drm might actually be useful ... 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] [RFC PATCH v2 1/4] drm/i915: add i915_ved.c to setup bridge for VED
On Wed, Oct 22, 2014 at 07:11:21AM +, Cheng, Yao wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, October 21, 2014 8:09 PM To: Cheng, Yao Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; Kelley, Sean V; Vetter, Daniel; Abel, Michael J; Jiang, Fei; Rao, Ram R Subject: Re: [Intel-gfx] [RFC PATCH v2 1/4] drm/i915: add i915_ved.c to setup bridge for VED On Tue, Oct 21, 2014 at 02:36:41PM +0800, Yao Cheng wrote: Setup minimum required resources during i915_driver_load: 1. Create a platform device to share MMIO/IRQ resources 2. Make the platform device child of i915 device for runtime PM. 3. Create IRQ chip to forward the VED irqs. VED driver (a standalone drm driver) probes the VED device and creates a new dri card on install. Currently only supports VED on valleyview. Kerneldoc is updated for i915_ved.c. Signed-off-by: Yao Cheng yao.ch...@intel.com Please resend with a patch changelog to account for my review comments. And Ville's. Plus cc us both. And if there's anything you didn't address, you must reply to the review and we need to further discuss this. Daniel, I see, thanks for the instruction. Do you mean resending the [RFC PATCH v2] with changelog and cc list? Or adding changelog/cc when sending [RFC PATCH v3]? I think you could just add the per-patch changelog for both v2 and v3 when sending out v3. -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] [RFC PATCH v2 2/4] drm/ipvr: drm driver for VED
On Wed, Oct 22, 2014 at 06:37:16AM +, Cheng, Yao wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, October 21, 2014 5:08 PM To: Cheng, Yao Cc: intel-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; Kelley, Sean V; Vetter, Daniel; Abel, Michael J; Jiang, Fei; Rao, Ram R Subject: Re: [Intel-gfx] [RFC PATCH v2 2/4] drm/ipvr: drm driver for VED On Tue, Oct 21, 2014 at 02:36:42PM +0800, Yao Cheng wrote: Probes VED and creates a new drm device for hardware accelerated video decoding. Currently support VP8 decoding on valleyview. Signed-off-by: Yao Cheng yao.ch...@intel.com The in-patch changelog here is missing, and there's also no indication in the cover letter for what changes you've made. On a quick look you've incorporated some of David's feedback, but not all of it. That's not good, since if you only partially apply review feedback then you essentially force reviewers to read the entire patch again, which is a good way to driver them away. Also you should Cc: (in the sob section of the patch) all the people who have commented on your patch already. Oops, sorry for not following the upstreaming rules :( I might have overlooked some of David's comment..have to learn more about the rules. For this version, I'll add changelog by replying my patch with cc to those commenters, I assume this is not too late With that out of the way some high-level review: - I think we need the full libva implementation to review the interfaces properly. At least the little libdrm test program doesn't seem to fully exercise it all. The libva driver need some time to be fully open sourced, but I can upload the code to Sean's private github repo for your access. I'll sync with Sean and you internally. It doesn't need to be the final libva driver of course, just something so that people can look at the userspace side. So upload to some github account is perfectly ok. Or do you mean we still have legal review pending on those patches? In that case I think we need to wait for that to complete first. - The ioctl structs need to be cleaned up. You can't use uint32_t and similar typedefs since they can clash with userspace. You must use __u32 and friends. Also, some of the padding fields arent' really required - if you only have 4byte types then you don't need to align to 8 bytes. - Input validation on ioctls looks spotty at best. E.g. if you have any padding fields you need to check that they are 0, otherwise we can't ever reuse them as flags fields. And on principle _all_ input fields must be validated first. For some good guidelines for ioctls see http://blog.ffwll.ch/2013/11/botching-up-ioctls.html Thanks for pointing me to the ioctl instruction... I'll read it carefully and update the ioctl interfaces... - Locking seems to be inexistent in places, at least some of the idr manipulation very much looks like it's done lock-free. That doesn't work well. Yes, probably we haven't considered all the scenarios carefully, is it possible to review them in an internal discussion? Imo no need for private review since I didn't spot anything fundamentally wrong. It's just a lot of small details, and for those I think m-l review is a good tool. But someone needs to do that, and I don't really have the time for it. - You implement file-descriptor based fences, but then also have the more gem-traditional wait ioctl working on buffer objects. That's a bit a funky mix of implicit and explicit fencing. Furthermore adding new private fence objects isn't a good idea now that everyon is talking about de-staging android syncpts as the official userspace interface. Also, your userspace patches don't use this, so maybe we can just rip it all out? Currently the libdrm_ipvr.so uses both the WAIT IOCTL and FD style fence... At beginning, both drm_ipvr_gem_bo_alloc() and drm_ipvr_gem_bo_wait() use the WAIT IOCTL. In drm_ipvr_gem_bo_alloc(), libdrm_ipvr tries to return an existing free BO instead of requesting kernel via IOCTL, like libdrm_intel does. Eventually we think the status query on multiple BOs is inefficient, so we added the FD style fence to let libdrm_ipvr call select() to do a batch query. I'm fine to drop one and keep the other. Which one is preferred by GEM? The WAIT_IOCTL or the FD fence? Or do you suggest directly use the Android syncpts? The wait ioctl is the usual approach with gem drivers. Explicit fencing is still in flux like I've said, so charging ahead and locking down an interface doesn't seem like a good idea. And I'd be _really_ surprised if you can benchmark the benefits of explicit fencing, so I don't think you can even justify the added complexity. - I'm a bit unclear on your usage of vxd_/pvr_ prefixes. Thanks for pointing out
Re: [Intel-gfx] [PATCH] drm/i915: add missing forcewake put on i915_wa_registers()
On 22/10/2014 08:35, Ville Syrjälä wrote: On Tue, Oct 21, 2014 at 07:40:35PM +0200, Daniel Vetter wrote: On Tue, Oct 21, 2014 at 02:58:08PM -0200, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Otherwise, a simple cat to the debugfs file can make the machine use much more power than needed, and prevent it from runtime suspending. Related commit: commit 8452e1d173a16d9812422a2272c4ab0f0ba81057 Author: Mika Kuoppala mika.kuopp...@linux.intel.com Date: Tue Oct 7 17:21:26 2014 +0300 drm/i915: Build workaround list in ring initialization Cc: Mika Kuoppala mika.kuopp...@linux.intel.com Cc: Arun Siluvery arun.siluv...@linux.intel.com Testcase: igt/pm_rpm/debugfs-read Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com tbh I'm not even sure we want to do the manual forcewake get here - I915_READ will do it for us, and this is a debug interface. So no one should care about perf. Mika, is that right? If so I'd like to merge the inverse patch which drops the fw_get. Don't we still need the idle msg disable+poll CSPWRFSM trick here on gen8? That also needs forcewake around it. I had a chat with Mika on this yesterday and he seem to agree that forcewake is probably not required here. I couldn't send the patch yesterday but as per Ville's comments looks like we need forcewake here? regards Arun -Daniel --- drivers/gpu/drm/i915/i915_debugfs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 9600285..36a4baa 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2671,6 +2671,7 @@ static int i915_wa_registers(struct seq_file *m, void *unused) addr, value, mask, read, ok ? OK : FAIL); } + gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL); intel_runtime_pm_put(dev_priv); mutex_unlock(dev-struct_mutex); -- 2.1.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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/6] drm/tile: expose the tile property to userspace (v2)
On Wed, Oct 22, 2014 at 12:32:06PM +1000, Dave Airlie wrote:
From: Dave Airlie airl...@redhat.com
This takes the tiling info from the connector and
exposes it to userspace, as a blob object in a
connector property.
The contents of the blob is ABI.
v2: add property + function documentation.
Signed-off-by: Dave Airlie airl...@redhat.com
---
Documentation/DocBook/drm.tmpl | 9 +++-
drivers/gpu/drm/drm_crtc.c | 44
+
drivers/gpu/drm/i915/intel_dp_mst.c | 2 ++
include/drm/drm_crtc.h | 4
4 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 5ea6289..1f19340 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -2513,7 +2513,7 @@ void intel_crt_init(struct drm_device *dev)
/tr
tr
td rowspan=21 valign=top DRM/td
- td rowspan=3 valign=top Generic/td
+ td rowspan=4 valign=top Generic/td
td valign=top “EDID”/td
td valign=top BLOB | IMMUTABLE/td
td valign=top 0/td
@@ -2535,6 +2535,13 @@ void intel_crt_init(struct drm_device *dev)
td valign=top Contains topology path to a connector./td
/tr
tr
+ td valign=top “TILE”/td
+ td valign=top BLOB | IMMUTABLE/td
+ td valign=top 0/td
+ td valign=top Connector/td
+ td valign=top Contains tiling information for a connector./td
+ /tr
+ tr
td rowspan=1 valign=top Plane/td
td valign=top “type”/td
td valign=top ENUM | IMMUTABLE/td
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index f3e082d..1c64f5f 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1319,6 +1319,11 @@ static int
drm_mode_create_standard_connector_properties(struct drm_device *dev)
PATH, 0);
dev-mode_config.path_property = dev_path;
+ dev-mode_config.tile_property = drm_property_create(dev,
+ DRM_MODE_PROP_BLOB
|
+
DRM_MODE_PROP_IMMUTABLE,
+ TILE, 0);
+
return 0;
}
@@ -4015,6 +4020,45 @@ int drm_mode_connector_set_path_property(struct
drm_connector *connector,
EXPORT_SYMBOL(drm_mode_connector_set_path_property);
/**
+ * drm_mode_connector_set_tile_property - set tile property on connector
+ * @connector: connector to set property on.
+ *
+ * This looks up the tile information for a connector, and creates a
+ * property for userspace to parse if it exists. The property is of
+ * the form of 8 integers using ':' as a separator.
+ */
Again return value boilerplate missing.
+int drm_mode_connector_set_tile_property(struct drm_connector *connector)
+{
+ struct drm_device *dev = connector-dev;
+ int ret, size;
+ char tile[256];
+
+ if (connector-tile_blob_ptr)
+ drm_property_destroy_blob(dev, connector-tile_blob_ptr);
+
+ if (!connector-has_tile) {
+ connector-tile_blob_ptr = NULL;
+ ret = drm_object_property_set_value(connector-base,
+
dev-mode_config.tile_property, 0);
+ return ret;
+ }
+
+ snprintf(tile, 256, %d:%d:%d:%d:%d:%d:%d:%d,
connector-tile_group-id, connector-tile_is_single_monitor,
connector-num_h_tile, connector-num_v_tile, connector-tile_h_loc,
connector-tile_v_loc, connector-tile_h_size, connector-tile_v_size);
Long line.
With both nits fixed this is Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch
+ size = strlen(tile) + 1;
+
+ connector-tile_blob_ptr = drm_property_create_blob(connector-dev,
+ size, tile);
+ if (!connector-tile_blob_ptr)
+ return -EINVAL;
+
+ ret = drm_object_property_set_value(connector-base,
+ dev-mode_config.tile_property,
+ connector-tile_blob_ptr-base.id);
+ return ret;
+}
+EXPORT_SYMBOL(drm_mode_connector_set_tile_property);
+
+/**
* drm_mode_connector_update_edid_property - update the edid property of a
connector
* @connector: drm connector
* @edid: new value of the edid property
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index c66e73a..c5529ff 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -422,6 +422,8 @@ static struct drm_connector
*intel_dp_add_mst_connector(struct drm_dp_mst_topolo
intel_dp_add_properties(intel_dp, connector);
drm_object_attach_property(connector-base,
dev-mode_config.path_property, 0);
+ drm_object_attach_property(connector-base,
Re: [Intel-gfx] [PATCH 3/6] drm/mst: cached EDID for logical ports
On Wed, Oct 22, 2014 at 12:32:04PM +1000, Dave Airlie wrote:
From: Dave Airlie airl...@redhat.com
Logical ports are never going to have EDID changes,
they are used for the internal ports on MST monitors.
We cache the EDIDs from these to save time at MST probe.
Signed-off-by: Dave Airlie airl...@redhat.com
---
drivers/gpu/drm/drm_dp_mst_topology.c | 20 ++--
drivers/gpu/drm/i915/intel_dp_mst.c | 2 +-
include/drm/drm_dp_mst_helper.h | 4 +++-
3 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 50926db..ce1113c 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -858,6 +858,8 @@ static void drm_dp_destroy_port(struct kref *kref)
struct drm_dp_mst_topology_mgr *mgr = port-mgr;
if (!port-input) {
port-vcpi.num_slots = 0;
+
+ kfree(port-cached_edid);
if (port-connector)
(*port-mgr-cbs-destroy_connector)(mgr,
port-connector);
drm_dp_port_teardown_pdt(port, port-pdt);
@@ -1096,6 +1098,10 @@ static void drm_dp_add_port(struct drm_dp_mst_branch
*mstb,
char proppath[255];
build_mst_prop_path(port, mstb, proppath);
port-connector = (*mstb-mgr-cbs-add_connector)(mstb-mgr,
port, proppath);
+
+ if (port-port_num = 8) {
+ port-cached_edid = drm_get_edid(port-connector,
port-aux.ddc);
+ }
I'm confused about how this works ... the tile property gets added in the
intel -add_connector callback already, but that relies upon drm_get_edid
having parsed the displayid stuff. What am I missing?
-Daniel
}
/* put reference to this port */
@@ -2149,7 +2155,8 @@ EXPORT_SYMBOL(drm_dp_mst_hpd_irq);
* This returns the current connection state for a port. It validates the
* port pointer still exists so the caller doesn't require a reference
*/
-enum drm_connector_status drm_dp_mst_detect_port(struct
drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port)
+enum drm_connector_status drm_dp_mst_detect_port(struct drm_connector
*connector,
+ struct drm_dp_mst_topology_mgr
*mgr, struct drm_dp_mst_port *port)
{
enum drm_connector_status status = connector_status_disconnected;
@@ -2168,6 +2175,10 @@ enum drm_connector_status
drm_dp_mst_detect_port(struct drm_dp_mst_topology_mgr
case DP_PEER_DEVICE_SST_SINK:
status = connector_status_connected;
+ /* for logical ports - cache the EDID */
+ if (port-port_num = 8 !port-cached_edid) {
+ port-cached_edid = drm_get_edid(connector,
port-aux.ddc);
+ }
break;
case DP_PEER_DEVICE_DP_LEGACY_CONV:
if (port-ldps)
@@ -2199,7 +2210,12 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector
*connector, struct drm_dp_
if (!port)
return NULL;
- edid = drm_get_edid(connector, port-aux.ddc);
+ if (port-cached_edid)
+ edid = drm_edid_duplicate(port-cached_edid);
+ else
+ edid = drm_get_edid(connector, port-aux.ddc);
+
+ drm_mode_connector_set_tile_property(connector);
drm_dp_put_port(port);
return edid;
}
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index d9a7a78..c66e73a 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -283,7 +283,7 @@ intel_mst_port_dp_detect(struct drm_connector *connector)
struct intel_connector *intel_connector = to_intel_connector(connector);
struct intel_dp *intel_dp = intel_connector-mst_port;
- return drm_dp_mst_detect_port(intel_dp-mst_mgr,
intel_connector-port);
+ return drm_dp_mst_detect_port(connector, intel_dp-mst_mgr,
intel_connector-port);
}
static enum drm_connector_status
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 338fc10..ee6fbad 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -92,6 +92,8 @@ struct drm_dp_mst_port {
struct drm_dp_vcpi vcpi;
struct drm_connector *connector;
struct drm_dp_mst_topology_mgr *mgr;
+
+ struct edid *cached_edid; /* for DP logical ports - make tiling work */
};
/**
@@ -474,7 +476,7 @@ int drm_dp_mst_topology_mgr_set_mst(struct
drm_dp_mst_topology_mgr *mgr, bool ms
int drm_dp_mst_hpd_irq(struct drm_dp_mst_topology_mgr *mgr, u8 *esi, bool
*handled);
-enum drm_connector_status drm_dp_mst_detect_port(struct
drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
+enum drm_connector_status drm_dp_mst_detect_port(struct drm_connector
*connector, struct drm_dp_mst_topology_mgr *mgr, struct
[Intel-gfx] [PATCH] drm/i915: Emit even number of dwords when emitting LRIs
The number of DWords should be even when doing ring emits as command sequences require QWord alignment. Cc: Mika Kuoppala mika.kuopp...@intel.com Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 12a546f..79211ae 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -680,7 +680,7 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring) if (ret) return ret; - ret = intel_ring_begin(ring, w-count * 3); + ret = intel_ring_begin(ring, w-count * 4); if (ret) return ret; @@ -688,6 +688,7 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring) intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); intel_ring_emit(ring, w-reg[i].addr); intel_ring_emit(ring, w-reg[i].value); + intel_ring_emit(ring, MI_NOOP); } intel_ring_advance(ring); -- 2.1.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] Revert drm/i915: Enable full PPGTT on gen7
This reverts commit 8c50f10d73b50139dcfe48bc22f2c8c7822c1983. It's not yet solid and Dave objected to pulling the tree in its current state. Cc: Michel Thierry michel.thie...@intel.com Cc: Dave Airlie airl...@gmail.com Cc: Chris Wilson ch...@chris-wilson.co.uk References: http://mid.mail-archive.com/CAPM=9ty2r1MLE=wzc-_vnsuzxvqayxiggocpsv9qop0gzpk...@mail.gmail.com References: http://lists.freedesktop.org/archives/intel-gfx/2014-October/053926.html Signed-off-by: Daniel Vetter daniel.vet...@intel.com --- 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 273dad964e1b..8ddc834f722f 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -67,7 +67,7 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt) return 0; } - return has_full_ppgtt ? 2 : has_aliasing_ppgtt ? 1 : 0; + return has_aliasing_ppgtt ? 1 : 0; } -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] Revert drm/i915: Enable full PPGTT on gen7
On Wed, Oct 22, 2014 at 11:23:03AM +0200, Daniel Vetter wrote: This reverts commit 8c50f10d73b50139dcfe48bc22f2c8c7822c1983. It's not yet solid and Dave objected to pulling the tree in its current state. Cc: Michel Thierry michel.thie...@intel.com Cc: Dave Airlie airl...@gmail.com Cc: Chris Wilson ch...@chris-wilson.co.uk References: http://mid.mail-archive.com/CAPM=9ty2r1MLE=wzc-_vnsuzxvqayxiggocpsv9qop0gzpk...@mail.gmail.com References: http://lists.freedesktop.org/archives/intel-gfx/2014-October/053926.html Signed-off-by: Daniel Vetter daniel.vet...@intel.com Acked-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 0/8] Add host i915 support for vGPU
On Tue, Sep 30, 2014 at 06:05:30PM +0800, Jike Song wrote: Intel GVT-g (previously known as XenGT), is a complete GPU virtualization solution with mediated pass-through for 4th generation Intel Core processors - Haswell platform. This technology presents a virtual full-fledged GPU to each Virtual Machine (VM). VMs can directly access performance-critical resources, without intervention from the hypervisor in most cases, while privileged operations from VMs are trap-and-emulated at minimal cost. For details, please refer to: https://01.org/xen/blogs/wangbo85/2014/intel-gvt-gxengt-pubic-release The patches of adding vGPU guest support for i915, is already posted at: http://lists.freedesktop.org/archives/intel-gfx/2014-September/052571.html This patch set is about to add vGPU host support. When running as vGPU host, the i915 driver can't simply occupy the whole GPU resources, it needs to proactively meidate the accesses to most hardware resources: MMIO, GTT, and interrupt. Only by handling the hardware resources, the vgt can have centralized management about sharing between VMs(including host and guest). This patch set adds: - an i915 extension, named vgt. vgt is the major part of Intel GVT-g implementation, it manages and shares GPU among VMs and host; - interfaces between i915 and vgt, including: Mediated MMIO access Mediated GTT access Mediated IRQ handling NOTE of RFC: - the vgt in-kernel GPU device-model is not yet integrated, though the interfaces between i915 and vgt are provided; - the host i915 driver has some logic same with the guest i915, e.g. the ballooning. When the guest patches(see above) are finalized, we need to rebase upon them; - vgt_suspend/vgt_resume are still under cleanup; - GPU reset is still under cleanup We send out the framework changes in this patch set for review at first, and meanwhile, vgt integration and cleanup are ongoing. So on a very high level I don't understand this design. For the guest side it's completely clear that we need a bunch of hooks over the driver to make paravirtualization work. But on the host side I expect the driver to be in full control of the hardware. I haven't really seen the other side but it looks like with vgt we actually have two drivers fighting over the hardware, which requires the various hooks to avoid disaster. The drm subsystem is littered with such attempts, and they all didn't end up in a pretty way. So way can't we have the vgt support for guests sit on top of i915, using the i915 functions to set up pagetables, contexts and reserve gtt areas for the guests? Then we'd have just one driver in control of the hardware, and vgt on the host side would just look like a really crazy interace layer between virtual hosts and the low-level driver, similar to who the execbuf ioctl is a really crazy interface between userspace and the low-level driver. 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 v2 0/8] Add enlightenments for vGPU
On Wed, Oct 22, 2014 at 03:03:56PM +0800, Yu, Zhang wrote: On 10/22/2014 12:51 AM, Daniel Vetter wrote: - Please pull all the new documentation together and integrate it into the i915 section of the drm docbook. A good place is probably a new subsection Paravirtualized Guest Support (vGPU) under the driver core section. How about subsection name Intel GVT-g Guest Support(vGPU)? :) Ack. Sounds a bit like marketing though ;-) 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] drm/i915: Emit even number of dwords when emitting LRIs
On Wed, Oct 22, 2014 at 10:09:49AM +0100, Arun Siluvery wrote: The number of DWords should be even when doing ring emits as command sequences require QWord alignment. It looks like we could just pad at the end of the batch instead of one NOP per register write? Also, It looks like we could use the LRI variant that can write more than one register in one go (separate patch)?. -- Damien Cc: Mika Kuoppala mika.kuopp...@intel.com Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/intel_ringbuffer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 12a546f..79211ae 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -680,7 +680,7 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring) if (ret) return ret; - ret = intel_ring_begin(ring, w-count * 3); + ret = intel_ring_begin(ring, w-count * 4); if (ret) return ret; @@ -688,6 +688,7 @@ static int intel_ring_workarounds_emit(struct intel_engine_cs *ring) intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); intel_ring_emit(ring, w-reg[i].addr); intel_ring_emit(ring, w-reg[i].value); + intel_ring_emit(ring, MI_NOOP); } intel_ring_advance(ring); -- 2.1.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Emit even number of dwords when emitting LRIs
On Wed, Oct 22, 2014 at 11:40:30AM +0100, Damien Lespiau wrote: On Wed, Oct 22, 2014 at 10:09:49AM +0100, Arun Siluvery wrote: The number of DWords should be even when doing ring emits as command sequences require QWord alignment. It looks like we could just pad at the end of the batch instead of one NOP per register write? Also, It looks like we could use the LRI variant that can write more than one register in one go (separate patch)?. Note that if you use the multi-register LRI variant, the number of dwords will always be odd, so we'll always need to pad a NOP, that makes it easy. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: run intel_uncore_early_sanitize earlier on resume on non-VLV
On Tue, 2014-10-21 at 19:05 +0200, Daniel Vetter wrote:
On Mon, Oct 20, 2014 at 01:20:50PM +0300, Imre Deak wrote:
On Fri, 2014-10-17 at 16:01 -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
As far as I understand, intel_uncore_early_sanitize() was supposed to
be ran before any register access, but currently
intel_resume_prepare() is ran earlier, and it does register
access. I don't think it should be safe to be calling
I915_{READ,WRITE} without calling intel_uncore_early_sanitize() first.
One of the problems we currently have is that when we suspend/resume
BDW, the FPGA_DBG_RM_NOCLAIM bit becomes 1, so we end up printing an
unclaimed register message on resume, but this message doesn't
really seem to have been triggered by our driver or user space, since
the bit was not there before suspending, and gets there just after
resuming, before any of our own register accesses. So calling
intel_uncore_early_sanitize() as a first thing will allow us to stop
printing the error message, fixing the bug.
v2: VLV is an exception to the early_sanitize() rule: it needs to do
stuff before calling early_sanitize(), so instead of calling it
earlier for every platform, we call it earlier for non-VLV by adding
the early_sanitize() call inside intel_resume_prepare(). This doesn't
look like the most-beautiful-solution-ever, but, well, at least it
fixes the bug. (Imre)
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Imre Deak imre.d...@intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83094
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index a05a1d0..f6d28f2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -669,7 +669,6 @@ static int i915_drm_thaw_early(struct drm_device *dev)
if (ret)
DRM_ERROR(Resume prepare failed: %d,Continuing resume\n, ret);
- intel_uncore_early_sanitize(dev, true);
intel_uncore_sanitize(dev);
intel_power_domains_init_hw(dev_priv);
@@ -1049,6 +1048,8 @@ static int snb_resume_prepare(struct
drm_i915_private *dev_priv,
if (rpm_resume)
intel_init_pch_refclk(dev);
+ else
+ intel_uncore_early_sanitize(dev, true);
return 0;
}
@@ -1056,6 +1057,9 @@ static int snb_resume_prepare(struct
drm_i915_private *dev_priv,
static int hsw_resume_prepare(struct drm_i915_private *dev_priv,
bool rpm_resume)
{
+ if (!rpm_resume)
+ intel_uncore_early_sanitize(dev_priv-dev, true);
+
hsw_disable_pc8(dev_priv);
return 0;
@@ -1421,6 +1425,9 @@ static int vlv_resume_prepare(struct
drm_i915_private *dev_priv,
i915_gem_restore_fences(dev);
}
+ if (!rpm_resume)
+ intel_uncore_early_sanitize(dev, true);
+
return ret;
}
You also need to call intel_uncore_early_sanitize() from
intel_resume_prepare() for the rest of the platforms. With that fixed:
Reviewed-by: Imre Deak imre.d...@intel.com
Looking at the result, I agree it's not the nicest, so yet another way
to reduce the clutter would be to have the following instead in
i915_drm_thaw_early():
intel_resume_early_prepare()
intel_uncore_early_sanitize()
intel_resume_prepare()
and do the early steps for VLV in intel_resume_early_prepare(). I'm ok
with both solutions.
This honestly starts to smell like a giant maintenance nightmare. We kinda
started off into the wrong direction with vlv rpm and it seems to get
worse by the day. And it looks like the situation is messy enough that we
can't even look down the ordering with copious amounts of warnings ...
But I also don't see any real solution, so just ranting for now. I'd
appreciate though if the revised version comes with a bunch of comments
attached in the code.
I blame it on the HW people. :) Seriously, the VLV PM code differs from
the rest of PM code in that we save/restore some HW state instead of
reinitializing it. That's where the above special casing of the ordering
stems from. I agree that it's not ideal, but I think having started with
that solution and moving towards the ideal was not that bad. In fact
s0ix doesn't yet work in the upstream kernel for reasons independent of
i915 (or at least I couldn't make it work), but we would need it to
fully validate all the suspend/resume paths.
--Imre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/chv: Use 16 and 32 for low and high drain latency precision.
On Wed, Oct 22, 2014 at 09:57:06AM +0300, Ville Syrjälä wrote:
On Fri, Oct 17, 2014 at 08:05:08AM -0700, Rodrigo Vivi wrote:
Current chv spec teels we can only use either 16 or 32 bits as precision.
Although in the past VLV went from 16/32 to 32/64 and spec might not be
updated,
these precision values brings stability and fixes some issues Wayne was
facing.
Lies, damned lies, specs!
Well in this case I guess the spec might be correct since it helps
Wayne.
Cc: Wayne Boyer wayne.bo...@intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 13 +++--
drivers/gpu/drm/i915/intel_pm.c | 40
+---
2 files changed, 32 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index 6db369a..dcd5650 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4054,17 +4054,18 @@ enum punit_power_well {
#define DSPFW_PLANEA_WM1_HI_MASK (10)
/* drain latency register values*/
+#define DRAIN_LATENCY_PRECISION_16 16
#define DRAIN_LATENCY_PRECISION_32 32
#define DRAIN_LATENCY_PRECISION_64 64
While you're poking at this stuff, could I trouble you to remove these
defines and just use the values directly? Could be a separate patch if
you prefer.
Separate patch imo.
#define VLV_DDL(pipe) (VLV_DISPLAY_BASE + 0x70050 + 4
* (pipe))
-#define DDL_CURSOR_PRECISION_64(131)
-#define DDL_CURSOR_PRECISION_32(031)
+#define DDL_CURSOR_PRECISION_HIGH (131)
+#define DDL_CURSOR_PRECISION_LOW (031)
#define DDL_CURSOR_SHIFT 24
-#define DDL_SPRITE_PRECISION_64(sprite)(1(15+8*(sprite)))
-#define DDL_SPRITE_PRECISION_32(sprite)(0(15+8*(sprite)))
+#define DDL_SPRITE_PRECISION_HIGH(sprite) (1(15+8*(sprite)))
+#define DDL_SPRITE_PRECISION_LOW(sprite) (0(15+8*(sprite)))
#define DDL_SPRITE_SHIFT(sprite) (8+8*(sprite))
-#define DDL_PLANE_PRECISION_64 (17)
-#define DDL_PLANE_PRECISION_32 (07)
+#define DDL_PLANE_PRECISION_HIGH (17)
+#define DDL_PLANE_PRECISION_LOW(07)
#define DDL_PLANE_SHIFT0
#define DRAIN_LATENCY_MASK 0x7f
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index daa99e7..50c3512 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1345,6 +1345,7 @@ static bool vlv_compute_drain_latency(struct drm_crtc
*crtc,
int *prec_mult,
int *drain_latency)
{
+ struct drm_device *dev = crtc-dev;
int entries;
int clock = to_intel_crtc(crtc)-config.adjusted_mode.crtc_clock;
@@ -1355,8 +1356,12 @@ static bool vlv_compute_drain_latency(struct
drm_crtc *crtc,
return false;
entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
- *prec_mult = (entries 128) ? DRAIN_LATENCY_PRECISION_64 :
- DRAIN_LATENCY_PRECISION_32;
+ if (IS_CHERRYVIEW(dev))
+ *prec_mult = (entries 128) ? DRAIN_LATENCY_PRECISION_32 :
+ DRAIN_LATENCY_PRECISION_16;
+ else
+ *prec_mult = (entries 128) ? DRAIN_LATENCY_PRECISION_64 :
+ DRAIN_LATENCY_PRECISION_32;
*drain_latency = (64 * (*prec_mult) * 4) / entries;
if (*drain_latency DRAIN_LATENCY_MASK)
@@ -1375,15 +1380,18 @@ static bool vlv_compute_drain_latency(struct
drm_crtc *crtc,
static void vlv_update_drain_latency(struct drm_crtc *crtc)
{
- struct drm_i915_private *dev_priv = crtc-dev-dev_private;
+ struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pixel_size;
int drain_latency;
enum pipe pipe = intel_crtc-pipe;
int plane_prec, prec_mult, plane_dl;
+ int high_precision = IS_CHERRYVIEW(dev) ? DRAIN_LATENCY_PRECISION_32 :
+ DRAIN_LATENCY_PRECISION_64;
Maybe const just help the reader remember that it's a constant.
- plane_dl = I915_READ(VLV_DDL(pipe)) ~(DDL_PLANE_PRECISION_64 |
- DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_64 |
+ plane_dl = I915_READ(VLV_DDL(pipe)) ~(DDL_PLANE_PRECISION_HIGH |
+ DRAIN_LATENCY_MASK | DDL_CURSOR_PRECISION_HIGH |
(DRAIN_LATENCY_MASK DDL_CURSOR_SHIFT));
if (!intel_crtc_active(crtc)) {
@@ -1394,9 +1402,9 @@ static void vlv_update_drain_latency(struct drm_crtc
*crtc)
/* Primary plane Drain Latency */
pixel_size = crtc-primary-fb-bits_per_pixel / 8; /* BPP */
if
Re: [Intel-gfx] [PATCH 3/8] drm/i915: Add infrastructure for choosing DPLLs before disabling crtcs
On 10/22/2014 11:33 AM, Ville Syrjälä wrote:
On Tue, Oct 21, 2014 at 04:02:04PM +0300, Ander Conselvan de Oliveira wrote:
It is possible for a mode set to fail if there aren't shared DPLLS that
match the new configuration requirement or other errors in clock
computation. If that step is executed after disabling crtcs, in the
failure case the hardware configuration is changed and needs to be
restored. Doing those things early will allow the mode set to fail
before actually touching the hardware.
Follow up patches will convert different platforms to use the new
infrastructure.
Signed-off-by: Ander Conselvan de Oliveira
ander.conselvan.de.olive...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/intel_ddi.c | 2 +
drivers/gpu/drm/i915/intel_display.c | 125 +++
3 files changed, 102 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d7412d9..5b2464f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -233,6 +233,8 @@ struct intel_shared_dpll_config {
struct intel_shared_dpll {
struct intel_shared_dpll_config config;
+ struct intel_shared_dpll_config *new_config;
+
int active; /* count of number of active CRTCs (i.e. DPMS on) */
bool on; /* is the PLL actually active? Disabled during modeset */
const char *name;
@@ -480,6 +482,7 @@ struct drm_i915_display_funcs {
struct intel_crtc_config *);
void (*get_plane_config)(struct intel_crtc *,
struct intel_plane_config *);
+ int (*crtc_compute_clock)(struct intel_crtc *crtc);
int (*crtc_mode_set)(struct intel_crtc *crtc,
int x, int y,
struct drm_framebuffer *old_fb);
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 5915a07..7b8c4b8 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1375,6 +1375,8 @@ static void hsw_shared_dplls_init(struct drm_i915_private
*dev_priv)
dev_priv-num_shared_dpll = 2;
for (i = 0; i dev_priv-num_shared_dpll; i++) {
+ dev_priv-shared_dplls[i].new_config =
+ dev_priv-shared_dplls[i].config;
dev_priv-shared_dplls[i].id = i;
dev_priv-shared_dplls[i].name = hsw_ddi_pll_names[i];
dev_priv-shared_dplls[i].disable = hsw_ddi_pll_disable;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index cdaf8ef..f2f7e8f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3851,15 +3851,9 @@ void intel_put_shared_dpll(struct intel_crtc *crtc)
struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
{
struct drm_i915_private *dev_priv = crtc-base.dev-dev_private;
- struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
+ struct intel_shared_dpll *pll;
enum intel_dpll_id i;
- if (pll) {
- DRM_DEBUG_KMS(CRTC:%d dropping existing %s\n,
- crtc-base.base.id, pll-name);
- intel_put_shared_dpll(crtc);
- }
-
if (HAS_PCH_IBX(dev_priv-dev)) {
/* Ironlake PCH has a fixed PLL-PCH pipe mapping. */
i = (enum intel_dpll_id) crtc-pipe;
@@ -3868,7 +3862,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct
intel_crtc *crtc)
DRM_DEBUG_KMS(CRTC:%d using pre-allocated %s\n,
crtc-base.base.id, pll-name);
- WARN_ON(pll-config.crtc_mask);
+ WARN_ON(pll-new_config-crtc_mask);
goto found;
}
@@ -3877,17 +3871,16 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct
intel_crtc *crtc)
pll = dev_priv-shared_dplls[i];
/* Only want to check enabled timings first */
- if (pll-config.crtc_mask == 0)
+ if (pll-new_config-crtc_mask == 0)
continue;
- if (memcmp(crtc-config.dpll_hw_state,
- pll-config.hw_state,
- sizeof(pll-config.hw_state)) == 0) {
- DRM_DEBUG_KMS(CRTC:%d sharing existing %s
- (crtc_mask 0x%08x, active %d)\n,
+ if (memcmp(crtc-new_config-dpll_hw_state,
+ pll-new_config-hw_state,
+ sizeof(pll-new_config-hw_state)) == 0) {
+ DRM_DEBUG_KMS(CRTC:%d sharing existing %s (crtc mask
0x%08x, ative %d)\n,
crtc-base.base.id, pll-name,
- pll-config.crtc_mask, pll-active);
-
+ pll-new_config-crtc_mask,
Re: [Intel-gfx] Regression in i915 intel_panel_setup_backligh
[This is in reply to https://lkml.org/lkml/2014/10/3/415 - I just don't have the message to reply to.] Daniel, the bug you describe is likely [1]. We're on it. Thanks, Jani. [1] https://bugzilla.kernel.org/show_bug.cgi?id=86551 -- 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 7/8] drm/i915: Create vgpu specific write MMIO to reduce traps
On 10/22/2014 12:40 AM, Daniel Vetter wrote:
On Thu, Oct 16, 2014 at 02:24:27PM +0800, Yu Zhang wrote:
In the virtualized environment, forcewake operations are not
necessory for the driver, because mmio accesses will be trapped
and emulated by the host side, and real forcewake operations are
also done in the host. New mmio write handlers are added to directly
call the __raw_i915_write, therefore will reduce many traps and
increase the overall performance for drivers runing in the VM
with Intel GVT-g enhancement.
Signed-off-by: Yu Zhang yu.c.zh...@linux.intel.com
Signed-off-by: Jike Song jike.s...@intel.com
Signed-off-by: Kevin Tian kevin.t...@intel.com
---
drivers/gpu/drm/i915/intel_uncore.c | 20
1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index d5f39f3..ec6d5ce 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -719,6 +719,14 @@ hsw_write##x(struct drm_i915_private *dev_priv, off_t reg,
u##x val, bool trace)
REG_WRITE_FOOTER; \
}
+#define __vgpu_write(x) \
+static void \
+vgpu_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool
trace) { \
+ REG_WRITE_HEADER; \
+ __raw_i915_write##x(dev_priv, reg, val); \
+ REG_WRITE_FOOTER; \
+}
+
static const u32 gen8_shadowed_regs[] = {
FORCEWAKE_MT,
GEN6_RPNSWREQ,
@@ -813,6 +821,10 @@ __gen4_write(8)
__gen4_write(16)
__gen4_write(32)
__gen4_write(64)
+__vgpu_write(8)
+__vgpu_write(16)
+__vgpu_write(32)
+__vgpu_write(64)
#undef __chv_write
#undef __gen8_write
@@ -820,6 +832,7 @@ __gen4_write(64)
#undef __gen6_write
#undef __gen5_write
#undef __gen4_write
+#undef __vgpu_write
#undef REG_WRITE_FOOTER
#undef REG_WRITE_HEADER
@@ -950,6 +963,13 @@ void intel_uncore_init(struct drm_device *dev)
dev_priv-uncore.funcs.mmio_readq = gen4_read64;
break;
}
+
+ if (intel_vgpu_active(dev)) {
+ dev_priv-uncore.funcs.mmio_writeb = vgpu_write8;
+ dev_priv-uncore.funcs.mmio_writew = vgpu_write16;
+ dev_priv-uncore.funcs.mmio_writel = vgpu_write32;
+ dev_priv-uncore.funcs.mmio_writeq = vgpu_write64;
Someone should write a cool macro which uses prepocessor string
concatenation so that we can compress this all to
ASSIGN_WRITE_MMIO_VFUNCS(vgpu)
Then throw in an ASSIGN_READ_MMIO_VFUNC which looks similarly and this
might actually be pretty. Just an idea for some follow-up cleanup.
-Daniel
Thanks Daniel.
Do you mean something like this:
#define ASSIGN_WRITE_MMIO_VFUNCS(x) \
do {\
dev_priv-uncore.funcs.mmio_writeb = x##_write8;\
dev_priv-uncore.funcs.mmio_writew = x##_write16; \
dev_priv-uncore.funcs.mmio_writel = x##_write32; \
dev_priv-uncore.funcs.mmio_writeq = x##_write64; \
} while (0)
and then we can use ASSIGN_WRITE_MMIO_VFUNCS(hsw) for hsw and
ASSIGN_WRITE_MMIO_VFUNCS(vgpu) for vgpu, etc?
+ }
}
void intel_uncore_fini(struct drm_device *dev)
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4] drm/i915: Add ppgtt create/release trace points
From: Daniele Ceraolo Spurio daniele.ceraolospu...@intel.com These tracepoints are useful for observing the creation and destruction of Full PPGTTs. v4: add DOC information Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospu...@intel.com --- drivers/gpu/drm/i915/i915_gem_gtt.c | 4 +++ drivers/gpu/drm/i915/i915_trace.h | 58 + 2 files changed, 62 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index e0bcba0..ed9ec67 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1174,6 +1174,8 @@ i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv) ppgtt-file_priv = fpriv; + trace_i915_ppgtt_create(ppgtt-base); + return ppgtt; } @@ -1182,6 +1184,8 @@ void i915_ppgtt_release(struct kref *kref) struct i915_hw_ppgtt *ppgtt = container_of(kref, struct i915_hw_ppgtt, ref); + trace_i915_ppgtt_release(ppgtt-base); + /* vmas should already be unbound */ WARN_ON(!list_empty(ppgtt-base.active_list)); WARN_ON(!list_empty(ppgtt-base.inactive_list)); diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index f5aa006..6cf5ca2 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -15,6 +15,14 @@ #define TRACE_SYSTEM_STRING __stringify(TRACE_SYSTEM) #define TRACE_INCLUDE_FILE i915_trace +/** + * DOC: i915 tracepoints + * + * This file has the fancy tracepoints available in the drm/i915 driver. + * I encourage people to update theirs! + * + */ + /* pipe updates */ TRACE_EVENT(i915_pipe_update_start, @@ -587,6 +595,56 @@ TRACE_EVENT(intel_gpu_freq_change, TP_printk(new_freq=%u, __entry-freq) ); +/** + * DOC: i915_ppgtt_create and i915_ppgtt_release tracepoints + * + * With full ppgtt enabled each process using drm will allocate at least one + * translation table. With these traces it is possible to keep track of the + * allocation and of the lifetime of the tables; this can be used during + * testing/debug to verify that we are not leaking ppgtts. + * These traces identify the ppgtt through the vm pointer, which is also printed + * by the i915_vma_bind and i915_vma_unbind tracepoints. + */ +TRACE_EVENT(i915_ppgtt_create, + TP_PROTO(struct i915_address_space *vm), + + TP_ARGS(vm), + + TP_STRUCT__entry( + __field(struct i915_address_space *, vm) + __field(u32, dev) + __field(int, pid) + ), + + TP_fast_assign( + __entry-vm = vm; + __entry-dev = vm-dev-primary-index; + __entry-pid = (int)task_pid_nr(current); + ), + + TP_printk(dev=%u, task_pid=%d, vm=%p, + __entry-dev, __entry-pid, __entry-vm) +); + +TRACE_EVENT(i915_ppgtt_release, + + TP_PROTO(struct i915_address_space *vm), + + TP_ARGS(vm), + + TP_STRUCT__entry( + __field(struct i915_address_space *, vm) + __field(u32, dev) + ), + + TP_fast_assign( + __entry-vm = vm; + __entry-dev = vm-dev-primary-index; + ), + + TP_printk(dev=%u, vm=%p, __entry-dev, __entry-vm) +); + #endif /* _I915_TRACE_H_ */ /* This part must be outside protection */ -- 1.8.5.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915: Fix GMBUSFREQ on vlv/chv
On Fri, 17 Oct 2014, Jani Nikula jani.nik...@linux.intel.com wrote: On Thu, 16 Oct 2014, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com vlv_cdclk_freq is in kHz but we need MHz for the GMBUSFREQ divider. This is a regression from: commit f8bf63fdcb1f82459dae7a3f22ee5ce92f3ea727 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Fri Jun 13 13:37:54 2014 +0300 drm/i915: Kill duplicated cdclk readout code from i2c cc: stable Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Pushed to drm-intel-fixes, thanks for the patch. 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] [PATCH 2/2] drm/i915: Ignore long hpds on eDP ports
On Tue, 21 Oct 2014, Daniel Vetter dan...@ffwll.ch wrote: On Fri, Oct 17, 2014 at 09:08:28AM -0700, Todd Previte wrote: On 10/17/2014 1:43 AM, Ville Syrjälä wrote: On Thu, Oct 16, 2014 at 12:38:55PM -0700, Todd Previte wrote: On 10/16/2014 10:46 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Turning vdd on/off can generate a long hpd pulse on eDP ports. In order to handle hpd we would need to turn on vdd to perform aux transfers. This would lead to an endless cycle of vdd off - long hpd - vdd on - detect - vdd off - ... So ignore long hpd pulses on eDP ports. eDP panels should be physically tied to the machine anyway so they should not actually disappear and thus don't need long hpd handling. Short hpds are still needed for link re-train and whatnot so we can't just turn off the hpd interrupt entirely for eDP ports. Perhaps we could turn it off whenever the panel is disabled, but just ignoring the long hpd seems sufficient. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com ... Anyway, Cc: sta...@vger.kernel.org and one for Jani. Pushed both patches to drm-intel-fixes, thanks for the patches and review. 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] [PATCH] drm/i915: Improve reliability for Displayport link training
On Thu, 09 Oct 2014, Todd Previte tprev...@gmail.com wrote:
Link training for Displayport can fail in many ways and at multiple different
points
during the training process. Previously, errors were logged but no additional
action
was taken based on them. Consequently, training attempts could continue even
after
errors have occured that would prevent successful link training. This patch
updates
the link training functions and where/how they're used to be more intelligent
about
failures and to stop trying to train the link when it's a lost cause.
Signed-off-by: Todd Previte tprev...@gmail.com
---
drivers/gpu/drm/i915/intel_ddi.c | 25 ++--
drivers/gpu/drm/i915/intel_dp.c | 88
++--
drivers/gpu/drm/i915/intel_drv.h | 10 +++--
3 files changed, 95 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c
b/drivers/gpu/drm/i915/intel_ddi.c
index cb5367c..718240f 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1119,6 +1119,8 @@ static void intel_ddi_pre_enable(struct intel_encoder
*intel_encoder)
struct intel_crtc *crtc = to_intel_crtc(encoder-crtc);
enum port port = intel_ddi_get_encoder_port(intel_encoder);
int type = intel_encoder-type;
+ uint8_t fail_code = 0;
+ char *msg;
if (crtc-config.has_audio) {
DRM_DEBUG_DRIVER(Audio on pipe %c on DDI\n,
@@ -1143,10 +1145,22 @@ static void intel_ddi_pre_enable(struct intel_encoder
*intel_encoder)
intel_ddi_init_dp_buf_reg(intel_encoder);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
- intel_dp_start_link_train(intel_dp);
- intel_dp_complete_link_train(intel_dp);
+ if (!intel_dp_start_link_train(intel_dp)) {
+ fail_code = INTEL_DP_CLOCKREC_FAILED;
+ msg = clock recovery;
+ goto failed;
+ }
+ if (!intel_dp_complete_link_train(intel_dp)) {
+ fail_code = INTEL_DP_CHANNELEQ_FAILED;
+ msg = channel equalization;
+ goto failed;
+ }
if (port != PORT_A)
- intel_dp_stop_link_train(intel_dp);
+ if (!intel_dp_stop_link_train(intel_dp)) {
+ fail_code = INTEL_DP_STOPTRAIN_FAILED;
+ msg = stop training;
+ goto failed;
+ }
} else if (type == INTEL_OUTPUT_HDMI) {
struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
@@ -1154,6 +1168,11 @@ static void intel_ddi_pre_enable(struct intel_encoder
*intel_encoder)
crtc-config.has_hdmi_sink,
crtc-config.adjusted_mode);
}
+
+ return;
+
+failed:
+ DRM_DEBUG_KMS(Failed to pre-enable DP, fail code %d\n, fail_code);
}
static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 64c8e04..a8352c4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2538,18 +2538,38 @@ static void intel_enable_dp(struct intel_encoder
*encoder)
struct drm_device *dev = encoder-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
uint32_t dp_reg = I915_READ(intel_dp-output_reg);
+ uint8_t fail_code = 0;
+ char *msg;
if (WARN_ON(dp_reg DP_PORT_EN))
- return;
+ return false;
intel_dp_enable_port(intel_dp);
intel_edp_panel_vdd_on(intel_dp);
intel_edp_panel_on(intel_dp);
intel_edp_panel_vdd_off(intel_dp, true);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
- intel_dp_start_link_train(intel_dp);
- intel_dp_complete_link_train(intel_dp);
- intel_dp_stop_link_train(intel_dp);
+ if (!intel_dp_start_link_train(intel_dp)) {
+ fail_code = INTEL_DP_CLOCKREC_FAILED;
+ msg = clock recovery;
+ goto failed;
+ }
+ if (!intel_dp_complete_link_train(intel_dp)) {
+ fail_code = INTEL_DP_CHANNELEQ_FAILED;
+ msg = channel equalization;
+ goto failed;
+ }
+ if (!intel_dp_stop_link_train(intel_dp)) {
+ fail_code = INTEL_DP_STOPTRAIN_FAILED;
+ msg = stop training;
+ goto failed;
+ }
In general I like failing fast and bailing out. However the users
probably like it better if we limp on and keep trying to get a picture
on screen. It's also much less stressful to work on bugs that cause a
warn in the logs instead of a black screen.
Case in point [1] which would end up in a black screen with this
patch. Unless we've managed to fix it by now.
BR,
Re: [Intel-gfx] [PATCH 08/17] drm/i915: Wait for PHY port ready before link training on VLV/CHV
On 10/16/2014 11:27 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com There's no point in checking if the data lanes came out of reset after link training. If the data lanes aren't ready link training will fail anyway. Suggested-by: Todd Previte tprev...@gmail.com Cc: Todd Previte tprev...@gmail.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- If Todd has a better patch with better description we can use that one instead of my version. But at least I think the spot where I put the vlv_wait_port_ready() is the right one. We could perhaps skip the link training attempt entirely if the port is already stuck. drivers/gpu/drm/i915/intel_dp.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 4c8f169..6f568b4 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2550,6 +2550,9 @@ static void intel_enable_dp(struct intel_encoder *encoder) pps_unlock(intel_dp); + if (IS_VALLEYVIEW(dev)) + vlv_wait_port_ready(dev_priv, dp_to_dig_port(intel_dp)); + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); intel_dp_start_link_train(intel_dp); intel_dp_complete_link_train(intel_dp); @@ -2689,8 +2692,6 @@ static void vlv_pre_enable_dp(struct intel_encoder *encoder) mutex_unlock(dev_priv-dpio_lock); intel_enable_dp(encoder); - - vlv_wait_port_ready(dev_priv, dport); } static void vlv_dp_pre_pll_enable(struct intel_encoder *encoder) @@ -2783,8 +2784,6 @@ static void chv_pre_enable_dp(struct intel_encoder *encoder) mutex_unlock(dev_priv-dpio_lock); intel_enable_dp(encoder); - - vlv_wait_port_ready(dev_priv, dport); } static void chv_dp_pre_pll_enable(struct intel_encoder *encoder) We should definitely skip link training if the PHYs are down. There's going to be a WARN when wait_port_ready() fails so we'll be well aware that something went wrong. Spamming dmesg with errors/WARNs from trying to train the link after that is really counterproductive, since we already know there's no way link training could succeed. -T ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH V2] drm/i915: Change order of operations for VLV/CHV to not train DP link before PHYs are ready
On 10/21/2014 7:41 AM, Daniel Vetter wrote: On Fri, Oct 17, 2014 at 11:41:12AM -0700, Todd Previte wrote: V2 changes:-+ - Moved the intel_dp_enable_port() call out of intel_dp_enable() and placed it before the calls to intel_dp_enable() and vlv_wait_port_ready() - Cleaned up a spacing issues with the code indents - Amended the commit message to be under 80 characters per line and expanded on the description of what the patch does The per-patch commit log should be part of the commit message, above the sob section. Some kernel maintainers want it below claiming it's noise, but I disagree. In any case it needs to be part of the patch when submitting it. The cover letter changelog is just for the big stuff spawning more than one patch when you have a big series. -Daniel Ville's patch (patch 07/17) in the CHV PPS fix sequence is going to pick up most of the necessary changes that are included here. The one aspect that is not covered is that link training needs to be skipped when the PHYs are down. If that change is integrated into his patch, this patch is not necessary. Otherwise, this patch will need to be updated to accommodate that change. -T ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page
On Tue, Oct 21, 2014 at 08:26:05AM -0700, Daniel Vetter wrote:
On Wed, Oct 15, 2014 at 02:52:41PM -0700, bradley.d.vol...@intel.com wrote:
From: Brad Volkin bradley.d.vol...@intel.com
The size of the batch buffer passed to the kernel is significantly
larger than the size of the batch buffer passed to the function. A
proposed optimization as part of the batch copy kernel series is to
use batch_len for the copy and parse operations, which leads to a
false batch without MI_BATCH_BUFFER_END failure for this test.
Signed-off-by: Brad Volkin bradley.d.vol...@intel.com
---
tests/gem_exec_parse.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index 05f271c..568bd4a 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -144,9 +144,10 @@ static void exec_split_batch(int fd, uint32_t *cmds,
struct drm_i915_gem_exec_object2 objs[1];
uint32_t cmd_bo;
uint32_t noop[1024] = { 0 };
+ const int alloc_size = 4096 * 2;
// Allocate and fill a 2-page batch with noops
- cmd_bo = gem_create(fd, 4096 * 2);
+ cmd_bo = gem_create(fd, alloc_size);
gem_write(fd, cmd_bo, 0, noop, sizeof(noop));
gem_write(fd, cmd_bo, 4096, noop, sizeof(noop));
@@ -167,7 +168,7 @@ static void exec_split_batch(int fd, uint32_t *cmds,
execbuf.buffers_ptr = (uintptr_t)objs;
execbuf.buffer_count = 1;
execbuf.batch_start_offset = 0;
Shouldn't we simply adjust batch_start_offset to be 4096-4, i.e. where
we've placed the batch? Would have the benifit of also testing offset
batches ;-)
-Daniel
Good suggestion. I'll go with that approach instead.
Brad
- execbuf.batch_len = size;
+ execbuf.batch_len = alloc_size;
execbuf.cliprects_ptr = 0;
execbuf.num_cliprects = 0;
execbuf.DR1 = 0;
--
1.9.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 v4] drm/i915: Add ppgtt create/release trace points
On Wed, Oct 22, 2014 at 02:28:45PM +0100, daniele.ceraolospu...@intel.com wrote: From: Daniele Ceraolo Spurio daniele.ceraolospu...@intel.com These tracepoints are useful for observing the creation and destruction of Full PPGTTs. v4: add DOC information Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospu...@intel.com --- drivers/gpu/drm/i915/i915_gem_gtt.c | 4 +++ drivers/gpu/drm/i915/i915_trace.h | 58 + 2 files changed, 62 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index e0bcba0..ed9ec67 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1174,6 +1174,8 @@ i915_ppgtt_create(struct drm_device *dev, struct drm_i915_file_private *fpriv) ppgtt-file_priv = fpriv; + trace_i915_ppgtt_create(ppgtt-base); + return ppgtt; } @@ -1182,6 +1184,8 @@ void i915_ppgtt_release(struct kref *kref) struct i915_hw_ppgtt *ppgtt = container_of(kref, struct i915_hw_ppgtt, ref); + trace_i915_ppgtt_release(ppgtt-base); + /* vmas should already be unbound */ WARN_ON(!list_empty(ppgtt-base.active_list)); WARN_ON(!list_empty(ppgtt-base.inactive_list)); diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index f5aa006..6cf5ca2 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -15,6 +15,14 @@ #define TRACE_SYSTEM_STRING __stringify(TRACE_SYSTEM) #define TRACE_INCLUDE_FILE i915_trace +/** + * DOC: i915 tracepoints + * + * This file has the fancy tracepoints available in the drm/i915 driver. + * I encourage people to update theirs! I think we cand drop this part, but to make these DOC sections really useful you also need to pull them into a new Tracing section in the i915 part of the drm DocBook. See Documentation/DocBook/drm.tmpl and lock at the recent commits from i915 people for examples. -Daniel + * + */ + /* pipe updates */ TRACE_EVENT(i915_pipe_update_start, @@ -587,6 +595,56 @@ TRACE_EVENT(intel_gpu_freq_change, TP_printk(new_freq=%u, __entry-freq) ); +/** + * DOC: i915_ppgtt_create and i915_ppgtt_release tracepoints + * + * With full ppgtt enabled each process using drm will allocate at least one + * translation table. With these traces it is possible to keep track of the + * allocation and of the lifetime of the tables; this can be used during + * testing/debug to verify that we are not leaking ppgtts. + * These traces identify the ppgtt through the vm pointer, which is also printed + * by the i915_vma_bind and i915_vma_unbind tracepoints. + */ +TRACE_EVENT(i915_ppgtt_create, + TP_PROTO(struct i915_address_space *vm), + + TP_ARGS(vm), + + TP_STRUCT__entry( + __field(struct i915_address_space *, vm) + __field(u32, dev) + __field(int, pid) + ), + + TP_fast_assign( + __entry-vm = vm; + __entry-dev = vm-dev-primary-index; + __entry-pid = (int)task_pid_nr(current); + ), + + TP_printk(dev=%u, task_pid=%d, vm=%p, + __entry-dev, __entry-pid, __entry-vm) +); + +TRACE_EVENT(i915_ppgtt_release, + + TP_PROTO(struct i915_address_space *vm), + + TP_ARGS(vm), + + TP_STRUCT__entry( + __field(struct i915_address_space *, vm) + __field(u32, dev) + ), + + TP_fast_assign( + __entry-vm = vm; + __entry-dev = vm-dev-primary-index; + ), + + TP_printk(dev=%u, vm=%p, __entry-dev, __entry-vm) +); + #endif /* _I915_TRACE_H_ */ /* This part must be outside protection */ -- 1.8.5.2 ___ 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 4/4] drm/i915: use current mode if the size matches the preferred mode
On Tue, 21 Oct 2014 16:53:02 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Thu, Oct 09, 2014 at 12:57:45PM -0700, Jesse Barnes wrote: From: Kristian Høgsberg hoegsb...@gmail.com The BIOS may set a native mode that doesn't quite match the preferred mode timings. It should be ok to use however if it uses the same size, so try to avoid a mode set in that case. Signed-off-by: Kristian Høgsberg hoegsb...@gmail.com Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org Why exactly does this fail? Is the clock off slightly or are the timings off? I just wonder whether we should check vrefresh to make sure the bios doesn't sneak a low refresh rate mode past us (from stuck drrs or whatever). /me thinking a bit paranoid again. Yeah you asked this same thing last time. IIRC it's not a lower clock rate like DRRS, but rather different timings. But Kristian will have to post them. Kristian, can you please post the mode lines from the EDID on this machine and the ones the BIOS sets at boot up? Thanks, -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Abort command parsing for chained batches
[snip]
On Tue, Oct 21, 2014 at 08:50:33AM -0700, Daniel Vetter wrote:
On Thu, Oct 16, 2014 at 12:24:42PM -0700, bradley.d.vol...@intel.com wrote:
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1a0611b..1ed5702 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1368,17 +1368,19 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
batch_obj,
args-batch_start_offset,
file-is_master);
- if (ret)
- goto err;
-
- /*
-* XXX: Actually do this when enabling batch copy...
-*
-* Set the DISPATCH_SECURE bit to remove the NON_SECURE bit
-* from MI_BATCH_BUFFER_START commands issued in the
-* dispatch_execbuffer implementations. We specifically don't
-* want that set when the command parser is enabled.
-*/
+ if (ret) {
+ if (ret != -EACCES)
+ goto err;
+ } else {
+ /*
+* XXX: Actually do this when enabling batch copy...
+*
+* Set the DISPATCH_SECURE bit to remove the NON_SECURE
bit
+* from MI_BATCH_BUFFER_START commands issued in the
+* dispatch_execbuffer implementations. We specifically
don't
+* want that set when the command parser is enabled.
+*/
+ }
Tbh this hunk here confuses me ... Why do we need to change anything here?
Yeah, it makes more sense with the batch copy code, it's just that this
patch has to go in before the patch where we set I915_DISPATCH_SECURE.
The final logic basically goes like this:
ret = i915_parse_cmds()
if ret == 0
dispatch shadow_batch_obj, flags = I915_DISPATCH_SECURE
else if ret == -EACCES // i.e. i915_parse_cmds() found an MI_BB_S
dispatch batch_obj, flags = 0
else
return error
The point is that there's a restriction that chained batches must have
the AddressSpace bit set to the same value as the parent batch (i.e.
GGTT when batch copy is present). But because of the way libva uses
chained batches we can't parse or copy the chained batch to safely put
it into GGTT. So we fall back to dispatching the userspace-supplied
batch from PPGTT. I should probably have mentioned this restriction in
the commit message.
As you suggest below, we could instead start to conditionally check
batches based on the flag from userspace. However, I thought we had
decided not to take that approach in general. Mesa already implements
all of the code that they need the command parser for, with a runtime
check as to whether hardware will nop their LRI, etc commands. So if
we run the parser on all batches, then once we switch to enabling mode
features magically work for them. Also, the I915_EXEC_SECURE flag is
currently root-only, so there's a bit of a semantic/API/whatever change
that we'd have to make there, or add a new flag I suppose. Maybe not a
big deal, but I think that the choice of running the parser on all
batches is the right direction.
Brad
And since we we currently scan batches unconditionally: Shouldn't we
filter out the -EACCESS at a higher level?
In the end I imagine the logic will be:
a) Userspace asks for secure batch
- Scan and reject or copy and run.
b) Userspace asks for normal batch
- Don't scan, but run without additional hw privs.
Or am I again completely missing the point?
Thanks, Daniel
}
/* snb/ivb/vlv conflate the batch in ppgtt bit with the non-secure
--
1.9.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
[Intel-gfx] [PATCH] drm/i915: Emit even number of dwords when emitting LRIs
The number of DWords should be even when doing ring emits as
command sequences require QWord alignment.
v2: user LRI variant that can write multiple regs in one go (Damien).
We can simply insert one NOP at the end instead of one per register write.
Cc: Mika Kuoppala mika.kuopp...@intel.com
Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 497b836..a8f72e8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -680,15 +680,16 @@ static int intel_ring_workarounds_emit(struct
intel_engine_cs *ring)
if (ret)
return ret;
- ret = intel_ring_begin(ring, w-count * 3);
+ ret = intel_ring_begin(ring, (w-count * 2 + 2));
if (ret)
return ret;
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(w-count));
for (i = 0; i w-count; i++) {
- intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
intel_ring_emit(ring, w-reg[i].addr);
intel_ring_emit(ring, w-reg[i].value);
}
+ intel_ring_emit(ring, MI_NOOP);
intel_ring_advance(ring);
--
2.1.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Regression: audit: x86: drop arch from __audit_syscall_entry() interface
Hi (Cc'ing everybody mentioned in the original patch) I work for Intel, on our Linux Graphics driver - aka i915.ko - and our QA team recently reported a regression on: commit b4f0d3755c5e9cc86292d5fd78261903b4f23d4a Author: Richard Guy Briggs Date: Tue Mar 4 10:38:06 2014 -0500 audit: x86: drop arch from __audit_syscall_entry() interface According to our QA, their i386 machine doesn't boot anymore. I tried to write my own revert for the patch, asked QA to test, and they confirmed it solves the problem. Here are the details of QA' s bug report: https://bugs.freedesktop.org/show_bug.cgi?id=85277 . The trees our QA tests are the development trees from i915.ko: http://cgit.freedesktop.org/drm-intel?h=drm-intel-fixes . I tried searching for other bug reports on the same patch, but couldn't find any. Forgive me if this bug was already reported. Feel free to continue this discussion on the bugzilla report if you want. Thanks, Paulo -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Regression: audit: x86: drop arch from __audit_syscall_entry() interface
That's really serious. Looking now. On Wed, 2014-10-22 at 16:08 -0200, Paulo Zanoni wrote: Hi (Cc'ing everybody mentioned in the original patch) I work for Intel, on our Linux Graphics driver - aka i915.ko - and our QA team recently reported a regression on: commit b4f0d3755c5e9cc86292d5fd78261903b4f23d4a Author: Richard Guy Briggs Date: Tue Mar 4 10:38:06 2014 -0500 audit: x86: drop arch from __audit_syscall_entry() interface According to our QA, their i386 machine doesn't boot anymore. I tried to write my own revert for the patch, asked QA to test, and they confirmed it solves the problem. Here are the details of QA' s bug report: https://bugs.freedesktop.org/show_bug.cgi?id=85277 . The trees our QA tests are the development trees from i915.ko: http://cgit.freedesktop.org/drm-intel?h=drm-intel-fixes . I tried searching for other bug reports on the same patch, but couldn't find any. Forgive me if this bug was already reported. Feel free to continue this discussion on the bugzilla report if you want. Thanks, Paulo ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] lib: fix #define max
On Tue, 21 Oct 2014, Mika Kuoppala mika.kuopp...@linux.intel.com wrote: Regression from: commit be4710a541b517b5f8663448bffed5656d59b47b Author: Thomas Wood thomas.w...@intel.com Date: Fri Oct 10 11:20:35 2014 +0100 lib: add common min and max macros Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85218 Tested-by: Guo Jinxian jinxianx@intel.com Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com Push it already! --- lib/igt_aux.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/igt_aux.h b/lib/igt_aux.h index 415614b..9b42918 100644 --- a/lib/igt_aux.h +++ b/lib/igt_aux.h @@ -85,6 +85,6 @@ bool intel_check_memory(uint32_t count, uint32_t size, unsigned mode); #define min(a, b) ((a) (b) ? (a) : (b)) -#define max(a, b) ((a) (b) ? (a) : (b)) +#define max(a, b) ((a) (b) ? (a) : (b)) #endif /* IGT_AUX_H */ -- 1.9.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 2/2] drm/i915: only run hsw_power_well_post_enable when really needed
On Tue, Oct 07, 2014 at 11:00:54PM +0300, Ville Syrjälä wrote: On Tue, Oct 07, 2014 at 04:11:11PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Only run it after we actually enable the power well. When we're booting the machine there are cases where we run hsw_power_well_post_enable without really needing, and even though this is not causing any real bugs, it is unneeded and causes confusion to people debugging interrupts. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Seems perfectly sensible. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Queued for -next, thanks for the patch. -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] i915.fastboot bug report - not working on coreboot
Hello Sorry for the late reply. On Thu, Sep 11, 2014 at 2:36 PM, Jesse Barnes jbar...@virtuousgeek.org wrote: Your config looks ok, but it sounds like the i915 driver may be doing a full mode set. The flickering suggested me it did. Doing a drm.debug=6 with fastboot enabled should give us clues about why in the dmesg. Here is the result of the exact same kernel, run in the exact same conditions (same coreboot version, etc). I'm sorry but I'm not familiar enough with video issues to understand why it does a full mode set (it could be failed to find VBIOS tables / no _DSM method for intel device) Let me know if you need anything else. [0.213810] Linux agpgart interface v0.103 [0.213860] agpgart-intel :00:00.0: Intel 945GM Chipset [0.213902] agpgart-intel :00:00.0: detected gtt size: 262144K total, 262144K mappable [0.214477] agpgart-intel :00:00.0: detected 8192K stolen memory [0.214829] agpgart-intel :00:00.0: AGP aperture is 256M @ 0xd000 [0.214870] Hangcheck: starting hangcheck timer 0.9.1 (tick is 180 seconds, margin is 60 seconds). [0.214872] Hangcheck: Using getrawmonotonic(). [0.214899] [drm] Initialized drm 1.1.0 20060810 [0.215413] [drm:i915_dump_device_info], i915 device info: gen=3, pciid=0x27a2 flags=is_mobile,is_i945gm,has_hotplug,cursor_needs_physical,has_overlay,overlay_needs_physical,supports_tv, [0.215605] [drm] Memory usable by graphics device = 256M [0.215608] [drm:i915_gem_gtt_init], GMADR size = 256M [0.215611] [drm:i915_gem_gtt_init], GTT stolen size = 8M [0.215618] i915 :00:02.0: setting latency timer to 64 [0.217121] [drm:intel_opregion_setup], graphic opregion physical addr: 0x0 [0.217127] [drm:intel_opregion_setup], ACPI OpRegion not supported! [0.217155] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010). [0.217178] [drm] Driver supports precise vblank timestamp query. [0.217180] [drm:init_vbt_defaults], Set default to SSC at 100MHz [0.217188] i915 :00:02.0: Invalid ROM contents [0.217194] [drm:intel_parse_bios], VBT signature missing [0.217216] [drm] failed to find VBIOS tables [0.217271] [drm:intel_dsm_pci_probe], no _DSM method for intel device [0.217311] [drm:intel_modeset_init], 2 display pipes available. [0.217317] [drm:intel_crtc_init], swapping pipes planes for FBC [0.217320] [drm:intel_modeset_init], pipe 0 plane 0 init failed: -19 [0.217325] [drm:intel_crtc_init], swapping pipes planes for FBC [0.217327] [drm:intel_modeset_init], pipe 1 plane 0 init failed: -19 [0.217330] [drm:intel_pch_pll_init], No PCH PLLs on this hardware, skipping initialisation [0.217336] vgaarb: device changed decodes: PCI::00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem [0.226977] ACPI: Deprecated procfs I/F for battery is loaded, please retry with CONFIG_ACPI_PROCFS_POWER cleared [0.226987] ACPI: Battery Slot [BAT0] (battery present) [0.227106] ACPI: Deprecated procfs I/F for battery is loaded, please retry with CONFIG_ACPI_PROCFS_POWER cleared [0.227115] ACPI: Battery Slot [BAT1] (battery absent) [0.268013] [drm] GMBUS [i915 gmbus panel] timed out, falling back to bit banging on pin 3 [0.296144] [drm:intel_lvds_init], using preferred mode from EDID: [0.296147] [drm:drm_mode_debug_printmodeline], Modeline 8:1024x768 60 65000 1024 1048 1184 1344 768 771 777 806 0x48 0xa [0.296155] [drm:intel_lvds_init], detected single-link lvds configuration [0.296192] [drm:intel_panel_get_backlight], get backlight PWM = 1 [0.296195] [drm:intel_panel_setup_backlight], Failed to get maximum backlight value [0.296268] [drm:i915_gem_setup_global_gtt], clearing unused GTT space: [0, 000] [0.298950] [drm] initialized overlay support [0.298954] [drm:intel_modeset_setup_hw_state], [CRTC:3] hw state readout: disabled [0.298957] [drm:intel_modeset_setup_hw_state], [CRTC:4] hw state readout: enabled [0.298961] [drm:intel_modeset_setup_hw_state], [ENCODER:6:LVDS-6] hw state readout: enabled, pipe=1 [0.298964] [drm:intel_modeset_setup_hw_state], [ENCODER:15:DAC-15] hw state readout: disabled, pipe=0 [0.298968] [drm:intel_modeset_setup_hw_state], [ENCODER:17:TV-17] hw state readout: disabled, pipe=0 [0.298972] [drm:intel_modeset_setup_hw_state], [CONNECTOR:5:LVDS-1] hw state readout: enabled [0.298976] [drm:intel_modeset_setup_hw_state], [CONNECTOR:14:VGA-1] hw state readout: disabled [0.298979] [drm:intel_modeset_setup_hw_state], [CONNECTOR:16:SVIDEO-1] hw state readout: disabled [0.298986] [drm:intel_connector_check_state], [CONNECTOR:5:LVDS-1] [0.298990] [drm:intel_modeset_check_state], [ENCODER:6:LVDS-6] [0.298993] [drm:intel_modeset_check_state], [ENCODER:15:DAC-15] [0.298996] [drm:intel_modeset_check_state], [ENCODER:17:TV-17] [0.298999] [drm:intel_modeset_check_state], [CRTC:3] [0.299011] [drm:intel_modeset_check_state], [CRTC:4] [
Re: [Intel-gfx] [PATCH] drm/i915: run intel_uncore_early_sanitize earlier on resume on non-VLV
2014-10-22 9:20 GMT-02:00 Imre Deak imre.d...@intel.com:
On Tue, 2014-10-21 at 19:05 +0200, Daniel Vetter wrote:
On Mon, Oct 20, 2014 at 01:20:50PM +0300, Imre Deak wrote:
On Fri, 2014-10-17 at 16:01 -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
As far as I understand, intel_uncore_early_sanitize() was supposed to
be ran before any register access, but currently
intel_resume_prepare() is ran earlier, and it does register
access. I don't think it should be safe to be calling
I915_{READ,WRITE} without calling intel_uncore_early_sanitize() first.
One of the problems we currently have is that when we suspend/resume
BDW, the FPGA_DBG_RM_NOCLAIM bit becomes 1, so we end up printing an
unclaimed register message on resume, but this message doesn't
really seem to have been triggered by our driver or user space, since
the bit was not there before suspending, and gets there just after
resuming, before any of our own register accesses. So calling
intel_uncore_early_sanitize() as a first thing will allow us to stop
printing the error message, fixing the bug.
v2: VLV is an exception to the early_sanitize() rule: it needs to do
stuff before calling early_sanitize(), so instead of calling it
earlier for every platform, we call it earlier for non-VLV by adding
the early_sanitize() call inside intel_resume_prepare(). This doesn't
look like the most-beautiful-solution-ever, but, well, at least it
fixes the bug. (Imre)
Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Imre Deak imre.d...@intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83094
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index a05a1d0..f6d28f2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -669,7 +669,6 @@ static int i915_drm_thaw_early(struct drm_device
*dev)
if (ret)
DRM_ERROR(Resume prepare failed: %d,Continuing resume\n,
ret);
- intel_uncore_early_sanitize(dev, true);
intel_uncore_sanitize(dev);
intel_power_domains_init_hw(dev_priv);
@@ -1049,6 +1048,8 @@ static int snb_resume_prepare(struct
drm_i915_private *dev_priv,
if (rpm_resume)
intel_init_pch_refclk(dev);
+ else
+ intel_uncore_early_sanitize(dev, true);
return 0;
}
@@ -1056,6 +1057,9 @@ static int snb_resume_prepare(struct
drm_i915_private *dev_priv,
static int hsw_resume_prepare(struct drm_i915_private *dev_priv,
bool rpm_resume)
{
+ if (!rpm_resume)
+ intel_uncore_early_sanitize(dev_priv-dev, true);
+
hsw_disable_pc8(dev_priv);
return 0;
@@ -1421,6 +1425,9 @@ static int vlv_resume_prepare(struct
drm_i915_private *dev_priv,
i915_gem_restore_fences(dev);
}
+ if (!rpm_resume)
+ intel_uncore_early_sanitize(dev, true);
+
return ret;
}
You also need to call intel_uncore_early_sanitize() from
intel_resume_prepare() for the rest of the platforms. With that fixed:
Reviewed-by: Imre Deak imre.d...@intel.com
Looking at the result, I agree it's not the nicest, so yet another way
to reduce the clutter would be to have the following instead in
i915_drm_thaw_early():
intel_resume_early_prepare()
intel_uncore_early_sanitize()
intel_resume_prepare()
and do the early steps for VLV in intel_resume_early_prepare(). I'm ok
with both solutions.
This honestly starts to smell like a giant maintenance nightmare. We kinda
started off into the wrong direction with vlv rpm and it seems to get
worse by the day. And it looks like the situation is messy enough that we
can't even look down the ordering with copious amounts of warnings ...
But I also don't see any real solution, so just ranting for now. I'd
appreciate though if the revised version comes with a bunch of comments
attached in the code.
I blame it on the HW people. :) Seriously, the VLV PM code differs from
the rest of PM code in that we save/restore some HW state instead of
reinitializing it. That's where the above special casing of the ordering
stems from. I agree that it's not ideal, but I think having started with
that solution and moving towards the ideal was not that bad. In fact
s0ix doesn't yet work in the upstream kernel for reasons independent of
i915 (or at least I couldn't make it work), but we would need it to
fully validate all the suspend/resume paths.
On a side note, even igt/pm_rpm/rte (the basic subtest) seems to be
broken on BYT since forever (at least according to QA, bug #82939), so
do we even want RPM enabled on BYT?
--Imre
--
Paulo Zanoni
___
Re: [Intel-gfx] Regression: audit: x86: drop arch from __audit_syscall_entry() interface
On 10/22/2014 11:23 AM, Eric Paris wrote: That's really serious. Looking now. On Wed, 2014-10-22 at 16:08 -0200, Paulo Zanoni wrote: Hi (Cc'ing everybody mentioned in the original patch) I work for Intel, on our Linux Graphics driver - aka i915.ko - and our QA team recently reported a regression on: commit b4f0d3755c5e9cc86292d5fd78261903b4f23d4a Author: Richard Guy Briggs Date: Tue Mar 4 10:38:06 2014 -0500 audit: x86: drop arch from __audit_syscall_entry() interface According to our QA, their i386 machine doesn't boot anymore. I tried to write my own revert for the patch, asked QA to test, and they confirmed it solves the problem. Here are the details of QA' s bug report: https://bugs.freedesktop.org/show_bug.cgi?id=85277 . The trees our QA tests are the development trees from i915.ko: http://cgit.freedesktop.org/drm-intel?h=drm-intel-fixes . I tried searching for other bug reports on the same patch, but couldn't find any. Forgive me if this bug was already reported. Feel free to continue this discussion on the bugzilla report if you want. This piece: movl %esi,4(%esp) /* 5th arg: 4th syscall arg */ movl %edx,(%esp)/* 4th arg: 3rd syscall arg */ looks like it's overwriting syscall arguments. This is clearly fixable, but an even better fix would be to drop the asm entirely and switch to two-phase tracing. Want to do it? I can test the seccomp bits if you switch over the asm :) --Andy ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Regression: audit: x86: drop arch from __audit_syscall_entry() interface
On 14/10/22, Andy Lutomirski wrote: On 10/22/2014 11:23 AM, Eric Paris wrote: That's really serious. Looking now. On Wed, 2014-10-22 at 16:08 -0200, Paulo Zanoni wrote: Hi (Cc'ing everybody mentioned in the original patch) I work for Intel, on our Linux Graphics driver - aka i915.ko - and our QA team recently reported a regression on: commit b4f0d3755c5e9cc86292d5fd78261903b4f23d4a Author: Richard Guy Briggs Date: Tue Mar 4 10:38:06 2014 -0500 audit: x86: drop arch from __audit_syscall_entry() interface According to our QA, their i386 machine doesn't boot anymore. I tried to write my own revert for the patch, asked QA to test, and they confirmed it solves the problem. Here are the details of QA' s bug report: https://bugs.freedesktop.org/show_bug.cgi?id=85277 . The trees our QA tests are the development trees from i915.ko: http://cgit.freedesktop.org/drm-intel?h=drm-intel-fixes . I tried searching for other bug reports on the same patch, but couldn't find any. Forgive me if this bug was already reported. Feel free to continue this discussion on the bugzilla report if you want. This piece: movl %esi,4(%esp) /* 5th arg: 4th syscall arg */ movl %edx,(%esp)/* 4th arg: 3rd syscall arg */ looks like it's overwriting syscall arguments. This is clearly fixable, but an even better fix would be to drop the asm entirely and switch to two-phase tracing. Want to do it? I can test the seccomp bits if you switch over the asm :) Like what you did for x86_64. That sounds worth investigating. I'll have a look at the asm, but I'm being distracted by a gunman loose 2km from me and my wife and kids under lockdown in two different locations on the other side of the shooting site. Had to cancel lunch today with two work colleagues 1/2km away from that site. ...not been a productive day. --Andy - RGB -- Richard Guy Briggs rbri...@redhat.com Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Regression: audit: x86: drop arch from __audit_syscall_entry() interface
On Wed, Oct 22, 2014 at 12:16 PM, Richard Guy Briggs r...@redhat.com wrote: On 14/10/22, Andy Lutomirski wrote: On 10/22/2014 11:23 AM, Eric Paris wrote: That's really serious. Looking now. On Wed, 2014-10-22 at 16:08 -0200, Paulo Zanoni wrote: Hi (Cc'ing everybody mentioned in the original patch) I work for Intel, on our Linux Graphics driver - aka i915.ko - and our QA team recently reported a regression on: commit b4f0d3755c5e9cc86292d5fd78261903b4f23d4a Author: Richard Guy Briggs Date: Tue Mar 4 10:38:06 2014 -0500 audit: x86: drop arch from __audit_syscall_entry() interface According to our QA, their i386 machine doesn't boot anymore. I tried to write my own revert for the patch, asked QA to test, and they confirmed it solves the problem. Here are the details of QA' s bug report: https://bugs.freedesktop.org/show_bug.cgi?id=85277 . The trees our QA tests are the development trees from i915.ko: http://cgit.freedesktop.org/drm-intel?h=drm-intel-fixes . I tried searching for other bug reports on the same patch, but couldn't find any. Forgive me if this bug was already reported. Feel free to continue this discussion on the bugzilla report if you want. This piece: movl %esi,4(%esp) /* 5th arg: 4th syscall arg */ movl %edx,(%esp)/* 4th arg: 3rd syscall arg */ looks like it's overwriting syscall arguments. This is clearly fixable, but an even better fix would be to drop the asm entirely and switch to two-phase tracing. Want to do it? I can test the seccomp bits if you switch over the asm :) Like what you did for x86_64. That sounds worth investigating. I'll have a look at the asm, but I'm being distracted by a gunman loose 2km from me and my wife and kids under lockdown in two different locations on the other side of the shooting site. Had to cancel lunch today with two work colleagues 1/2km away from that site. ...not been a productive day. That's putting it mildly. Stay safe. --Andy ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Regression: audit: x86: drop arch from __audit_syscall_entry() interface
On Wed, 2014-10-22 at 23:36 +0200, Thomas Gleixner wrote: On Wed, 22 Oct 2014, Eric Paris wrote: That's really serious. Looking now. Indeed its serious. And it's even more serious as this masterpiece of assembly wreckage was pulled in via your tree w/o having an acked-by one of the x86 maintainers. On Wed, 2014-10-22 at 16:08 -0200, Paulo Zanoni wrote: commit b4f0d3755c5e9cc86292d5fd78261903b4f23d4a Author: Richard Guy Briggs Date: Tue Mar 4 10:38:06 2014 -0500 audit: x86: drop arch from __audit_syscall_entry() interface According to our QA, their i386 machine doesn't boot anymore. I tried to write my own revert for the patch, asked QA to test, and they confirmed it solves the problem. diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S index 0d0c9d4ab6d5..f9e3fabc8716 100644 --- a/arch/x86/kernel/entry_32.S +++ b/arch/x86/kernel/entry_32.S @@ -449,12 +449,11 @@ sysenter_audit: jnz syscall_trace_entry addl $4,%esp CFI_ADJUST_CFA_OFFSET -4 - /* %esi already in 8(%esp) 6th arg: 4th syscall arg */ - /* %edx already in 4(%esp) 5th arg: 3rd syscall arg */ - /* %ecx already in 0(%esp) 4th arg: 2nd syscall arg */ - movl %ebx,%ecx /* 3rd arg: 1st syscall arg */ - movl %eax,%edx /* 2nd arg: syscall number */ - movl $AUDIT_ARCH_I386,%eax /* 1st arg: audit arch */ + movl %esi,4(%esp) /* 5th arg: 4th syscall arg */ + movl %edx,(%esp)/* 4th arg: 3rd syscall arg */ Bilndly overwriting the stack which holds the syscall arguments is really a brilliant way to ensure security. It was sent, numerous times, to the x86 list for reviews, and lived in -next for 2 complete devel cycles without a complaint. I'm trying to get an i386 system to test a fix. But yes, it's total crap. -Eric ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Regression: audit: x86: drop arch from __audit_syscall_entry() interface
On Wed, 2014-10-22 at 14:43 -0700, H. Peter Anvin wrote: On 10/22/2014 02:38 PM, Eric Paris wrote: It was sent, numerous times, to the x86 list for reviews, and lived in -next for 2 complete devel cycles without a complaint. I'm trying to get an i386 system to test a fix. But yes, it's total crap. You don't need an i386 system -- you can install an i386 distro on an x86-64 system, or in KVM. I'm currently building on i386 on KVM. That's what I meant by get ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Emit even number of dwords when emitting LRIs
On Wed, Oct 22, 2014 at 06:59:52PM +0100, Arun Siluvery wrote:
The number of DWords should be even when doing ring emits as
command sequences require QWord alignment.
v2: user LRI variant that can write multiple regs in one go (Damien).
We can simply insert one NOP at the end instead of one per register write.
Cc: Mika Kuoppala mika.kuopp...@intel.com
Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
Looks good to me (maybe without the extra '()' outlined below).
Reviewed-by: Damien Lespiau damien.lesp...@intel.com
--
Damien
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 497b836..a8f72e8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -680,15 +680,16 @@ static int intel_ring_workarounds_emit(struct
intel_engine_cs *ring)
if (ret)
return ret;
- ret = intel_ring_begin(ring, w-count * 3);
+ ret = intel_ring_begin(ring, (w-count * 2 + 2));
No need of the () around the second argument.
if (ret)
return ret;
+ intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(w-count));
for (i = 0; i w-count; i++) {
- intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
intel_ring_emit(ring, w-reg[i].addr);
intel_ring_emit(ring, w-reg[i].value);
}
+ intel_ring_emit(ring, MI_NOOP);
intel_ring_advance(ring);
--
2.1.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Regression: audit: x86: drop arch from __audit_syscall_entry() interface
On 10/22/2014 02:38 PM, Eric Paris wrote: It was sent, numerous times, to the x86 list for reviews, and lived in -next for 2 complete devel cycles without a complaint. I'm trying to get an i386 system to test a fix. But yes, it's total crap. You don't need an i386 system -- you can install an i386 distro on an x86-64 system, or in KVM. -hpa ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Regression: audit: x86: drop arch from __audit_syscall_entry() interface
On Wed, 2014-10-22 at 14:43 -0700, H. Peter Anvin wrote: On 10/22/2014 02:38 PM, Eric Paris wrote: It was sent, numerous times, to the x86 list for reviews, and lived in -next for 2 complete devel cycles without a complaint. I'm trying to get an i386 system to test a fix. But yes, it's total crap. You don't need an i386 system -- you can install an i386 distro on an x86-64 system, or in KVM. So I might still be an idiot, because I still haven't gotten a working kernel. But I can't get Linus' latest not panic even without CONFIG_AUDITSYSCALL. I kept blaming myself for not fixing this problem, but reverting the patch like the reporter didn't give me bootable kernels either. I just jumped back in time and am looking to get anything I build to boot... ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add rotation support for cursor plane
On Tue, Oct 07, 2014 at 08:43:46AM +, Jindal, Sonika wrote:
Hi,
Did anybody get a chance to look at this patch?
Thanks,
Sonika
Looks like we waited a bit too long and the codebase has evolved, so I
needed to make some tweaks to your patches to get them to apply cleanly
on the latest di-nightly. However the overall changes here look good to
me, and we don't seem to be missing any details from the bspec, so
Reviewed-by: Matt Roper matthew.d.ro...@intel.com
After tweaking your patches to apply against the latest di-nightly, your
i-g-t test runs properly for me on IVB, as well as another simple test I
wrote myself, so you can also put
Tested-by: Matt Roper matthew.d.ro...@intel.com
Let me know if you want the rebased copies of the patches I used for
testing.
Matt
-Original Message-
From: Jindal, Sonika
Sent: Monday, September 15, 2014 1:14 PM
To: intel-gfx@lists.freedesktop.org
Cc: Ville Syrjälä; Kamble, Sagar A; Jindal, Sonika
Subject: [PATCH] drm/i915: Add rotation support for cursor plane
From: Ville Syrjälä ville.syrj...@linux.intel.com
The cursor plane also supports 180 degree rotation. Add a new
cursor-rotation property on the crtc which controls this.
Unlike sprites, the cursor has a fixed size, so if you have a small cursor
image with the rest of the bo filled by transparent pixels, simply flipping
the rotation property will cause the visible part of the cursor to shift.
This is something to keep in mind when using cursor rotation.
v2: Fix gen4/vlv by offsetting the base address appropriately
v3: Removing cursor-rotation property and using rotation property on cursor
plane.
v4: Changing the author name back to Ville.
Testcase: kms_rotation_crc
Cc: Sagar Kamble sagar.a.kam...@intel.com
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h |1 +
drivers/gpu/drm/i915/intel_display.c | 26 +-
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h index 15c0eaa..5a9fab9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4162,6 +4162,7 @@ enum punit_power_well {
#define MCURSOR_PIPE_A 0x00
#define MCURSOR_PIPE_B (1 28)
#define MCURSOR_GAMMA_ENABLE (1 26)
+#define CURSOR_ROTATE_180 (115)
#define CURSOR_TRICKLE_FEED_DISABLE(1 14)
#define _CURABASE0x70084
#define _CURAPOS 0x70088
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 122ac6e..8c83bcc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8247,6 +8247,9 @@ static void i9xx_update_cursor(struct drm_crtc *crtc,
u32 base)
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
cntl |= CURSOR_PIPE_CSC_ENABLE;
+ if (to_intel_plane(crtc-cursor)-rotation == BIT(DRM_ROTATE_180))
+ cntl |= CURSOR_ROTATE_180;
+
if (intel_crtc-cursor_cntl != cntl) {
I915_WRITE(CURCNTR(pipe), cntl);
POSTING_READ(CURCNTR(pipe));
@@ -8302,6 +8305,13 @@ static void intel_crtc_update_cursor(struct drm_crtc
*crtc,
I915_WRITE(CURPOS(pipe), pos);
+ /* ILK+ do this automagically */
Minor note, but it might be nice to clarify this with a little more
detail. Something like Gen4 and Valley View expect the address to
point to the lower right corner for 180-rotated cursors. Other
platforms expect the address to point to the top-left corner regardless
of rotation.
+ if (HAS_GMCH_DISPLAY(dev)
+ to_intel_plane(crtc-cursor)-rotation == BIT(DRM_ROTATE_180)) {
+ base += (intel_crtc-cursor_height *
+ intel_crtc-cursor_width - 1) * 4;
+ }
+
if (IS_845G(dev) || IS_I865G(dev))
i845_update_cursor(crtc, base);
else
@@ -8453,7 +8463,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc
*crtc,
mutex_unlock(dev-struct_mutex);
old_width = intel_crtc-cursor_width;
-
Unintentional whitespace change?
intel_crtc-cursor_addr = addr;
intel_crtc-cursor_bo = obj;
intel_crtc-cursor_width = width;
@@ -12074,6 +12083,7 @@ static const struct drm_plane_funcs
intel_cursor_plane_funcs = {
.update_plane = intel_cursor_plane_update,
.disable_plane = intel_cursor_plane_disable,
.destroy = intel_plane_destroy,
+ .set_property = intel_plane_set_property,
};
static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
@@ -12089,12 +12099,26 @@ static struct drm_plane
*intel_cursor_plane_create(struct drm_device *dev,
cursor-max_downscale = 1;
cursor-pipe = pipe;
cursor-plane = pipe;
+ cursor-rotation = BIT(DRM_ROTATE_0);
Re: [Intel-gfx] [RFC PATCH 0/8] Add host i915 support for vGPU
On 10/22/2014 05:48 PM, Daniel Vetter wrote: So on a very high level I don't understand this design. For the guest side it's completely clear that we need a bunch of hooks over the driver to make paravirtualization work. But on the host side I expect the driver to be in full control of the hardware. I haven't really seen the other side but it looks like with vgt we actually have two drivers fighting over the hardware, which requires the various hooks to avoid disaster. The drm subsystem is littered with such attempts, and they all didn't end up in a pretty way. So way can't we have the vgt support for guests sit on top of i915, using the i915 functions to set up pagetables, contexts and reserve gtt areas for the guests? Then we'd have just one driver in control of the hardware, and vgt on the host side would just look like a really crazy interace layer between virtual hosts and the low-level driver, similar to who the execbuf ioctl is a really crazy interface between userspace and the low-level driver. Yes we can do this, but that also means lots of pollution to existing i915 codes, only for virtualization purpose. Currently vgt has pretty abstractions for both host i915 and guest graphics drivers, mixing vgt and host i915 means breaking that. I believe a vgt-integrated repository will be better to look at :) Cheers, Daniel -- Thanks, Jike ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
