Re: [Intel-gfx] [PATCH] drm/i915: do not disable backlight on vgaswitcheroo switch off
On Thu, 25 Jul 2013, Jani Nikula jani.nik...@intel.com wrote:
On muxed systems, the other vgaswitcheroo client may depend on i915 to
handle the backlight. We began switching off the backlight since
commit a261b246ebd552fd5d5a8ed84cc931bb821c427f
Author: Daniel Vetter daniel.vet...@ffwll.ch
Date: Thu Jul 26 19:21:47 2012 +0200
drm/i915: disable all crtcs at suspend time
breaking backlight on discreet graphics in (some) muxed systems.
Keep the backlight on when the state is changed through vgaswitcheroo.
Note: The alternative would be to add a quirk table to achieve the same
based on system identifiers, but AFAICS it would asymptotically approach
effectively the same as this patch as more IDs are added, but with the
maintenance burden of the quirk table.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=55311
Tested-by: Fede fed...@yahoo.com
Tested-by: Aximab laurent.deb...@gmail.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59785
Tested-by: sfievet sebastien.fie...@free.fr
Signed-off-by: Jani Nikula jani.nik...@intel.com
---
drivers/gpu/drm/i915/intel_panel.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 67e2c1f..1062154 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -515,6 +515,17 @@ void intel_panel_disable_backlight(struct drm_device
*dev)
struct drm_i915_private *dev_priv = dev-dev_private;
unsigned long flags;
+ /*
+ * Do not disable backlight on the vgaswitcheroo path. When switching
+ * away from i915, the other client may depend on i915 to handle the
+ * backlight. This will leave the backlight on unnecessarily when
+ * another client is not activated.
+ */
+ if (dev-switch_power_state == DRM_SWITCH_POWER_CHANGING) {
+ DRM_DEBUG_DRIVER(Skipping backlight disable on vga switch\n);
+ return;
+ }
+
spin_lock_irqsave(dev_priv-backlight.lock, flags);
dev_priv-backlight.enabled = false;
--
1.7.9.5
--
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] Second HDMI port not visible
On Wed, Aug 7, 2013 at 5:10 AM, Matsumura, Ryan ryan.matsum...@intel.com wrote: I have a BayTrail board with two HDMI ports and running the default Tizen 3.0M1 release. The first HDMI shows up just fine but I can't get the second screen to display anything. I tried enabling the second screen through the kernel command line parameters (video=HDMI-1:e video=HDMI-2:e) and running xrandr. This is my output from xrandr -q Iirc Baytrail still has a bunch of hardcoded ports ... Jesse? -Daniel Screen 0: minimum 320 x 200, current 640 x 480, maximum 8192 x 8192 VGA1 connected 640x480+0+0 (normal left inverted right x axis y axis) 0mm x 0mm 1024x768 60.0 800x60060.3 56.2 848x48060.0 640x48059.9* HDMI1 connected 640x480+0+0 (normal left inverted right x axis y axis) 256mm x 144mm 1920x1080 60.0 + 60.0 50.0 59.9 40.0 1920x1080i 60.1 50.0 60.0 1280x720 60.0 50.0 59.9 1440x576i 50.1 1440x480i 60.1 60.1 720x57650.0 720x48060.0 59.9 640x48060.0 59.9* DP1 disconnected (normal left inverted right x axis y axis) Is there some other configuration I need? I tried this on both X and Wayland. Seems more like a DRM issue at this point. -Ryan ___ 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] [PULL] drm-intel-next
On Wed, Aug 07, 2013 at 10:27:35AM +1000, Dave Airlie wrote: On Mon, Aug 5, 2013 at 5:35 AM, Daniel Vetter dan...@ffwll.ch wrote: Hi Dave, Okay since I missed this, then I merged patches from the list from David Herrmann fixing up drm_mm usage, then I merged this and it all fell to pieces. CC [M] drivers/gpu/drm/i915/i915_gem_gtt.o CC [M] drivers/gpu/drm/i915/i915_gem_stolen.o /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i915/i915_gem_stolen.c: In function ‘i915_gem_object_create_stolen_for_preallocated’: /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/i915/i915_gem_stolen.c:426:2: error: implicit declaration of function ‘drm_mm_put_block’ [-Werror=implicit-function-declaration] drm_mm_put_block(stolen); ^ cc1: some warnings being treated as errors Smash a patch on top of this if you like or whatever you it is you want. Indeed I've merged a little error path fix which added a new put_block call, and I've missed that this will cause a new conflict. Below the fixup I've squashed into the nightly tree, can you please squash that into your merge commit? Thanks, Daniel diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c index 0d0a3b1..ba4dabc 100644 --- a/drivers/gpu/drm/i915/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c @@ -423,7 +423,8 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, return obj; err_out: - drm_mm_put_block(stolen); + drm_mm_remove_node(stolen); + kfree(stolen); drm_gem_object_unreference(obj-base); return NULL; } -- 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: do not disable backlight on vgaswitcheroo switch off
On Wed, Aug 07, 2013 at 09:26:34AM +0300, Jani Nikula wrote: On Thu, 25 Jul 2013, Jani Nikula jani.nik...@intel.com wrote: On muxed systems, the other vgaswitcheroo client may depend on i915 to handle the backlight. We began switching off the backlight since commit a261b246ebd552fd5d5a8ed84cc931bb821c427f Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Thu Jul 26 19:21:47 2012 +0200 drm/i915: disable all crtcs at suspend time breaking backlight on discreet graphics in (some) muxed systems. Keep the backlight on when the state is changed through vgaswitcheroo. Note: The alternative would be to add a quirk table to achieve the same based on system identifiers, but AFAICS it would asymptotically approach effectively the same as this patch as more IDs are added, but with the maintenance burden of the quirk table. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=55311 Tested-by: Fede fed...@yahoo.com Tested-by: Aximab laurent.deb...@gmail.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59785 Tested-by: sfievet sebastien.fie...@free.fr Picked up for -fixes (with a cc: stable), 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] [PATCH 01/12] video/hdmi: Replace the payload length by their defines
On Tue, Aug 06, 2013 at 10:06:16PM -0400, Alex Deucher wrote: Patches 1-5, 10, 11 are: Reviewed-by: Alex Deucher alexander.deuc...@amd.com Entire series merged to drm-intel-next-queue with Dave's irc ack. Thanks for the patches and review. -Daniel On Tue, Aug 6, 2013 at 3:32 PM, Damien Lespiau damien.lesp...@intel.com wrote: Cc: Thierry Reding thierry.red...@avionic-design.de Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/video/hdmi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index 4017833..dbd882f 100644 --- a/drivers/video/hdmi.c +++ b/drivers/video/hdmi.c @@ -52,7 +52,7 @@ int hdmi_avi_infoframe_init(struct hdmi_avi_infoframe *frame) frame-type = HDMI_INFOFRAME_TYPE_AVI; frame-version = 2; - frame-length = 13; + frame-length = HDMI_AVI_INFOFRAME_SIZE; return 0; } @@ -151,7 +151,7 @@ int hdmi_spd_infoframe_init(struct hdmi_spd_infoframe *frame, frame-type = HDMI_INFOFRAME_TYPE_SPD; frame-version = 1; - frame-length = 25; + frame-length = HDMI_SPD_INFOFRAME_SIZE; strncpy(frame-vendor, vendor, sizeof(frame-vendor)); strncpy(frame-product, product, sizeof(frame-product)); @@ -218,7 +218,7 @@ int hdmi_audio_infoframe_init(struct hdmi_audio_infoframe *frame) frame-type = HDMI_INFOFRAME_TYPE_AUDIO; frame-version = 1; - frame-length = 10; + frame-length = HDMI_AUDIO_INFOFRAME_SIZE; return 0; } -- 1.8.3.1 ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/13] drm/i915: Rename hsw_data_buf_partitioning to intel_ddb_partitioning
On Tue, Aug 06, 2013 at 09:31:33PM +0100, Chris Wilson wrote: On Tue, Aug 06, 2013 at 10:24:04PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com We're going to use the 1/2 vs. 5/6 split option already on IVB so the HSW name is not proper. Just give it an intel_ prefix and move it to i915_drv.h so that we can use it there later. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com I do prefer DDB here, so Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk Merged up to this patch with the exception of patch 2. Since I didn't yet take that one follow-up patches started to rain conflicts, so I think I'll wait for the new version. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Backlight control only in the kernel?
On 08/07/2013 03:44 PM, Borislav Petkov wrote: On Sun, Jun 09, 2013 at 07:01:36PM -0400, Matthew Garrett wrote: Windows 8 introduced new policy for backlight control by pushing it out to graphics drivers. This appears to have coincided with a range of vendors adding Windows 8 checks to their backlight control code which trigger either awkward behaviour (Lenovo) or complete brokenness (some Dells). The simplest thing to do would be to just disable ACPI backlight control entirely if the firmware indicates Windows 8 support, but it's entirely possible that individual graphics drivers might still make use of the ACPI functionality in preference to native control. Maybe tangential, so Aaron and I were wondering on https://bugzilla.kernel.org/show_bug.cgi?id=60680 whether it would make sense to handle the backlight control strictly in the kernel, without going to userspace and back? I think this would require the kernel has the knowledge of which backlight interface this system is using or should be using, or it wouldn't know which interface should receive and process the event... -Aaron Background is that on my x230, I needed to connect the Fn-Fx backlight hotkey presses to a script to write to /sys/class/backlight/intel_backlight/brightness because Fluxbox doesn't do that (and maybe doesn't have to). So, without presuming any ACPI or backlight knowledge, can we make the backlight control work only in the kernel by connecting the hotkey presses to some backlight controlling interface which backlight-capable devices implement so that it works regardless of userspace environment? Even if the machine is not running X? Hmmm. Thanks. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Cancel outstanding modeset workers before suspend
On Tue, Aug 06, 2013 at 04:19:00PM +0200, Daniel Vetter wrote: Thinking about this some more we already cancel the rps work items (with the exception of the vlv_work) in intel_disable_gt_powersave, which is called already from i915_save_state. So I think we just need to add the cancel_work for vlv_work at the right spot. For the hotplug work item we also have the dev_priv-enable_hotplug_processing state changes (required to get the ordering right on the resume side of things). I'd prefer if the hotplug cancel_work_sync call is right next to this (maybe as part of a irq disable function?). That leaves the catchall for unpin work and similar stuff. I think that suits best as part of i915_gem_idle next to cancelling the retire_work handler - with unpin am slightly afraid that if it runs later on while gem is already quiescent it'll create havoc. So roughly three patches, with the hotplug one for -fixes/stable since we have real reports about it. What do you think? The ordering did bother me. However, I like the paranoid safety that one big barrier before teardown offers. It does wonders stating that after this point there shall be no outstanding work. We can do the fine grained shutdown, and convert this barrier into a sanity check just by using WARN_ON(cancel_delayed_work_sync()). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/3] Small i915/exynos prime cleanup
Hi all, I need to get my prime fixes in since they're blocking QA from running -nightly prime tests. Which is a prerequisite of mine before I start touching dma-buf for real to look at fencing and ww-mutex integration for i915. These three patches are just a bit of prep cleanup and one bugfix that Maarten spotted. Cheers, Daniel Daniel Vetter (3): drm: use common drm_gem_dmabuf_release in i915/exynos drivers drm/i915: unpin backing storage in dmabuf_unmap drm/i915: explicit store base gem object in dma_buf-priv drivers/gpu/drm/drm_prime.c| 3 ++- drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 23 +--- drivers/gpu/drm/i915/i915_gem_dmabuf.c | 34 +- include/drm/drmP.h | 1 + 4 files changed, 19 insertions(+), 42 deletions(-) -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
Note that this is slightly tricky since both drivers store their
native objects in dma_buf-priv. But both also embed the base
drm_gem_object at the first position, so the implicit cast is ok.
To use the release helper we need to export it, too.
Cc: Inki Dae inki@samsung.com
Cc: Intel Graphics Development intel-gfx@lists.freedesktop.org
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/drm_prime.c| 3 ++-
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 23 +--
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 13 +
include/drm/drmP.h | 1 +
4 files changed, 5 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 85e450e..a35f206 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -192,7 +192,7 @@ static void drm_gem_unmap_dma_buf(struct dma_buf_attachment
*attach,
/* nothing to be done here */
}
-static void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
+void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
{
struct drm_gem_object *obj = dma_buf-priv;
@@ -202,6 +202,7 @@ static void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
drm_gem_object_unreference_unlocked(obj);
}
}
+EXPORT_SYMBOL(drm_gem_dmabuf_release);
static void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf)
{
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index a0f997e..3cd56e1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -127,27 +127,6 @@ static void exynos_gem_unmap_dma_buf(struct
dma_buf_attachment *attach,
/* Nothing to do. */
}
-static void exynos_dmabuf_release(struct dma_buf *dmabuf)
-{
- struct exynos_drm_gem_obj *exynos_gem_obj = dmabuf-priv;
-
- /*
-* exynos_dmabuf_release() call means that file object's
-* f_count is 0 and it calls drm_gem_object_handle_unreference()
-* to drop the references that these values had been increased
-* at drm_prime_handle_to_fd()
-*/
- if (exynos_gem_obj-base.export_dma_buf == dmabuf) {
- exynos_gem_obj-base.export_dma_buf = NULL;
-
- /*
-* drop this gem object refcount to release allocated buffer
-* and resources.
-*/
- drm_gem_object_unreference_unlocked(exynos_gem_obj-base);
- }
-}
-
static void *exynos_gem_dmabuf_kmap_atomic(struct dma_buf *dma_buf,
unsigned long page_num)
{
@@ -193,7 +172,7 @@ static struct dma_buf_ops exynos_dmabuf_ops = {
.kunmap = exynos_gem_dmabuf_kunmap,
.kunmap_atomic = exynos_gem_dmabuf_kunmap_atomic,
.mmap = exynos_gem_dmabuf_mmap,
- .release= exynos_dmabuf_release,
+ .release= drm_gem_dmabuf_release,
};
struct dma_buf *exynos_dmabuf_prime_export(struct drm_device *drm_dev,
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index f2e185c..63ee1a9 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -90,17 +90,6 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment
*attachment,
kfree(sg);
}
-static void i915_gem_dmabuf_release(struct dma_buf *dma_buf)
-{
- struct drm_i915_gem_object *obj = dma_buf-priv;
-
- if (obj-base.export_dma_buf == dma_buf) {
- /* drop the reference on the export fd holds */
- obj-base.export_dma_buf = NULL;
- drm_gem_object_unreference_unlocked(obj-base);
- }
-}
-
static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
{
struct drm_i915_gem_object *obj = dma_buf-priv;
@@ -211,7 +200,7 @@ static int i915_gem_begin_cpu_access(struct dma_buf
*dma_buf, size_t start, size
static const struct dma_buf_ops i915_dmabuf_ops = {
.map_dma_buf = i915_gem_map_dma_buf,
.unmap_dma_buf = i915_gem_unmap_dma_buf,
- .release = i915_gem_dmabuf_release,
+ .release = drm_gem_dmabuf_release,
.kmap = i915_gem_dmabuf_kmap,
.kmap_atomic = i915_gem_dmabuf_kmap_atomic,
.kunmap = i915_gem_dmabuf_kunmap,
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 4b518e0..cc991a2 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1537,6 +1537,7 @@ extern struct drm_gem_object *drm_gem_prime_import(struct
drm_device *dev,
struct dma_buf *dma_buf);
extern int drm_gem_prime_fd_to_handle(struct drm_device *dev,
struct drm_file *file_priv, int prime_fd, uint32_t *handle);
+extern void drm_gem_dmabuf_release(struct dma_buf *dma_buf);
extern int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data,
[Intel-gfx] [PATCH 2/3] drm/i915: unpin backing storage in dmabuf_unmap
This fixes a WARN in i915_gem_free_object when the
obj-pages_pin_count isn't 0.
Reported-by: Maarten Lankhorst maarten.lankho...@canonical.com
Cc: Maarten Lankhorst maarten.lankho...@canonical.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 63ee1a9..0bf3d51 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -85,9 +85,13 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment
*attachment,
struct sg_table *sg,
enum dma_data_direction dir)
{
+ struct drm_i915_gem_object *obj = attachment-dmabuf-priv;
+
dma_unmap_sg(attachment-dev, sg-sgl, sg-nents, dir);
sg_free_table(sg);
kfree(sg);
+
+ i915_gem_object_unpin_pages(obj);
}
static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: explicit store base gem object in dma_buf-priv
Makes it more obviously correct what tricks we play by reusing the drm
prime release helper.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 0bf3d51..3c0edaf 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -27,10 +27,15 @@
#include i915_drv.h
#include linux/dma-buf.h
+static struct drm_i915_gem_object *dma_buf_to_obj(struct dma_buf *buf)
+{
+ return to_intel_bo(buf-priv);
+}
+
static struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment
*attachment,
enum dma_data_direction dir)
{
- struct drm_i915_gem_object *obj = attachment-dmabuf-priv;
+ struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment-dmabuf);
struct sg_table *st;
struct scatterlist *src, *dst;
int ret, i;
@@ -85,7 +90,7 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment
*attachment,
struct sg_table *sg,
enum dma_data_direction dir)
{
- struct drm_i915_gem_object *obj = attachment-dmabuf-priv;
+ struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment-dmabuf);
dma_unmap_sg(attachment-dev, sg-sgl, sg-nents, dir);
sg_free_table(sg);
@@ -96,7 +101,7 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment
*attachment,
static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
{
- struct drm_i915_gem_object *obj = dma_buf-priv;
+ struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
struct drm_device *dev = obj-base.dev;
struct sg_page_iter sg_iter;
struct page **pages;
@@ -144,7 +149,7 @@ error:
static void i915_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr)
{
- struct drm_i915_gem_object *obj = dma_buf-priv;
+ struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
struct drm_device *dev = obj-base.dev;
int ret;
@@ -187,7 +192,7 @@ static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf,
struct vm_area_struct *
static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start,
size_t length, enum dma_data_direction direction)
{
- struct drm_i915_gem_object *obj = dma_buf-priv;
+ struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
struct drm_device *dev = obj-base.dev;
int ret;
bool write = (direction == DMA_BIDIRECTIONAL || direction ==
DMA_TO_DEVICE);
@@ -218,9 +223,7 @@ static const struct dma_buf_ops i915_dmabuf_ops = {
struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
struct drm_gem_object *gem_obj, int flags)
{
- struct drm_i915_gem_object *obj = to_intel_bo(gem_obj);
-
- return dma_buf_export(obj, i915_dmabuf_ops, obj-base.size, flags);
+ return dma_buf_export(gem_obj, i915_dmabuf_ops, gem_obj-size, flags);
}
static int i915_gem_object_get_pages_dmabuf(struct drm_i915_gem_object *obj)
@@ -257,7 +260,7 @@ struct drm_gem_object *i915_gem_prime_import(struct
drm_device *dev,
/* is this one of own objects? */
if (dma_buf-ops == i915_dmabuf_ops) {
- obj = dma_buf-priv;
+ obj = dma_buf_to_obj(dma_buf);
/* is it from our device? */
if (obj-base.dev == dev) {
/*
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: unpin backing storage in dmabuf_unmap
On Wed, Aug 07, 2013 at 11:15:07AM +0200, Daniel Vetter wrote:
This fixes a WARN in i915_gem_free_object when the
obj-pages_pin_count isn't 0.
Reported-by: Maarten Lankhorst maarten.lankho...@canonical.com
Cc: Maarten Lankhorst maarten.lankho...@canonical.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67391
---
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 63ee1a9..0bf3d51 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -85,9 +85,13 @@ static void i915_gem_unmap_dma_buf(struct
dma_buf_attachment *attachment,
struct sg_table *sg,
enum dma_data_direction dir)
{
+ struct drm_i915_gem_object *obj = attachment-dmabuf-priv;
+
dma_unmap_sg(attachment-dev, sg-sgl, sg-nents, dir);
sg_free_table(sg);
kfree(sg);
+
+ i915_gem_object_unpin_pages(obj);
}
static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
--
1.8.3.2
--
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 2/2] drm/i915: Introduce a new create ioctl for user specified placement
Despite being a unified memory architecture (UMA) some bits of memory
are more equal than others. In particular we have the thorny issue of
stolen memory, memory stolen from the system by the BIOS and reserved
for igfx use. Stolen memory is required for some functions of the GPU
and display engine, but in general it goes wasted. Whilst we cannot
return it back to the system, we need to find some other method for
utilising it. As we do not support direct access to the physical address
in the stolen region, it behaves like a different class of memory,
closer in kin to local GPU memory. This strongly suggests that we need a
placement model like TTM if we are to fully utilize these discrete
chunks of differing memory.
This new create ioctl therefore exists to allow the user to create these
second class buffer objects from stolen memory. At the moment direct
access by the CPU through mmaps and pread/pwrite are verboten on the
objects, and so the user must be aware of the limitations of the objects
created. Yet, those limitations rarely reduce the desired functionality
in many use cases and so the user should be able to easily fill the
stolen memory and so help to reduce overall memory pressure.
The most obvious use case for stolen memory is for the creation of objects
for the display engine which already have very similar restrictions on
access. However, we want a reasonably general ioctl in order to cater
for diverse scenarios beyond the author's imagination.
v2: Expand the struct slightly to include cache domains, and ensure that
all memory allocated by the kernel for userspace is zeroed.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_dma.c| 5 +-
drivers/gpu/drm/i915/i915_drv.h| 15 +++--
drivers/gpu/drm/i915/i915_gem.c| 102 +--
drivers/gpu/drm/i915/i915_gem_tiling.c | 107 ++---
include/uapi/drm/i915_drm.h| 80
5 files changed, 252 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index e4e98df..ef69c68 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1869,8 +1869,8 @@ struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_MMAP_GTT, i915_gem_mmap_gtt_ioctl,
DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_GEM_SET_DOMAIN, i915_gem_set_domain_ioctl,
DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_GEM_SW_FINISH, i915_gem_sw_finish_ioctl,
DRM_UNLOCKED),
- DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling,
DRM_UNLOCKED),
- DRM_IOCTL_DEF_DRV(I915_GEM_GET_TILING, i915_gem_get_tiling,
DRM_UNLOCKED),
+ DRM_IOCTL_DEF_DRV(I915_GEM_SET_TILING, i915_gem_set_tiling_ioctl,
DRM_UNLOCKED),
+ DRM_IOCTL_DEF_DRV(I915_GEM_GET_TILING, i915_gem_get_tiling_ioctl,
DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl,
DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_GET_PIPE_FROM_CRTC_ID,
intel_get_pipe_from_crtc_id, DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_GEM_MADVISE, i915_gem_madvise_ioctl,
DRM_UNLOCKED),
@@ -1882,6 +1882,7 @@ struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE,
i915_gem_context_create_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY,
i915_gem_context_destroy_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED),
+ DRM_IOCTL_DEF_DRV(I915_GEM_CREATE2, i915_gem_create2_ioctl,
DRM_AUTH|DRM_UNLOCKED),
};
int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 29ff248..546d907 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1671,6 +1671,8 @@ int i915_gem_init_ioctl(struct drm_device *dev, void
*data,
struct drm_file *file_priv);
int i915_gem_create_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
+int i915_gem_create2_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_priv);
int i915_gem_pread_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
int i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
@@ -1705,10 +1707,10 @@ int i915_gem_entervt_ioctl(struct drm_device *dev, void
*data,
struct drm_file *file_priv);
int i915_gem_leavevt_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv);
-int i915_gem_set_tiling(struct drm_device *dev, void *data,
- struct drm_file *file_priv);
-int i915_gem_get_tiling(struct drm_device *dev, void *data,
- struct drm_file *file_priv);
+int i915_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
+
[Intel-gfx] [PATCH 1/2] drm/i915: Support in-kernel GPU command execution
There are a few simple operations that we would like to offload onto the
GPU for the benefit of running asynchronously. The first is to clear the
backing storage for an object.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/Makefile| 1 +
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/i915_gem_exec.c | 120 +++
3 files changed, 124 insertions(+)
create mode 100644 drivers/gpu/drm/i915/i915_gem_exec.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index b8449a8..9d498e5 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -11,6 +11,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \
i915_gem_context.o \
i915_gem_debug.o \
i915_gem_evict.o \
+ i915_gem_exec.o \
i915_gem_execbuffer.o \
i915_gem_gtt.o \
i915_gem_stolen.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1ad8a42..29ff248 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1951,6 +1951,9 @@ int i915_gem_context_create_ioctl(struct drm_device *dev,
void *data,
int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
struct drm_file *file);
+/* i915_gem_exec.c */
+int i915_gem_exec_clear_object(struct drm_i915_gem_object *obj);
+
/* i915_gem_gtt.c */
void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev);
void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
diff --git a/drivers/gpu/drm/i915/i915_gem_exec.c
b/drivers/gpu/drm/i915/i915_gem_exec.c
new file mode 100644
index 000..d2ac077
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_gem_exec.c
@@ -0,0 +1,120 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Chris Wilson ch...@chris-wilson.co.uk
+ *
+ */
+
+#include drm/drmP.h
+#include drm/i915_drm.h
+#include i915_drv.h
+
+#define COLOR_BLT_CMD (229 | 0x4022)
+#define BLT_WRITE_ALPHA (121)
+#define BLT_WRITE_RGB (120)
+#define BLT_WRITE_RGBA (BLT_WRITE_RGB|BLT_WRITE_ALPHA)
+
+#define BPP_8 0
+#define BPP_16 (124)
+#define BPP_32 (125 | 124)
+
+#define ROP_FILL_COPY (0xf0 16)
+
+static int i915_gem_exec_flush_object(struct drm_i915_gem_object *obj,
+ struct intel_ring_buffer *ring)
+{
+ int ret;
+
+ ret = i915_gem_object_sync(obj, ring);
+ if (ret)
+ return ret;
+
+ if (obj-base.write_domain I915_GEM_DOMAIN_CPU) {
+ i915_gem_clflush_object(obj);
+ i915_gem_chipset_flush(obj-base.dev);
+ obj-base.write_domain = ~I915_GEM_DOMAIN_CPU;
+ }
+ if (obj-base.write_domain I915_GEM_DOMAIN_GTT) {
+ wmb();
+ obj-base.write_domain = ~I915_GEM_DOMAIN_GTT;
+ }
+
+ return intel_ring_invalidate_all_caches(ring);
+}
+
+static void i915_gem_exec_dirty_object(struct drm_i915_gem_object *obj,
+ struct intel_ring_buffer *ring)
+{
+ obj-fenced_gpu_access = false;
+ obj-base.read_domains = I915_GEM_DOMAIN_RENDER;
+ obj-base.write_domain = I915_GEM_DOMAIN_RENDER;
+ i915_gem_object_move_to_active(obj, ring);
+ obj-last_write_seqno = intel_ring_get_seqno(ring);
+ obj-dirty = 1;
+
+ ring-gpu_caches_dirty = true;
+}
+
+int i915_gem_exec_clear_object(struct drm_i915_gem_object *obj)
+{
+ struct drm_device *dev = obj-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_ring_buffer *ring;
+ int ret;
+
+ lockdep_assert_held(dev-struct_mutex);
+
+ ring = dev_priv-ring[HAS_BLT(dev) ? BCS : RCS];
+
+ ret = i915_gem_obj_ggtt_pin(obj, 0, false, false);
+ if (ret)
+ return ret;
Re: [Intel-gfx] [PATCH 2/3] drm/i915: unpin backing storage in dmabuf_unmap
On Wed, Aug 07, 2013 at 11:15:07AM +0200, Daniel Vetter wrote: This fixes a WARN in i915_gem_free_object when the obj-pages_pin_count isn't 0. Reported-by: Maarten Lankhorst maarten.lankho...@canonical.com Cc: Maarten Lankhorst maarten.lankho...@canonical.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Papers over the WARN with iffy locking. Not all callers hold struct_mutex, right? Worse some do, some don't... What's the long term plan here? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
On Wed, Aug 07, 2013 at 11:15:06AM +0200, Daniel Vetter wrote: Note that this is slightly tricky since both drivers store their native objects in dma_buf-priv. But both also embed the base drm_gem_object at the first position, so the implicit cast is ok. To use the release helper we need to export it, too. Cc: Inki Dae inki@samsung.com Cc: Intel Graphics Development intel-gfx@lists.freedesktop.org Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Any perverse driver could always call into drm_gem_dmabuf_release() with container_of(). Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch]
Sent: Wednesday, August 07, 2013 6:15 PM
To: DRI Development
Cc: Intel Graphics Development; Daniel Vetter; Inki Dae
Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos
drivers
Note that this is slightly tricky since both drivers store their
native objects in dma_buf-priv. But both also embed the base
drm_gem_object at the first position, so the implicit cast is ok.
To use the release helper we need to export it, too.
Yeah, may I repost this patch with additional work? We also need to export
with a gem object instead of specific one like you did.
Thanks,
Inki Dae
Cc: Inki Dae inki@samsung.com
Cc: Intel Graphics Development intel-gfx@lists.freedesktop.org
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/drm_prime.c| 3 ++-
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 23 +--
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 13 +
include/drm/drmP.h | 1 +
4 files changed, 5 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 85e450e..a35f206 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -192,7 +192,7 @@ static void drm_gem_unmap_dma_buf(struct
dma_buf_attachment *attach,
/* nothing to be done here */
}
-static void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
+void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
{
struct drm_gem_object *obj = dma_buf-priv;
@@ -202,6 +202,7 @@ static void drm_gem_dmabuf_release(struct dma_buf
*dma_buf)
drm_gem_object_unreference_unlocked(obj);
}
}
+EXPORT_SYMBOL(drm_gem_dmabuf_release);
static void *drm_gem_dmabuf_vmap(struct dma_buf *dma_buf)
{
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index a0f997e..3cd56e1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -127,27 +127,6 @@ static void exynos_gem_unmap_dma_buf(struct
dma_buf_attachment *attach,
/* Nothing to do. */
}
-static void exynos_dmabuf_release(struct dma_buf *dmabuf)
-{
- struct exynos_drm_gem_obj *exynos_gem_obj = dmabuf-priv;
-
- /*
- * exynos_dmabuf_release() call means that file object's
- * f_count is 0 and it calls drm_gem_object_handle_unreference()
- * to drop the references that these values had been increased
- * at drm_prime_handle_to_fd()
- */
- if (exynos_gem_obj-base.export_dma_buf == dmabuf) {
- exynos_gem_obj-base.export_dma_buf = NULL;
-
- /*
- * drop this gem object refcount to release allocated buffer
- * and resources.
- */
- drm_gem_object_unreference_unlocked(exynos_gem_obj-base);
- }
-}
-
static void *exynos_gem_dmabuf_kmap_atomic(struct dma_buf *dma_buf,
unsigned long page_num)
{
@@ -193,7 +172,7 @@ static struct dma_buf_ops exynos_dmabuf_ops = {
.kunmap = exynos_gem_dmabuf_kunmap,
.kunmap_atomic = exynos_gem_dmabuf_kunmap_atomic,
.mmap = exynos_gem_dmabuf_mmap,
- .release= exynos_dmabuf_release,
+ .release= drm_gem_dmabuf_release,
};
struct dma_buf *exynos_dmabuf_prime_export(struct drm_device *drm_dev,
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index f2e185c..63ee1a9 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -90,17 +90,6 @@ static void i915_gem_unmap_dma_buf(struct
dma_buf_attachment *attachment,
kfree(sg);
}
-static void i915_gem_dmabuf_release(struct dma_buf *dma_buf)
-{
- struct drm_i915_gem_object *obj = dma_buf-priv;
-
- if (obj-base.export_dma_buf == dma_buf) {
- /* drop the reference on the export fd holds */
- obj-base.export_dma_buf = NULL;
- drm_gem_object_unreference_unlocked(obj-base);
- }
-}
-
static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
{
struct drm_i915_gem_object *obj = dma_buf-priv;
@@ -211,7 +200,7 @@ static int i915_gem_begin_cpu_access(struct dma_buf
*dma_buf, size_t start, size
static const struct dma_buf_ops i915_dmabuf_ops = {
.map_dma_buf = i915_gem_map_dma_buf,
.unmap_dma_buf = i915_gem_unmap_dma_buf,
- .release = i915_gem_dmabuf_release,
+ .release = drm_gem_dmabuf_release,
.kmap = i915_gem_dmabuf_kmap,
.kmap_atomic = i915_gem_dmabuf_kmap_atomic,
.kunmap = i915_gem_dmabuf_kunmap,
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 4b518e0..cc991a2 100644
--- a/include/drm/drmP.h
+++
Re: [Intel-gfx] [PATCH 3/3] drm/i915: explicit store base gem object in dma_buf-priv
On Wed, Aug 07, 2013 at 11:15:08AM +0200, Daniel Vetter wrote: Makes it more obviously correct what tricks we play by reusing the drm prime release helper. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch I do like these little typesafe cast helpers. Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: unpin backing storage in dmabuf_unmap
On Wed, Aug 7, 2013 at 11:29 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 07, 2013 at 11:15:07AM +0200, Daniel Vetter wrote: This fixes a WARN in i915_gem_free_object when the obj-pages_pin_count isn't 0. Reported-by: Maarten Lankhorst maarten.lankho...@canonical.com Cc: Maarten Lankhorst maarten.lankho...@canonical.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Papers over the WARN with iffy locking. Not all callers hold struct_mutex, right? Worse some do, some don't... Oops, that needs mutex locking here. I've checked the code and none of the callers here should ever hold our own dev-struct_mutex (due to the self-import checks we bypass dma-buf for our own objects) so no immediate deadlock. But it's easy to create circles and piss off lockded ofc. What's the long term plan here? Per-bo locking with ww mutexes. Locking is done by the callers of map/unmap, sonce only those can properly do the ww locking dance on all relevent buffers of a batch upfront. It's going to be fun ;-) Looking closer I've also spotted that our map_buf callback has a call to i915_mutex_lock_interruptible, which means the map can fail with -EINTR. Currently it seems unspec'ed whether map is allowed to do that, but current callers certainly can't cope with this. I'll throw a 2nd patch on top. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
On Wed, Aug 7, 2013 at 11:40 AM, Inki Dae inki@samsung.com wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] Sent: Wednesday, August 07, 2013 6:15 PM To: DRI Development Cc: Intel Graphics Development; Daniel Vetter; Inki Dae Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers Note that this is slightly tricky since both drivers store their native objects in dma_buf-priv. But both also embed the base drm_gem_object at the first position, so the implicit cast is ok. To use the release helper we need to export it, too. Yeah, may I repost this patch with additional work? We also need to export with a gem object instead of specific one like you did. I'm confused here what you mean, so pls just submit the patch. That usually helps ;-) -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 03/13] drm/i915: Split watermark level computation from the code
On Tue, Aug 06, 2013 at 09:56:31PM +0100, Chris Wilson wrote: On Tue, Aug 06, 2013 at 10:24:02PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Refactor the watermarks computation for one level to a separate function. This function will now set the -enable flat to true, s/flat/flag/ Though I did think you meant a flatten value at one point. even if the watermark level wasn't actually checked yet. In the future we will delay the checking so we must consider all unchecked watermarks as possibly valid. v2: Preserve comment about latency units Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com I don't have the context here to confirm passing level to ilk_compute_pri_wm is exactly what you want, and it seems that level is implicitly greater than 0 in the original code. The transformation looks fine by itself. If you can calm my quirms over the value of level (in the changelog is fine), Looks like Daniel already took this one. But yeah, I guess we should really pass 'level 0' to make it clearer that it's a boolean. If you recall I had a patch in the original series to pass level all the way down, but I dropped it since it didn't actually do any good. As far as level=0, I didn't convert the hsw_compute_wm_pipe() to use the new function. I could do that as a followup patch. Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/13] drm/i915: Kill fbc_enable from hsw_lp_wm_results
On Tue, Aug 06, 2013 at 09:45:11PM +0100, Chris Wilson wrote: On Tue, Aug 06, 2013 at 10:24:03PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com We don't need to store the FBC WM enabled status in each watermark level. We anyway have to reduce it down to a single boolean, so just delay checking the FBC WM limit until we're computing the final value. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com The pay-off simply being the reduction of one bool in a temporary struct [x3]? I don't remember anymore if I had a better reason originally. Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: unpin backing storage in dmabuf_unmap
This fixes a WARN in i915_gem_free_object when the
obj-pages_pin_count isn't 0.
v2: Add locking to unmap, noticed by Chris Wilson. Note that even
though we call unmap with our own dev-struct_mutex held that won't
result in an immediate deadlock since we never go through the dma_buf
interfaces for our own, reimported buffers. But it's still easy to
blow up and anger lockdep, but that's already the case with our -map
implementation. Fixing this for real will involve per dma-buf ww mutex
locking by the callers. And lots of fun. So go with the duct-tape
approach for now.
Cc: Chris Wilson ch...@chris-wilson.co.uk
Reported-by: Maarten Lankhorst maarten.lankho...@canonical.com
Cc: Maarten Lankhorst maarten.lankho...@canonical.com
Tested-by: Armin K. kre...@email.com (v1)
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 63ee1a9..f7e1682 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -85,9 +85,17 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment
*attachment,
struct sg_table *sg,
enum dma_data_direction dir)
{
+ struct drm_i915_gem_object *obj = attachment-dmabuf-priv;
+
+ mutex_lock(obj-base.dev-struct_mutex);
+
dma_unmap_sg(attachment-dev, sg-sgl, sg-nents, dir);
sg_free_table(sg);
kfree(sg);
+
+ i915_gem_object_unpin_pages(obj);
+
+ mutex_unlock(obj-base.dev-struct_mutex);
}
static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: no interruptible locking for dma_buf-map
It's unclear whether -map is allowed to fail with -EINTR, but
looking at current callers it's pretty clear that they don't
expect this to happen. So use a blocking mutex_lock call. Since
we don't wait for the gpu in our -map callback the lack of the
gpu hang checks doesn't matter.
Furthermore the goal is to eventually have per dma-buf locking done
by callers with ww mutexes, so this will then be removed.
Cc: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index f7e1682..63c0818 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -35,9 +35,7 @@ static struct sg_table *i915_gem_map_dma_buf(struct
dma_buf_attachment *attachme
struct scatterlist *src, *dst;
int ret, i;
- ret = i915_mutex_lock_interruptible(obj-base.dev);
- if (ret)
- return ERR_PTR(ret);
+ mutex_lock(obj-base.dev-struct_mutex);
ret = i915_gem_object_get_pages(obj);
if (ret) {
--
1.8.3.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
On Wed, Aug 07, 2013 at 07:18:45PM +0900, Joonyoung Shim wrote: On 08/07/2013 06:55 PM, Daniel Vetter wrote: On Wed, Aug 7, 2013 at 11:40 AM, Inki Dae inki@samsung.com wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] Sent: Wednesday, August 07, 2013 6:15 PM To: DRI Development Cc: Intel Graphics Development; Daniel Vetter; Inki Dae Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers Note that this is slightly tricky since both drivers store their native objects in dma_buf-priv. But both also embed the base drm_gem_object at the first position, so the implicit cast is ok. To use the release helper we need to export it, too. Yeah, may I repost this patch with additional work? We also need to export with a gem object instead of specific one like you did. I think dmabuf stuff of exynos can be replaced to common drm_gem_dmabuf. Already dmabuf stuff of drm_gem_cma_helper.c was substituted to common drm_gem_dmabuf with low-level hook functions to use prime helpers. Ah, but that can easily be done on top of this, right? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Pull watermark level validity check out
From: Ville Syrjälä ville.syrj...@linux.intel.com
Refactor the code a bit to split the watermark level validity check into
a separate function.
Also add hack there that allows us to use it even for LP0 watermarks.
ATM we don't pre-compute/check the LP0 watermarks, so we just have to
clamp them to the maximum and hope things work out.
v2: Add some debug prints when we exceed max WM0
Kill pointless ret = false' assignment.
Include the check for the already disabled 'result' which
got shuffled around when the patchs got reorderd
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 51 +++--
1 file changed, 44 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a5a9959..1dd8f30 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2278,6 +2278,49 @@ static uint32_t ilk_compute_fbc_wm(struct
hsw_pipe_wm_parameters *params,
params-pri_bytes_per_pixel);
}
+static bool ilk_check_wm(int level,
+const struct hsw_wm_maximums *max,
+struct hsw_lp_wm_result *result)
+{
+ bool ret;
+
+ /* already determined to be invalid? */
+ if (!result-enable)
+ return false;
+
+ result-enable = result-pri_val = max-pri
+result-spr_val = max-spr
+result-cur_val = max-cur;
+
+ ret = result-enable;
+
+ /*
+* HACK until we can pre-compute everything,
+* and thus fail gracefully if LP0 watermarks
+* are exceeded...
+*/
+ if (level == 0 !result-enable) {
+ if (result-pri_val max-pri)
+ DRM_DEBUG_KMS(Primary WM%d too large %u (max %u)\n,
+ level, result-pri_val, max-pri);
+ if (result-spr_val max-spr)
+ DRM_DEBUG_KMS(Sprite WM%d too large %u (max %u)\n,
+ level, result-spr_val, max-spr);
+ if (result-cur_val max-cur)
+ DRM_DEBUG_KMS(Cursor WM%d too large %u (max %u)\n,
+ level, result-cur_val, max-cur);
+
+ result-pri_val = min_t(uint32_t, result-pri_val, max-pri);
+ result-spr_val = min_t(uint32_t, result-spr_val, max-spr);
+ result-cur_val = min_t(uint32_t, result-cur_val, max-cur);
+ result-enable = true;
+ }
+
+ DRM_DEBUG_KMS(WM%d: %sabled\n, level, result-enable ? en : dis);
+
+ return ret;
+}
+
static void ilk_compute_wm_level(struct drm_i915_private *dev_priv,
int level,
struct hsw_pipe_wm_parameters *p,
@@ -2318,13 +2361,7 @@ static bool hsw_compute_lp_wm(struct drm_i915_private
*dev_priv,
result-fbc_val = max3(res[0].fbc_val, res[1].fbc_val, res[2].fbc_val);
result-enable = true;
- if (!result-enable)
- return false;
-
- result-enable = result-pri_val = max-pri
-result-spr_val = max-spr
-result-cur_val = max-cur;
- return result-enable;
+ return ilk_check_wm(level, max, result);
}
static uint32_t hsw_compute_wm_pipe(struct drm_i915_private *dev_priv,
--
1.8.1.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Calculate max watermark levels for ILK+
From: Ville Syrjälä ville.syrj...@linux.intel.com
There are quite a few variables we need to take into account to
determine the maximum watermark levels, so it feels a bit cleaner
to calculate those rather than just have a bunch of what look like
magic numbers.
v2: s/pipes_active/num_pipes_active
s/othwewise/otherwise
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 119
1 file changed, 107 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d10d4c7..5daa32a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2270,6 +2270,104 @@ static uint32_t ilk_compute_fbc_wm(struct
hsw_pipe_wm_parameters *params,
params-pri_bytes_per_pixel);
}
+static unsigned int ilk_display_fifo_size(const struct drm_device *dev)
+{
+ if (INTEL_INFO(dev)-gen = 7)
+ return 768;
+ else
+ return 512;
+}
+
+/* Calculate the maximum primary/sprite plane watermark */
+static unsigned int ilk_plane_wm_max(const struct drm_device *dev,
+int level,
+unsigned int num_pipes_active,
+bool sprite_enabled,
+enum intel_ddb_partitioning
ddb_partitioning,
+bool is_sprite)
+{
+ unsigned int fifo_size = ilk_display_fifo_size(dev);
+ unsigned int max;
+
+ /* if sprites aren't enabled, sprites get nothing */
+ if (is_sprite !sprite_enabled)
+ return 0;
+
+ /* HSW allows LP1+ watermarks even with multiple pipes */
+ if (level == 0 || num_pipes_active 1) {
+ fifo_size /= INTEL_INFO(dev)-num_pipes;
+
+ /*
+* For some reason the non self refresh
+* FIFO size is only half of the self
+* refresh FIFO size on ILK/SNB.
+*/
+ if (INTEL_INFO(dev)-gen = 6)
+ fifo_size /= 2;
+ }
+
+ if (sprite_enabled) {
+ /* level 0 is always calculated with 1:1 split */
+ if (level 0 ddb_partitioning == INTEL_DDB_PART_5_6) {
+ if (is_sprite)
+ fifo_size *= 5;
+ fifo_size /= 6;
+ } else {
+ fifo_size /= 2;
+ }
+ }
+
+ /* clamp to max that the registers can hold */
+ if (INTEL_INFO(dev)-gen = 7)
+ /* IVB/HSW primary/sprite plane watermarks */
+ max = level == 0 ? 127 : 1023;
+ else if (!is_sprite)
+ /* ILK/SNB primary plane watermarks */
+ max = level == 0 ? 127 : 511;
+ else
+ /* ILK/SNB sprite plane watermarks */
+ max = level == 0 ? 63 : 255;
+
+ return min(fifo_size, max);
+}
+
+/* Calculate the maximum cursor plane watermark */
+static unsigned int ilk_cursor_wm_max(const struct drm_device *dev,
+ int level, unsigned int num_pipes_active)
+{
+ /* HSW LP1+ watermarks w/ multiple pipes */
+ if (level 0 num_pipes_active 1)
+ return 64;
+
+ /* otherwise just report max that registers can hold */
+ if (INTEL_INFO(dev)-gen = 7)
+ return level == 0 ? 63 : 255;
+ else
+ return level == 0 ? 31 : 63;
+}
+
+/* Calculate the maximum FBC watermark */
+static unsigned int ilk_fbc_wm_max(void)
+{
+ /* max that registers can hold */
+ return 15;
+}
+
+static void ilk_wm_max(struct drm_device *dev,
+ int level,
+ unsigned int num_pipes_active,
+ bool sprite_enabled,
+ enum intel_ddb_partitioning ddb_partitioning,
+ struct hsw_wm_maximums *max)
+{
+ max-pri = ilk_plane_wm_max(dev, level, num_pipes_active,
+ sprite_enabled, ddb_partitioning, false);
+ max-spr = ilk_plane_wm_max(dev, level, num_pipes_active,
+ sprite_enabled, ddb_partitioning, true);
+ max-cur = ilk_cursor_wm_max(dev, level, num_pipes_active);
+ max-fbc = ilk_fbc_wm_max();
+}
+
static bool ilk_check_wm(int level,
const struct hsw_wm_maximums *max,
struct intel_wm_level *result)
@@ -2555,18 +2653,15 @@ static void hsw_compute_wm_parameters(struct drm_device
*dev,
sprites_enabled++;
}
- if (pipes_active 1) {
- lp_max_1_2-pri = lp_max_5_6-pri = sprites_enabled ? 128 : 256;
- lp_max_1_2-spr = lp_max_5_6-spr = 128;
- lp_max_1_2-cur =
[Intel-gfx] [PATCH v2] drm/i915: Pull some watermarks state into a separate structure
From: Ville Syrjälä ville.syrj...@linux.intel.com
There is a bunch of global state that needs to be considered when
checking watermarks for validity. Move most of that to a new
structure intel_wm_config, to avoid having to pass around so
many variables.
One notable thing left out is the DDB partitioning information,
since we often anyway need to check the same watermarks against
both 1/2 and 5/6 DDB partitioning layouts.
v2: s/pipes_active/num_pipes_active
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 48 +
1 file changed, 25 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5daa32a..9d087be 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2188,6 +2188,14 @@ struct hsw_wm_values {
bool enable_fbc_wm;
};
+/* used in computing the new watermarks state */
+struct intel_wm_config {
+ unsigned int num_pipes_active;
+ bool sprites_enabled;
+ bool sprites_scaled;
+ bool fbc_wm_enabled;
+};
+
/*
* For both WM_PIPE and WM_LP.
* mem_value must be in 0.1us units.
@@ -2281,8 +2289,7 @@ static unsigned int ilk_display_fifo_size(const struct
drm_device *dev)
/* Calculate the maximum primary/sprite plane watermark */
static unsigned int ilk_plane_wm_max(const struct drm_device *dev,
int level,
-unsigned int num_pipes_active,
-bool sprite_enabled,
+const struct intel_wm_config *config,
enum intel_ddb_partitioning
ddb_partitioning,
bool is_sprite)
{
@@ -2290,11 +2297,11 @@ static unsigned int ilk_plane_wm_max(const struct
drm_device *dev,
unsigned int max;
/* if sprites aren't enabled, sprites get nothing */
- if (is_sprite !sprite_enabled)
+ if (is_sprite !config-sprites_enabled)
return 0;
/* HSW allows LP1+ watermarks even with multiple pipes */
- if (level == 0 || num_pipes_active 1) {
+ if (level == 0 || config-num_pipes_active 1) {
fifo_size /= INTEL_INFO(dev)-num_pipes;
/*
@@ -2306,7 +2313,7 @@ static unsigned int ilk_plane_wm_max(const struct
drm_device *dev,
fifo_size /= 2;
}
- if (sprite_enabled) {
+ if (config-sprites_enabled) {
/* level 0 is always calculated with 1:1 split */
if (level 0 ddb_partitioning == INTEL_DDB_PART_5_6) {
if (is_sprite)
@@ -2333,10 +2340,11 @@ static unsigned int ilk_plane_wm_max(const struct
drm_device *dev,
/* Calculate the maximum cursor plane watermark */
static unsigned int ilk_cursor_wm_max(const struct drm_device *dev,
- int level, unsigned int num_pipes_active)
+ int level,
+ const struct intel_wm_config *config)
{
/* HSW LP1+ watermarks w/ multiple pipes */
- if (level 0 num_pipes_active 1)
+ if (level 0 config-num_pipes_active 1)
return 64;
/* otherwise just report max that registers can hold */
@@ -2355,16 +2363,13 @@ static unsigned int ilk_fbc_wm_max(void)
static void ilk_wm_max(struct drm_device *dev,
int level,
- unsigned int num_pipes_active,
- bool sprite_enabled,
+ const struct intel_wm_config *config,
enum intel_ddb_partitioning ddb_partitioning,
struct hsw_wm_maximums *max)
{
- max-pri = ilk_plane_wm_max(dev, level, num_pipes_active,
- sprite_enabled, ddb_partitioning, false);
- max-spr = ilk_plane_wm_max(dev, level, num_pipes_active,
- sprite_enabled, ddb_partitioning, true);
- max-cur = ilk_cursor_wm_max(dev, level, num_pipes_active);
+ max-pri = ilk_plane_wm_max(dev, level, config, ddb_partitioning,
false);
+ max-spr = ilk_plane_wm_max(dev, level, config, ddb_partitioning, true);
+ max-cur = ilk_cursor_wm_max(dev, level, config);
max-fbc = ilk_fbc_wm_max();
}
@@ -2614,7 +2619,7 @@ static void hsw_compute_wm_parameters(struct drm_device
*dev,
struct drm_crtc *crtc;
struct drm_plane *plane;
enum pipe pipe;
- int pipes_active = 0, sprites_enabled = 0;
+ struct intel_wm_config config = {};
list_for_each_entry(crtc, dev-mode_config.crtc_list, head) {
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -2627,7 +2632,7 @@ static void hsw_compute_wm_parameters(struct drm_device
[Intel-gfx] [PATCH v2] drm/i915: Split plane watermark parameters into a separate struct
From: Ville Syrjälä ville.syrj...@linux.intel.com
Give a name to the plane watermark related data we have currently
stored under intel_plane-wm.
We also observe that this data is more or less the same that we have
in the hsw_pipe_wm_parameters structure, so use it there as well.
v2: Make pahole happier
Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_drv.h | 14 +-
drivers/gpu/drm/i915/intel_pm.c | 57 +++-
2 files changed, 35 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7df662b..3ea8e5f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -331,6 +331,13 @@ struct intel_crtc {
bool pch_fifo_underrun_disabled;
};
+struct intel_plane_wm_parameters {
+ uint32_t horiz_pixels;
+ uint8_t bytes_per_pixel;
+ bool enabled;
+ bool scaled;
+};
+
struct intel_plane {
struct drm_plane base;
int plane;
@@ -349,12 +356,7 @@ struct intel_plane {
* as the other pieces of the struct may not reflect the values we want
* for the watermark calculations. Currently only Haswell uses this.
*/
- struct {
- bool enabled;
- bool scaled;
- uint8_t bytes_per_pixel;
- uint32_t horiz_pixels;
- } wm;
+ struct intel_plane_wm_parameters wm;
void (*update_plane)(struct drm_plane *plane,
struct drm_framebuffer *fb,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 9d087be..af030e8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2162,15 +2162,11 @@ static uint32_t ilk_wm_fbc(uint32_t pri_val, uint32_t
horiz_pixels,
struct hsw_pipe_wm_parameters {
bool active;
- bool sprite_enabled;
- uint8_t pri_bytes_per_pixel;
- uint8_t spr_bytes_per_pixel;
- uint8_t cur_bytes_per_pixel;
- uint32_t pri_horiz_pixels;
- uint32_t spr_horiz_pixels;
- uint32_t cur_horiz_pixels;
uint32_t pipe_htotal;
uint32_t pixel_rate;
+ struct intel_plane_wm_parameters pri;
+ struct intel_plane_wm_parameters spr;
+ struct intel_plane_wm_parameters cur;
};
struct hsw_wm_maximums {
@@ -2206,12 +2202,11 @@ static uint32_t ilk_compute_pri_wm(struct
hsw_pipe_wm_parameters *params,
{
uint32_t method1, method2;
- /* TODO: for now, assume the primary plane is always enabled. */
- if (!params-active)
+ if (!params-active || !params-pri.enabled)
return 0;
method1 = ilk_wm_method1(params-pixel_rate,
-params-pri_bytes_per_pixel,
+params-pri.bytes_per_pixel,
mem_value);
if (!is_lp)
@@ -2219,8 +2214,8 @@ static uint32_t ilk_compute_pri_wm(struct
hsw_pipe_wm_parameters *params,
method2 = ilk_wm_method2(params-pixel_rate,
params-pipe_htotal,
-params-pri_horiz_pixels,
-params-pri_bytes_per_pixel,
+params-pri.horiz_pixels,
+params-pri.bytes_per_pixel,
mem_value);
return min(method1, method2);
@@ -2235,16 +2230,16 @@ static uint32_t ilk_compute_spr_wm(struct
hsw_pipe_wm_parameters *params,
{
uint32_t method1, method2;
- if (!params-active || !params-sprite_enabled)
+ if (!params-active || !params-spr.enabled)
return 0;
method1 = ilk_wm_method1(params-pixel_rate,
-params-spr_bytes_per_pixel,
+params-spr.bytes_per_pixel,
mem_value);
method2 = ilk_wm_method2(params-pixel_rate,
params-pipe_htotal,
-params-spr_horiz_pixels,
-params-spr_bytes_per_pixel,
+params-spr.horiz_pixels,
+params-spr.bytes_per_pixel,
mem_value);
return min(method1, method2);
}
@@ -2256,13 +2251,13 @@ static uint32_t ilk_compute_spr_wm(struct
hsw_pipe_wm_parameters *params,
static uint32_t ilk_compute_cur_wm(struct hsw_pipe_wm_parameters *params,
uint32_t mem_value)
{
- if (!params-active)
+ if (!params-active || !params-cur.enabled)
return 0;
return ilk_wm_method2(params-pixel_rate,
params-pipe_htotal,
- params-cur_horiz_pixels,
-
Re: [Intel-gfx] [PATCH 2/2] drm/i915: no interruptible locking for dma_buf-map
On Wed, Aug 07, 2013 at 12:09:33PM +0200, Daniel Vetter wrote: It's unclear whether -map is allowed to fail with -EINTR, but looking at current callers it's pretty clear that they don't expect this to happen. So use a blocking mutex_lock call. Since we don't wait for the gpu in our -map callback the lack of the gpu hang checks doesn't matter. Furthermore the goal is to eventually have per dma-buf locking done by callers with ww mutexes, so this will then be removed. Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Ugh, who can't handle EINTR here but can handle all the other errors? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2] drm/i915: Don't try to disable plane if it's already disabled
From: Ville Syrjälä ville.syrj...@linux.intel.com Check plane-fb in intel_disable_plane() to determine if the plane is already disabled. If the plane has an fb, then it must also have a crtc, so we can drop the plane-crtc check and just call intel_enable_primary() directly. v2: WARN and bail if the plane doesn't have a crtc when it should Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_sprite.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index d4e0592..0a174d7 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -863,8 +863,13 @@ intel_disable_plane(struct drm_plane *plane) struct intel_plane *intel_plane = to_intel_plane(plane); int ret = 0; - if (plane-crtc) - intel_enable_primary(plane-crtc); + if (!plane-fb) + return 0; + + if (WARN_ON(!plane-crtc)) + return -EINVAL; + + intel_enable_primary(plane-crtc); intel_plane-disable_plane(plane, plane-crtc); if (!intel_plane-obj) -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Backlight control only in the kernel?
On Wed, 2013-08-07 at 09:44 +0200, Borislav Petkov wrote: Background is that on my x230, I needed to connect the Fn-Fx backlight hotkey presses to a script to write to /sys/class/backlight/intel_backlight/brightness because Fluxbox doesn't do that (and maybe doesn't have to). So, without presuming any ACPI or backlight knowledge, can we make the backlight control work only in the kernel by connecting the hotkey presses to some backlight controlling interface which backlight-capable devices implement so that it works regardless of userspace environment? Even if the machine is not running X? Not really. The ACPI driver is able to do this because the events and the backlight are associated with the same device. We could potentially make something work with GPU backlight drivers since their PCI device should also be associated with the backlight device, but things get complicated quickly - once ddcci support is hooked up you'll also have kernel backlight devices for some external monitors, and now you need to make a policy decision about which display should be controlled in response to the keypress. One reasonable choice would be whichever display contains the currently focused window, which is obviously out of scope for the kernel. So if we're going to do this then we need a generalised mechanism in-kernel for connecting input devices to backlight devices, and we need a mechanism for disabling it when userspace knows better. This sounds like a lot of work for something that should really just be handled by userspace already. -- Matthew Garrett | mj...@srcf.ucam.org ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: unpin backing storage in dmabuf_unmap
Op 07-08-13 12:09, Daniel Vetter schreef: This fixes a WARN in i915_gem_free_object when the obj-pages_pin_count isn't 0. v2: Add locking to unmap, noticed by Chris Wilson. Note that even though we call unmap with our own dev-struct_mutex held that won't result in an immediate deadlock since we never go through the dma_buf interfaces for our own, reimported buffers. But it's still easy to blow up and anger lockdep, but that's already the case with our -map implementation. Fixing this for real will involve per dma-buf ww mutex locking by the callers. And lots of fun. So go with the duct-tape approach for now. Cc: Chris Wilson ch...@chris-wilson.co.uk Reported-by: Maarten Lankhorst maarten.lankho...@canonical.com Cc: Maarten Lankhorst maarten.lankho...@canonical.com Tested-by: Armin K. kre...@email.com (v1) Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Acked, this was my original patch to solve the issue. I want to note that locking struct_mutex here will break lockdep, but it's a problem in drm, not this patch. ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Second HDMI port not visible
On Wed, Aug 07, 2013 at 09:41:39AM +0200, Daniel Vetter wrote: On Wed, Aug 7, 2013 at 5:10 AM, Matsumura, Ryan ryan.matsum...@intel.com wrote: I have a BayTrail board with two HDMI ports and running the default Tizen 3.0M1 release. The first HDMI shows up just fine but I can't get the second screen to display anything. I tried enabling the second screen through the kernel command line parameters (video=HDMI-1:e video=HDMI-2:e) and running xrandr. This is my output from xrandr -q Iirc Baytrail still has a bunch of hardcoded ports ... Jesse? Currently the code assumes that port C is eDP, not DP/HDMI. The specs say that physically either port should be able to function in any role (eDP/DP/HDMI). Not sure if there's some way to detect which way the board is wired (some VBT stuff like the eDP on port D on HSW?), or maybe we should just register the DP/HDMI ports if eDP init fails? -Daniel Screen 0: minimum 320 x 200, current 640 x 480, maximum 8192 x 8192 VGA1 connected 640x480+0+0 (normal left inverted right x axis y axis) 0mm x 0mm 1024x768 60.0 800x60060.3 56.2 848x48060.0 640x48059.9* HDMI1 connected 640x480+0+0 (normal left inverted right x axis y axis) 256mm x 144mm 1920x1080 60.0 + 60.0 50.0 59.9 40.0 1920x1080i 60.1 50.0 60.0 1280x720 60.0 50.0 59.9 1440x576i 50.1 1440x480i 60.1 60.1 720x57650.0 720x48060.0 59.9 640x48060.0 59.9* DP1 disconnected (normal left inverted right x axis y axis) Is there some other configuration I need? I tried this on both X and Wayland. Seems more like a DRM issue at this point. -Ryan ___ 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] [PATCH 2/2] drm/i915: no interruptible locking for dma_buf-map
On Wed, Aug 7, 2013 at 12:30 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Wed, Aug 07, 2013 at 12:09:33PM +0200, Daniel Vetter wrote: It's unclear whether -map is allowed to fail with -EINTR, but looking at current callers it's pretty clear that they don't expect this to happen. So use a blocking mutex_lock call. Since we don't wait for the gpu in our -map callback the lack of the gpu hang checks doesn't matter. Furthermore the goal is to eventually have per dma-buf locking done by callers with ww mutexes, so this will then be removed. Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Ugh, who can't handle EINTR here but can handle all the other errors? Ok, I've re-read the code and I think callers can actually cope. I'm just freaked out that we don't have test coverage for these case, but that should be a moot point once all the locking is converted over to ww mutexes. So I'll drop this patch here. -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 04/12] video/hmdi: Clear the whole incoming buffer, not just the infoframe size
On Tue, Aug 06, 2013 at 08:32:16PM +0100, Damien Lespiau wrote: If the user if this API is providing a bigger buffer than the infoframe size, it could be for a could reason. For instance it could be because it gives the buffer that will be written to the hardware, up to the maximum of an infoframe size. Instead of just zeroing up to the infoframe size, let's zero the whole incoming buffer as those extra bytes are also used to compute the ECC and need to be 0. Signed-off-by: Damien Lespiau damien.lesp...@intel.com One concern that came to mind was someone needing to preserve the buffer contents beyond the infoframe. But I guess if someone really needs to do that, they can go and figure out the exact length of the infoframe and pass that. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/video/hdmi.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index f7a85e5..635d569 100644 --- a/drivers/video/hdmi.c +++ b/drivers/video/hdmi.c @@ -84,7 +84,7 @@ ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe *frame, void *buffer, if (size length) return -ENOSPC; - memset(buffer, 0, length); + memset(buffer, 0, size); ptr[0] = frame-type; ptr[1] = frame-version; @@ -186,7 +186,7 @@ ssize_t hdmi_spd_infoframe_pack(struct hdmi_spd_infoframe *frame, void *buffer, if (size length) return -ENOSPC; - memset(buffer, 0, length); + memset(buffer, 0, size); ptr[0] = frame-type; ptr[1] = frame-version; @@ -251,7 +251,7 @@ ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame, if (size length) return -ENOSPC; - memset(buffer, 0, length); + memset(buffer, 0, size); if (frame-channels = 2) channels = frame-channels - 1; @@ -308,7 +308,7 @@ ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame, if (size length) return -ENOSPC; - memset(buffer, 0, length); + memset(buffer, 0, size); ptr[0] = frame-type; ptr[1] = frame-version; -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/12] drm/i915/hdmi: Change the write_infoframe vfunc to take a buffer and a type
On Tue, Aug 06, 2013 at 08:32:18PM +0100, Damien Lespiau wrote:
First step in the move to the shared infoframe infrastructure, let's
move the different infoframe helpers and the write_infoframe() vfunc to
a type (enum hdmi_infoframe_type) and a buffer + len instead of using
our struct dip_infoframe.
v2: constify the infoframe pointer and don't mix signs (Ville Syrjälä)
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Signed-off-by: Paulo Zanoni paulo.r.zanoni at intel.com
Signed-off-by: Thierry Reding thierry.reding at avionic-design.de
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_drv.h | 4 +-
drivers/gpu/drm/i915/intel_hdmi.c | 106
--
2 files changed, 59 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 474797b..273acfd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -26,6 +26,7 @@
#define __INTEL_DRV_H__
#include linux/i2c.h
+#include linux/hdmi.h
#include drm/i915_drm.h
#include i915_drv.h
#include drm/drm_crtc.h
@@ -463,7 +464,8 @@ struct intel_hdmi {
enum hdmi_force_audio force_audio;
bool rgb_quant_range_selectable;
void (*write_infoframe)(struct drm_encoder *encoder,
- struct dip_infoframe *frame);
+ enum hdmi_infoframe_type type,
+ const uint8_t *frame, ssize_t len);
void (*set_infoframes)(struct drm_encoder *encoder,
struct drm_display_mode *adjusted_mode);
};
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index e82cd81..ee67e23 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -29,6 +29,7 @@
#include linux/i2c.h
#include linux/slab.h
#include linux/delay.h
+#include linux/hdmi.h
#include drm/drmP.h
#include drm/drm_crtc.h
#include drm/drm_edid.h
@@ -81,74 +82,75 @@ void intel_dip_infoframe_csum(struct dip_infoframe *frame)
frame-checksum = 0x100 - sum;
}
-static u32 g4x_infoframe_index(struct dip_infoframe *frame)
+static u32 g4x_infoframe_index(enum hdmi_infoframe_type type)
{
- switch (frame-type) {
- case DIP_TYPE_AVI:
+ switch (type) {
+ case HDMI_INFOFRAME_TYPE_AVI:
return VIDEO_DIP_SELECT_AVI;
- case DIP_TYPE_SPD:
+ case HDMI_INFOFRAME_TYPE_SPD:
return VIDEO_DIP_SELECT_SPD;
default:
- DRM_DEBUG_DRIVER(unknown info frame type %d\n, frame-type);
+ DRM_DEBUG_DRIVER(unknown info frame type %d\n, type);
return 0;
}
}
-static u32 g4x_infoframe_enable(struct dip_infoframe *frame)
+static u32 g4x_infoframe_enable(enum hdmi_infoframe_type type)
{
- switch (frame-type) {
- case DIP_TYPE_AVI:
+ switch (type) {
+ case HDMI_INFOFRAME_TYPE_AVI:
return VIDEO_DIP_ENABLE_AVI;
- case DIP_TYPE_SPD:
+ case HDMI_INFOFRAME_TYPE_SPD:
return VIDEO_DIP_ENABLE_SPD;
default:
- DRM_DEBUG_DRIVER(unknown info frame type %d\n, frame-type);
+ DRM_DEBUG_DRIVER(unknown info frame type %d\n, type);
return 0;
}
}
-static u32 hsw_infoframe_enable(struct dip_infoframe *frame)
+static u32 hsw_infoframe_enable(enum hdmi_infoframe_type type)
{
- switch (frame-type) {
- case DIP_TYPE_AVI:
+ switch (type) {
+ case HDMI_INFOFRAME_TYPE_AVI:
return VIDEO_DIP_ENABLE_AVI_HSW;
- case DIP_TYPE_SPD:
+ case HDMI_INFOFRAME_TYPE_SPD:
return VIDEO_DIP_ENABLE_SPD_HSW;
default:
- DRM_DEBUG_DRIVER(unknown info frame type %d\n, frame-type);
+ DRM_DEBUG_DRIVER(unknown info frame type %d\n, type);
return 0;
}
}
-static u32 hsw_infoframe_data_reg(struct dip_infoframe *frame,
+static u32 hsw_infoframe_data_reg(enum hdmi_infoframe_type type,
enum transcoder cpu_transcoder)
{
- switch (frame-type) {
- case DIP_TYPE_AVI:
+ switch (type) {
+ case HDMI_INFOFRAME_TYPE_AVI:
return HSW_TVIDEO_DIP_AVI_DATA(cpu_transcoder);
- case DIP_TYPE_SPD:
+ case HDMI_INFOFRAME_TYPE_SPD:
return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder);
default:
- DRM_DEBUG_DRIVER(unknown info frame type %d\n, frame-type);
+ DRM_DEBUG_DRIVER(unknown info frame type %d\n, type);
return 0;
}
}
static void g4x_write_infoframe(struct drm_encoder *encoder,
- struct dip_infoframe *frame)
+ enum hdmi_infoframe_type type,
+ const uint8_t *frame, ssize_t len)
{
uint32_t *data = (uint32_t *)frame;
Re: [Intel-gfx] [PATCH 07/12] drm/i915/hdmi: Port the infoframe code to the common hdmi helpers
On Tue, Aug 06, 2013 at 08:32:19PM +0100, Damien Lespiau wrote:
Let's use the drivers/video/hmdi.c and drm infoframe helpers to build
our infoframes.
v2: Simplify the logic to compute the buffer size. We can just take the
maximum infoframe size rounded to 32, which happens to be what the
hardware let us write anyway.
v3: Remove unnecessary memset() (Ville Syrjälä)
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_hdmi.c | 82
+++
1 file changed, 58 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index ee67e23..455dfa7 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -325,14 +325,43 @@ static void hsw_write_infoframe(struct drm_encoder
*encoder,
POSTING_READ(ctl_reg);
}
+/*
+ * The data we write to the DIP data buffer registers is 1 byte bigger than
the
+ * HDMI infoframe size because of an ECC/reserved byte at position 3
(starting
+ * at 0). It's also a byte used by DisplayPort so the same DIP registers can
be
+ * used for both technologies.
+ *
+ * DW0: Reserved/ECC/DP | HB2 | HB1 | HB0
+ * DW1: DB3 | DB2 | DB1 | DB0
+ * DW2: DB7 | DB6 | DB5 | DB4
+ * DW3: ...
+ *
+ * (HB is Header Byte, DB is Data Byte)
+ *
+ * The hdmi pack() functions don't know about that hardware specific hole so
we
+ * trick them by giving an offset into the buffer and moving back the header
+ * bytes by one.
+ */
static void intel_set_infoframe(struct drm_encoder *encoder,
- struct dip_infoframe *frame)
+ union hdmi_infoframe *frame)
{
struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+ uint8_t buffer[VIDEO_DIP_DATA_SIZE];
+ ssize_t len;
- intel_dip_infoframe_csum(frame);
- intel_hdmi-write_infoframe(encoder, frame-type, (uint8_t *)frame,
- DIP_HEADER_SIZE + frame-len);
+ /* see comment above for the reason for this offset */
+ len = hdmi_infoframe_pack(frame, buffer + 1, sizeof(buffer) - 1);
+ if (len 0)
+ return;
+
+ /* Insert the 'hole' (see big comment above) at position 3 */
+ buffer[0] = buffer[1];
+ buffer[1] = buffer[2];
+ buffer[2] = buffer[3];
+ buffer[3] = 0;
+ len++;
+
+ intel_hdmi-write_infoframe(encoder, frame-any.type, buffer, len);
}
static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
@@ -340,40 +369,45 @@ static void intel_hdmi_set_avi_infoframe(struct
drm_encoder *encoder,
{
struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
struct intel_crtc *intel_crtc = to_intel_crtc(encoder-crtc);
- struct dip_infoframe avi_if = {
- .type = DIP_TYPE_AVI,
- .ver = DIP_VERSION_AVI,
- .len = DIP_LEN_AVI,
- };
+ union hdmi_infoframe frame;
+ int ret;
+
+ ret = drm_hdmi_avi_infoframe_from_display_mode(frame.avi,
+adjusted_mode);
+ if (ret 0) {
+ DRM_ERROR(couldn't fill AVI infoframe\n);
+ return;
+ }
if (adjusted_mode-flags DRM_MODE_FLAG_DBLCLK)
- avi_if.body.avi.YQ_CN_PR |= DIP_AVI_PR_2;
+ frame.avi.pixel_repeat = 1;
if (intel_hdmi-rgb_quant_range_selectable) {
if (intel_crtc-config.limited_color_range)
- avi_if.body.avi.ITC_EC_Q_SC |=
DIP_AVI_RGB_QUANT_RANGE_LIMITED;
+ frame.avi.quantization_range =
+ HDMI_QUANTIZATION_RANGE_LIMITED;
else
- avi_if.body.avi.ITC_EC_Q_SC |=
DIP_AVI_RGB_QUANT_RANGE_FULL;
+ frame.avi.quantization_range =
+ HDMI_QUANTIZATION_RANGE_FULL;
}
- avi_if.body.avi.VIC = drm_match_cea_mode(adjusted_mode);
-
- intel_set_infoframe(encoder, avi_if);
+ intel_set_infoframe(encoder, frame);
}
static void intel_hdmi_set_spd_infoframe(struct drm_encoder *encoder)
{
- struct dip_infoframe spd_if;
+ union hdmi_infoframe frame;
+ int ret;
+
+ ret = hdmi_spd_infoframe_init(frame.spd, Intel, Integrated gfx);
+ if (ret 0) {
+ DRM_ERROR(couldn't fill SPD infoframe\n);
+ return;
+ }
- memset(spd_if, 0, sizeof(spd_if));
- spd_if.type = DIP_TYPE_SPD;
- spd_if.ver = DIP_VERSION_SPD;
- spd_if.len = DIP_LEN_SPD;
- strcpy(spd_if.body.spd.vn, Intel);
- strcpy(spd_if.body.spd.pd, Integrated gfx);
- spd_if.body.spd.sdi = DIP_SPD_PC;
+ frame.spd.sdi = HDMI_SPD_SDI_PC;
- intel_set_infoframe(encoder, spd_if);
+ intel_set_infoframe(encoder, frame);
Re: [Intel-gfx] [PATCH 04/12] video/hmdi: Clear the whole incoming buffer, not just the infoframe size
On Wed, Aug 07, 2013 at 01:56:58PM +0300, Ville Syrjälä wrote: On Tue, Aug 06, 2013 at 08:32:16PM +0100, Damien Lespiau wrote: If the user if this API is providing a bigger buffer than the infoframe size, it could be for a could reason. For instance it could be because it gives the buffer that will be written to the hardware, up to the maximum of an infoframe size. Instead of just zeroing up to the infoframe size, let's zero the whole incoming buffer as those extra bytes are also used to compute the ECC and need to be 0. Signed-off-by: Damien Lespiau damien.lesp...@intel.com One concern that came to mind was someone needing to preserve the buffer contents beyond the infoframe. But I guess if someone really needs to do that, they can go and figure out the exact length of the infoframe and pass that. Right, that was my thinking as well. We even have a macro for that now. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
2013/8/7 Daniel Vetter dan...@ffwll.ch On Wed, Aug 07, 2013 at 07:18:45PM +0900, Joonyoung Shim wrote: On 08/07/2013 06:55 PM, Daniel Vetter wrote: On Wed, Aug 7, 2013 at 11:40 AM, Inki Dae inki@samsung.com wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] Sent: Wednesday, August 07, 2013 6:15 PM To: DRI Development Cc: Intel Graphics Development; Daniel Vetter; Inki Dae Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers Note that this is slightly tricky since both drivers store their native objects in dma_buf-priv. But both also embed the base drm_gem_object at the first position, so the implicit cast is ok. To use the release helper we need to export it, too. Yeah, may I repost this patch with additional work? We also need to export with a gem object instead of specific one like you did. I think dmabuf stuff of exynos can be replaced to common drm_gem_dmabuf. Already dmabuf stuff of drm_gem_cma_helper.c was substituted to common drm_gem_dmabuf with low-level hook functions to use prime helpers. Ah, but that can easily be done on top of this, right? Daniel, could you remove exynos related codes from your patch set? Your patch set would make exynos broken because you didn't consider exporting with a gem object for exynos like [PATCH 3/3] drm/i915: explicit store base gem object in dma_buf-priv. So I think your patch set is not complete set, and That is why exynos needs the additional work I mentioned above. So I just wanted to repost your patch set + new one. However, I think not only exynos could go to common drm_gem_dmabuf directly but also it would make your patch set to be complete set if you remove exynos related codes from your patch set. Otherwise, we have to work twice. one is the additional work for resolving exynos broken issue by your patch set, and other is to replace existing dmabuf stuff of exynos to common drm_gem_dmabuf. Thanks, Inki Dae -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
On Wed, Aug 7, 2013 at 2:01 PM, Inki Dae inki@samsung.com wrote: 2013/8/7 Daniel Vetter dan...@ffwll.ch On Wed, Aug 07, 2013 at 07:18:45PM +0900, Joonyoung Shim wrote: On 08/07/2013 06:55 PM, Daniel Vetter wrote: On Wed, Aug 7, 2013 at 11:40 AM, Inki Dae inki@samsung.com wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] Sent: Wednesday, August 07, 2013 6:15 PM To: DRI Development Cc: Intel Graphics Development; Daniel Vetter; Inki Dae Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers Note that this is slightly tricky since both drivers store their native objects in dma_buf-priv. But both also embed the base drm_gem_object at the first position, so the implicit cast is ok. To use the release helper we need to export it, too. Yeah, may I repost this patch with additional work? We also need to export with a gem object instead of specific one like you did. I think dmabuf stuff of exynos can be replaced to common drm_gem_dmabuf. Already dmabuf stuff of drm_gem_cma_helper.c was substituted to common drm_gem_dmabuf with low-level hook functions to use prime helpers. Ah, but that can easily be done on top of this, right? Daniel, could you remove exynos related codes from your patch set? Your patch set would make exynos broken because you didn't consider exporting with a gem object for exynos like [PATCH 3/3] drm/i915: explicit store base gem object in dma_buf-priv. So I think your patch set is not complete set, and That is why exynos needs the additional work I mentioned above. So I just wanted to repost your patch set + new one. Nope, my patch should not break exynos since the base gem_object is the first member of the exynos object, so we don't have any issues with upcasting in exynos dma-buf code. The same applies to i915 dma-buf code, my follow-up patch just makes the code a bit safer. However, I think not only exynos could go to common drm_gem_dmabuf directly but also it would make your patch set to be complete set if you remove exynos related codes from your patch set. Otherwise, we have to work twice. one is the additional work for resolving exynos broken issue by your patch set, and other is to replace existing dmabuf stuff of exynos to common drm_gem_dmabuf. Yeah np, I'll drop exynos then. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Silence a sparse warning
From: Ville Syrjälä ville.syrj...@linux.intel.com
drivers/gpu/drm/i915/i915_debugfs.c:2136:3: warning: symbol
'i915_debugfs_files' was not declared. Should it be static?
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index d2935b4..5d52a23 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2130,7 +2130,7 @@ static struct drm_info_list i915_debugfs_list[] = {
};
#define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
-struct i915_debugfs_files {
+static struct i915_debugfs_files {
const char *name;
const struct file_operations *fops;
} i915_debugfs_files[] = {
--
1.8.1.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Silence a sparse warning
On Wed, Aug 07, 2013 at 03:11:52PM +0300, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
drivers/gpu/drm/i915/i915_debugfs.c:2136:3: warning: symbol
'i915_debugfs_files' was not declared. Should it be static?
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
Queued for -next, thanks for the patch.
-Daniel
---
drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index d2935b4..5d52a23 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2130,7 +2130,7 @@ static struct drm_info_list i915_debugfs_list[] = {
};
#define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
-struct i915_debugfs_files {
+static struct i915_debugfs_files {
const char *name;
const struct file_operations *fops;
} i915_debugfs_files[] = {
--
1.8.1.5
___
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 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
2013/8/7 Daniel Vetter dan...@ffwll.ch On Wed, Aug 7, 2013 at 2:01 PM, Inki Dae inki@samsung.com wrote: 2013/8/7 Daniel Vetter dan...@ffwll.ch On Wed, Aug 07, 2013 at 07:18:45PM +0900, Joonyoung Shim wrote: On 08/07/2013 06:55 PM, Daniel Vetter wrote: On Wed, Aug 7, 2013 at 11:40 AM, Inki Dae inki@samsung.com wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] Sent: Wednesday, August 07, 2013 6:15 PM To: DRI Development Cc: Intel Graphics Development; Daniel Vetter; Inki Dae Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers Note that this is slightly tricky since both drivers store their native objects in dma_buf-priv. But both also embed the base drm_gem_object at the first position, so the implicit cast is ok. To use the release helper we need to export it, too. Yeah, may I repost this patch with additional work? We also need to export with a gem object instead of specific one like you did. I think dmabuf stuff of exynos can be replaced to common drm_gem_dmabuf. Already dmabuf stuff of drm_gem_cma_helper.c was substituted to common drm_gem_dmabuf with low-level hook functions to use prime helpers. Ah, but that can easily be done on top of this, right? Daniel, could you remove exynos related codes from your patch set? Your patch set would make exynos broken because you didn't consider exporting with a gem object for exynos like [PATCH 3/3] drm/i915: explicit store base gem object in dma_buf-priv. So I think your patch set is not complete set, and That is why exynos needs the additional work I mentioned above. So I just wanted to repost your patch set + new one. Nope, my patch should not break exynos since the base gem_object is the first member of the exynos object, so we don't have any issues Ah, right. However, it does not seem like good way. with upcasting in exynos dma-buf code. The same applies to i915 dma-buf code, my follow-up patch just makes the code a bit safer. However, I think not only exynos could go to common drm_gem_dmabuf directly but also it would make your patch set to be complete set if you remove exynos related codes from your patch set. Otherwise, we have to work twice. one is the additional work for resolving exynos broken issue by your patch set, and other is to replace existing dmabuf stuff of exynos to common drm_gem_dmabuf. Yeah np, I'll drop exynos then. Thanks a lot. :) Thanks, Inki Dae -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/9] Haswell Package C8+
2013/8/6 Daniel Vetter dan...@ffwll.ch: On Tue, Aug 06, 2013 at 06:57:10PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Hi Here's my next attempt at the Haswell PC8+ feature. What's new? 1. I hope I implemented the feedback from Chris. Many function renames, a few style changes. Chris' biggest concern was about the interrupts. So in this series you'll see patches 2-6 moving some interrupt code around, so all changes to each IMR register should go through a single function. With this change we can easily detect when someone wants to change IMR but interrupts are disabled. As he suggested, we also don't enable the interrupts in that case. 2. Another difference is that now we also disable PC8 when there's GPU work to do. The lack of interrupts made things like glxgears work at 2 FPS. I talked to Ben about this and what I understood is that the lack of interrupts is not really a problem, it just makes things slower. But still I decided to write the code to disable PC8 when we have GPU work to do, so background apps can run faster than 2 FPS. Ben just mislead you here, this is only true due to an implemenation detail on our hw simulation enviroment. On real hw running gem workloads without interrupt support very much blows up as soon as something hits __wait_seqno and actually goes to sleep on the waitqueue. If you pimp the your igt testcase in the test_batch code with the dummy workload and wait_rendering stuff you should be able to hit this fairly easily. I haven't looked yet but I think adding a are interrupts working/am I out of pc8+ check to __wait_seqno would be good. Anyway, the current patches are not supposed to be broken since we try to disable PC8 when there's stuff to render, but I'll add the check and test everything. -Daniel 3. Another difference is that now we enable PC8 on a delayed work function that only gets called if we stay with 0 refcount for at least 5 seconds. So when someone runs xrandr we won't enable/disable PC8 dozens of times. Also, on xset dpms force off we'll only get to PC8 if the desktop environment decides to not do rendering every second. Not getting to PC8? Fix your desktop environment! 4. Due to 3 and the fact that Daniel didn't seem to like my do DP AUX and GMBUS without interrupts approach, the previous patches that implemented this just got dropped: we now have the delayed work thing which should replace them. 5. I also added code to wake up from PC8 when doing suspend, we need this. Despite this, the other thing to discuss is about the size of patch 7. I can certainly try to split it, but I really think that if all you want is to take a brief look at the code just and drop some bikesheds, then having a big patch that implements everything in one piece seems better. I can split this later if needed. I'm also open to ideas on how to really split this patch. Also notice that even if the patch is big, it doesn't remove a single line of the current code. And it adds PC8 disabled by default, so if git bisect points to that patch the surface to look will be small. Another thing worth mentioning is that all this code doesn't have IS_HASWELL checks, and on non-Haswell platforms the refcount will never reach 0, so we won't ever try to enable PC8. I'm not sure if that's what we want, so please comment on that. Even if we decide to delay patch 7, we could try to merge patches 1-6 if they look acceptable. I'm also not really sure if we want the last patch yet, but it's there just in case. Also, I couldn't do i-g-t regression testing since the i-g-t test suite is quite unreliable on Haswell right now. Thanks, Paulo Paulo Zanoni (9): drm/i915: add the FCLK case to intel_ddi_get_cdclk_freq drm/i915: wrap GTIMR changes drm/i915: wrap GEN6_PMIMR changes drm/i915: don't update GEN6_PMIMR when it's not needed drm/i915: add dev_priv-pm_irq_mask drm/i915: don't disable/reenable IVB error interrupts when not needed drm/i915: allow package C8+ states on Haswell (disabled) drm/i915: add i915_pc8_status debugfs file drm/i915: enable Package C8+ by default drivers/gpu/drm/i915/i915_debugfs.c | 25 drivers/gpu/drm/i915/i915_dma.c | 10 ++ drivers/gpu/drm/i915/i915_drv.c | 11 ++ drivers/gpu/drm/i915/i915_drv.h | 73 drivers/gpu/drm/i915/i915_irq.c | 202 +--- drivers/gpu/drm/i915/intel_ddi.c| 9 +- drivers/gpu/drm/i915/intel_display.c| 164 ++ drivers/gpu/drm/i915/intel_dp.c | 3 + drivers/gpu/drm/i915/intel_drv.h| 14 +++ drivers/gpu/drm/i915/intel_i2c.c| 2 + drivers/gpu/drm/i915/intel_pm.c | 13 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 32 ++--- 12 files changed, 518 insertions(+),
Re: [Intel-gfx] [PATCH 4/9] drm/i915: don't update GEN6_PMIMR when it's not needed
2013/8/6 Chris Wilson ch...@chris-wilson.co.uk: On Tue, Aug 06, 2013 at 06:57:14PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com I did some brief tests and the new_val = pmimr condition usually happens a few times after exiting games. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com I'm not sure of the value of this patch by itself. It did make me wonder what you were micro-optimising, and then I saw patch 5 and it made more sense. Patches 4 and 5 are just micro optimizations and shouldn't be needed for the PC8+, but I thought they would be useful. If you think they're not worth it, we can discard them. I was trying to make the code similar to the other IMR-changing functions. If we massage the code a little bit more we could make all the IMR-changing functions share the same code -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/9] drm/i915: allow package C8+ states on Haswell (disabled)
2013/8/6 Chris Wilson ch...@chris-wilson.co.uk:
On Tue, Aug 06, 2013 at 06:57:17PM -0300, Paulo Zanoni wrote:
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2ef4335..7f6ec9e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1590,6 +1590,8 @@ void intel_ring_advance(struct intel_ring_buffer *ring)
{
struct drm_i915_private *dev_priv = ring-dev-dev_private;
+ hsw_package_c8_gpu_busy(dev_priv);
+
Ahem, what is this doing here? If only we had something that was the
opposite of intel_mark_idle... At this point, I am going to get some
sleep...
You're suggesting me to use intel_mark_busy? I have to admit I saw it,
but I noticed intel_mark_busy is called after intel_ring_advance, and
I was trying to follow what Ben suggested me. If you're suggesting
this, I guess it's ok, so I will test this possibility.
Overall it looks much, much better. I am still uneasy about the
interrupt race whilst disabling them, so I need to think some more about
that. The new DRM_ERRORs should be WARN and a few other minor niggles. I
really do like the new names though, thanks.
Thanks for the reviews! I will change the errors to WARNs.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Some edid-decode patches
A bit more context than the previous patch that was a bit alone. This series parses yet a bit more of the EDID: - The HDMI VICs in the HDMI vendor specific block (HDMI 1.4, 4k modes) - The CEA Video Capability Data Block It also includes 2 EDIDs of TVs that have those bits. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [edid-decode 1/7] Print the resolutions next to the CEA VICs
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
edid-decode.c | 84 +--
1 file changed, 82 insertions(+), 2 deletions(-)
diff --git a/edid-decode.c b/edid-decode.c
index 9840db6..7515181 100644
--- a/edid-decode.c
+++ b/edid-decode.c
@@ -32,6 +32,8 @@
#include time.h
#include ctype.h
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
+
static int claims_one_point_oh = 0;
static int claims_one_point_two = 0;
static int claims_one_point_three = 0;
@@ -569,14 +571,92 @@ cea_audio_block(unsigned char *x)
}
}
+static const char *edid_cea_modes[] = {
+640x480@60Hz,
+720x480@60Hz,
+720x480@60Hz,
+1280x720@60Hz,
+1920x1080i@60Hz,
+1440x480i@60Hz,
+1440x480i@60Hz,
+1440x240@60Hz,
+1440x240@60Hz,
+2880x480i@60Hz,
+2880x480i@60Hz
+2880x240@60Hz,
+2880x240@60Hz,
+1440x480@60Hz,
+1440x480@60Hz,
+1920x1080@60Hz,
+720x576@50Hz,
+720x576@50Hz,
+1280x720@50Hz,
+1920x1080i@50Hz,
+1440x576i@50Hz,
+1440x576i@50Hz,
+1440x288@50Hz,
+1440x288@50Hz,
+2880x576i@50Hz,
+2880x576i@50Hz,
+2880x288@50Hz,
+2880x288@50Hz,
+1440x576@50Hz,
+1440x576@50Hz,
+1920x1080@50Hz,
+1920x1080@24Hz,
+1920x1080@25Hz,
+1920x1080@30Hz,
+2880x480@60Hz,
+2880x480@60Hz,
+2880x576@50Hz,
+2880x576@50Hz,
+1920x1080i@50Hz,
+1920x1080i@100Hz,
+1280x720@100Hz,
+720x576@100Hz,
+720x576@100Hz,
+1440x576@100Hz,
+1440x576@100Hz,
+1920x1080i@120Hz,
+1280x720@120Hz,
+720x480@120Hz,
+720x480@120Hz,
+1440x480i@120Hz,
+1440x480i@120Hz,
+720x576@200Hz,
+720x576@200Hz,
+1440x576i@200Hz,
+1440x576i@200Hz,
+720x480@240Hz,
+720x480@240Hz,
+1440x480i@240Hz,
+1440x480i@240Hz,
+1280x720@24Hz,
+1280x720@25Hz,
+1280x720@30Hz,
+1920x1080@120Hz,
+1920x1080@100Hz,
+};
+
static void
cea_video_block(unsigned char *x)
{
int i;
int length = x[0] 0x1f;
-for (i = 1; i length; i++)
- printf(VIC %02d %s\n, x[i] 0x7f, x[i] 0x80 ? (native) : );
+for (i = 1; i length; i++) {
+ unsigned char vic = x[i] 0x7f;
+ unsigned char native = x[i] 0x80;
+ const char *mode;
+
+ vic--;
+ if (vic ARRAY_SIZE(edid_cea_modes))
+ mode = edid_cea_modes[vic];
+ else
+ mode = Unknown mode;
+
+ printf(VIC %02d %s %s\n, vic, mode, native ? (native) : );
+}
}
static void
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [edid-decode 2/7] Add Skyworth 50E780U 50 edid (4k TV)
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
data/skyworth-50e780u-hdmi | Bin 0 - 256 bytes
1 file changed, 0 insertions(+), 0 deletions(-)
create mode 100644 data/skyworth-50e780u-hdmi
diff --git a/data/skyworth-50e780u-hdmi b/data/skyworth-50e780u-hdmi
new file mode 100644
index
..b618a6ff5efbbe15141183266d961ba2c48dfd0c
GIT binary patch
literal 256
zcmZSh4+adr%|rB3=9l1VvNiUHcAy-yeAigyU$P;^6=UJo`De^$TAcKUXWAB(+Fia
z(x=G4Ajc@%AWkIrVyc{3K5wHH0%#UaCT*Reo;w=La2ue7f|FcgP{+903QRF0vIrE
zWMbAV@oo{2XB8GWD;Z(RpYE@7OaO2DAenTNrEVTX-=LjofcGoviSo*x=J2XNa
z;8tEJ0$M4cquKx#nb*U0p-bU`u7H5uGzMXivm}5b6BHZ_6?hy%p~#AOR)-?WZ{b
literal 0
HcmV?d1
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [edid-decode 3/7] Decode HDMI 1.4 4k VICs
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
edid-decode.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/edid-decode.c b/edid-decode.c
index 7515181..55e48a7 100644
--- a/edid-decode.c
+++ b/edid-decode.c
@@ -705,7 +705,7 @@ cea_hdmi_block(unsigned char *x)
if (x[8] 0x20) {
int mask = 0, formats = 0;
- int len_xx, len_3d;
+ int len_vic, len_3d;
printf(Extended HDMI video details:\n);
if (x[9 + b] 0x80)
printf( 3D present\n);
@@ -730,14 +730,17 @@ cea_hdmi_block(unsigned char *x)
printf( Base EDID image size is in units of 5cm\n);
break;
}
- len_xx = (x[10 + b] 0xe0) 5;
+ len_vic = (x[10 + b] 0xe0) 5;
len_3d = (x[10 + b] 0x1f) 0;
b += 2;
- if (len_xx) {
- printf( Skipping %d bytes that HDMI refuses to publicly
- document\n, len_xx);
- b += len_xx;
+ if (len_vic) {
+ int i;
+
+ for (i = 0; i len_vic; i++)
+ printf( HDMI VIC %d\n, x[9 + b + i]);
+
+ b += len_vic;
}
if (len_3d) {
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [edid-decode 4/7] Print the HDMI resolution next to the HDMI VICs
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
edid-decode.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/edid-decode.c b/edid-decode.c
index 55e48a7..5061228 100644
--- a/edid-decode.c
+++ b/edid-decode.c
@@ -659,6 +659,13 @@ cea_video_block(unsigned char *x)
}
}
+static const char *edid_cea_hdmi_modes[] = {
+3840x2160@30Hz,
+3840x2160@25Hz,
+3840x2160@24Hz,
+4096x2160@24Hz,
+};
+
static void
cea_hdmi_block(unsigned char *x)
{
@@ -737,8 +744,18 @@ cea_hdmi_block(unsigned char *x)
if (len_vic) {
int i;
- for (i = 0; i len_vic; i++)
- printf( HDMI VIC %d\n, x[9 + b + i]);
+ for (i = 0; i len_vic; i++) {
+unsigned char vic = x[9 + b + i];
+const char *mode;
+
+vic--;
+if (vic ARRAY_SIZE(edid_cea_hdmi_modes))
+mode = edid_cea_hdmi_modes[vic];
+else
+mode = Unknown mode;
+
+ printf( HDMI VIC %d %s\n, vic, mode);
+}
b += len_vic;
}
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [edid-decode 5/7] Add the EDID of a Samsung TV that has a VCDB
---
data/samsung-UE40D8000YU-hmdi | Bin 0 - 256 bytes
1 file changed, 0 insertions(+), 0 deletions(-)
create mode 100644 data/samsung-UE40D8000YU-hmdi
diff --git a/data/samsung-UE40D8000YU-hmdi b/data/samsung-UE40D8000YU-hmdi
new file mode 100644
index
..ba87cb02837173ede00d01206808dd4705eff699
GIT binary patch
literal 256
zcmZSh4+acAx}a=85kJ!LQSHB8@7z-c4K_;xki?KOki9`-VdQMutX*#)hd3Q~5VD
ztaMqYLFj~=E=ab;K#1=JrWmjxkU`qOp}-6(0u=qrAmJ?)D9*s800uyjKMcW+
zzQLh?hqly2qtEukKPmHS%g_dn1vJ+m6SQz*_(k5dBni~n3b8ah?$4MfnkTP!UYaS
z2X=dhfCqLc3t0+AW~t+5oYeQI=s(;b#9U7qyFcF{{88HO`9aV^$c|g0dferx8
CDLnxI
literal 0
HcmV?d1
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [edid-decode 6/7] Add a small framework to decode fields generically
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
edid-decode.c | 69 +--
1 file changed, 67 insertions(+), 2 deletions(-)
diff --git a/edid-decode.c b/edid-decode.c
index 5061228..7aed3c6 100644
--- a/edid-decode.c
+++ b/edid-decode.c
@@ -32,8 +32,6 @@
#include time.h
#include ctype.h
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
-
static int claims_one_point_oh = 0;
static int claims_one_point_two = 0;
static int claims_one_point_three = 0;
@@ -65,6 +63,73 @@ static int warning_zero_preferred_refresh = 0;
static int conformant = 1;
+struct value {
+int value;
+const char *description;
+};
+
+struct field {
+const char *name;
+int start, end;
+struct value *values;
+int n_values;
+};
+
+#define DEFINE_FIELD(n, var, s, e, ...)\
+static struct value var##_values[] = {\
+__VA_ARGS__\
+}; \
+static struct field var = {\
+.name = n, \
+.start = s,\
+.end = e, \
+.values = var##_values,\
+.n_values = ARRAY_SIZE(var##_values), \
+}
+
+static void
+decode_value(struct field *field, int val, const char *prefix)
+{
+struct value *v;
+int i;
+
+for (i = 0; i field-n_values; i++) {
+v = field-values[i];
+
+if (v-value == val)
+ break;
+}
+
+if (i == field-n_values) {
+ printf(%s%s: %d\n, prefix, field-name, val);
+ return;
+}
+
+printf(%s%s: %s (%d)\n, prefix, field-name, v-description, val);
+}
+
+static void
+_decode(struct field **fields, int n_fields, int data, const char *prefix)
+{
+int i;
+
+for (i = 0; i n_fields; i++) {
+ struct field *f = fields[i];
+ int field_length = f-end - f-start + 1;
+ int val;
+
+if (field_length == 32)
+val = data;
+else
+val = (data f-start) ((1 field_length) - 1);
+
+decode_value(f, val, prefix);
+}
+}
+
+#define decode(fields, data, prefix)\
+_decode(fields, ARRAY_SIZE(fields), data, prefix)
+
static char *manufacturer_name(unsigned char *x)
{
static char name[4];
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [edid-decode 7/7] Decode the Video Capability Data Block
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
edid-decode.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/edid-decode.c b/edid-decode.c
index 7aed3c6..58297c9 100644
--- a/edid-decode.c
+++ b/edid-decode.c
@@ -861,6 +861,44 @@ cea_hdmi_block(unsigned char *x)
}
}
+DEFINE_FIELD(YCbCr quantization, YCbCr_quantization, 7, 7,
+ { 0, No Data },
+ { 1, Selectable (via AVI YQ) });
+DEFINE_FIELD(RGB quantization, RGB_quantization, 6, 6,
+ { 0, No Data },
+ { 1, Selectable (via AVI Q) });
+DEFINE_FIELD(PT scan behaviour, PT_scan, 4, 5,
+ { 0, No Data },
+ { 1, Always Overscannned },
+ { 2, Always Underscanned },
+ { 3, Support both over- and underscan });
+DEFINE_FIELD(IT scan behaviour, IT_scan, 2, 3,
+ { 0, IT video formats not supported },
+ { 1, Always Overscannned },
+ { 2, Always Underscanned },
+ { 3, Support both over- and underscan });
+DEFINE_FIELD(CE scan behaviour, CE_scan, 0, 1,
+ { 0, CE video formats not supported },
+ { 1, Always Overscannned },
+ { 2, Always Underscanned },
+ { 3, Support both over- and underscan });
+
+static struct field *vcdb_fields[] = {
+YCbCr_quantization,
+RGB_quantization,
+PT_scan,
+IT_scan,
+CE_scan,
+};
+
+static void
+cea_vcdb(unsigned char *x)
+{
+unsigned char d = x[2];
+
+decode(vcdb_fields, d, );
+}
+
static void
cea_block(unsigned char *x)
{
@@ -895,6 +933,7 @@ cea_block(unsigned char *x)
switch (x[1]) {
case 0x00:
printf(video capability data block\n);
+ cea_vcdb(x);
break;
case 0x01:
printf(vendor-specific video data block\n);
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/9] drm/i915: don't update GEN6_PMIMR when it's not needed
On Wed, Aug 07, 2013 at 10:34:11AM -0300, Paulo Zanoni wrote: 2013/8/6 Chris Wilson ch...@chris-wilson.co.uk: On Tue, Aug 06, 2013 at 06:57:14PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com I did some brief tests and the new_val = pmimr condition usually happens a few times after exiting games. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com I'm not sure of the value of this patch by itself. It did make me wonder what you were micro-optimising, and then I saw patch 5 and it made more sense. Patches 4 and 5 are just micro optimizations and shouldn't be needed for the PC8+, but I thought they would be useful. If you think they're not worth it, we can discard them. I was trying to make the code similar to the other IMR-changing functions. Combined together, I think the micro-optimisation makes sense and would say it was less of a micro-optimisation than a consistent design to use the bookkeeping instead of touching registers. Just on its own this patch caused me to do a double-take and question what your motivation was. If we massage the code a little bit more we could make all the IMR-changing functions share the same code Sure, that may be worthwhile. Probably borderline though, I envisage it will take more code to setup than it will save. -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 7/9] drm/i915: allow package C8+ states on Haswell (disabled)
On Wed, Aug 07, 2013 at 10:38:19AM -0300, Paulo Zanoni wrote:
2013/8/6 Chris Wilson ch...@chris-wilson.co.uk:
On Tue, Aug 06, 2013 at 06:57:17PM -0300, Paulo Zanoni wrote:
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2ef4335..7f6ec9e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1590,6 +1590,8 @@ void intel_ring_advance(struct intel_ring_buffer
*ring)
{
struct drm_i915_private *dev_priv = ring-dev-dev_private;
+ hsw_package_c8_gpu_busy(dev_priv);
+
Ahem, what is this doing here? If only we had something that was the
opposite of intel_mark_idle... At this point, I am going to get some
sleep...
You're suggesting me to use intel_mark_busy? I have to admit I saw it,
but I noticed intel_mark_busy is called after intel_ring_advance, and
I was trying to follow what Ben suggested me. If you're suggesting
this, I guess it's ok, so I will test this possibility.
intel_mark_busy() is where we note the transition in userspace activity,
it should be where we put all the little hooks and pm tweaks required.
If you however need the pc8 disable earlier than the ring register write,
we should do it in intel_ring_begin(). Ultimately, though we should be
able to squash it into a single operation such that we never submit a
command before intel_mark_busy(). Hmm, easier in fact just to move
intel_mark_busy() to intel_ring_begin() and I think everybody is happy.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [edid-decode] Add a small framework to decode fields generically
v2: Fix rebase fail that removed a necessary hunk
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
edid-decode.c | 67 +++
1 file changed, 67 insertions(+)
diff --git a/edid-decode.c b/edid-decode.c
index 5061228..083ddd9 100644
--- a/edid-decode.c
+++ b/edid-decode.c
@@ -65,6 +65,73 @@ static int warning_zero_preferred_refresh = 0;
static int conformant = 1;
+struct value {
+int value;
+const char *description;
+};
+
+struct field {
+const char *name;
+int start, end;
+struct value *values;
+int n_values;
+};
+
+#define DEFINE_FIELD(n, var, s, e, ...)\
+static struct value var##_values[] = {\
+__VA_ARGS__\
+}; \
+static struct field var = {\
+.name = n, \
+.start = s,\
+.end = e, \
+.values = var##_values,\
+.n_values = ARRAY_SIZE(var##_values), \
+}
+
+static void
+decode_value(struct field *field, int val, const char *prefix)
+{
+struct value *v;
+int i;
+
+for (i = 0; i field-n_values; i++) {
+v = field-values[i];
+
+if (v-value == val)
+ break;
+}
+
+if (i == field-n_values) {
+ printf(%s%s: %d\n, prefix, field-name, val);
+ return;
+}
+
+printf(%s%s: %s (%d)\n, prefix, field-name, v-description, val);
+}
+
+static void
+_decode(struct field **fields, int n_fields, int data, const char *prefix)
+{
+int i;
+
+for (i = 0; i n_fields; i++) {
+ struct field *f = fields[i];
+ int field_length = f-end - f-start + 1;
+ int val;
+
+if (field_length == 32)
+val = data;
+else
+val = (data f-start) ((1 field_length) - 1);
+
+decode_value(f, val, prefix);
+}
+}
+
+#define decode(fields, data, prefix)\
+_decode(fields, ARRAY_SIZE(fields), data, prefix)
+
static char *manufacturer_name(unsigned char *x)
{
static char name[4];
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Second HDMI port not visible
On Wed, 7 Aug 2013 09:41:39 +0200
Daniel Vetter dan...@ffwll.ch wrote:
On Wed, Aug 7, 2013 at 5:10 AM, Matsumura, Ryan
ryan.matsum...@intel.com wrote:
I have a BayTrail board with two HDMI ports and running the default Tizen
3.0M1 release. The first HDMI shows up just fine but I can't get the
second screen to display anything. I tried enabling the second screen
through the kernel command line parameters (video=HDMI-1:e video=HDMI-2:e)
and running xrandr. This is my output from xrandr -q
Iirc Baytrail still has a bunch of hardcoded ports ... Jesse?
I don't know of any boards with two HDMI ports, but if they're wired up
correctly something like this might work:
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_d
index 468dbc9..81e86af 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9304,8 +9304,13 @@ static void intel_setup_outputs(struct drm_device *dev)
intel_dp_init(dev, PCH_DP_D, PORT_D);
} else if (IS_VALLEYVIEW(dev)) {
/* Check for built-in panel first. Shares lanes with HDMI on SDV
- if (I915_READ(VLV_DISPLAY_BASE + DP_C) DP_DETECTED)
- intel_dp_init(dev, VLV_DISPLAY_BASE + DP_C, PORT_C);
+ if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIC) SDVO_DETECTED) {
+ intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIC,
+ PORT_C);
+ if (I915_READ(VLV_DISPLAY_BASE + DP_C) DP_DETECTED)
+ intel_dp_init(dev, VLV_DISPLAY_BASE + DP_C,
+ PORT_C);
+ }
if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIB) SDVO_DETECTED) {
intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIB,
--
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 7/9] drm/i915: allow package C8+ states on Haswell (disabled)
On Wed, Aug 07, 2013 at 03:20:09PM +0100, Chris Wilson wrote:
On Wed, Aug 07, 2013 at 10:38:19AM -0300, Paulo Zanoni wrote:
2013/8/6 Chris Wilson ch...@chris-wilson.co.uk:
On Tue, Aug 06, 2013 at 06:57:17PM -0300, Paulo Zanoni wrote:
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2ef4335..7f6ec9e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1590,6 +1590,8 @@ void intel_ring_advance(struct intel_ring_buffer
*ring)
{
struct drm_i915_private *dev_priv = ring-dev-dev_private;
+ hsw_package_c8_gpu_busy(dev_priv);
+
Ahem, what is this doing here? If only we had something that was the
opposite of intel_mark_idle... At this point, I am going to get some
sleep...
You're suggesting me to use intel_mark_busy? I have to admit I saw it,
but I noticed intel_mark_busy is called after intel_ring_advance, and
I was trying to follow what Ben suggested me. If you're suggesting
this, I guess it's ok, so I will test this possibility.
intel_mark_busy() is where we note the transition in userspace activity,
it should be where we put all the little hooks and pm tweaks required.
If you however need the pc8 disable earlier than the ring register write,
we should do it in intel_ring_begin(). Ultimately, though we should be
able to squash it into a single operation such that we never submit a
command before intel_mark_busy(). Hmm, easier in fact just to move
intel_mark_busy() to intel_ring_begin() and I think everybody is happy.
We have oddball places where we add stuff to the ring but not a request,
so we'd miss the eventual mark_idle. But since this is only to make sure
we have working interrupts the current place of mark_busy should be good
enough.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
On 08/07/2013 06:55 PM, Daniel Vetter wrote: On Wed, Aug 7, 2013 at 11:40 AM, Inki Dae inki@samsung.com wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] Sent: Wednesday, August 07, 2013 6:15 PM To: DRI Development Cc: Intel Graphics Development; Daniel Vetter; Inki Dae Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers Note that this is slightly tricky since both drivers store their native objects in dma_buf-priv. But both also embed the base drm_gem_object at the first position, so the implicit cast is ok. To use the release helper we need to export it, too. Yeah, may I repost this patch with additional work? We also need to export with a gem object instead of specific one like you did. I think dmabuf stuff of exynos can be replaced to common drm_gem_dmabuf. Already dmabuf stuff of drm_gem_cma_helper.c was substituted to common drm_gem_dmabuf with low-level hook functions to use prime helpers. Thanks. I'm confused here what you mean, so pls just submit the patch. That usually helps ;-) -Daniel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
On 08/07/2013 07:21 PM, Daniel Vetter wrote: On Wed, Aug 07, 2013 at 07:18:45PM +0900, Joonyoung Shim wrote: On 08/07/2013 06:55 PM, Daniel Vetter wrote: On Wed, Aug 7, 2013 at 11:40 AM, Inki Dae inki@samsung.com wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] Sent: Wednesday, August 07, 2013 6:15 PM To: DRI Development Cc: Intel Graphics Development; Daniel Vetter; Inki Dae Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers Note that this is slightly tricky since both drivers store their native objects in dma_buf-priv. But both also embed the base drm_gem_object at the first position, so the implicit cast is ok. To use the release helper we need to export it, too. Yeah, may I repost this patch with additional work? We also need to export with a gem object instead of specific one like you did. I think dmabuf stuff of exynos can be replaced to common drm_gem_dmabuf. Already dmabuf stuff of drm_gem_cma_helper.c was substituted to common drm_gem_dmabuf with low-level hook functions to use prime helpers. Ah, but that can easily be done on top of this, right? -Daniel I think it doesn't matter. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: List objects allocated from stolen memory in debugfs
I was curious as to what objects were currently allocated from stolen
memory, and so exported it from debugfs.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
drivers/gpu/drm/i915/i915_debugfs.c | 63 +
1 file changed, 63 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 07f5896..4243b63 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -30,6 +30,7 @@
#include linux/debugfs.h
#include linux/slab.h
#include linux/export.h
+#include linux/list_sort.h
#include drm/drmP.h
#include intel_drv.h
#include intel_ringbuffer.h
@@ -187,6 +188,67 @@ static int i915_gem_object_list_info(struct seq_file *m,
void *data)
return 0;
}
+static int obj_rank_by_stolen(void *priv,
+ struct list_head *A, struct list_head *B)
+{
+ struct drm_i915_gem_object *a =
+ container_of(A, struct drm_i915_gem_object, exec_list);
+ struct drm_i915_gem_object *b =
+ container_of(B, struct drm_i915_gem_object, exec_list);
+
+ return a-stolen-start - b-stolen-start;
+}
+
+static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
+{
+ struct drm_info_node *node = (struct drm_info_node *) m-private;
+ struct drm_device *dev = node-minor-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct drm_i915_gem_object *obj;
+ size_t total_obj_size, total_gtt_size;
+ LIST_HEAD(stolen);
+ int count, ret;
+
+ ret = mutex_lock_interruptible(dev-struct_mutex);
+ if (ret)
+ return ret;
+
+ total_obj_size = total_gtt_size = count = 0;
+ list_for_each_entry(obj, dev_priv-mm.bound_list, global_list) {
+ if (obj-stolen == NULL)
+ continue;
+
+ list_add(obj-exec_list, stolen);
+
+ total_obj_size += obj-base.size;
+ total_gtt_size += i915_gem_obj_ggtt_size(obj);
+ count++;
+ }
+ list_for_each_entry(obj, dev_priv-mm.unbound_list, global_list) {
+ if (obj-stolen == NULL)
+ continue;
+
+ list_add(obj-exec_list, stolen);
+
+ total_obj_size += obj-base.size;
+ count++;
+ }
+ list_sort(NULL, stolen, obj_rank_by_stolen);
+ seq_puts(m, Stolen:\n);
+ while (!list_empty(stolen)) {
+ obj = list_first_entry(stolen, typeof(*obj), exec_list);
+ seq_puts(m,);
+ describe_obj(m, obj);
+ seq_putc(m, '\n');
+ list_del_init(obj-exec_list);
+ }
+ mutex_unlock(dev-struct_mutex);
+
+ seq_printf(m, Total %d objects, %zu bytes, %zu GTT size\n,
+ count, total_obj_size, total_gtt_size);
+ return 0;
+}
+
#define count_objects(list, member) do { \
list_for_each_entry(obj, list, member) { \
size += i915_gem_obj_ggtt_size(obj); \
@@ -2092,6 +2154,7 @@ static struct drm_info_list i915_debugfs_list[] = {
{i915_gem_pinned, i915_gem_gtt_info, 0, (void *) PINNED_LIST},
{i915_gem_active, i915_gem_object_list_info, 0, (void *) ACTIVE_LIST},
{i915_gem_inactive, i915_gem_object_list_info, 0, (void *)
INACTIVE_LIST},
+ {i915_gem_stolen, i915_gem_stolen_list_info },
{i915_gem_pageflip, i915_gem_pageflip_info, 0},
{i915_gem_request, i915_gem_request_info, 0},
{i915_gem_seqno, i915_gem_seqno_info, 0},
--
1.8.1.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 18/29] drm/i915: Use new bind/unbind in eviction code
On Wed, Aug 07, 2013 at 01:44:58AM +0200, Daniel Vetter wrote: On Wed, Aug 7, 2013 at 1:25 AM, Ben Widawsky b...@bwidawsk.net wrote: On Wed, Aug 07, 2013 at 12:59:29AM +0200, Daniel Vetter wrote: On Wed, Aug 7, 2013 at 12:57 AM, Ben Widawsky b...@bwidawsk.net wrote: We need evict_everything for #1. For #2, we call evict_something already for the vm when we go through the out of space path. If that failed, evicting everything for a specific VM is just the same operation. But maybe I've glossed over something in the details. Please correct if I'm wrong. Is there a case where evict something might fail with ENOSPC, and evict everything in a VM would help? Yes, when we've terminally fragmented the gtt we kick out everything and start over. That's the 3rd usecase. I am not seeing it. To me evict_something is what you want, and the fix for wherever the 3rd usecase is (please point it out, I'm dense) is it should call evict_something, not evict_everything. See the call to evict_everything in i915_gem_execbuffer.c:i915_gem_execbuffer_reserve As I was saying in the first response - you only hit this if evict_something() for a vm fails, right? That's the way ret == ENOSPC AFAICT. Like I've said if we can't fit a batch we do a last ditch effort of evicting everything and starting over anew. That's also what the retry logic in there is for. This happens after evict_something failed. Dunno what exactly isn't clear or what's confusing ... -Daniel Okay, sorted this out on IRC. You'll get a new patch as described with a new function for per vm eviction (which will just idle, and call evict_something() with proper args) -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 20/29] drm/i915: Fix up map and fenceable for VMA
On Tue, Aug 06, 2013 at 09:11:54PM +0200, Daniel Vetter wrote:
On Wed, Jul 31, 2013 at 05:00:13PM -0700, Ben Widawsky wrote:
formerly: drm/i915: Create VMAs (part 3.5) - map and fenceable
tracking
The map_and_fenceable tracking is per object. GTT mapping, and fences
only apply to global GTT. As such, object operations which are not
performed on the global GTT should not effect mappable or fenceable
characteristics.
Functionally, this commit could very well be squashed in to a previous
patch which updated object operations to take a VM argument. This
commit is split out because it's a bit tricky (or at least it was for
me).
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index d4d6444..ec23a5c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2626,7 +2626,7 @@ int i915_vma_unbind(struct i915_vma *vma)
trace_i915_vma_unbind(vma);
- if (obj-has_global_gtt_mapping)
+ if (obj-has_global_gtt_mapping i915_is_ggtt(vma-vm))
i915_gem_gtt_unbind_object(obj);
if (obj-has_aliasing_ppgtt_mapping) {
i915_ppgtt_unbind_object(dev_priv-mm.aliasing_ppgtt, obj);
Hm, shouldn't we do the is_ggtt check for both? After all only the global
ggtt can be aliased ever ... This would also be more symmetric with some
of the other global gtt checks I've spotted. You're take or will that run
afoul of your Great Plan?
-Daniel
You're right. The check makes sense for both cases. In both the original
series, and n a few patches, this code turns into:
vma-vm-unbind_vma(vma);
This ugliness is a result of bad rebasing on my part.
--
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] HDMI 4k support
This series parses one of the dark corners of the EDID to expose 4k x 2k modes to userspace. Those modes are part of HDMI 1.4. To complete the 4k HDMI support, one needs: * Hardware able to output a 300 MHz TMDS clock (Haswell can do that) and Daniel's patch to bump that limit in the kernel (already queued). Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Mon Jul 22 18:02:39 2013 +0200 drm/i915: fix hdmi portclock limits * The 2 patches attached here to parse the HDMI VICs in the HDMI vendor specific CEA block. * A DDX patch (for UXA) already merged (and in 2.21.14) Author: Damien Lespiau damien.lesp...@intel.com Date: Wed Jul 31 18:50:51 2013 +0100 uxa/display: Keep the EDID blob around for the lifetime of an output * My Use the TMDS maximum frequency to check mode dot clock xserver series: http://lists.x.org/archives/xorg-devel/2013-August/037297.html The bug referencing all that and the testing by QA: https://bugs.freedesktop.org/show_bug.cgi?id=67030 Additionally, edid-decode has been taught about those modes as well: http://lists.freedesktop.org/archives/dri-devel/2013-August/043078.html -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/edid: Fix add_cea_modes() style issues
A few styles issues have creept in here, fix them before touching this
code again.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/drm_edid.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 95d6f4b..51342c4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2442,10 +2442,10 @@ add_alternate_cea_modes(struct drm_connector
*connector, struct edid *edid)
}
static int
-do_cea_modes (struct drm_connector *connector, u8 *db, u8 len)
+do_cea_modes(struct drm_connector *connector, u8 *db, u8 len)
{
struct drm_device *dev = connector-dev;
- u8 * mode, cea_mode;
+ u8 *mode, cea_mode;
int modes = 0;
for (mode = db; mode db + len; mode++) {
@@ -2502,8 +2502,8 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
static int
add_cea_modes(struct drm_connector *connector, struct edid *edid)
{
- u8 * cea = drm_find_cea_extension(edid);
- u8 * db, dbl;
+ u8 *cea = drm_find_cea_extension(edid);
+ u8 *db, dbl;
int modes = 0;
if (cea cea_revision(cea) = 3) {
@@ -2517,7 +2517,7 @@ add_cea_modes(struct drm_connector *connector, struct
edid *edid)
dbl = cea_db_payload_len(db);
if (cea_db_tag(db) == VIDEO_BLOCK)
- modes += do_cea_modes (connector, db+1, dbl);
+ modes += do_cea_modes(connector, db + 1, dbl);
}
}
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/edid: Parse the HDMI CEA block and look for 4k modes
HDMI 1.4 adds 4 4k x 2k modes in the the CEA vendor specific block.
With this commit, we now parse this block and expose the 4k modes that
we find there.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Tested-by: Cancan Feng cancan.f...@intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67030
---
drivers/gpu/drm/drm_edid.c | 115 +++--
1 file changed, 100 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 51342c4..b43d64f 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -931,6 +931,36 @@ static const struct drm_display_mode edid_cea_modes[] = {
.vrefresh = 100, },
};
+/*
+ * HDMI 1.4 4k modes.
+ */
+static const struct drm_display_mode edid_4k_modes[] = {
+ /* 1 - 3840x2160@30Hz */
+ { DRM_MODE(3840x2160, DRM_MODE_TYPE_DRIVER, 297000,
+ 3840, 4016, 4104, 4400, 0,
+ 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 30, },
+ /* 2 - 3840x2160@25Hz */
+ { DRM_MODE(3840x2160, DRM_MODE_TYPE_DRIVER, 297000,
+ 3840, 4896, 4984, 5280, 0,
+ 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 25, },
+ /* 3 - 3840x2160@24Hz */
+ { DRM_MODE(3840x2160, DRM_MODE_TYPE_DRIVER, 297000,
+ 3840, 5116, 5204, 5500, 0,
+ 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, },
+ /* 4 - 4096x2160@24Hz (SMPTE) */
+ { DRM_MODE(3840x2160, DRM_MODE_TYPE_DRIVER, 297000,
+ 4096, 5116, 5204, 5500, 0,
+ 2160, 2168, 2178, 2250, 0,
+ DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, },
+};
+
/*** DDC fetch and block validation ***/
static const u8 edid_header[] = {
@@ -2465,6 +2495,59 @@ do_cea_modes(struct drm_connector *connector, u8 *db, u8
len)
return modes;
}
+static int do_hdmi_vsdb_modes(struct drm_connector *connector, u8 *db, u8 len)
+{
+ struct drm_device *dev = connector-dev;
+ int modes = 0, offset = 0, i;
+ u8 vic_len;
+
+ if (len 8)
+ goto out;
+
+ /* no HDMI_Video_Present */
+ if (!(db[8] (1 5)))
+ goto out;
+
+ /* Latency_Fields_Present */
+ if (db[8] (1 7))
+ offset += 2;
+
+ /* I_Latency_Fields_Present */
+ if (db[8] (1 6))
+ offset += 2;
+
+ /* the declared length is not long enough for the 2 first bytes
+* of additional video format capabilities */
+ if (len (10 + offset))
+ goto out;
+
+ vic_len = db[10 + offset] 5;
+ offset += 2;
+
+ for (i = 0; i vic_len len = (9 + offset + i); i++) {
+ struct drm_display_mode *newmode;
+ u8 vic;
+
+ vic = db[9 + offset + i];
+
+ vic--; /* VICs start at 1 */
+ if (vic = ARRAY_SIZE(edid_4k_modes)) {
+ DRM_ERROR(Unknow HDMI VIC: %d\n, vic);
+ continue;
+ }
+
+ newmode = drm_mode_duplicate(dev, edid_4k_modes[vic]);
+ if (!newmode)
+ continue;
+
+ drm_mode_probed_add(connector, newmode);
+ modes++;
+ }
+
+out:
+ return modes;
+}
+
static int
cea_db_payload_len(const u8 *db)
{
@@ -2496,6 +2579,21 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
return 0;
}
+static bool cea_db_is_hdmi_vsdb(const u8 *db)
+{
+ int hdmi_id;
+
+ if (cea_db_tag(db) != VENDOR_BLOCK)
+ return false;
+
+ if (cea_db_payload_len(db) 5)
+ return false;
+
+ hdmi_id = db[1] | (db[2] 8) | (db[3] 16);
+
+ return hdmi_id == HDMI_IDENTIFIER;
+}
+
#define for_each_cea_db(cea, i, start, end) \
for ((i) = (start); (i) (end) (i) +
cea_db_payload_len((cea)[(i)]) (end); (i) += cea_db_payload_len((cea)[(i)])
+ 1)
@@ -2518,6 +2616,8 @@ add_cea_modes(struct drm_connector *connector, struct
edid *edid)
if (cea_db_tag(db) == VIDEO_BLOCK)
modes += do_cea_modes(connector, db + 1, dbl);
+ else if (cea_db_is_hdmi_vsdb(db))
+ modes += do_hdmi_vsdb_modes(connector, db, dbl);
}
}
@@ -2570,21 +2670,6 @@ monitor_name(struct detailed_timing *t, void *data)
*(u8 **)data = t-data.other_data.data.str.str;
}
-static bool cea_db_is_hdmi_vsdb(const u8 *db)
-{
- int hdmi_id;
-
- if (cea_db_tag(db) != VENDOR_BLOCK)
- return false;
-
- if (cea_db_payload_len(db) 5)
- return false;
-
-
Re: [Intel-gfx] Second HDMI port not visible
Awesome, that worked thanks Jesse! Will this be just a hack or will you push
this as a fix in future releases?
-Ryan
-Original Message-
From: Jesse Barnes [mailto:jbar...@virtuousgeek.org]
Sent: Wednesday, August 07, 2013 8:49 AM
To: Daniel Vetter
Cc: Matsumura, Ryan; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] Second HDMI port not visible
On Wed, 7 Aug 2013 09:41:39 +0200
Daniel Vetter dan...@ffwll.ch wrote:
On Wed, Aug 7, 2013 at 5:10 AM, Matsumura, Ryan
ryan.matsum...@intel.com wrote:
I have a BayTrail board with two HDMI ports and running the default Tizen
3.0M1 release. The first HDMI shows up just fine but I can't get the
second screen to display anything. I tried enabling the second screen
through the kernel command line parameters (video=HDMI-1:e video=HDMI-2:e)
and running xrandr. This is my output from xrandr -q
Iirc Baytrail still has a bunch of hardcoded ports ... Jesse?
I don't know of any boards with two HDMI ports, but if they're wired up
correctly something like this might work:
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_d
index 468dbc9..81e86af 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9304,8 +9304,13 @@ static void intel_setup_outputs(struct drm_device *dev)
intel_dp_init(dev, PCH_DP_D, PORT_D);
} else if (IS_VALLEYVIEW(dev)) {
/* Check for built-in panel first. Shares lanes with HDMI on SDV
- if (I915_READ(VLV_DISPLAY_BASE + DP_C) DP_DETECTED)
- intel_dp_init(dev, VLV_DISPLAY_BASE + DP_C, PORT_C);
+ if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIC) SDVO_DETECTED) {
+ intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIC,
+ PORT_C);
+ if (I915_READ(VLV_DISPLAY_BASE + DP_C) DP_DETECTED)
+ intel_dp_init(dev, VLV_DISPLAY_BASE + DP_C,
+ PORT_C);
+ }
if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIB) SDVO_DETECTED) {
intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIB,
--
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 2/2] drm/edid: Parse the HDMI CEA block and look for 4k modes
On Wed, Aug 07, 2013 at 07:39:14PM +0100, Damien Lespiau wrote:
HDMI 1.4 adds 4 4k x 2k modes in the the CEA vendor specific block.
With this commit, we now parse this block and expose the 4k modes that
we find there.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Tested-by: Cancan Feng cancan.f...@intel.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67030
---
drivers/gpu/drm/drm_edid.c | 115
+++--
1 file changed, 100 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 51342c4..b43d64f 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -931,6 +931,36 @@ static const struct drm_display_mode edid_cea_modes[] = {
.vrefresh = 100, },
};
+/*
+ * HDMI 1.4 4k modes.
+ */
+static const struct drm_display_mode edid_4k_modes[] = {
+ /* 1 - 3840x2160@30Hz */
+ { DRM_MODE(3840x2160, DRM_MODE_TYPE_DRIVER, 297000,
+3840, 4016, 4104, 4400, 0,
+2160, 2168, 2178, 2250, 0,
+DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 30, },
+ /* 2 - 3840x2160@25Hz */
+ { DRM_MODE(3840x2160, DRM_MODE_TYPE_DRIVER, 297000,
+3840, 4896, 4984, 5280, 0,
+2160, 2168, 2178, 2250, 0,
+DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 25, },
+ /* 3 - 3840x2160@24Hz */
+ { DRM_MODE(3840x2160, DRM_MODE_TYPE_DRIVER, 297000,
+3840, 5116, 5204, 5500, 0,
+2160, 2168, 2178, 2250, 0,
+DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, },
+ /* 4 - 4096x2160@24Hz (SMPTE) */
+ { DRM_MODE(3840x2160, DRM_MODE_TYPE_DRIVER, 297000,
+4096, 5116, 5204, 5500, 0,
+2160, 2168, 2178, 2250, 0,
+DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
+ .vrefresh = 24, },
We should also add the 1000/1001 variants for #1 and #3. But we
don't want drm_match_cea_mode() to return the HDMI VIC, so a bit of
gymnastics are going to be required. I guess we want some kind of
drm_match_hdmi_mode() since we should also send the HDMI VIC in the
HDMI infoframe (if and when someone adds support for such a thing).
Also the mode #4 isn't supposed to have a 23.97Hz variant, so if we
want to utilize cea_mode_alternate_clock() for drm_match_hdmi_mode()
we need to add an exception there.
+};
+
/*** DDC fetch and block validation ***/
static const u8 edid_header[] = {
@@ -2465,6 +2495,59 @@ do_cea_modes(struct drm_connector *connector, u8 *db,
u8 len)
return modes;
}
+static int do_hdmi_vsdb_modes(struct drm_connector *connector, u8 *db, u8
len)
+{
Could be 'const u8 *db'
+ struct drm_device *dev = connector-dev;
+ int modes = 0, offset = 0, i;
+ u8 vic_len;
+
+ if (len 8)
+ goto out;
We skip the 'tag/len' byte for do_cea_modes(), but here we include it.
The resulting indexing is maybe a bit easier to compare w/ the spec,
but one must always keep in mind that the payload length doesn't include
byte 0. Not sure if we should just increase len by 1, or skip the tag
byte here too. Or maybe just add a comment explaining the situation.
+
+ /* no HDMI_Video_Present */
+ if (!(db[8] (1 5)))
+ goto out;
+
+ /* Latency_Fields_Present */
+ if (db[8] (1 7))
+ offset += 2;
+
+ /* I_Latency_Fields_Present */
+ if (db[8] (1 6))
+ offset += 2;
+
Maybe do the final 'offset += 2' already here? It would mean we could
say '8 + offset' below, which would somewhat nicely match the fact that
there are 8 fixed bytes in the payload, and the rest are shifty.
+ /* the declared length is not long enough for the 2 first bytes
+ * of additional video format capabilities */
+ if (len (10 + offset))
+ goto out;
+
+ vic_len = db[10 + offset] 5;
+ offset += 2;
+
+ for (i = 0; i vic_len len = (9 + offset + i); i++) {
+ struct drm_display_mode *newmode;
+ u8 vic;
+
+ vic = db[9 + offset + i];
+
+ vic--; /* VICs start at 1 */
+ if (vic = ARRAY_SIZE(edid_4k_modes)) {
+ DRM_ERROR(Unknow HDMI VIC: %d\n, vic);
+ continue;
+ }
+
+ newmode = drm_mode_duplicate(dev, edid_4k_modes[vic]);
+ if (!newmode)
+ continue;
+
+ drm_mode_probed_add(connector, newmode);
+ modes++;
+ }
+
+out:
+ return modes;
+}
+
static int
cea_db_payload_len(const u8 *db)
{
@@ -2496,6 +2579,21 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
return 0;
}
+static bool cea_db_is_hdmi_vsdb(const u8 *db)
+{
+ int hdmi_id;
+
+ if
Re: [Intel-gfx] [PATCH 1/2] drm/edid: Fix add_cea_modes() style issues
On Wed, Aug 07, 2013 at 07:39:13PM +0100, Damien Lespiau wrote:
A few styles issues have creept in here, fix them before touching this
code again.
You could also sprinke a bit of constness into these functions while
you're touching them.
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/drm_edid.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 95d6f4b..51342c4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2442,10 +2442,10 @@ add_alternate_cea_modes(struct drm_connector
*connector, struct edid *edid)
}
static int
-do_cea_modes (struct drm_connector *connector, u8 *db, u8 len)
+do_cea_modes(struct drm_connector *connector, u8 *db, u8 len)
{
struct drm_device *dev = connector-dev;
- u8 * mode, cea_mode;
+ u8 *mode, cea_mode;
int modes = 0;
for (mode = db; mode db + len; mode++) {
@@ -2502,8 +2502,8 @@ cea_db_offsets(const u8 *cea, int *start, int *end)
static int
add_cea_modes(struct drm_connector *connector, struct edid *edid)
{
- u8 * cea = drm_find_cea_extension(edid);
- u8 * db, dbl;
+ u8 *cea = drm_find_cea_extension(edid);
+ u8 *db, dbl;
int modes = 0;
if (cea cea_revision(cea) = 3) {
@@ -2517,7 +2517,7 @@ add_cea_modes(struct drm_connector *connector, struct
edid *edid)
dbl = cea_db_payload_len(db);
if (cea_db_tag(db) == VIDEO_BLOCK)
- modes += do_cea_modes (connector, db+1, dbl);
+ modes += do_cea_modes(connector, db + 1, dbl);
}
}
--
1.8.3.1
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: check that the i965g/gm 4G limit is really obeyed
In truly crazy circumstances shmem might give us the wrong type of page. So be a bit paranoid and double check this. Cc: Rob Clark robdcl...@gmail.com References: http://lkml.org/lkml/2011/7/11/238 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_gem.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 8508b3d..55fdd20 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1827,6 +1827,9 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj) sg-length += PAGE_SIZE; } last_pfn = page_to_pfn(page); + + /* Check that the i965g/gm workaround works. */ + WARN_ON((gfp __GFP_DMA32) (last_pfn = 0x0010UL)); } #ifdef CONFIG_SWIOTLB if (!swiotlb_nr_tbl()) -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Second HDMI port not visible
Chris's machine would be a good regression test for this. If it works
for him too, I think we should push it.
Thanks,
Jesse
On Wed, 7 Aug 2013 19:15:10 +
Matsumura, Ryan ryan.matsum...@intel.com wrote:
Awesome, that worked thanks Jesse! Will this be just a hack or will you
push this as a fix in future releases?
-Ryan
-Original Message-
From: Jesse Barnes [mailto:jbar...@virtuousgeek.org]
Sent: Wednesday, August 07, 2013 8:49 AM
To: Daniel Vetter
Cc: Matsumura, Ryan; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] Second HDMI port not visible
On Wed, 7 Aug 2013 09:41:39 +0200
Daniel Vetter dan...@ffwll.ch wrote:
On Wed, Aug 7, 2013 at 5:10 AM, Matsumura, Ryan
ryan.matsum...@intel.com wrote:
I have a BayTrail board with two HDMI ports and running the default Tizen
3.0M1 release. The first HDMI shows up just fine but I can't get the
second screen to display anything. I tried enabling the second screen
through the kernel command line parameters (video=HDMI-1:e
video=HDMI-2:e) and running xrandr. This is my output from xrandr -q
Iirc Baytrail still has a bunch of hardcoded ports ... Jesse?
I don't know of any boards with two HDMI ports, but if they're wired up
correctly something like this might work:
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_d
index 468dbc9..81e86af 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9304,8 +9304,13 @@ static void intel_setup_outputs(struct drm_device *dev)
intel_dp_init(dev, PCH_DP_D, PORT_D);
} else if (IS_VALLEYVIEW(dev)) {
/* Check for built-in panel first. Shares lanes with HDMI on
SDV
- if (I915_READ(VLV_DISPLAY_BASE + DP_C) DP_DETECTED)
- intel_dp_init(dev, VLV_DISPLAY_BASE + DP_C, PORT_C);
+ if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIC) SDVO_DETECTED)
{
+ intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIC,
+ PORT_C);
+ if (I915_READ(VLV_DISPLAY_BASE + DP_C) DP_DETECTED)
+ intel_dp_init(dev, VLV_DISPLAY_BASE + DP_C,
+ PORT_C);
+ }
if (I915_READ(VLV_DISPLAY_BASE + GEN4_HDMIB) SDVO_DETECTED)
{
intel_hdmi_init(dev, VLV_DISPLAY_BASE + GEN4_HDMIB,
--
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 20/29] drm/i915: Fix up map and fenceable for VMA
On Wed, Aug 07, 2013 at 11:37:04AM -0700, Ben Widawsky wrote:
On Tue, Aug 06, 2013 at 09:11:54PM +0200, Daniel Vetter wrote:
On Wed, Jul 31, 2013 at 05:00:13PM -0700, Ben Widawsky wrote:
formerly: drm/i915: Create VMAs (part 3.5) - map and fenceable
tracking
The map_and_fenceable tracking is per object. GTT mapping, and fences
only apply to global GTT. As such, object operations which are not
performed on the global GTT should not effect mappable or fenceable
characteristics.
Functionally, this commit could very well be squashed in to a previous
patch which updated object operations to take a VM argument. This
commit is split out because it's a bit tricky (or at least it was for
me).
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index d4d6444..ec23a5c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2626,7 +2626,7 @@ int i915_vma_unbind(struct i915_vma *vma)
trace_i915_vma_unbind(vma);
- if (obj-has_global_gtt_mapping)
+ if (obj-has_global_gtt_mapping i915_is_ggtt(vma-vm))
i915_gem_gtt_unbind_object(obj);
if (obj-has_aliasing_ppgtt_mapping) {
i915_ppgtt_unbind_object(dev_priv-mm.aliasing_ppgtt, obj);
Hm, shouldn't we do the is_ggtt check for both? After all only the global
ggtt can be aliased ever ... This would also be more symmetric with some
of the other global gtt checks I've spotted. You're take or will that run
afoul of your Great Plan?
-Daniel
You're right. The check makes sense for both cases. In both the original
series, and n a few patches, this code turns into:
vma-vm-unbind_vma(vma);
Ok, I've killed it and merged 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
[Intel-gfx] Optimization in intersect functions
This patch avoids the calculation of next points if unnecessary. Is this correct?? If yes, can someone commit?? Thanks Raul Fernandes rgfernan...@gmail.com opt-of-intersect-functions.patch Description: Binary data ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 21/29] drm/i915: mm_list is per VMA
On Tue, Aug 06, 2013 at 05:28:06PM -0700, Ben Widawsky wrote:
On Tue, Aug 06, 2013 at 09:38:41PM +0200, Daniel Vetter wrote:
On Wed, Jul 31, 2013 at 05:00:14PM -0700, Ben Widawsky wrote:
formerly: drm/i915: Create VMAs (part 5) - move mm_list
The mm_list is used for the active/inactive LRUs. Since those LRUs are
per address space, the link should be per VMx .
Because we'll only ever have 1 VMA before this point, it's not incorrect
to defer this change until this point in the patch series, and doing it
here makes the change much easier to understand.
Shamelessly manipulated out of Daniel:
active/inactive stuff is used by eviction when we run out of address
space, so needs to be per-vma and per-address space. Bound/unbound otoh
is used by the shrinker which only cares about the amount of memory used
and not one bit about in which address space this memory is all used in.
Of course to actual kick out an object we need to unbind it from every
address space, but for that we have the per-object list of vmas.
v2: only bump GGTT LRU in i915_gem_object_set_to_gtt_domain (Chris)
v3: Moved earlier in the series
v4: Add dropped message from v3
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Some comments below for this one. The lru changes look a bit strange so
I'll wait for your confirmation that the do_switch hunk has the same
reasons s the one in execbuf with the FIXME comment.
-Daniel
---
drivers/gpu/drm/i915/i915_debugfs.c| 53
--
drivers/gpu/drm/i915/i915_drv.h| 5 +--
drivers/gpu/drm/i915/i915_gem.c| 37 +++--
drivers/gpu/drm/i915/i915_gem_context.c| 3 ++
drivers/gpu/drm/i915/i915_gem_evict.c | 14
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 ++
drivers/gpu/drm/i915/i915_gem_stolen.c | 2 +-
drivers/gpu/drm/i915/i915_gpu_error.c | 37 -
8 files changed, 91 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 6d5ca85bd..181e5a6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -149,7 +149,7 @@ static int i915_gem_object_list_info(struct seq_file
*m, void *data)
struct drm_device *dev = node-minor-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct i915_address_space *vm = dev_priv-gtt.base;
- struct drm_i915_gem_object *obj;
+ struct i915_vma *vma;
size_t total_obj_size, total_gtt_size;
int count, ret;
@@ -157,6 +157,7 @@ static int i915_gem_object_list_info(struct seq_file
*m, void *data)
if (ret)
return ret;
+ /* FIXME: the user of this interface might want more than just GGTT */
switch (list) {
case ACTIVE_LIST:
seq_puts(m, Active:\n);
@@ -172,12 +173,12 @@ static int i915_gem_object_list_info(struct
seq_file *m, void *data)
}
total_obj_size = total_gtt_size = count = 0;
- list_for_each_entry(obj, head, mm_list) {
- seq_puts(m,);
- describe_obj(m, obj);
- seq_putc(m, '\n');
- total_obj_size += obj-base.size;
- total_gtt_size += i915_gem_obj_ggtt_size(obj);
+ list_for_each_entry(vma, head, mm_list) {
+ seq_printf(m,);
+ describe_obj(m, vma-obj);
+ seq_printf(m, \n);
+ total_obj_size += vma-obj-base.size;
+ total_gtt_size += i915_gem_obj_size(vma-obj, vma-vm);
Why not use vma-node.size? If you don't disagree I'll bikeshed this while
applying.
I think in terms of the diff, it's more logical to do it how I did. The
result should damn well be the same though, so go right ahead. When I
set about writing the series, I really didn't want to use
node.size/start directly as much as possible - so we can sneak things
into the helpers as needed.
I've applied this bikeshed, but the patch required some wiggling in and
conflict resolution. I've checked with your branch and that seems to be
due to the removel of the inactive list walking to adjust the gpu domains
in i915_gem_reset. Please check that I didn't botch the patch rebasing
with your tree.
-Daniel
count++;
}
mutex_unlock(dev-struct_mutex);
@@ -224,7 +225,18 @@ static int per_file_stats(int id, void *ptr, void
*data)
return 0;
}
-static int i915_gem_object_info(struct seq_file *m, void *data)
+#define count_vmas(list, member) do { \
+ list_for_each_entry(vma, list, member) { \
+ size += i915_gem_obj_ggtt_size(vma-obj); \
+ ++count; \
+ if (vma-obj-map_and_fenceable) { \
+ mappable_size += i915_gem_obj_ggtt_size(vma-obj); \
+ ++mappable_count; \
+ } \
+ } \
+} while
Re: [Intel-gfx] [PATCH 24/29] drm/i915: create vmas at execbuf
On Wed, Jul 31, 2013 at 05:00:17PM -0700, Ben Widawsky wrote:
In order to transition more of our code over to using a VMA instead of
an OBJ, VM pair - we must have the vma accessible at execbuf time. Up
until now, we've only had a VMA when actually binding an object.
The previous patch helped handle the distinction on bound vs. unbound.
This patch will help us catch leaks, and other issues before we actually
shuffle a bunch of stuff around.
The subsequent patch to fix up the rest of execbuf should be mostly just
moving code around, and this is the major functional change.
v2: Release table_lock earlier so vma allocation needn't be atomic.
(Chris)
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Merged up to this patch to dinq, thanks.
-Daniel
---
drivers/gpu/drm/i915/i915_drv.h| 3 +++
drivers/gpu/drm/i915/i915_gem.c| 25 ++---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 18 +-
3 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f6c2812..c0eb7fd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1857,6 +1857,9 @@ unsigned long i915_gem_obj_size(struct
drm_i915_gem_object *o,
struct i915_address_space *vm);
struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
struct i915_address_space *vm);
+struct i915_vma *
+i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
+ struct i915_address_space *vm);
/* Some GGTT VM helpers */
#define obj_to_ggtt(obj) \
(((struct drm_i915_private *)(obj)-base.dev-dev_private)-gtt.base)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 21331d8..72bd53c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3101,8 +3101,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object
*obj,
struct i915_vma *vma;
int ret;
- if (WARN_ON(!list_empty(obj-vma_list)))
- return -EBUSY;
+ BUG_ON(!i915_is_ggtt(vm));
fence_size = i915_gem_get_gtt_size(dev,
obj-base.size,
@@ -3142,16 +3141,15 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object
*obj,
i915_gem_object_pin_pages(obj);
- /* FIXME: For now we only ever use 1 VMA per object */
- BUG_ON(!i915_is_ggtt(vm));
- WARN_ON(!list_empty(obj-vma_list));
-
- vma = i915_gem_vma_create(obj, vm);
+ vma = i915_gem_obj_lookup_or_create_vma(obj, vm);
if (IS_ERR(vma)) {
i915_gem_object_unpin_pages(obj);
return PTR_ERR(vma);
}
+ /* For now we only ever use 1 vma per object */
+ WARN_ON(!list_is_singular(obj-vma_list));
+
search_free:
ret = drm_mm_insert_node_in_range_generic(vm-mm, vma-node,
size, alignment,
@@ -4800,3 +4798,16 @@ struct i915_vma *i915_gem_obj_to_vma(struct
drm_i915_gem_object *obj,
return NULL;
}
+
+struct i915_vma *
+i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
+ struct i915_address_space *vm)
+{
+ struct i915_vma *vma;
+
+ vma = i915_gem_obj_to_vma(obj, vm);
+ if (!vma)
+ vma = i915_gem_vma_create(obj, vm);
+
+ return vma;
+}
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 0f21702..3f17a55 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -85,14 +85,14 @@ static int
eb_lookup_objects(struct eb_objects *eb,
struct drm_i915_gem_exec_object2 *exec,
const struct drm_i915_gem_execbuffer2 *args,
+ struct i915_address_space *vm,
struct drm_file *file)
{
+ struct drm_i915_gem_object *obj;
int i;
spin_lock(file-table_lock);
for (i = 0; i args-buffer_count; i++) {
- struct drm_i915_gem_object *obj;
-
obj = to_intel_bo(idr_find(file-object_idr, exec[i].handle));
if (obj == NULL) {
spin_unlock(file-table_lock);
@@ -110,6 +110,15 @@ eb_lookup_objects(struct eb_objects *eb,
drm_gem_object_reference(obj-base);
list_add_tail(obj-exec_list, eb-objects);
+ }
+ spin_unlock(file-table_lock);
+
+ list_for_each_entry(obj, eb-objects, exec_list) {
+ struct i915_vma *vma;
+
+ vma = i915_gem_obj_lookup_or_create_vma(obj, vm);
+ if (IS_ERR(vma))
+ return PTR_ERR(vma);
obj-exec_entry = exec[i];
if (eb-and 0) {
@@ -121,7 +130,6 @@ eb_lookup_objects(struct
[Intel-gfx] [PATCH] drm/i915: Do not add an interrupt for a context switch
We use the request to ensure we hold a reference to the context for the
duration that it remains in use by the ring. Each request only holds a
reference to the current context, hence we emit a request after
switching contexts with the final reference to the old context. However,
the extra interrupt caused by that request is not useful (no timing
critical function will wait for the context object), instead the overhead
of servicing the IRQ shows up in some (lightweight) benchmarks. In order
to keep the useful property of using the request to manage the context
lifetime, we want to add a dummy request that is associated with the
interrupt from the subsequent real request following the batch.
The extra interrupt was added as a side-effect of using
i915_add_request() in
commit 112522f6789581824903f6f72082b5b841a7f0f9
Author: Chris Wilson ch...@chris-wilson.co.uk
Date: Thu May 2 16:48:07 2013 +0300
drm/i915: put context upon switching
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ben Widawsky b...@bwidawsk.net
Cc: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 32 +++-
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9a8fac3..e822390 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1873,6 +1873,7 @@ int __i915_add_request(struct intel_ring_buffer *ring,
struct drm_file *file,
struct drm_i915_gem_object *batch_obj,
u32 *seqno);
+int __i915_add_request_marker(struct intel_ring_buffer *ring);
#define i915_add_request(ring, seqno) \
__i915_add_request(ring, NULL, NULL, seqno)
int __must_check i915_wait_seqno(struct intel_ring_buffer *ring,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0fa45e0..7568bf0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2110,6 +2110,37 @@ i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
return 0;
}
+int
+__i915_add_request_marker(struct intel_ring_buffer *ring)
+{
+ struct drm_i915_gem_request *request;
+
+ request = kmalloc(sizeof(*request), GFP_KERNEL);
+ if (request == NULL)
+ return -ENOMEM;
+
+ request-ring = ring;
+ request-seqno = intel_ring_get_seqno(ring);
+ request-tail = request-head = intel_ring_get_tail(ring);
+ request-batch_obj = NULL;
+
+ /* Whilst this request exists, batch_obj will be on the
+* active_list, and so will hold the active reference. Only when this
+* request is retired will the the batch_obj be moved onto the
+* inactive_list and lose its active reference. Hence we do not need
+* to explicitly hold another reference here.
+*/
+ request-ctx = ring-last_context;
+ if (request-ctx)
+ i915_gem_context_reference(request-ctx);
+
+ request-emitted_jiffies = jiffies;
+ list_add_tail(request-list, ring-request_list);
+ request-file_priv = NULL;
+
+ return 0;
+}
+
int __i915_add_request(struct intel_ring_buffer *ring,
struct drm_file *file,
struct drm_i915_gem_object *obj,
@@ -2137,7 +2168,6 @@ int __i915_add_request(struct intel_ring_buffer *ring,
if (request == NULL)
return -ENOMEM;
-
/* Record the position of the start of the request so that
* should we detect the updated seqno part-way through the
* GPU processing the request, we never over-estimate the
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index db94aca..88a9425 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -466,7 +466,7 @@ static int do_switch(struct i915_hw_context *to)
from-obj-dirty = 1;
BUG_ON(from-obj-ring != ring);
- ret = i915_add_request(ring, NULL);
+ ret = __i915_add_request_marker(ring);
if (ret) {
/* Too late, we've already scheduled a context switch.
* Try to undo the change so that the hw state is
--
1.8.4.rc1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: List objects allocated from stolen memory in debugfs
On Wed, Aug 07, 2013 at 06:30:54PM +0100, Chris Wilson wrote:
I was curious as to what objects were currently allocated from stolen
memory, and so exported it from debugfs.
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
I liike this, but since I've just merged Ben's exec_list vma conversion it
doesn't apply any more cleanly. Can you please rebase and resend?
-Daniel
---
drivers/gpu/drm/i915/i915_debugfs.c | 63
+
1 file changed, 63 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 07f5896..4243b63 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -30,6 +30,7 @@
#include linux/debugfs.h
#include linux/slab.h
#include linux/export.h
+#include linux/list_sort.h
#include drm/drmP.h
#include intel_drv.h
#include intel_ringbuffer.h
@@ -187,6 +188,67 @@ static int i915_gem_object_list_info(struct seq_file *m,
void *data)
return 0;
}
+static int obj_rank_by_stolen(void *priv,
+ struct list_head *A, struct list_head *B)
+{
+ struct drm_i915_gem_object *a =
+ container_of(A, struct drm_i915_gem_object, exec_list);
+ struct drm_i915_gem_object *b =
+ container_of(B, struct drm_i915_gem_object, exec_list);
+
+ return a-stolen-start - b-stolen-start;
+}
+
+static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
+{
+ struct drm_info_node *node = (struct drm_info_node *) m-private;
+ struct drm_device *dev = node-minor-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct drm_i915_gem_object *obj;
+ size_t total_obj_size, total_gtt_size;
+ LIST_HEAD(stolen);
+ int count, ret;
+
+ ret = mutex_lock_interruptible(dev-struct_mutex);
+ if (ret)
+ return ret;
+
+ total_obj_size = total_gtt_size = count = 0;
+ list_for_each_entry(obj, dev_priv-mm.bound_list, global_list) {
+ if (obj-stolen == NULL)
+ continue;
+
+ list_add(obj-exec_list, stolen);
+
+ total_obj_size += obj-base.size;
+ total_gtt_size += i915_gem_obj_ggtt_size(obj);
+ count++;
+ }
+ list_for_each_entry(obj, dev_priv-mm.unbound_list, global_list) {
+ if (obj-stolen == NULL)
+ continue;
+
+ list_add(obj-exec_list, stolen);
+
+ total_obj_size += obj-base.size;
+ count++;
+ }
+ list_sort(NULL, stolen, obj_rank_by_stolen);
+ seq_puts(m, Stolen:\n);
+ while (!list_empty(stolen)) {
+ obj = list_first_entry(stolen, typeof(*obj), exec_list);
+ seq_puts(m,);
+ describe_obj(m, obj);
+ seq_putc(m, '\n');
+ list_del_init(obj-exec_list);
+ }
+ mutex_unlock(dev-struct_mutex);
+
+ seq_printf(m, Total %d objects, %zu bytes, %zu GTT size\n,
+count, total_obj_size, total_gtt_size);
+ return 0;
+}
+
#define count_objects(list, member) do { \
list_for_each_entry(obj, list, member) { \
size += i915_gem_obj_ggtt_size(obj); \
@@ -2092,6 +2154,7 @@ static struct drm_info_list i915_debugfs_list[] = {
{i915_gem_pinned, i915_gem_gtt_info, 0, (void *) PINNED_LIST},
{i915_gem_active, i915_gem_object_list_info, 0, (void *) ACTIVE_LIST},
{i915_gem_inactive, i915_gem_object_list_info, 0, (void *)
INACTIVE_LIST},
+ {i915_gem_stolen, i915_gem_stolen_list_info },
{i915_gem_pageflip, i915_gem_pageflip_info, 0},
{i915_gem_request, i915_gem_request_info, 0},
{i915_gem_seqno, i915_gem_seqno_info, 0},
--
1.8.1.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] drm/i915: Do not add an interrupt for a context switch
On Wed, Aug 07, 2013 at 10:29:05PM +0100, Chris Wilson wrote:
We use the request to ensure we hold a reference to the context for the
duration that it remains in use by the ring. Each request only holds a
reference to the current context, hence we emit a request after
switching contexts with the final reference to the old context. However,
the extra interrupt caused by that request is not useful (no timing
critical function will wait for the context object), instead the overhead
of servicing the IRQ shows up in some (lightweight) benchmarks. In order
to keep the useful property of using the request to manage the context
lifetime, we want to add a dummy request that is associated with the
interrupt from the subsequent real request following the batch.
The extra interrupt was added as a side-effect of using
i915_add_request() in
commit 112522f6789581824903f6f72082b5b841a7f0f9
Author: Chris Wilson ch...@chris-wilson.co.uk
Date: Thu May 2 16:48:07 2013 +0300
drm/i915: put context upon switching
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ben Widawsky b...@bwidawsk.net
Cc: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 32 +++-
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9a8fac3..e822390 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1873,6 +1873,7 @@ int __i915_add_request(struct intel_ring_buffer *ring,
struct drm_file *file,
struct drm_i915_gem_object *batch_obj,
u32 *seqno);
+int __i915_add_request_marker(struct intel_ring_buffer *ring);
#define i915_add_request(ring, seqno) \
__i915_add_request(ring, NULL, NULL, seqno)
int __must_check i915_wait_seqno(struct intel_ring_buffer *ring,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0fa45e0..7568bf0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2110,6 +2110,37 @@ i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
return 0;
}
+int
+__i915_add_request_marker(struct intel_ring_buffer *ring)
+{
+ struct drm_i915_gem_request *request;
+
+ request = kmalloc(sizeof(*request), GFP_KERNEL);
+ if (request == NULL)
+ return -ENOMEM;
+
+ request-ring = ring;
+ request-seqno = intel_ring_get_seqno(ring);
+ request-tail = request-head = intel_ring_get_tail(ring);
+ request-batch_obj = NULL;
+
+ /* Whilst this request exists, batch_obj will be on the
+ * active_list, and so will hold the active reference. Only when this
+ * request is retired will the the batch_obj be moved onto the
+ * inactive_list and lose its active reference. Hence we do not need
+ * to explicitly hold another reference here.
+ */
+ request-ctx = ring-last_context;
+ if (request-ctx)
+ i915_gem_context_reference(request-ctx);
+
+ request-emitted_jiffies = jiffies;
+ list_add_tail(request-list, ring-request_list);
+ request-file_priv = NULL;
+
+ return 0;
+}
+
int __i915_add_request(struct intel_ring_buffer *ring,
struct drm_file *file,
struct drm_i915_gem_object *obj,
@@ -2137,7 +2168,6 @@ int __i915_add_request(struct intel_ring_buffer *ring,
if (request == NULL)
return -ENOMEM;
-
/* Record the position of the start of the request so that
* should we detect the updated seqno part-way through the
* GPU processing the request, we never over-estimate the
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index db94aca..88a9425 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -466,7 +466,7 @@ static int do_switch(struct i915_hw_context *to)
from-obj-dirty = 1;
BUG_ON(from-obj-ring != ring);
- ret = i915_add_request(ring, NULL);
+ ret = __i915_add_request_marker(ring);
Can't we just ditch the add_request here? The actual backing storage of
the old context won't get reaped too early (due to the oustanding lazy
request logic) and as long as we don't schedule a batch we don't care one
bit that there's no request with a context linked to it. Only once we emit
batches we care, since otherwise the hangcheck code can't assign the
blame.
Ripping out the add_request would also allow us to ditch the scary WARN +
mi_set_context stuff below.
Or do I massively miss something here?
-Daniel
if (ret) {
/* Too late, we've already scheduled a context switch.
Re: [Intel-gfx] [PATCH] drm/i915: Do not add an interrupt for a context switch
On Wed, Aug 07, 2013 at 11:58:21PM +0200, Daniel Vetter wrote:
On Wed, Aug 07, 2013 at 10:29:05PM +0100, Chris Wilson wrote:
We use the request to ensure we hold a reference to the context for the
duration that it remains in use by the ring. Each request only holds a
reference to the current context, hence we emit a request after
switching contexts with the final reference to the old context. However,
the extra interrupt caused by that request is not useful (no timing
critical function will wait for the context object), instead the overhead
of servicing the IRQ shows up in some (lightweight) benchmarks. In order
to keep the useful property of using the request to manage the context
lifetime, we want to add a dummy request that is associated with the
interrupt from the subsequent real request following the batch.
The extra interrupt was added as a side-effect of using
i915_add_request() in
commit 112522f6789581824903f6f72082b5b841a7f0f9
Author: Chris Wilson ch...@chris-wilson.co.uk
Date: Thu May 2 16:48:07 2013 +0300
drm/i915: put context upon switching
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ben Widawsky b...@bwidawsk.net
Cc: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 32
+++-
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 9a8fac3..e822390 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1873,6 +1873,7 @@ int __i915_add_request(struct intel_ring_buffer *ring,
struct drm_file *file,
struct drm_i915_gem_object *batch_obj,
u32 *seqno);
+int __i915_add_request_marker(struct intel_ring_buffer *ring);
#define i915_add_request(ring, seqno) \
__i915_add_request(ring, NULL, NULL, seqno)
int __must_check i915_wait_seqno(struct intel_ring_buffer *ring,
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index 0fa45e0..7568bf0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2110,6 +2110,37 @@ i915_gem_get_seqno(struct drm_device *dev, u32
*seqno)
return 0;
}
+int
+__i915_add_request_marker(struct intel_ring_buffer *ring)
+{
+ struct drm_i915_gem_request *request;
+
+ request = kmalloc(sizeof(*request), GFP_KERNEL);
+ if (request == NULL)
+ return -ENOMEM;
+
+ request-ring = ring;
+ request-seqno = intel_ring_get_seqno(ring);
+ request-tail = request-head = intel_ring_get_tail(ring);
+ request-batch_obj = NULL;
+
+ /* Whilst this request exists, batch_obj will be on the
+* active_list, and so will hold the active reference. Only when this
+* request is retired will the the batch_obj be moved onto the
+* inactive_list and lose its active reference. Hence we do not need
+* to explicitly hold another reference here.
+*/
+ request-ctx = ring-last_context;
+ if (request-ctx)
+ i915_gem_context_reference(request-ctx);
+
+ request-emitted_jiffies = jiffies;
+ list_add_tail(request-list, ring-request_list);
+ request-file_priv = NULL;
+
+ return 0;
+}
+
int __i915_add_request(struct intel_ring_buffer *ring,
struct drm_file *file,
struct drm_i915_gem_object *obj,
@@ -2137,7 +2168,6 @@ int __i915_add_request(struct intel_ring_buffer *ring,
if (request == NULL)
return -ENOMEM;
-
/* Record the position of the start of the request so that
* should we detect the updated seqno part-way through the
* GPU processing the request, we never over-estimate the
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index db94aca..88a9425 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -466,7 +466,7 @@ static int do_switch(struct i915_hw_context *to)
from-obj-dirty = 1;
BUG_ON(from-obj-ring != ring);
- ret = i915_add_request(ring, NULL);
+ ret = __i915_add_request_marker(ring);
Can't we just ditch the add_request here? The actual backing storage of
the old context won't get reaped too early (due to the oustanding lazy
request logic) and as long as we don't schedule a batch we don't care one
bit that there's no request with a context linked to it. Only once we emit
batches we care, since otherwise the hangcheck code can't assign the
blame.
The request holds the last reference to the context-obj, and each
request only has a single context slot. The olr and next batch only
preserves
Re: [Intel-gfx] [PATCH] drm/i915: Do not add an interrupt for a context switch
On Thu, Aug 08, 2013 at 12:10:22AM +0100, Chris Wilson wrote:
On Wed, Aug 07, 2013 at 11:58:21PM +0200, Daniel Vetter wrote:
On Wed, Aug 07, 2013 at 10:29:05PM +0100, Chris Wilson wrote:
We use the request to ensure we hold a reference to the context for the
duration that it remains in use by the ring. Each request only holds a
reference to the current context, hence we emit a request after
switching contexts with the final reference to the old context. However,
the extra interrupt caused by that request is not useful (no timing
critical function will wait for the context object), instead the overhead
of servicing the IRQ shows up in some (lightweight) benchmarks. In order
to keep the useful property of using the request to manage the context
lifetime, we want to add a dummy request that is associated with the
interrupt from the subsequent real request following the batch.
The extra interrupt was added as a side-effect of using
i915_add_request() in
commit 112522f6789581824903f6f72082b5b841a7f0f9
Author: Chris Wilson ch...@chris-wilson.co.uk
Date: Thu May 2 16:48:07 2013 +0300
drm/i915: put context upon switching
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ben Widawsky b...@bwidawsk.net
Cc: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 32
+++-
drivers/gpu/drm/i915/i915_gem_context.c | 2 +-
3 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 9a8fac3..e822390 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1873,6 +1873,7 @@ int __i915_add_request(struct intel_ring_buffer
*ring,
struct drm_file *file,
struct drm_i915_gem_object *batch_obj,
u32 *seqno);
+int __i915_add_request_marker(struct intel_ring_buffer *ring);
#define i915_add_request(ring, seqno) \
__i915_add_request(ring, NULL, NULL, seqno)
int __must_check i915_wait_seqno(struct intel_ring_buffer *ring,
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index 0fa45e0..7568bf0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2110,6 +2110,37 @@ i915_gem_get_seqno(struct drm_device *dev, u32
*seqno)
return 0;
}
+int
+__i915_add_request_marker(struct intel_ring_buffer *ring)
+{
+ struct drm_i915_gem_request *request;
+
+ request = kmalloc(sizeof(*request), GFP_KERNEL);
+ if (request == NULL)
+ return -ENOMEM;
+
+ request-ring = ring;
+ request-seqno = intel_ring_get_seqno(ring);
+ request-tail = request-head = intel_ring_get_tail(ring);
+ request-batch_obj = NULL;
+
+ /* Whilst this request exists, batch_obj will be on the
+ * active_list, and so will hold the active reference. Only when this
+ * request is retired will the the batch_obj be moved onto the
+ * inactive_list and lose its active reference. Hence we do not need
+ * to explicitly hold another reference here.
+ */
+ request-ctx = ring-last_context;
+ if (request-ctx)
+ i915_gem_context_reference(request-ctx);
+
+ request-emitted_jiffies = jiffies;
+ list_add_tail(request-list, ring-request_list);
+ request-file_priv = NULL;
+
+ return 0;
+}
+
int __i915_add_request(struct intel_ring_buffer *ring,
struct drm_file *file,
struct drm_i915_gem_object *obj,
@@ -2137,7 +2168,6 @@ int __i915_add_request(struct intel_ring_buffer
*ring,
if (request == NULL)
return -ENOMEM;
-
/* Record the position of the start of the request so that
* should we detect the updated seqno part-way through the
* GPU processing the request, we never over-estimate the
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index db94aca..88a9425 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -466,7 +466,7 @@ static int do_switch(struct i915_hw_context *to)
from-obj-dirty = 1;
BUG_ON(from-obj-ring != ring);
- ret = i915_add_request(ring, NULL);
+ ret = __i915_add_request_marker(ring);
Can't we just ditch the add_request here? The actual backing storage of
the old context won't get reaped too early (due to the oustanding lazy
request logic) and as long as we don't schedule a batch we don't care one
bit that there's no request with a context linked to it. Only once we emit
batches we care, since otherwise the hangcheck code can't assign the
blame.
The request holds the last reference to the
Re: [Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
On Wed, Aug 07, 2013 at 09:37:52PM +0900, Inki Dae wrote: 2013/8/7 Daniel Vetter dan...@ffwll.ch On Wed, Aug 7, 2013 at 2:01 PM, Inki Dae inki@samsung.com wrote: 2013/8/7 Daniel Vetter dan...@ffwll.ch On Wed, Aug 07, 2013 at 07:18:45PM +0900, Joonyoung Shim wrote: On 08/07/2013 06:55 PM, Daniel Vetter wrote: On Wed, Aug 7, 2013 at 11:40 AM, Inki Dae inki@samsung.com wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] Sent: Wednesday, August 07, 2013 6:15 PM To: DRI Development Cc: Intel Graphics Development; Daniel Vetter; Inki Dae Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers Note that this is slightly tricky since both drivers store their native objects in dma_buf-priv. But both also embed the base drm_gem_object at the first position, so the implicit cast is ok. To use the release helper we need to export it, too. Yeah, may I repost this patch with additional work? We also need to export with a gem object instead of specific one like you did. I think dmabuf stuff of exynos can be replaced to common drm_gem_dmabuf. Already dmabuf stuff of drm_gem_cma_helper.c was substituted to common drm_gem_dmabuf with low-level hook functions to use prime helpers. Ah, but that can easily be done on top of this, right? Daniel, could you remove exynos related codes from your patch set? Your patch set would make exynos broken because you didn't consider exporting with a gem object for exynos like [PATCH 3/3] drm/i915: explicit store base gem object in dma_buf-priv. So I think your patch set is not complete set, and That is why exynos needs the additional work I mentioned above. So I just wanted to repost your patch set + new one. Nope, my patch should not break exynos since the base gem_object is the first member of the exynos object, so we don't have any issues Ah, right. However, it does not seem like good way. with upcasting in exynos dma-buf code. The same applies to i915 dma-buf code, my follow-up patch just makes the code a bit safer. However, I think not only exynos could go to common drm_gem_dmabuf directly but also it would make your patch set to be complete set if you remove exynos related codes from your patch set. Otherwise, we have to work twice. one is the additional work for resolving exynos broken issue by your patch set, and other is to replace existing dmabuf stuff of exynos to common drm_gem_dmabuf. Yeah np, I'll drop exynos then. Thanks a lot. :) Ah, I remember again why I want to also convert over exynos to the common dma buf release function: Later patches in my prime locking series will change things in there to avoid a userspace-triggerable oops. If we leave out exynos it'll break rather badly for dma-buf export. I need to think a bit more about what stuff looks like atm, but if I resend those parts I'll include exynos. It's a bit tricky that it still works, but that way you can fix it up without the introduction of a bisect failure point. -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: Do not add an interrupt for a context switch
On Thu, Aug 08, 2013 at 01:18:18AM +0200, Daniel Vetter wrote: Afaict the request holds a ref on the context, and that a ref on the hw context bo. But the active list itself (thanks to the move_to_active) should also hold a ref to the hw context bo, so I don't think we really need full request here. The old context might disappear, but no one really cares if it disappears a notch too early since the backing storage will survive long enough. The next request holds a ref to the new context. We care about holding a ref to the old context until the hw has finished writing to it. That is the purpose of taking a request here, to bump the old_ctx-bo-active for the context save. Otherwise the backing storage is liable to disappear too early (and the hw write to a random location). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: unpin backing storage in dmabuf_unmap
On Wed, Aug 07, 2013 at 12:09:32PM +0200, Daniel Vetter wrote:
This fixes a WARN in i915_gem_free_object when the
obj-pages_pin_count isn't 0.
v2: Add locking to unmap, noticed by Chris Wilson. Note that even
though we call unmap with our own dev-struct_mutex held that won't
result in an immediate deadlock since we never go through the dma_buf
interfaces for our own, reimported buffers. But it's still easy to
blow up and anger lockdep, but that's already the case with our -map
implementation. Fixing this for real will involve per dma-buf ww mutex
locking by the callers. And lots of fun. So go with the duct-tape
approach for now.
Cc: Chris Wilson ch...@chris-wilson.co.uk
Reported-by: Maarten Lankhorst maarten.lankho...@canonical.com
Cc: Maarten Lankhorst maarten.lankho...@canonical.com
Tested-by: Armin K. kre...@email.com (v1)
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 63ee1a9..f7e1682 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -85,9 +85,17 @@ static void i915_gem_unmap_dma_buf(struct
dma_buf_attachment *attachment,
struct sg_table *sg,
enum dma_data_direction dir)
{
+ struct drm_i915_gem_object *obj = attachment-dmabuf-priv;
+
+ mutex_lock(obj-base.dev-struct_mutex);
+
dma_unmap_sg(attachment-dev, sg-sgl, sg-nents, dir);
sg_free_table(sg);
kfree(sg);
+
+ i915_gem_object_unpin_pages(obj);
I am curious - would it logic of first unpinning, and then doing the
dma_unmap_sg
make more sense? As in, in the map path we do:
dma_map
pin
And in here you do the same:
dma_unmap
unpin
But I would have thought that on a unroll you would do it in reverse
order, so:
unpin
dma_unmap
+
+ mutex_unlock(obj-base.dev-struct_mutex);
}
static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
--
1.8.3.2
___
dri-devel mailing list
dri-de...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: unpin backing storage in dmabuf_unmap
On Wed, Aug 07, 2013 at 08:50:20PM -0400, Konrad Rzeszutek Wilk wrote:
On Wed, Aug 07, 2013 at 12:09:32PM +0200, Daniel Vetter wrote:
This fixes a WARN in i915_gem_free_object when the
obj-pages_pin_count isn't 0.
v2: Add locking to unmap, noticed by Chris Wilson. Note that even
though we call unmap with our own dev-struct_mutex held that won't
result in an immediate deadlock since we never go through the dma_buf
interfaces for our own, reimported buffers. But it's still easy to
blow up and anger lockdep, but that's already the case with our -map
implementation. Fixing this for real will involve per dma-buf ww mutex
locking by the callers. And lots of fun. So go with the duct-tape
approach for now.
Cc: Chris Wilson ch...@chris-wilson.co.uk
Reported-by: Maarten Lankhorst maarten.lankho...@canonical.com
Cc: Maarten Lankhorst maarten.lankho...@canonical.com
Tested-by: Armin K. kre...@email.com (v1)
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 63ee1a9..f7e1682 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -85,9 +85,17 @@ static void i915_gem_unmap_dma_buf(struct
dma_buf_attachment *attachment,
struct sg_table *sg,
enum dma_data_direction dir)
{
+ struct drm_i915_gem_object *obj = attachment-dmabuf-priv;
+
+ mutex_lock(obj-base.dev-struct_mutex);
+
dma_unmap_sg(attachment-dev, sg-sgl, sg-nents, dir);
sg_free_table(sg);
kfree(sg);
+
+ i915_gem_object_unpin_pages(obj);
I am curious - would it logic of first unpinning, and then doing the
dma_unmap_sg
make more sense? As in, in the map path we do:
dma_map
pin
Actually this is the wrong way around, and is a potential issue.
Hindsight is a powerful tool.
-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 21/29] drm/i915: mm_list is per VMA
On Wed, Aug 07, 2013 at 10:52:14PM +0200, Daniel Vetter wrote:
On Tue, Aug 06, 2013 at 05:28:06PM -0700, Ben Widawsky wrote:
On Tue, Aug 06, 2013 at 09:38:41PM +0200, Daniel Vetter wrote:
On Wed, Jul 31, 2013 at 05:00:14PM -0700, Ben Widawsky wrote:
formerly: drm/i915: Create VMAs (part 5) - move mm_list
The mm_list is used for the active/inactive LRUs. Since those LRUs are
per address space, the link should be per VMx .
Because we'll only ever have 1 VMA before this point, it's not incorrect
to defer this change until this point in the patch series, and doing it
here makes the change much easier to understand.
Shamelessly manipulated out of Daniel:
active/inactive stuff is used by eviction when we run out of address
space, so needs to be per-vma and per-address space. Bound/unbound otoh
is used by the shrinker which only cares about the amount of memory used
and not one bit about in which address space this memory is all used in.
Of course to actual kick out an object we need to unbind it from every
address space, but for that we have the per-object list of vmas.
v2: only bump GGTT LRU in i915_gem_object_set_to_gtt_domain (Chris)
v3: Moved earlier in the series
v4: Add dropped message from v3
Signed-off-by: Ben Widawsky b...@bwidawsk.net
Some comments below for this one. The lru changes look a bit strange so
I'll wait for your confirmation that the do_switch hunk has the same
reasons s the one in execbuf with the FIXME comment.
-Daniel
---
drivers/gpu/drm/i915/i915_debugfs.c| 53
--
drivers/gpu/drm/i915/i915_drv.h| 5 +--
drivers/gpu/drm/i915/i915_gem.c| 37 +++--
drivers/gpu/drm/i915/i915_gem_context.c| 3 ++
drivers/gpu/drm/i915/i915_gem_evict.c | 14
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 ++
drivers/gpu/drm/i915/i915_gem_stolen.c | 2 +-
drivers/gpu/drm/i915/i915_gpu_error.c | 37 -
8 files changed, 91 insertions(+), 62 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 6d5ca85bd..181e5a6 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -149,7 +149,7 @@ static int i915_gem_object_list_info(struct
seq_file *m, void *data)
struct drm_device *dev = node-minor-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct i915_address_space *vm = dev_priv-gtt.base;
- struct drm_i915_gem_object *obj;
+ struct i915_vma *vma;
size_t total_obj_size, total_gtt_size;
int count, ret;
@@ -157,6 +157,7 @@ static int i915_gem_object_list_info(struct
seq_file *m, void *data)
if (ret)
return ret;
+ /* FIXME: the user of this interface might want more than just
GGTT */
switch (list) {
case ACTIVE_LIST:
seq_puts(m, Active:\n);
@@ -172,12 +173,12 @@ static int i915_gem_object_list_info(struct
seq_file *m, void *data)
}
total_obj_size = total_gtt_size = count = 0;
- list_for_each_entry(obj, head, mm_list) {
- seq_puts(m,);
- describe_obj(m, obj);
- seq_putc(m, '\n');
- total_obj_size += obj-base.size;
- total_gtt_size += i915_gem_obj_ggtt_size(obj);
+ list_for_each_entry(vma, head, mm_list) {
+ seq_printf(m,);
+ describe_obj(m, vma-obj);
+ seq_printf(m, \n);
+ total_obj_size += vma-obj-base.size;
+ total_gtt_size += i915_gem_obj_size(vma-obj, vma-vm);
Why not use vma-node.size? If you don't disagree I'll bikeshed this while
applying.
I think in terms of the diff, it's more logical to do it how I did. The
result should damn well be the same though, so go right ahead. When I
set about writing the series, I really didn't want to use
node.size/start directly as much as possible - so we can sneak things
into the helpers as needed.
I've applied this bikeshed, but the patch required some wiggling in and
conflict resolution. I've checked with your branch and that seems to be
due to the removel of the inactive list walking to adjust the gpu domains
in i915_gem_reset. Please check that I didn't botch the patch rebasing
with your tree.
-Daniel
You killed a BUG in i915_gem_retire_requests_ring, shouldn't that be a WARN or
are you in the business of completely killing assertions now :p?
Otherwise, it looks good to me. There are enough diffs because of some
other patches you merged (like watermarks) - that I may have well
Re: [Intel-gfx] [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers
-Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Thursday, August 08, 2013 8:21 AM To: Inki Dae Cc: Daniel Vetter; Intel Graphics Development; DRI Development Subject: Re: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers On Wed, Aug 07, 2013 at 09:37:52PM +0900, Inki Dae wrote: 2013/8/7 Daniel Vetter dan...@ffwll.ch On Wed, Aug 7, 2013 at 2:01 PM, Inki Dae inki@samsung.com wrote: 2013/8/7 Daniel Vetter dan...@ffwll.ch On Wed, Aug 07, 2013 at 07:18:45PM +0900, Joonyoung Shim wrote: On 08/07/2013 06:55 PM, Daniel Vetter wrote: On Wed, Aug 7, 2013 at 11:40 AM, Inki Dae inki@samsung.com wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] Sent: Wednesday, August 07, 2013 6:15 PM To: DRI Development Cc: Intel Graphics Development; Daniel Vetter; Inki Dae Subject: [PATCH 1/3] drm: use common drm_gem_dmabuf_release in i915/exynos drivers Note that this is slightly tricky since both drivers store their native objects in dma_buf-priv. But both also embed the base drm_gem_object at the first position, so the implicit cast is ok. To use the release helper we need to export it, too. Yeah, may I repost this patch with additional work? We also need to export with a gem object instead of specific one like you did. I think dmabuf stuff of exynos can be replaced to common drm_gem_dmabuf. Already dmabuf stuff of drm_gem_cma_helper.c was substituted to common drm_gem_dmabuf with low-level hook functions to use prime helpers. Ah, but that can easily be done on top of this, right? Daniel, could you remove exynos related codes from your patch set? Your patch set would make exynos broken because you didn't consider exporting with a gem object for exynos like [PATCH 3/3] drm/i915: explicit store base gem object in dma_buf-priv. So I think your patch set is not complete set, and That is why exynos needs the additional work I mentioned above. So I just wanted to repost your patch set + new one. Nope, my patch should not break exynos since the base gem_object is the first member of the exynos object, so we don't have any issues Ah, right. However, it does not seem like good way. with upcasting in exynos dma-buf code. The same applies to i915 dma-buf code, my follow-up patch just makes the code a bit safer. However, I think not only exynos could go to common drm_gem_dmabuf directly but also it would make your patch set to be complete set if you remove exynos related codes from your patch set. Otherwise, we have to work twice. one is the additional work for resolving exynos broken issue by your patch set, and other is to replace existing dmabuf stuff of exynos to common drm_gem_dmabuf. Yeah np, I'll drop exynos then. Thanks a lot. :) Ah, I remember again why I want to also convert over exynos to the common dma buf release function: Later patches in my prime locking series will change things in there to avoid a userspace-triggerable oops. If we leave out exynos it'll break rather badly for dma-buf export. I need to think a bit more about what stuff looks like atm, but if I resend those parts I'll include exynos. It's a bit tricky that it still works, but that way you can fix it up without the introduction of a bisect failure point. I'll repost your patch set + new one that exports to a common gem object; already worked and tested. I think it's good for they to be one set because only using the patch 1/3 doesn't look good even though Exynos works fine with the path 1/3. So I'll repost it like below if you agree with me, [PATCH 0/4] Small i915/exynos prime cleanup [PATCH 1/4] drm: use common drm_gem_dmabuf_release in i915/exynos drivers [PATCH 2/4] drm/i915: unpin backing storage in dmabuf_unmap [PATCH 3/4] drm/i915: explicit store base gem object in dma_buf-priv [PATCH 4/4] drm/exynos: explicit store base gem object in dma_buf-priv After this, you can take care of them until merged to next. Or you can repost this patch set including my patch again. What you proper doesn't matter to me. :) And it seems better that exynos keeps using existing dmabuf interfaces without replacing them to common drm_gem_dmabuf ones because we may need features only for Exynos. Actually, now exynos dmabuf includes some features related to v4l2 and gpu driver for more performance. Thanks, Inki Dae -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/exynos: explicit store base gem object in dma_buf-priv
Signed-off-by: Inki Dae inki@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 12
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index 3cd56e1..fd76449 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -22,6 +22,11 @@ struct exynos_drm_dmabuf_attachment {
bool is_mapped;
};
+static struct exynos_drm_gem_obj *dma_buf_to_obj(struct dma_buf *buf)
+{
+ return to_exynos_gem_obj(buf-priv);
+}
+
static int exynos_gem_attach_dma_buf(struct dma_buf *dmabuf,
struct device *dev,
struct dma_buf_attachment *attach)
@@ -63,7 +68,7 @@ static struct sg_table *
enum dma_data_direction dir)
{
struct exynos_drm_dmabuf_attachment *exynos_attach = attach-priv;
- struct exynos_drm_gem_obj *gem_obj = attach-dmabuf-priv;
+ struct exynos_drm_gem_obj *gem_obj = dma_buf_to_obj(attach-dmabuf);
struct drm_device *dev = gem_obj-base.dev;
struct exynos_drm_gem_buf *buf;
struct scatterlist *rd, *wr;
@@ -180,7 +185,7 @@ struct dma_buf *exynos_dmabuf_prime_export(struct
drm_device *drm_dev,
{
struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
- return dma_buf_export(exynos_gem_obj, exynos_dmabuf_ops,
+ return dma_buf_export(obj, exynos_dmabuf_ops,
exynos_gem_obj-base.size, flags);
}
@@ -198,8 +203,7 @@ struct drm_gem_object *exynos_dmabuf_prime_import(struct
drm_device *drm_dev,
if (dma_buf-ops == exynos_dmabuf_ops) {
struct drm_gem_object *obj;
- exynos_gem_obj = dma_buf-priv;
- obj = exynos_gem_obj-base;
+ obj = dma_buf-priv;
/* is it from our device? */
if (obj-dev == drm_dev) {
--
1.7.5.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/exynos: explicit store base gem object in dma_buf-priv
Hi, Daniel. You can repost your patch set including the below my patch. And
my patch numbering is wrong. Sorry about that.
[PATCH 1/4] - [PATCH 4/4]
Thanks,
Inki Dae
-Original Message-
From: Inki Dae [mailto:inki@samsung.com]
Sent: Thursday, August 08, 2013 1:40 PM
To: dan...@ffwll.ch
Cc: dri-de...@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Inki
Dae; Kyungmin Park
Subject: [PATCH 1/4] drm/exynos: explicit store base gem object in
dma_buf-priv
Signed-off-by: Inki Dae inki@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 12
1 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index 3cd56e1..fd76449 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -22,6 +22,11 @@ struct exynos_drm_dmabuf_attachment {
bool is_mapped;
};
+static struct exynos_drm_gem_obj *dma_buf_to_obj(struct dma_buf *buf)
+{
+ return to_exynos_gem_obj(buf-priv);
+}
+
static int exynos_gem_attach_dma_buf(struct dma_buf *dmabuf,
struct device *dev,
struct dma_buf_attachment *attach)
@@ -63,7 +68,7 @@ static struct sg_table *
enum dma_data_direction dir)
{
struct exynos_drm_dmabuf_attachment *exynos_attach = attach-priv;
- struct exynos_drm_gem_obj *gem_obj = attach-dmabuf-priv;
+ struct exynos_drm_gem_obj *gem_obj = dma_buf_to_obj(attach-dmabuf);
struct drm_device *dev = gem_obj-base.dev;
struct exynos_drm_gem_buf *buf;
struct scatterlist *rd, *wr;
@@ -180,7 +185,7 @@ struct dma_buf *exynos_dmabuf_prime_export(struct
drm_device *drm_dev,
{
struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
- return dma_buf_export(exynos_gem_obj, exynos_dmabuf_ops,
+ return dma_buf_export(obj, exynos_dmabuf_ops,
exynos_gem_obj-base.size, flags);
}
@@ -198,8 +203,7 @@ struct drm_gem_object
*exynos_dmabuf_prime_import(struct drm_device *drm_dev,
if (dma_buf-ops == exynos_dmabuf_ops) {
struct drm_gem_object *obj;
- exynos_gem_obj = dma_buf-priv;
- obj = exynos_gem_obj-base;
+ obj = dma_buf-priv;
/* is it from our device? */
if (obj-dev == drm_dev) {
--
1.7.5.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
