[Intel-gfx] [igt PATCH] gen9: fix gem_render_copy 3d state setup
Without emitting the default 3DSTATE_WM_DEPTH_STENCIL state the test
will fail.
Signed-off-by: Imre Deak
---
lib/rendercopy_gen9.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/rendercopy_gen9.c b/lib/rendercopy_gen9.c
index e20a84f..b7b133c 100644
--- a/lib/rendercopy_gen9.c
+++ b/lib/rendercopy_gen9.c
@@ -821,7 +821,13 @@ gen8_emit_ps(struct intel_batchbuffer *batch, uint32_t
kernel) {
}
static void
-gen8_emit_depth(struct intel_batchbuffer *batch) {
+gen9_emit_depth(struct intel_batchbuffer *batch)
+{
+ OUT_BATCH(GEN8_3DSTATE_WM_DEPTH_STENCIL | (4 - 2));
+ OUT_BATCH(0);
+ OUT_BATCH(0);
+ OUT_BATCH(0);
+
OUT_BATCH(GEN7_3DSTATE_DEPTH_BUFFER | (8-2));
OUT_BATCH(0);
OUT_BATCH(0);
@@ -999,7 +1005,7 @@ void gen9_render_copyfunc(struct intel_batchbuffer *batch,
OUT_BATCH(GEN6_3DSTATE_SCISSOR_STATE_POINTERS);
OUT_BATCH(scissor_state);
- gen8_emit_depth(batch);
+ gen9_emit_depth(batch);
gen7_emit_clear(batch);
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] dma-buf: cleanup dma_buf_export() to make it easily extensible
Em Wed, 28 Jan 2015 18:24:03 +0530
Sumit Semwal escreveu:
> +/**
> + * helper macro for exporters; zeros and fills in most common values
> + */
> +#define DEFINE_DMA_BUF_EXPORT_INFO(a)\
> + struct dma_buf_export_info a = { .exp_name = KBUILD_MODNAME }
> +
I suspect that this will let the other fields not initialized.
You likely need to do:
#define DEFINE_DMA_BUF_EXPORT_INFO(a) \
struct dma_buf_export_info a = {\
.exp_name = KBUILD_MODNAME; \
.fields = 0;\
...
}
Regards,
Mauro
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] memcontrol.c BUG
On Wed, Jan 28, 2015 at 03:32:43PM +0100, Michal Hocko wrote: > On Wed 28-01-15 08:48:52, Chris Wilson wrote: > > On Wed, Jan 28, 2015 at 08:13:06AM +1000, Dave Airlie wrote: > > > https://bugzilla.redhat.com/show_bug.cgi?id=1165369 > > > > > > ov 18 09:23:22 elissa.gathman.org kernel: page:f5e36a40 count:2 > > > mapcount:0 mapping: (null) index:0x0 > > > Nov 18 09:23:22 elissa.gathman.org kernel: page flags: > > > 0x80090029(locked|uptodate|lru|swapcache|swapbacked) > > > Nov 18 09:23:22 elissa.gathman.org kernel: page dumped because: > > > VM_BUG_ON_PAGE(!lrucare && PageLRU(oldpage)) > > > Nov 18 09:23:23 elissa.gathman.org kernel: [ cut here > > > ] > > > Nov 18 09:23:23 elissa.gathman.org kernel: kernel BUG at > > > mm/memcontrol.c:6733! > > I guess this matches the following bugon in your kernel: > VM_BUG_ON_PAGE(!lrucare && PageLRU(oldpage), oldpage); > > so the oldpage is on the LRU list already. I am completely unfamiliar > with 965GM but is the page perhaps shared with somebody with a different > gfp mask requirement (e.g. userspace accessing the memory via mmap)? So > the other (racing) caller didn't need to move the page and put it on > LRU. Generally, yes. The shmemfs filp is exported through a vm_mmap() as well as pinned into the GPU via shmem_read_mapping_page_gfp(). But I would not expect that to be the case very often, if at all, on 965GM as the two access paths are incoherent. Still it sounds promising, hopefully Dave can put it into a fedora kernel for testing? -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] [PULL] drm-intel-fixes
Hi Dave - i915 fixes all around, mostly cc: stable. Was surprised to see your pull request already on Tuesday, are you planning on doing another one before -rc7? BR, Jani. The following changes since commit 26bc420b59a38e4e6685a73345a0def461136dce: Linux 3.19-rc6 (2015-01-25 20:04:41 -0800) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2015-01-29 for you to fetch changes up to 6b96d705f3cf435b0b8835b12c9742513c77fed6: drm/i915: BDW Fix Halo PCI IDs marked as ULT. (2015-01-26 11:00:34 +0200) Bob Paauwe (1): drm/i915: Only fence tiled region of object. David Woodhouse (1): drm/i915: Init PPGTT before context enable Jeremiah Mahler (1): drm/i915: fix inconsistent brightness after resume Rodrigo Vivi (2): drm/i915: Fix and clean BDW PCH identification drm/i915: BDW Fix Halo PCI IDs marked as ULT. drivers/gpu/drm/i915/i915_drv.c| 14 -- drivers/gpu/drm/i915/i915_drv.h| 3 +-- drivers/gpu/drm/i915/i915_gem.c| 26 +- drivers/gpu/drm/i915/intel_panel.c | 2 +- 4 files changed, 19 insertions(+), 26 deletions(-) -- 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] [igt PATCH] gen9: fix gem_render_copy 3d state setup
On Thu, Jan 29, 2015 at 12:03:19AM -0800, Imre Deak wrote:
> Without emitting the default 3DSTATE_WM_DEPTH_STENCIL state the test
> will fail.
>
> Signed-off-by: Imre Deak
Question: Wasn't the golden context supposed to paper over those?
--
Damien
> ---
> lib/rendercopy_gen9.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/lib/rendercopy_gen9.c b/lib/rendercopy_gen9.c
> index e20a84f..b7b133c 100644
> --- a/lib/rendercopy_gen9.c
> +++ b/lib/rendercopy_gen9.c
> @@ -821,7 +821,13 @@ gen8_emit_ps(struct intel_batchbuffer *batch, uint32_t
> kernel) {
> }
>
> static void
> -gen8_emit_depth(struct intel_batchbuffer *batch) {
> +gen9_emit_depth(struct intel_batchbuffer *batch)
> +{
> + OUT_BATCH(GEN8_3DSTATE_WM_DEPTH_STENCIL | (4 - 2));
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> + OUT_BATCH(0);
> +
> OUT_BATCH(GEN7_3DSTATE_DEPTH_BUFFER | (8-2));
> OUT_BATCH(0);
> OUT_BATCH(0);
> @@ -999,7 +1005,7 @@ void gen9_render_copyfunc(struct intel_batchbuffer
> *batch,
> OUT_BATCH(GEN6_3DSTATE_SCISSOR_STATE_POINTERS);
> OUT_BATCH(scissor_state);
>
> - gen8_emit_depth(batch);
> + gen9_emit_depth(batch);
>
> gen7_emit_clear(batch);
>
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt PATCH] gen9: fix gem_render_copy 3d state setup
On Thu, 2015-01-29 at 10:51 +, Damien Lespiau wrote: > On Thu, Jan 29, 2015 at 12:03:19AM -0800, Imre Deak wrote: > > Without emitting the default 3DSTATE_WM_DEPTH_STENCIL state the test > > will fail. > > > > Signed-off-by: Imre Deak > > Question: Wasn't the golden context supposed to paper over those? Perhaps, currently the golden context doesn't include this. I put it here to match what GEN8 copy_render does, but I agree that eventually we should move it to the golden context. I noticed that a bunch of other 3DSTATE commands are missing, which could explain the failures I see with other tests (at least piglit-mesa). I'm experimenting now with an updated golden context to see if it solves those failures. --Imre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bdw: Implement WaForceContextSaveRestoreNonCoherent
On Wed, Jan 28, 2015 at 11:21:17AM +0200, Jani Nikula wrote: > On Tue, 27 Jan 2015, Damien Lespiau wrote: > > Missing commit message. I need some description to decide whether this > is required for fixes/stable or not. This patch wasn't prompted by an actual problem I witnessed, but from going through the list of W/As for SKL. The need for that W/A was itself discovered through a preemption test, there's no indication if it could occur with the non-preempted context switch we do today. I'd leave it out of fixes/stable then. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt PATCH] gen9: fix gem_render_copy 3d state setup
On Thu, Jan 29, 2015 at 03:01:50AM -0800, Imre Deak wrote: > On Thu, 2015-01-29 at 10:51 +, Damien Lespiau wrote: > > On Thu, Jan 29, 2015 at 12:03:19AM -0800, Imre Deak wrote: > > > Without emitting the default 3DSTATE_WM_DEPTH_STENCIL state the test > > > will fail. > > > > > > Signed-off-by: Imre Deak > > > > Question: Wasn't the golden context supposed to paper over those? > > Perhaps, currently the golden context doesn't include this. Today, you cannot rely on the initial contents of the context even with the golden render state. There is no pristine context, every client is responsible for configuring the hardware exactly as they intend to use - at least as regards the untrusted commands (e.g. 3DSTATE). -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] [igt PATCH] gen9: fix gem_render_copy 3d state setup
On Thu, Jan 29, 2015 at 11:12:46AM +, Chris Wilson wrote: > On Thu, Jan 29, 2015 at 03:01:50AM -0800, Imre Deak wrote: > > On Thu, 2015-01-29 at 10:51 +, Damien Lespiau wrote: > > > On Thu, Jan 29, 2015 at 12:03:19AM -0800, Imre Deak wrote: > > > > Without emitting the default 3DSTATE_WM_DEPTH_STENCIL state the test > > > > will fail. > > > > > > > > Signed-off-by: Imre Deak > > > > > > Question: Wasn't the golden context supposed to paper over those? > > > > Perhaps, currently the golden context doesn't include this. > > Today, you cannot rely on the initial contents of the context even with > the golden render state. There is no pristine context, every client is > responsible for configuring the hardware exactly as they intend to use - > at least as regards the untrusted commands (e.g. 3DSTATE). Right. Now the question is, do we want to change that and have the golden context with sane defaults? -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] tools/intel_reg_read:Adding lib calls for CHT/VLV
From: meghanelogal
Calling the library functions for reg read and write
Signed-off-by: meghanelogal
---
tools/intel_reg_read.c | 17 +++--
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/tools/intel_reg_read.c b/tools/intel_reg_read.c
index c550b02..bdff92c 100644
--- a/tools/intel_reg_read.c
+++ b/tools/intel_reg_read.c
@@ -48,13 +48,9 @@ static void bit_decode(uint32_t reg)
static void dump_range(uint32_t start, uint32_t end)
{
- int i, reg = 0;
- struct pci_device *dev = intel_get_pci_device();
- if (IS_CHERRYVIEW(dev->device_id) || IS_VALLEYVIEW(dev->device_id))
- reg = 0x18;
+ int i;
for (i = start; i < end; i += 4)
- printf("0x%X : 0x%X\n", i,
- *(volatile uint32_t *)((volatile char*)mmio + i + reg));
+ printf("0x%X : 0x%X\n", i, intel_register_read(i+0x18));
}
static void usage(char *cmdname)
@@ -76,7 +72,6 @@ int main(int argc, char** argv)
int full_dump = 0;
int decode_bits = 0;
int dwords = 1;
-
while ((ch = getopt(argc, argv, "dfhc:")) != -1) {
switch(ch) {
case 'd':
@@ -132,13 +127,7 @@ int main(int argc, char** argv)
dump_range(reg, reg + (dwords * 4));
if (decode_bits) {
- struct pci_device *dev = intel_get_pci_device();
- if (IS_CHERRYVIEW(dev->device_id) ||
- IS_VALLEYVIEW(dev->device_id)) {
- reg += 0x18;
- bit_decode(*(volatile uint32_t *)
- ((volatile char*)mmio + reg));
- }
+ bit_decode(intel_register_read(0x18));
}
}
}
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2 (v1.5)
On Wed, Jan 28, 2015 at 01:46:53PM -0500, Rob Clark wrote:
> On Wed, Jan 28, 2015 at 12:37 PM, Daniel Vetter
> wrote:
> > From: Rob Clark
> >
> > In DRM/KMS we are lacking a good way to deal with tiled/compressed
> > formats. Especially in the case of dmabuf/prime buffer sharing, where
> > we cannot always rely on under-the-hood flags passed to driver specific
> > gem-create ioctl to pass around these extra flags.
> >
> > The proposal is to add a per-plane format modifier. This allows to, if
> > necessary, use different tiling patters for sub-sampled planes, etc.
> > The format modifiers are added at the end of the ioctl struct, so for
> > legacy userspace it will be zero padded.
> >
> > TODO how best to deal with assignment of modifier token values? The
> > rough idea was to namespace things with an 8bit vendor-id, and then
> > beyond that it is treated as an opaque value. But that was a relatively
> > arbitrary choice. There are cases where same tiling pattern and/or
> > compression is supported by various different vendors. So we should
> > standardize to use the vendor-id and value of the first one who
> > documents the format?
>
> iirc, I think we decided that drm_fourcc.h in drm-next is a sufficient
> authority for coordinating assignment of modifier token values, so we
> could probably drop this TODO. Perhaps it wouldn't hurt to document
> somewhere that, as with fourcc/format values, new additions are
> expected to come with some description of the format?
Oh right forgotten to drop the TODO. I'll add a comment to the header
file.
>
> >
> > v1: original
> > v1.5: increase modifier to 64b
> >
> > v2: Incorporate review comments from the big thread, plus a few more.
> >
> > - Add a getcap so that userspace doesn't have to jump through hoops.
> > - Allow modifiers only when a flag is set. That way drivers know when
> > they're dealing with old userspace and need to fish out e.g. tiling
> > from other information.
> > - After rolling out checks for ->modifier to all drivers I've decided
> > that this is way too fragile and needs an explicit opt-in flag. So
> > do that instead.
> > - Add a define (just for documentation really) for the "NONE"
> > modifier. Imo we don't need to add mask #defines since drivers
> > really should only do exact matches against values defined with
> > fourcc_mod_code.
> > - Drop the Samsung tiling modifier on Rob's request since he's not yet
> > sure whether that one is accurate.
> >
> > Cc: Rob Clark
> > Cc: Tvrtko Ursulin
> > Cc: Laurent Pinchart
> > Cc: Daniel Stone
> > Cc: Ville Syrjälä
> > Cc: Michel Dänzer
> > Signed-off-by: Rob Clark (v1.5)
> > Signed-off-by: Daniel Vetter
> >
>
> you forgot to change subject line to (v2).. but other than that, looks good
Ah, I generally don't keep a patch revision in the subject and forgot to
update it ;-)
>
> Reviewed-by: Rob Clark
>
> > --
> >
> > I've picked this up since we want to push out some fancy new tiling
> > modes soonish. No defines yet, but Tvrkto is working on the i915 parts
> > of this.
> >
> > I think the only part I haven't done from the discussion is exposing a
> > list of supported modifiers. Not sure that's really useful though
> > since all this is highly hw specific.
> >
> > And a note to driver writes: They need to check or the flag and in its
> > absence make a reasonable choice about the internal layet (e.g. for
> > i915 consult the tiling mode of the underlying bo).
> > -Daniel
> > ---
> > drivers/gpu/drm/drm_crtc.c| 14 +-
> > drivers/gpu/drm/drm_ioctl.c | 3 +++
> > include/drm/drm_crtc.h| 3 +++
> > include/uapi/drm/drm.h| 1 +
> > include/uapi/drm/drm_fourcc.h | 26 ++
> > include/uapi/drm/drm_mode.h | 9 +
> > 6 files changed, 55 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 419f9d915c78..8090e3d64f67 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -3324,6 +3324,12 @@ static int framebuffer_check(const struct
> > drm_mode_fb_cmd2 *r)
> > DRM_DEBUG_KMS("bad pitch %u for plane %d\n",
> > r->pitches[i], i);
> > return -EINVAL;
> > }
> > +
> > + if (r->modifier[i] && !(r->flags & DRM_MODE_FB_MODIFIERS)) {
> > + DRM_DEBUG_KMS("bad fb modifier %llu for plane %d\n",
> > + r->modifier[i], i);
> > + return -EINVAL;
> > + }
> > }
> >
> > return 0;
> > @@ -3337,7 +3343,7 @@ static struct drm_framebuffer
> > *add_framebuffer_internal(struct drm_device *dev,
> > struct drm_framebuffer *fb;
> > int ret;
> >
> > - if (r->flags & ~DRM_MODE_FB_INTERLACED) {
> > + if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {
> > DRM_DEBUG_KMS("bad framebuffer fla
Re: [Intel-gfx] [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2 (v1.5)
On Wed, Jan 28, 2015 at 05:57:56PM +, Tvrtko Ursulin wrote: > > On 01/28/2015 05:37 PM, Daniel Vetter wrote: > >From: Rob Clark > > > >In DRM/KMS we are lacking a good way to deal with tiled/compressed > >formats. Especially in the case of dmabuf/prime buffer sharing, where > >we cannot always rely on under-the-hood flags passed to driver specific > >gem-create ioctl to pass around these extra flags. > > > >The proposal is to add a per-plane format modifier. This allows to, if > >necessary, use different tiling patters for sub-sampled planes, etc. > >The format modifiers are added at the end of the ioctl struct, so for > >legacy userspace it will be zero padded. > > > >TODO how best to deal with assignment of modifier token values? The > >rough idea was to namespace things with an 8bit vendor-id, and then > >beyond that it is treated as an opaque value. But that was a relatively > >arbitrary choice. There are cases where same tiling pattern and/or > >compression is supported by various different vendors. So we should > >standardize to use the vendor-id and value of the first one who > >documents the format? > > Maybe: > __u64 modifier[4]; > __u64 vendor_modifier[4]; Seems rendundant since the modifier added in this patch is already vendor specific. Or what exactly are you trying to accomplish here? -Daniel > > ? > > Regards, > > Tvrtko -- 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] [igt PATCH] gen9: fix gem_render_copy 3d state setup
On Thu, Jan 29, 2015 at 11:17:04AM +, Damien Lespiau wrote: > On Thu, Jan 29, 2015 at 11:12:46AM +, Chris Wilson wrote: > > On Thu, Jan 29, 2015 at 03:01:50AM -0800, Imre Deak wrote: > > > On Thu, 2015-01-29 at 10:51 +, Damien Lespiau wrote: > > > > On Thu, Jan 29, 2015 at 12:03:19AM -0800, Imre Deak wrote: > > > > > Without emitting the default 3DSTATE_WM_DEPTH_STENCIL state the test > > > > > will fail. > > > > > > > > > > Signed-off-by: Imre Deak > > > > > > > > Question: Wasn't the golden context supposed to paper over those? > > > > > > Perhaps, currently the golden context doesn't include this. > > > > Today, you cannot rely on the initial contents of the context even with > > the golden render state. There is no pristine context, every client is > > responsible for configuring the hardware exactly as they intend to use - > > at least as regards the untrusted commands (e.g. 3DSTATE). > > Right. Now the question is, do we want to change that and have the > golden context with sane defaults? You missed the point. The point is that we don't keep initialise every context from scratch. And there still doesn't seem to be any reason to be papering over userspace bugs. -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] [igt PATCH] gen9: fix gem_render_copy 3d state setup
On Thu, Jan 29, 2015 at 11:32:46AM +, Chris Wilson wrote: > On Thu, Jan 29, 2015 at 11:17:04AM +, Damien Lespiau wrote: > > On Thu, Jan 29, 2015 at 11:12:46AM +, Chris Wilson wrote: > > > On Thu, Jan 29, 2015 at 03:01:50AM -0800, Imre Deak wrote: > > > > On Thu, 2015-01-29 at 10:51 +, Damien Lespiau wrote: > > > > > On Thu, Jan 29, 2015 at 12:03:19AM -0800, Imre Deak wrote: > > > > > > Without emitting the default 3DSTATE_WM_DEPTH_STENCIL state the test > > > > > > will fail. > > > > > > > > > > > > Signed-off-by: Imre Deak > > > > > > > > > > Question: Wasn't the golden context supposed to paper over those? > > > > > > > > Perhaps, currently the golden context doesn't include this. > > > > > > Today, you cannot rely on the initial contents of the context even with > > > the golden render state. There is no pristine context, every client is > > > responsible for configuring the hardware exactly as they intend to use - > > > at least as regards the untrusted commands (e.g. 3DSTATE). > > > > Right. Now the question is, do we want to change that and have the > > golden context with sane defaults? > > You missed the point. The point is that we don't keep initialise every > context from scratch. And there still doesn't seem to be any reason to > be papering over userspace bugs. That's because I still think the end of the journey is a fully initialized golden context image + copy of that context on context creation. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2 (v1.5)
On 01/29/2015 11:30 AM, Daniel Vetter wrote: On Wed, Jan 28, 2015 at 05:57:56PM +, Tvrtko Ursulin wrote: On 01/28/2015 05:37 PM, Daniel Vetter wrote: From: Rob Clark In DRM/KMS we are lacking a good way to deal with tiled/compressed formats. Especially in the case of dmabuf/prime buffer sharing, where we cannot always rely on under-the-hood flags passed to driver specific gem-create ioctl to pass around these extra flags. The proposal is to add a per-plane format modifier. This allows to, if necessary, use different tiling patters for sub-sampled planes, etc. The format modifiers are added at the end of the ioctl struct, so for legacy userspace it will be zero padded. TODO how best to deal with assignment of modifier token values? The rough idea was to namespace things with an 8bit vendor-id, and then beyond that it is treated as an opaque value. But that was a relatively arbitrary choice. There are cases where same tiling pattern and/or compression is supported by various different vendors. So we should standardize to use the vendor-id and value of the first one who documents the format? Maybe: __u64 modifier[4]; __u64 vendor_modifier[4]; Seems rendundant since the modifier added in this patch is already vendor specific. Or what exactly are you trying to accomplish here? I am trying to avoid packet-in-a-packet (bitmasks) mumbo-jumbo and vendor id on the head followed by maybe standardized or maybe vendor specific tag. Feels funny. Would it not be simpler to put a struct in there? But I was not following this from the start so maybe I am missing something.. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2 (v1.5)
On Thu, Jan 29, 2015 at 11:43:07AM +, Tvrtko Ursulin wrote: > > On 01/29/2015 11:30 AM, Daniel Vetter wrote: > >On Wed, Jan 28, 2015 at 05:57:56PM +, Tvrtko Ursulin wrote: > >> > >>On 01/28/2015 05:37 PM, Daniel Vetter wrote: > >>>From: Rob Clark > >>> > >>>In DRM/KMS we are lacking a good way to deal with tiled/compressed > >>>formats. Especially in the case of dmabuf/prime buffer sharing, where > >>>we cannot always rely on under-the-hood flags passed to driver specific > >>>gem-create ioctl to pass around these extra flags. > >>> > >>>The proposal is to add a per-plane format modifier. This allows to, if > >>>necessary, use different tiling patters for sub-sampled planes, etc. > >>>The format modifiers are added at the end of the ioctl struct, so for > >>>legacy userspace it will be zero padded. > >>> > >>>TODO how best to deal with assignment of modifier token values? The > >>>rough idea was to namespace things with an 8bit vendor-id, and then > >>>beyond that it is treated as an opaque value. But that was a relatively > >>>arbitrary choice. There are cases where same tiling pattern and/or > >>>compression is supported by various different vendors. So we should > >>>standardize to use the vendor-id and value of the first one who > >>>documents the format? > >> > >>Maybe: > >>__u64 modifier[4]; > >>__u64 vendor_modifier[4]; > > > >Seems rendundant since the modifier added in this patch is already vendor > >specific. Or what exactly are you trying to accomplish here? > > I am trying to avoid packet-in-a-packet (bitmasks) mumbo-jumbo and vendor id > on the head followed by maybe standardized or maybe vendor specific tag. > Feels funny. Would it not be simpler to put a struct in there? The u64 modifier is just an opaque thing, with 8 bit to identifier the vendor (for easier number management really) and the low 56 bits can be whatever we want them. On i915 I think we should just enumerate all the various tiling modes we have. And if the modifiers aren't set we use the tiling mode of the underlying gem bo. We already have code in place to guarantee that the underlying bo's tiling can't change as long as there's a kms fb around, which means all code which checks for tiling can switch over to these flags. struct won't work since by definition this is vendor-specific, and every vendor is slightly insane in a different way. -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 2/2] drm/i915: Introduce intel_set_rps()
On Wed, Jan 28, 2015 at 10:29:06AM +, Chris Wilson wrote:
> On Tue, Jan 27, 2015 at 04:36:16PM +0200, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > Replace the valleyview_set_rps() and gen6_set_rps() calls with
> > intel_set_rps() which itself does the IS_VALLEYVIEW() check. The
> > code becomes simpler since the callers don't have to do this check
> > themselves.
> >
> > Most of the change was performe with the following semantic patch:
> > @@
> > expression E1, E2;
> > @@
> > (
> > - valleyview_set_rps(E1, E2)
> > + intel_set_rps(E1, E2)
> > |
> > - gen6_set_rps(E1, E2)
> > + intel_set_rps(E1, E2)
> > )
> >
> > @@
> > expression E1, E2, E3;
> > @@
> > - if (IS_VALLEYVIEW(E1)) {
> > - intel_set_rps(E2, E3);
> > - } else {
> > - intel_set_rps(E2, E3);
> > - }
> > + intel_set_rps(E2, E3);
> >
> > Adding intel_set_rps() and making valleyview_set_rps() and gen6_set_rps()
> > static was done manually.
> >
> > Cc: Chris Wilson
> > Suggested-by: Chris Wilson
> > Signed-off-by: Ville Syrjälä
>
> Hmm, I like half of them :|
>
> The external callers from i915_debugfs.c, i915_irq.c are good. But
> inside intel_pm.c, it is more mixed. gen6_rps_boost() clearly wants
> intel_set_rps(), but from within the gen specific lowlevel functions, it
> looks odder to callback into intel_set_rps.
The semantic patch was greedy, and I figured I'd leave it that way to
avoid someone accidentally copy-pasting the wrong thing. But I can
change them back if that's the preferred way.
>
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > index 6ece663..2bad1e8 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3750,7 +3750,7 @@ static u32 gen6_rps_pm_mask(struct drm_i915_private
> > *dev_priv, u8 val)
> > /* gen6_set_rps is called to update the frequency request, but should also
> > be
> > * called when the range (min_delay and max_delay) is modified so that we
> > can
> > * update the GEN6_RP_INTERRUPT_LIMITS register accordingly. */
> > -void gen6_set_rps(struct drm_device *dev, u8 val)
> > +static void gen6_set_rps(struct drm_device *dev, u8 val)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > @@ -3801,7 +3801,7 @@ static void vlv_set_rps_idle(struct drm_i915_private
> > *dev_priv)
> >
> > /* CHV and latest VLV don't need to force the gfx clock */
> > if (IS_CHERRYVIEW(dev) || dev->pdev->revision >= 0xd) {
> > - valleyview_set_rps(dev_priv->dev,
> > dev_priv->rps.min_freq_softlimit);
> > + intel_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> > return;
> > }
> >
> > @@ -3842,7 +3842,8 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv)
> > if (IS_VALLEYVIEW(dev))
> > vlv_set_rps_idle(dev_priv);
> > else
> > - gen6_set_rps(dev_priv->dev,
> > dev_priv->rps.min_freq_softlimit);
> > + intel_set_rps(dev_priv->dev,
> > + dev_priv->rps.min_freq_softlimit);
> > dev_priv->rps.last_adj = 0;
> > }
> > mutex_unlock(&dev_priv->rps.hw_lock);
>
> These two are the most dubious for me.
>
> > static void gen9_disable_rps(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -4176,7 +4180,7 @@ static void gen8_enable_rps(struct drm_device *dev)
> > /* 6: Ring frequency + overclocking (our driver does this later */
> >
> > dev_priv->rps.power = HIGH_POWER; /* force a reset */
> > - gen6_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> > + intel_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> >
> > intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> > }
> > @@ -4270,7 +4274,7 @@ static void gen6_enable_rps(struct drm_device *dev)
> > }
> >
> > dev_priv->rps.power = HIGH_POWER; /* force a reset */
> > - gen6_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> > + intel_set_rps(dev_priv->dev, dev_priv->rps.min_freq_softlimit);
> >
> > rc6vids = 0;
> > ret = sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS,
> > &rc6vids);
> > @@ -4812,7 +4816,7 @@ static void cherryview_enable_rps(struct drm_device
> > *dev)
> > intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq),
> > dev_priv->rps.efficient_freq);
> >
> > - valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
> > + intel_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
> >
> > intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
> > }
> > @@ -4896,7 +4900,7 @@ static void valleyview_enable_rps(struct drm_device
> > *dev)
> > intel_gpu_freq(dev_priv, dev_priv->rps.efficient_freq),
> > dev_priv->rps.efficient_freq);
> >
> > - valleyview_set_rps(dev_priv->dev, dev_priv->rps.efficient_freq);
>
[Intel-gfx] [PATCH] drm/i915/skl: Enable eDRAM for gen9 as well
Suggested-by: Daniel Vetter
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/intel_uncore.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 0e9bf82..0a1089b 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -329,7 +329,8 @@ static void __intel_uncore_early_sanitize(struct drm_device
*dev,
if (HAS_FPGA_DBG_UNCLAIMED(dev))
__raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM);
- if ((IS_HASWELL(dev) || IS_BROADWELL(dev)) &&
+ if ((IS_HASWELL(dev) || IS_BROADWELL(dev) ||
+INTEL_INFO(dev)->gen >= 9) &&
(__raw_i915_read32(dev_priv, HSW_EDRAM_PRESENT) == 1)) {
/* The docs do not explain exactly how the calculation can be
* made. It is somewhat guessable, but for now, it's always
--
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] RFC: drm: add support for tiled/compressed/etc modifier in addfb2 (v1.5)
On 01/29/2015 11:57 AM, Daniel Vetter wrote: On Thu, Jan 29, 2015 at 11:43:07AM +, Tvrtko Ursulin wrote: On 01/29/2015 11:30 AM, Daniel Vetter wrote: On Wed, Jan 28, 2015 at 05:57:56PM +, Tvrtko Ursulin wrote: On 01/28/2015 05:37 PM, Daniel Vetter wrote: From: Rob Clark In DRM/KMS we are lacking a good way to deal with tiled/compressed formats. Especially in the case of dmabuf/prime buffer sharing, where we cannot always rely on under-the-hood flags passed to driver specific gem-create ioctl to pass around these extra flags. The proposal is to add a per-plane format modifier. This allows to, if necessary, use different tiling patters for sub-sampled planes, etc. The format modifiers are added at the end of the ioctl struct, so for legacy userspace it will be zero padded. TODO how best to deal with assignment of modifier token values? The rough idea was to namespace things with an 8bit vendor-id, and then beyond that it is treated as an opaque value. But that was a relatively arbitrary choice. There are cases where same tiling pattern and/or compression is supported by various different vendors. So we should standardize to use the vendor-id and value of the first one who documents the format? Maybe: __u64 modifier[4]; __u64 vendor_modifier[4]; Seems rendundant since the modifier added in this patch is already vendor specific. Or what exactly are you trying to accomplish here? I am trying to avoid packet-in-a-packet (bitmasks) mumbo-jumbo and vendor id on the head followed by maybe standardized or maybe vendor specific tag. Feels funny. Would it not be simpler to put a struct in there? The u64 modifier is just an opaque thing, with 8 bit to identifier the vendor (for easier number management really) and the low 56 bits can be whatever we want them. On i915 I think we should just enumerate all the various tiling modes we have. And if the modifiers aren't set we use the tiling mode of the underlying gem bo. We already have code in place to guarantee that the underlying bo's tiling can't change as long as there's a kms fb around, which means all code which checks for tiling can switch over to these flags. struct won't work since by definition this is vendor-specific, and every vendor is slightly insane in a different way. Well 8 + 56 bits is a "struct" already and not fully opaque. Are the bits expensive? :) That was first part of my point. Secondly, "vendor who registers first" part of discussion is what came to my attention as well. With this, a hypothetical standard format would be under a vendor id which looks funny to me. While you could split standard formats/modifiers and vendor specific modifiers. I don't care really, it was just an idea, but I don't get this arguments why it is, not only not better, but worse than a bitfield. :) Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2 (v1.5)
On Thu, Jan 29, 2015 at 12:55:48PM +, Tvrtko Ursulin wrote: > > On 01/29/2015 11:57 AM, Daniel Vetter wrote: > >On Thu, Jan 29, 2015 at 11:43:07AM +, Tvrtko Ursulin wrote: > >> > >>On 01/29/2015 11:30 AM, Daniel Vetter wrote: > >>>On Wed, Jan 28, 2015 at 05:57:56PM +, Tvrtko Ursulin wrote: > > On 01/28/2015 05:37 PM, Daniel Vetter wrote: > >From: Rob Clark > > > >In DRM/KMS we are lacking a good way to deal with tiled/compressed > >formats. Especially in the case of dmabuf/prime buffer sharing, where > >we cannot always rely on under-the-hood flags passed to driver specific > >gem-create ioctl to pass around these extra flags. > > > >The proposal is to add a per-plane format modifier. This allows to, if > >necessary, use different tiling patters for sub-sampled planes, etc. > >The format modifiers are added at the end of the ioctl struct, so for > >legacy userspace it will be zero padded. > > > >TODO how best to deal with assignment of modifier token values? The > >rough idea was to namespace things with an 8bit vendor-id, and then > >beyond that it is treated as an opaque value. But that was a relatively > >arbitrary choice. There are cases where same tiling pattern and/or > >compression is supported by various different vendors. So we should > >standardize to use the vendor-id and value of the first one who > >documents the format? > > Maybe: > __u64 modifier[4]; > __u64 vendor_modifier[4]; > >>> > >>>Seems rendundant since the modifier added in this patch is already vendor > >>>specific. Or what exactly are you trying to accomplish here? > >> > >>I am trying to avoid packet-in-a-packet (bitmasks) mumbo-jumbo and vendor id > >>on the head followed by maybe standardized or maybe vendor specific tag. > >>Feels funny. Would it not be simpler to put a struct in there? > > > >The u64 modifier is just an opaque thing, with 8 bit to identifier the > >vendor (for easier number management really) and the low 56 bits can be > >whatever we want them. On i915 I think we should just enumerate all the > >various tiling modes we have. And if the modifiers aren't set we use the > >tiling mode of the underlying gem bo. We already have code in place to > >guarantee that the underlying bo's tiling can't change as long as there's > >a kms fb around, which means all code which checks for tiling can switch > >over to these flags. > > > >struct won't work since by definition this is vendor-specific, and every > >vendor is slightly insane in a different way. > > Well 8 + 56 bits is a "struct" already and not fully opaque. Are the bits > expensive? :) That was first part of my point. > > Secondly, "vendor who registers first" part of discussion is what came to my > attention as well. > > With this, a hypothetical standard format would be under a vendor id which > looks funny to me. While you could split standard formats/modifiers and > vendor specific modifiers. If some standard body actually manages to pull this off we can always add a new enum value for KHRONOS or MICROSOFT or ISO or whatever it ends up being. The split really is just to make number assignements less conflit-y. > I don't care really, it was just an idea, but I don't get this arguments why > it is, not only not better, but worse than a bitfield. :) Just from your struct without any explanation what your idea was (namely modifiers which are standardized across vendors it seems) it looked like both a plain modifier and a vendor_modifier was possible. Which didn't make a lot of sense to me, so I asked. Going with an opaque u64 has the benefits that it matches kms property values. So if we ever extend this and allow generic properties for framebuffers it'll still fit. A struct is more painful. The idea of fb properties was one of the longer-term ideas tossed around in the v1 thread. -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 2/2] tests/gem_reset_stats: add tests for ban period ioctl
Test parameter set/get for ban periods.
Test actual impact on banning.
Signed-off-by: Mika Kuoppala
---
tests/gem_reset_stats.c | 101 +++-
1 file changed, 100 insertions(+), 1 deletion(-)
diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
index 646d6da..7b04fb7 100644
--- a/tests/gem_reset_stats.c
+++ b/tests/gem_reset_stats.c
@@ -52,6 +52,8 @@
#define RS_BATCH_PENDING (1 << 1)
#define RS_UNKNOWN (1 << 2)
+IGT_TEST_DESCRIPTION("Checks for RESET_STATS_IOCTL, context banning and reset
recovery");
+
static uint32_t devid;
static bool hw_contexts;
@@ -1047,6 +1049,98 @@ static void defer_hangcheck(int ring_num)
close(fd);
}
+static bool was_banned_in_period(int fd, int ctx, int seconds)
+{
+ int h1,h2,h3,h4;
+ bool banned;
+
+ h1 = inject_hang_no_ban_error(fd, ctx);
+ igt_assert(h1 >= 0);
+
+ sleep(seconds);
+
+ h2 = exec_valid(fd, ctx);
+ igt_assert(h2 >= 0);
+
+ h3 = inject_hang_no_ban_error(fd, ctx);
+ igt_assert(h3 >= 0);
+
+ gem_sync(fd, h3);
+
+ h4 = exec_valid(fd, ctx);
+ banned = (h4 == -EIO);
+
+ gem_close(fd, h1);
+ gem_close(fd, h2);
+ gem_close(fd, h3);
+ if (h4 >= 0)
+ gem_close(fd, h4);
+
+ return banned;
+}
+
+static int get_ban_period(int fd, uint32_t ctx)
+{
+ struct local_i915_gem_context_param p;
+
+ p.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
+ p.size = rand();
+ p.context = ctx;
+ p.value = ((uint64_t)rand() << 32) | rand();
+
+ igt_assert(gem_context_get_param(fd, &p) == 0);
+
+ return p.value;
+}
+
+static void set_ban_period(int fd, uint32_t ctx, int period)
+{
+ struct local_i915_gem_context_param p;
+
+ p.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
+ p.size = 0;
+ p.context = ctx;
+ p.value = period;
+ igt_assert(gem_context_set_param(fd, &p) == 0);
+}
+
+static void test_ban_period(bool new_ctx)
+{
+ int fd, period;
+ uint32_t ctx;
+
+ fd = drm_open_any();
+
+ igt_require(gem_context_has_param(fd, LOCAL_CONTEXT_PARAM_BAN_PERIOD));
+
+ if (new_ctx)
+ ctx = context_create(fd);
+ else
+ ctx = 0;
+
+ period = get_ban_period(fd, ctx);
+ igt_assert(period > 2);
+
+ period += 2;
+
+ set_ban_period(fd, ctx, period);
+
+ igt_assert(was_banned_in_period(fd, ctx, period + 2) == false);
+
+ set_ban_period(fd, ctx, 0);
+
+ igt_assert(was_banned_in_period(fd, ctx, 0) == false);
+
+ /* We just hanged, wait for a while */
+ sleep(period + 2);
+
+ set_ban_period(fd, ctx, period);
+
+ igt_assert(was_banned_in_period(fd, ctx, period / 4) == true);
+
+ close(fd);
+}
+
static bool gem_has_hw_contexts(int fd)
{
struct local_drm_i915_gem_context_create create;
@@ -,7 +1205,7 @@ static void check_gpu_ok(void)
#define RING_HAS_CONTEXTS (current_ring->contexts(current_ring))
#define RUN_TEST(...) do { check_gpu_ok(); __VA_ARGS__; check_gpu_ok(); }
while (0)
-#define RUN_CTX_TEST(...) do { igt_skip_on(RING_HAS_CONTEXTS == false);
RUN_TEST(__VA_ARGS__); } while (0)
+#define RUN_CTX_TEST(...) do { igt_require(RING_HAS_CONTEXTS);
RUN_TEST(__VA_ARGS__); } while (0)
igt_main
{
@@ -1191,5 +1285,10 @@ igt_main
igt_subtest_f("defer-hangcheck-%s", name)
RUN_TEST(defer_hangcheck(i));
+ igt_subtest_f("ban-period-%s", name)
+ RUN_TEST(test_ban_period(false));
+
+ igt_subtest_f("ban-period-ctx-%s", name)
+ RUN_CTX_TEST(test_ban_period(true));
}
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] tests: Add gem_ctx_params
Add gem_ctx_params to check context parameters ioctl test for
set and getting ban periods.
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
tests/Makefile.sources | 1 +
tests/gem_ctx_params.c | 211 +
2 files changed, 212 insertions(+)
create mode 100644 tests/gem_ctx_params.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 74deec3..54bea34 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -25,6 +25,7 @@ TESTS_progs_M = \
gem_cs_tlb \
gem_ctx_bad_exec \
gem_ctx_exec \
+ gem_ctx_params \
gem_dummy_reloc_loop \
gem_evict_alignment \
gem_evict_everything \
diff --git a/tests/gem_ctx_params.c b/tests/gem_ctx_params.c
new file mode 100644
index 000..f831ecd
--- /dev/null
+++ b/tests/gem_ctx_params.c
@@ -0,0 +1,211 @@
+/*
+ * Copyright © 2015 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:
+ *Mika Kuoppala
+ *
+ */
+
+#include "drmtest.h"
+#include "ioctl_wrappers.h"
+#include "igt_aux.h"
+
+IGT_TEST_DESCRIPTION("Checks for context parameters");
+
+struct local_drm_i915_gem_context_create {
+ __u32 ctx_id;
+ __u32 pad;
+};
+
+struct local_drm_i915_gem_context_destroy {
+ __u32 ctx_id;
+ __u32 pad;
+};
+
+#define CONTEXT_CREATE_IOCTL DRM_IOWR(DRM_COMMAND_BASE + 0x2d, struct
local_drm_i915_gem_context_create)
+#define CONTEXT_DESTROY_IOCTL DRM_IOWR(DRM_COMMAND_BASE + 0x2e, struct
local_drm_i915_gem_context_destroy)
+
+static uint32_t context_create(int fd)
+{
+ struct local_drm_i915_gem_context_create create;
+ int ret;
+
+ create.ctx_id = rand();
+ create.pad = rand();
+
+ ret = drmIoctl(fd, CONTEXT_CREATE_IOCTL, &create);
+ igt_assert(ret == 0);
+
+ return create.ctx_id;
+}
+
+static int context_destroy(int fd, uint32_t ctx_id)
+{
+ int ret;
+ struct local_drm_i915_gem_context_destroy destroy;
+
+ destroy.ctx_id = ctx_id;
+ destroy.pad = rand();
+
+ ret = drmIoctl(fd, CONTEXT_DESTROY_IOCTL, &destroy);
+ if (ret != 0)
+ return -errno;
+
+ return 0;
+}
+
+static int _get_ban_period(int fd, struct local_i915_gem_context_param *p)
+{
+ int r;
+
+ r = gem_context_get_param(fd, p);
+ if (r == -1)
+ return errno;
+
+ return 0;
+}
+
+static int get_ban_period(int fd, int ctx)
+{
+ struct local_i915_gem_context_param p;
+
+ p.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
+ p.size = rand();
+ p.context = rand();
+ if (p.context == ctx)
+ p.context = ctx + 1;
+ p.value = ((uint64_t)rand() << 32) | rand();
+
+ igt_assert(gem_context_get_param(fd, &p) == -1);
+ igt_assert(errno == ENOENT);
+
+ p.context = ctx;
+ p.param = 0xdeadf00d;
+
+ igt_assert(gem_context_get_param(fd, &p) == -1);
+ igt_assert(errno == EINVAL);
+
+ p.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
+ igt_assert(gem_context_get_param(fd, &p) == 0);
+ igt_assert(errno == 0);
+ igt_assert(p.size == 0);
+
+ return p.value;
+}
+
+static int set_ban_period(int fd, struct local_i915_gem_context_param *p)
+{
+ int r;
+
+ r = gem_context_set_param(fd, p);
+ if (r == -1)
+ return errno;
+
+ return 0;
+}
+
+static void test_ctx_params_invalid(bool new_ctx)
+{
+ struct local_i915_gem_context_param p;
+ int fd, period;
+ uint32_t ctx;
+
+ fd = drm_open_any();
+
+ igt_require(gem_context_has_param(fd, LOCAL_CONTEXT_PARAM_BAN_PERIOD));
+
+ if (new_ctx)
+ ctx = context_create(fd);
+ else
+ ctx = 0;
+
+ period = get_ban_period(fd, ctx);
+ igt_assert(period > 2);
+
+ p.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
+ p.size = 0xdeadf00d;
+
[Intel-gfx] [PATCH] tests: Add gem_copy_align_blt test
Copy a block into destination object with varying dst/src offsets.
Put guard area before and after the blit target to see that it didn't
touch memory out of blit boundaries.
v2: Test description, git add and gitignore (Thomas)
Strip it out from gem_userptr (Chris)
References: https://bugs.freedesktop.org/show_bug.cgi?id=79053
Cc: Thomas Wood
Cc: Chris Wilson
Signed-off-by: Mika Kuoppala
---
tests/.gitignore | 1 +
tests/Makefile.sources | 1 +
tests/gem_copy_align_blt.c | 225 +
3 files changed, 227 insertions(+)
create mode 100644 tests/gem_copy_align_blt.c
diff --git a/tests/.gitignore b/tests/.gitignore
index 88a6405..d5f2907 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -98,6 +98,7 @@ gem_storedw_loop_vebox
gem_stress
gem_threaded_access_tiled
gem_tiled_blits
+gem_copy_align_blt
gem_tiled_fence_blits
gem_tiled_partial_pwrite_pread
gem_tiled_pread
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 74deec3..0b17b8a 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -60,6 +60,7 @@ TESTS_progs_M = \
gem_set_tiling_vs_blt \
gem_storedw_batches_loop \
gem_tiled_blits \
+ gem_copy_align_blt \
gem_tiled_partial_pwrite_pread \
gem_userptr_blits \
gem_write_read_ring_switch \
diff --git a/tests/gem_copy_align_blt.c b/tests/gem_copy_align_blt.c
new file mode 100644
index 000..6376d70
--- /dev/null
+++ b/tests/gem_copy_align_blt.c
@@ -0,0 +1,225 @@
+/*
+ * Copyright © 2014 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
+ *Mika Kuoppala
+ *
+ */
+
+#include
+#include
+#include
+#include "i915_drm.h"
+#include "intel_chipset.h"
+#include "intel_reg.h"
+#include "ioctl_wrappers.h"
+#include "drm.h"
+#include "drmtest.h"
+
+IGT_TEST_DESCRIPTION("Verify blt copying with boundary checks");
+
+static int gen = 0;
+
+#define WIDTH 32
+#define HEIGHT 32
+
+static uint32_t linear[WIDTH*HEIGHT];
+
+static void
+copy_align(int fd,
+ uint32_t dst, uint32_t dst_offset,
+ uint32_t src, uint32_t src_offset,
+ uint32_t w, uint32_t h)
+{
+ uint32_t batch[12];
+ struct drm_i915_gem_relocation_entry reloc[2];
+ struct drm_i915_gem_exec_object2 obj[3];
+ struct drm_i915_gem_execbuffer2 exec;
+ uint32_t handle;
+ int i=0;
+
+ batch[i++] = XY_SRC_COPY_BLT_CMD |
+ XY_SRC_COPY_BLT_WRITE_ALPHA |
+ XY_SRC_COPY_BLT_WRITE_RGB;
+ if (gen >= 8)
+ batch[i - 1] |= 8;
+ else
+ batch[i - 1] |= 6;
+
+ batch[i++] = (3 << 24) | /* 32 bits */
+ (0xcc << 16) | /* copy ROP */
+ w * 4;
+
+ /* The >= gen8 blitter needs to have dst/src base
+* addresses aligned to 4k. So we need to handle the
+* offsets with with dst/src coordinates */
+ batch[i++] = dst_offset; /* dst, x1,y2 */
+ batch[i++] = ((h) << 16) | (w + dst_offset); /* dst x2,y2 */
+ batch[i++] = 0;
+ if (gen >= 8)
+ batch[i++] = 0;
+ batch[i++] = src_offset; /* src x1,y1 */
+ batch[i++] = w * 4;
+
+ batch[i++] = 0;
+ if (gen >= 8)
+ batch[i++] = 0;
+
+ batch[i++] = MI_BATCH_BUFFER_END;
+ batch[i++] = MI_NOOP;
+
+ handle = gem_create(fd, 4096);
+ gem_write(fd, handle, 0, batch, sizeof(batch));
+
+ reloc[0].target_handle = dst;
+ reloc[0].delta = 0;
+ reloc[0].offset = 4 * sizeof(batch[0]);
+ reloc[0].presumed_offset = 0;
+ reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
+ reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
+
+ reloc[1].target_handle = src;
+ reloc[1].delta = 0;
+ reloc[1].offset = 7 * sizeof(batch[0]);
+ if (gen >= 8)
+ relo
[Intel-gfx] [PATCH 2/3] drm/i915/skl: Declare that GT3 has a second VCS
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/i915_drv.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6484229..9fdaf64 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -356,7 +356,7 @@ static const struct intel_device_info intel_cherryview_info
= {
CURSOR_OFFSETS,
};
-static const struct intel_device_info intel_skylake_info = {
+static const struct intel_device_info intel_skylake_gt12_info = {
.is_preliminary = 1,
.is_skylake = 1,
.gen = 9, .num_pipes = 3,
@@ -369,6 +369,19 @@ static const struct intel_device_info intel_skylake_info =
{
IVB_CURSOR_OFFSETS,
};
+static const struct intel_device_info intel_skylake_gt3_info = {
+ .is_preliminary = 1,
+ .is_skylake = 1,
+ .gen = 9, .num_pipes = 3,
+ .need_gfx_hws = 1, .has_hotplug = 1,
+ .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
+ .has_llc = 1,
+ .has_ddi = 1,
+ .has_fbc = 1,
+ GEN_DEFAULT_PIPEOFFSETS,
+ IVB_CURSOR_OFFSETS,
+};
+
/*
* Make sure any device matches here are from most specific to most
* general. For example, since the Quanta match is based on the subsystem
@@ -406,7 +419,9 @@ static const struct intel_device_info intel_skylake_info = {
INTEL_BDW_GT3M_IDS(&intel_broadwell_gt3m_info), \
INTEL_BDW_GT3D_IDS(&intel_broadwell_gt3d_info), \
INTEL_CHV_IDS(&intel_cherryview_info), \
- INTEL_SKL_IDS(&intel_skylake_info)
+ INTEL_SKL_GT1_IDS(&intel_skylake_gt12_info),\
+ INTEL_SKL_GT2_IDS(&intel_skylake_gt12_info),\
+ INTEL_SKL_GT3_IDS(&intel_skylake_gt3_info) \
static const struct pci_device_id pciidlist[] = { /* aka */
INTEL_PCI_IDS,
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915/skl: Split the SKL PCI ids by GT
We need to have a separate GT3 struct intel_device_info to declare they have a second VCS. Let's start by splitting the PCI ids per-GT. Signed-off-by: Damien Lespiau --- include/drm/i915_pciids.h | 28 +++- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 180ad0e..38a7c80 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -259,21 +259,31 @@ INTEL_VGA_DEVICE(0x22b2, info), \ INTEL_VGA_DEVICE(0x22b3, info) -#define INTEL_SKL_IDS(info) \ - INTEL_VGA_DEVICE(0x1916, info), /* ULT GT2 */ \ +#define INTEL_SKL_GT1_IDS(info)\ INTEL_VGA_DEVICE(0x1906, info), /* ULT GT1 */ \ - INTEL_VGA_DEVICE(0x1926, info), /* ULT GT3 */ \ - INTEL_VGA_DEVICE(0x1921, info), /* ULT GT2F */ \ INTEL_VGA_DEVICE(0x190E, info), /* ULX GT1 */ \ + INTEL_VGA_DEVICE(0x1902, info), /* DT GT1 */ \ + INTEL_VGA_DEVICE(0x190B, info), /* Halo GT1 */ \ + INTEL_VGA_DEVICE(0x190A, info) /* SRV GT1 */ + +#define INTEL_SKL_GT2_IDS(info)\ + INTEL_VGA_DEVICE(0x1916, info), /* ULT GT2 */ \ + INTEL_VGA_DEVICE(0x1921, info), /* ULT GT2F */ \ INTEL_VGA_DEVICE(0x191E, info), /* ULX GT2 */ \ INTEL_VGA_DEVICE(0x1912, info), /* DT GT2 */ \ - INTEL_VGA_DEVICE(0x1902, info), /* DT GT1 */ \ INTEL_VGA_DEVICE(0x191B, info), /* Halo GT2 */ \ - INTEL_VGA_DEVICE(0x192B, info), /* Halo GT3 */ \ - INTEL_VGA_DEVICE(0x190B, info), /* Halo GT1 */ \ INTEL_VGA_DEVICE(0x191A, info), /* SRV GT2 */ \ - INTEL_VGA_DEVICE(0x192A, info), /* SRV GT3 */ \ - INTEL_VGA_DEVICE(0x190A, info), /* SRV GT1 */ \ INTEL_VGA_DEVICE(0x191D, info) /* WKS GT2 */ +#define INTEL_SKL_GT3_IDS(info) \ + INTEL_VGA_DEVICE(0x1926, info), /* ULT GT3 */ \ + INTEL_VGA_DEVICE(0x192B, info), /* Halo GT3 */ \ + INTEL_VGA_DEVICE(0x192A, info) /* SRV GT3 */ \ + +#define INTEL_SKL_IDS(info) \ + INTEL_SKL_GT1_IDS(info), \ + INTEL_SKL_GT2_IDS(info), \ + INTEL_SKL_GT3_IDS(info) + + #endif /* _I915_PCIIDS_H */ -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915/skl: Remove the check enforcing VCS2 to be gen8 only
Signed-off-by: Damien Lespiau
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index d393026..bbe439d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2630,19 +2630,13 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
}
/**
- * Initialize the second BSD ring for Broadwell GT3.
- * It is noted that this only exists on Broadwell GT3.
+ * Initialize the second BSD ring (eg. Broadwell GT3, Skylake GT3)
*/
int intel_init_bsd2_ring_buffer(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_engine_cs *ring = &dev_priv->ring[VCS2];
- if ((INTEL_INFO(dev)->gen != 8)) {
- DRM_ERROR("No dual-BSD ring on non-BDW machine\n");
- return -EINVAL;
- }
-
ring->name = "bsd2 ring";
ring->id = VCS2;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt PATCH] gen9: fix gem_render_copy 3d state setup
Damien Lespiau writes: > On Thu, Jan 29, 2015 at 11:32:46AM +, Chris Wilson wrote: >> On Thu, Jan 29, 2015 at 11:17:04AM +, Damien Lespiau wrote: >> > On Thu, Jan 29, 2015 at 11:12:46AM +, Chris Wilson wrote: >> > > On Thu, Jan 29, 2015 at 03:01:50AM -0800, Imre Deak wrote: >> > > > On Thu, 2015-01-29 at 10:51 +, Damien Lespiau wrote: >> > > > > On Thu, Jan 29, 2015 at 12:03:19AM -0800, Imre Deak wrote: >> > > > > > Without emitting the default 3DSTATE_WM_DEPTH_STENCIL state the >> > > > > > test >> > > > > > will fail. >> > > > > > >> > > > > > Signed-off-by: Imre Deak >> > > > > >> > > > > Question: Wasn't the golden context supposed to paper over those? >> > > > >> > > > Perhaps, currently the golden context doesn't include this. >> > > >> > > Today, you cannot rely on the initial contents of the context even with >> > > the golden render state. There is no pristine context, every client is >> > > responsible for configuring the hardware exactly as they intend to use - >> > > at least as regards the untrusted commands (e.g. 3DSTATE). >> > >> > Right. Now the question is, do we want to change that and have the >> > golden context with sane defaults? >> >> You missed the point. The point is that we don't keep initialise every >> context from scratch. And there still doesn't seem to be any reason to >> be papering over userspace bugs. > > That's because I still think the end of the journey is a fully > initialized golden context image + copy of that context on context > creation. > For me the end journey has looked like this: For first (default) context: - run golden/null batch - emit workarounds - take a master copy from ctx_obj Then: - copy from master (with gpu or cpu) for every new fd/ctx -Mika > -- > Damien > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 0/2] printing log messages on test failure
This series replaces the earlier one dealing with printing recent log messages and avoids any issues with signal handlers by only printing the log on a test failure. Thomas Wood (2): lib: add a ring buffer for log entries lib: print recent log messages to stderr when a test or subtest fails lib/Makefile.am | 3 +- lib/igt_core.c | 112 +--- 2 files changed, 100 insertions(+), 15 deletions(-) -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/2] lib: add a ring buffer for log entries
Signed-off-by: Thomas Wood
---
lib/Makefile.am | 3 ++-
lib/igt_core.c | 72 -
2 files changed, 63 insertions(+), 12 deletions(-)
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 3826a1c..a5a4390 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -11,7 +11,8 @@ noinst_HEADERS = check-ndebug.h
AM_CPPFLAGS = -I$(top_srcdir)
AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) \
-DIGT_DATADIR=\""$(abs_top_srcdir)/tests"\" \
- -DIGT_LOG_DOMAIN=\""$(subst _,-,$*)"\"
+ -DIGT_LOG_DOMAIN=\""$(subst _,-,$*)"\" \
+ -pthread
LDADD = $(CAIRO_LIBS)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 7b47b32..b03b7df 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -47,9 +47,8 @@
#include
#ifdef __linux__
#include
-#else
-#include
#endif
+#include
#include
#include
#include
@@ -238,6 +237,35 @@ enum {
};
static char* igt_log_domain_filter;
+static struct {
+ char *entries[256];
+ uint8_t start, end;
+} log_buffer;
+static pthread_mutex_t log_buffer_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+static void _igt_log_buffer_append(char *line)
+{
+ pthread_mutex_lock(&log_buffer_mutex);
+
+ free(log_buffer.entries[log_buffer.end]);
+ log_buffer.entries[log_buffer.end] = line;
+ log_buffer.end++;
+ if (log_buffer.end == log_buffer.start)
+ log_buffer.start++;
+
+ pthread_mutex_unlock(&log_buffer_mutex);
+}
+
+static void _igt_log_buffer_reset(void)
+{
+ pthread_mutex_lock(&log_buffer_mutex);
+
+ log_buffer.start = log_buffer.end = 0;
+
+ pthread_mutex_unlock(&log_buffer_mutex);
+}
+
+
__attribute__((format(printf, 1, 2)))
static void kmsg(const char *format, ...)
@@ -718,6 +746,8 @@ bool __igt_run_subtest(const char *subtest_name)
kmsg(KERN_INFO "%s: starting subtest %s\n", command_str, subtest_name);
+ _igt_log_buffer_reset();
+
gettime(&subtest_time);
return (in_subtest = subtest_name);
}
@@ -1490,6 +1520,7 @@ void igt_log(const char *domain, enum igt_log_level
level, const char *format, .
void igt_vlog(const char *domain, enum igt_log_level level, const char
*format, va_list args)
{
FILE *file;
+ char *line, *formatted_line;
const char *program_name;
const char *igt_log_level_str[] = {
"DEBUG",
@@ -1510,18 +1541,33 @@ void igt_vlog(const char *domain, enum igt_log_level
level, const char *format,
if (list_subtests)
return;
- if (igt_log_level > level)
+ if (vasprintf(&line, format, args) == -1)
return;
+ if (asprintf(&formatted_line, "(%s:%d) %s%s%s: %s", program_name,
+getpid(), (domain) ? domain : "", (domain) ? "-" : "",
+igt_log_level_str[level], line) == -1) {
+ goto out;
+ }
+
+ /* append log buffer */
+ _igt_log_buffer_append(formatted_line);
+
+ /* check print log level */
+ if (igt_log_level > level)
+ goto out;
+
+ /* check domain filter */
if (igt_log_domain_filter) {
/* if null domain and filter is not "application", return */
if (!domain && strcmp(igt_log_domain_filter, "application"))
- return;
+ goto out;
/* else if domain and filter do not match, return */
else if (domain && strcmp(igt_log_domain_filter, domain))
- return;
+ goto out;
}
+ /* use stderr for warning messages and above */
if (level > IGT_LOG_WARN) {
file = stderr;
fflush(stdout);
@@ -1529,12 +1575,16 @@ void igt_vlog(const char *domain, enum igt_log_level
level, const char *format,
else
file = stdout;
- if (level != IGT_LOG_INFO) {
- fprintf(file, "(%s:%d) %s%s%s: ", program_name, getpid(),
- (domain) ? domain : "", (domain) ? "-" : "",
- igt_log_level_str[level]);
- }
- vfprintf(file, format, args);
+ /* prepend all except information messages with process, domain and log
+* level information */
+ if (level != IGT_LOG_INFO)
+ fwrite(formatted_line, sizeof(char), strlen(formatted_line),
+ file);
+ else
+ fwrite(line, sizeof(char), strlen(line), file);
+
+out:
+ free(line);
}
static void igt_alarm_handler(int signal)
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/2] lib: print recent log messages to stderr when a test or subtest fails
Signed-off-by: Thomas Wood
---
lib/igt_core.c | 40 +---
1 file changed, 37 insertions(+), 3 deletions(-)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index b03b7df..596ab77 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -236,6 +236,9 @@ enum {
OPT_HELP = 'h'
};
+static int igt_exitcode = IGT_EXIT_SUCCESS;
+static const char *command_str;
+
static char* igt_log_domain_filter;
static struct {
char *entries[256];
@@ -265,7 +268,39 @@ static void _igt_log_buffer_reset(void)
pthread_mutex_unlock(&log_buffer_mutex);
}
+static void _igt_log_buffer_dump(void)
+{
+ uint8_t i;
+
+ if (in_subtest)
+ fprintf(stderr, "Subtest %s failed.\n", in_subtest);
+ else
+ fprintf(stderr, "Test %s failed.\n", command_str);
+
+ if (log_buffer.start == log_buffer.end) {
+ fprintf(stderr, "No log.\n");
+ return;
+ }
+
+ pthread_mutex_lock(&log_buffer_mutex);
+ fprintf(stderr, "Log Start\n");
+
+ i = log_buffer.start;
+ do {
+ char *last_line = log_buffer.entries[i];
+ fprintf(stderr, "%s%s", last_line,
+ (last_line[strlen(last_line)-1] != '\n') ? "\n" : "");
+ i++;
+ } while (i != log_buffer.start && i != log_buffer.end);
+
+ /* reset the buffer */
+ log_buffer.start = log_buffer.end = 0;
+
+ pthread_mutex_unlock(&log_buffer_mutex);
+
+ fprintf(stderr, "Log End\n");
+}
__attribute__((format(printf, 1, 2)))
static void kmsg(const char *format, ...)
@@ -422,8 +457,6 @@ static void print_version(void)
uts.sysname, uts.release, uts.machine);
}
-static const char *command_str;
-
static void print_usage(const char *help_str, bool output_on_stderr)
{
FILE *f = output_on_stderr ? stderr : stdout;
@@ -777,7 +810,6 @@ bool igt_only_list_subtests(void)
static bool skipped_one = false;
static bool succeeded_one = false;
static bool failed_one = false;
-static int igt_exitcode = IGT_EXIT_SUCCESS;
static void exit_subtest(const char *) __attribute__((noreturn));
static void exit_subtest(const char *result)
@@ -910,6 +942,8 @@ void igt_fail(int exitcode)
if (test_child)
exit(exitcode);
+ _igt_log_buffer_dump();
+
if (in_subtest) {
if (exitcode == IGT_EXIT_TIMEOUT)
exit_subtest("TIMEOUT");
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915: Split shared dpll setup out of __intel_set_mode()
This simplifies __intel_set_mode() a little.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_display.c | 50
1 file changed, 33 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 423ef95..3d220a6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11021,6 +11021,36 @@ out:
return pipe_config;
}
+static int __intel_set_mode_setup_plls(struct drm_device *dev,
+ unsigned modeset_pipes,
+ unsigned disable_pipes)
+{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ unsigned clear_pipes = modeset_pipes | disable_pipes;
+ struct intel_crtc *intel_crtc;
+ int ret = 0;
+
+ if (!dev_priv->display.crtc_compute_clock)
+ return 0;
+
+ ret = intel_shared_dpll_start_config(dev_priv, clear_pipes);
+ if (ret)
+ goto done;
+
+ for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
+ struct intel_crtc_state *state = intel_crtc->new_config;
+ ret = dev_priv->display.crtc_compute_clock(intel_crtc,
+ state);
+ if (ret) {
+ intel_shared_dpll_abort_config(dev_priv);
+ goto done;
+ }
+ }
+
+done:
+ return ret;
+}
+
static int __intel_set_mode(struct drm_crtc *crtc,
struct drm_display_mode *mode,
int x, int y, struct drm_framebuffer *fb,
@@ -11058,23 +11088,9 @@ static int __intel_set_mode(struct drm_crtc *crtc,
prepare_pipes &= ~disable_pipes;
}
- if (dev_priv->display.crtc_compute_clock) {
- unsigned clear_pipes = modeset_pipes | disable_pipes;
-
- ret = intel_shared_dpll_start_config(dev_priv, clear_pipes);
- if (ret)
- goto done;
-
- for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
- struct intel_crtc_state *state = intel_crtc->new_config;
- ret = dev_priv->display.crtc_compute_clock(intel_crtc,
- state);
- if (ret) {
- intel_shared_dpll_abort_config(dev_priv);
- goto done;
- }
- }
- }
+ ret = __intel_set_mode_setup_plls(dev, modeset_pipes, disable_pipes);
+ if (ret)
+ goto done;
for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
intel_crtc_disable(&intel_crtc->base);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: Move pll new_config field into intel_atomic_state
In order to implement atomic mode sets, we'll need to hold state shared
by multiple crtcs in the drm_atomic_state struct. This patch moves
towards that goal by introducing struct intel_atomic_state for that
purpose and moving the staged pll configuration into it. Current state
will be moved in a follow up patch.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/i915_drv.h | 3 +-
drivers/gpu/drm/i915/intel_ddi.c | 11 ++--
drivers/gpu/drm/i915/intel_display.c | 104 +++
drivers/gpu/drm/i915/intel_drv.h | 8 ++-
4 files changed, 59 insertions(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b09173f..862edc4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -297,7 +297,6 @@ struct intel_shared_dpll_config {
struct intel_shared_dpll {
struct intel_shared_dpll_config config;
- struct intel_shared_dpll_config *new_config;
int active; /* count of number of active CRTCs (i.e. DPMS on) */
bool on; /* is the PLL actually active? Disabled during modeset */
@@ -504,6 +503,7 @@ struct drm_i915_error_state {
struct intel_connector;
struct intel_encoder;
struct intel_crtc_state;
+struct intel_atomic_state;
struct intel_initial_plane_config;
struct intel_crtc;
struct intel_limit;
@@ -546,6 +546,7 @@ struct drm_i915_display_funcs {
void (*get_initial_plane_config)(struct intel_crtc *,
struct intel_initial_plane_config *);
int (*crtc_compute_clock)(struct intel_crtc *crtc,
+ struct intel_atomic_state *state,
struct intel_crtc_state *crtc_state);
void (*crtc_enable)(struct drm_crtc *crtc);
void (*crtc_disable)(struct drm_crtc *crtc);
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index ad8b73d..1cd541f 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -909,6 +909,7 @@ hsw_ddi_calculate_wrpll(int clock /* in Hz */,
static bool
hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
+ struct intel_atomic_state *state,
struct intel_crtc_state *crtc_state,
struct intel_encoder *intel_encoder,
int clock)
@@ -926,7 +927,7 @@ hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
crtc_state->dpll_hw_state.wrpll = val;
- pll = intel_get_shared_dpll(intel_crtc, crtc_state);
+ pll = intel_get_shared_dpll(intel_crtc, state, crtc_state);
if (pll == NULL) {
DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
pipe_name(intel_crtc->pipe));
@@ -1096,6 +1097,7 @@ found:
static bool
skl_ddi_pll_select(struct intel_crtc *intel_crtc,
+ struct intel_atomic_state *state,
struct intel_crtc_state *crtc_state,
struct intel_encoder *intel_encoder,
int clock)
@@ -1150,7 +1152,7 @@ skl_ddi_pll_select(struct intel_crtc *intel_crtc,
crtc_state->dpll_hw_state.cfgcr1 = cfgcr1;
crtc_state->dpll_hw_state.cfgcr2 = cfgcr2;
- pll = intel_get_shared_dpll(intel_crtc, crtc_state);
+ pll = intel_get_shared_dpll(intel_crtc, state, crtc_state);
if (pll == NULL) {
DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
pipe_name(intel_crtc->pipe));
@@ -1171,6 +1173,7 @@ skl_ddi_pll_select(struct intel_crtc *intel_crtc,
* function should be folded into compute_config() eventually.
*/
bool intel_ddi_pll_select(struct intel_crtc *intel_crtc,
+ struct intel_atomic_state *state,
struct intel_crtc_state *crtc_state)
{
struct drm_device *dev = intel_crtc->base.dev;
@@ -1179,10 +1182,10 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc,
int clock = crtc_state->port_clock;
if (IS_SKYLAKE(dev))
- return skl_ddi_pll_select(intel_crtc, crtc_state,
+ return skl_ddi_pll_select(intel_crtc, state, crtc_state,
intel_encoder, clock);
else
- return hsw_ddi_pll_select(intel_crtc, crtc_state,
+ return hsw_ddi_pll_select(intel_crtc, state, crtc_state,
intel_encoder, clock);
}
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 3d220a6..159e6c8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3891,6 +3891,7 @@ void intel_put_shared_dpll(struct intel_crtc *crtc)
}
struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
+ struct intel_at
[Intel-gfx] [PATCH 3/4] drm/i915: Move current pll config to shared global state
This patch adds a display_state pointer to drm_i915_private and moves
the current pll config into it.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/i915_debugfs.c | 15 ---
drivers/gpu/drm/i915/i915_drv.h | 4 +-
drivers/gpu/drm/i915/intel_ddi.c | 13 --
drivers/gpu/drm/i915/intel_display.c | 86 +---
4 files changed, 79 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 3b332a4..eb18a99 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2765,17 +2765,20 @@ static int i915_shared_dplls_info(struct seq_file *m,
void *unused)
drm_modeset_lock_all(dev);
for (i = 0; i < dev_priv->num_shared_dpll; i++) {
struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
+ struct intel_shared_dpll_config *pll_config;
+
+ pll_config = &dev_priv->display_state->shared_dpll[i];
seq_printf(m, "DPLL%i: %s, id: %i\n", i, pll->name, pll->id);
seq_printf(m, " crtc_mask: 0x%08x, active: %d, on: %s\n",
- pll->config.crtc_mask, pll->active, yesno(pll->on));
+ pll_config->crtc_mask, pll->active, yesno(pll->on));
seq_printf(m, " tracked hardware state:\n");
- seq_printf(m, " dpll:0x%08x\n", pll->config.hw_state.dpll);
+ seq_printf(m, " dpll:0x%08x\n", pll_config->hw_state.dpll);
seq_printf(m, " dpll_md: 0x%08x\n",
- pll->config.hw_state.dpll_md);
- seq_printf(m, " fp0: 0x%08x\n", pll->config.hw_state.fp0);
- seq_printf(m, " fp1: 0x%08x\n", pll->config.hw_state.fp1);
- seq_printf(m, " wrpll: 0x%08x\n", pll->config.hw_state.wrpll);
+ pll_config->hw_state.dpll_md);
+ seq_printf(m, " fp0: 0x%08x\n", pll_config->hw_state.fp0);
+ seq_printf(m, " fp1: 0x%08x\n", pll_config->hw_state.fp1);
+ seq_printf(m, " wrpll: 0x%08x\n", pll_config->hw_state.wrpll);
}
drm_modeset_unlock_all(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 862edc4..132eb7b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -296,8 +296,6 @@ struct intel_shared_dpll_config {
};
struct intel_shared_dpll {
- struct intel_shared_dpll_config config;
-
int active; /* count of number of active CRTCs (i.e. DPMS on) */
bool on; /* is the PLL actually active? Disabled during modeset */
const char *name;
@@ -1784,6 +1782,8 @@ struct drm_i915_private {
struct intel_pipe_crc pipe_crc[I915_MAX_PIPES];
#endif
+ struct intel_atomic_state *display_state;
+
int num_shared_dpll;
struct intel_shared_dpll shared_dplls[I915_NUM_PLLS];
int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 1cd541f..ff2197c 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1747,7 +1747,10 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private
*dev_priv)
static void hsw_ddi_pll_enable(struct drm_i915_private *dev_priv,
struct intel_shared_dpll *pll)
{
- I915_WRITE(WRPLL_CTL(pll->id), pll->config.hw_state.wrpll);
+ struct intel_shared_dpll_config *pll_config =
+ &dev_priv->display_state->shared_dpll[pll->id];
+
+ I915_WRITE(WRPLL_CTL(pll->id), pll_config->hw_state.wrpll);
POSTING_READ(WRPLL_CTL(pll->id));
udelay(20);
}
@@ -1836,6 +1839,8 @@ static void skl_ddi_pll_enable(struct drm_i915_private
*dev_priv,
uint32_t val;
unsigned int dpll;
const struct skl_dpll_regs *regs = skl_dpll_regs;
+ struct intel_shared_dpll_config *pll_config =
+ &dev_priv->display_state->shared_dpll[pll->id];
/* DPLL0 is not part of the shared DPLLs, so pll->id is 0 for DPLL1 */
dpll = pll->id + 1;
@@ -1844,13 +1849,13 @@ static void skl_ddi_pll_enable(struct drm_i915_private
*dev_priv,
val &= ~(DPLL_CTRL1_HDMI_MODE(dpll) | DPLL_CTRL1_SSC(dpll) |
DPLL_CRTL1_LINK_RATE_MASK(dpll));
- val |= pll->config.hw_state.ctrl1 << (dpll * 6);
+ val |= pll_config->hw_state.ctrl1 << (dpll * 6);
I915_WRITE(DPLL_CTRL1, val);
POSTING_READ(DPLL_CTRL1);
- I915_WRITE(regs[pll->id].cfgcr1, pll->config.hw_state.cfgcr1);
- I915_WRITE(regs[pll->id].cfgcr2, pll->config.hw_state.cfgcr2);
+ I915_WRITE(regs[pll->id].cfgcr1, pll_config->hw_state.cfgcr1);
+ I915_WRITE(regs[pll->id].cfgcr2, pll_config->hw_state.cfgcr2);
POSTING_READ(regs[pll->id].cfgcr1);
POSTING_READ(regs[pll->id].cfgcr2);
diff
Re: [Intel-gfx] i915: WARN_ON(val > dev_priv->rps.max_freq_softlimit)
On Wed, Jan 28, 2015 at 10:36:02PM -0800, O'Rourke, Tom wrote:
> On Wed, Jan 28, 2015 at 01:28:58PM +0200, Ville Syrjälä wrote:
> > On Wed, Jan 28, 2015 at 09:58:15AM +, Chris Wilson wrote:
> > > On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote:
> > > > Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) resulted in
> > > > this WARN at boot (and pretty frequently afterwards):
> > > >
> > > > WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377
> > > > gen6_set_rps+0x371/0x3c0()
> > > > WARN_ON(val > dev_priv->rps.max_freq_softlimit)
> > >
> > > [snip]
> > >
> > > > I'm not at all familiar with this hardware, but I took a quick look into
> > > > what changed with this commit for my laptop. Before the commit,
> > > > rps.min_freq_softlimit is 4 (from rps.min_freq) and
> > > > rps.max_freq_softlimit is 22.
> > > >
> > > > After the commit, rps.min_freq_softlimit is set to the
> > > > rps.efficient_freq value read from pcode, which is 34 on my laptop.
> > > > So later when gen6_set_rps() is called with rps.min_freq_softlimit that
> > > > warning is hit.
> > > >
> > > > Any thoughts? It certainly seems fishy that this commit causes
> > > > rps.min_freq_softlimit to be greater than rps.max_freq_softlimit.
> > >
> > > Very fishy indeed. Moral of this story, never trust hw.
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > b/drivers/gpu/drm/i915/intel_pm.c
> > > index 3e630feb18e4..bbedd2901c54 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -4007,7 +4007,10 @@ static void gen6_init_rps_frequencies(struct
> > > drm_device *dev)
> > > &ddcc_status);
> > > if (0 == ret)
> > > dev_priv->rps.efficient_freq =
> > > - (ddcc_status >> 8) & 0xff;
> > > + clamp_t(u8,
> > > + (ddcc_status >> 8) & 0xff,
> > > + dev_priv->rps.min_freq,
> > > + dev_priv->rps.max_freq);
> >
> > Maybe better to fall back to rp1_freq if this is bogus?
> >
> [TOR:] Michael, Thank you for bringing this problem to our attention.
>
> Yes, this function needs some range checking to maintain
> RPn <= RPe <= RP0.
>
> A value of 34 seems too high for RPe.
> What values does the Carbon X1 (Haswell) have for RPn and RP0?
RPn is 4, and RP0 is 22.
> I agree with Ville that a bogus value from the pcode read should
> not be used. Simple clamping would set the min_freq to RP0;
> probably incorrect.
>
> Tom O'Rourke
>
> > > }
> > >
> > > /* Preserve min/max settings in case of re-init */
> > >
> > > But really it is probably just best to disable the query for hsw:
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > > b/drivers/gpu/drm/i915/intel_pm.c
> > > index 3e630feb18e4..01bd508e81f6 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -4001,7 +4001,7 @@ static void gen6_init_rps_frequencies(struct
> > > drm_device *dev)
> > > dev_priv->rps.max_freq = dev_priv->rps.rp0_freq;
> > >
> > > dev_priv->rps.efficient_freq = dev_priv->rps.rp1_freq;
> > > - if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
> > > + if (IS_BROADWELL(dev)) {
> > > ret = sandybridge_pcode_read(dev_priv,
> > >
> > > HSW_PCODE_DYNAMIC_DUTY_CYCLE_CONTROL,
> > > &ddcc_status);
> > >
> > > Paranoia says we do both.
> > > -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
> >
> > --
> > 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 4/4] drm/i915: Simplify pll state commit by swapping new and old state
This deletes some code and is closer to what the logic will look like
with atomic mode setting.
Signed-off-by: Ander Conselvan de Oliveira
---
drivers/gpu/drm/i915/intel_display.c | 58
1 file changed, 12 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index fecffbb..96176c1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3963,46 +3963,11 @@ found:
return pll;
}
-/**
- * intel_shared_dpll_start_config - start a new PLL staged config
- * @dev_priv: DRM device
- * @clear_pipes: mask of pipes that will have their PLLs freed
- *
- * Starts a new PLL staged config, copying the current config but
- * releasing the references of pipes specified in clear_pipes.
- */
-static int intel_shared_dpll_start_config(struct drm_i915_private *dev_priv,
- struct intel_atomic_state *state,
- unsigned clear_pipes)
-{
- struct intel_shared_dpll_config *pll_config;
- enum intel_dpll_id i;
-
- /* FIXME: convert this to a simple memdup */
-
- for (i = 0; i < dev_priv->num_shared_dpll; i++) {
- pll_config = &dev_priv->display_state->shared_dpll[i];
-
- memcpy(&state->shared_dpll[i], pll_config,
- sizeof *pll_config);
- state->shared_dpll[i].crtc_mask &= ~clear_pipes;
- }
-
- return 0;
-}
-
-static void intel_shared_dpll_commit(struct drm_i915_private *dev_priv,
-struct intel_atomic_state *state)
+static struct intel_atomic_state *
+intel_atomic_state_duplicate(struct drm_i915_private *dev_priv)
{
- struct intel_shared_dpll_config *pll_config;
- enum intel_dpll_id i;
-
- /* FIXME: convert this to a poiner swap */
-
- for (i = 0; i < dev_priv->num_shared_dpll; i++) {
- pll_config = &dev_priv->display_state->shared_dpll[i];
- *pll_config = state->shared_dpll[i];
- }
+ return kmemdup(dev_priv->display_state,
+ sizeof *dev_priv->display_state, GFP_KERNEL);
}
static void cpt_verify_modeset(struct drm_device *dev, int pipe)
@@ -10405,14 +10370,15 @@ static bool intel_crtc_in_use(struct drm_crtc *crtc)
static void
intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes,
- struct intel_atomic_state *state)
+ struct intel_atomic_state **state)
{
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_encoder *intel_encoder;
struct intel_crtc *intel_crtc;
struct drm_connector *connector;
- intel_shared_dpll_commit(dev_priv, state);
+ /* Commit PLL and other global state */
+ swap(dev_priv->display_state, *state);
for_each_intel_encoder(dev, intel_encoder) {
if (!intel_encoder->base.crtc)
@@ -11020,13 +10986,13 @@ static int __intel_set_mode_setup_plls(struct
drm_device *dev,
unsigned clear_pipes = modeset_pipes | disable_pipes;
struct intel_crtc *intel_crtc;
int ret = 0;
+ int i;
if (!dev_priv->display.crtc_compute_clock)
return 0;
- ret = intel_shared_dpll_start_config(dev_priv, state, clear_pipes);
- if (ret)
- return ret;
+ for (i = 0; i < dev_priv->num_shared_dpll; i++)
+ state->shared_dpll[i].crtc_mask &= ~clear_pipes;
for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
struct intel_crtc_state *crtc_state = intel_crtc->new_config;
@@ -11077,7 +11043,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
prepare_pipes &= ~disable_pipes;
}
- state = kzalloc(sizeof *state, GFP_KERNEL);
+ state = intel_atomic_state_duplicate(dev_priv);
if (!state) {
ret = -ENOMEM;
goto done;
@@ -11120,7 +11086,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
/* Only after disabling all output pipelines that will be changed can we
* update the the output configuration. */
- intel_modeset_update_state(dev, prepare_pipes, state);
+ intel_modeset_update_state(dev, prepare_pipes, &state);
modeset_update_crtc_power_domains(dev);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2 (v1.5)
On Thu, Jan 29, 2015 at 7:55 AM, Tvrtko Ursulin wrote: > > On 01/29/2015 11:57 AM, Daniel Vetter wrote: >> >> On Thu, Jan 29, 2015 at 11:43:07AM +, Tvrtko Ursulin wrote: >>> >>> >>> On 01/29/2015 11:30 AM, Daniel Vetter wrote: On Wed, Jan 28, 2015 at 05:57:56PM +, Tvrtko Ursulin wrote: > > > On 01/28/2015 05:37 PM, Daniel Vetter wrote: >> >> From: Rob Clark >> >> In DRM/KMS we are lacking a good way to deal with tiled/compressed >> formats. Especially in the case of dmabuf/prime buffer sharing, where >> we cannot always rely on under-the-hood flags passed to driver >> specific >> gem-create ioctl to pass around these extra flags. >> >> The proposal is to add a per-plane format modifier. This allows to, >> if >> necessary, use different tiling patters for sub-sampled planes, etc. >> The format modifiers are added at the end of the ioctl struct, so for >> legacy userspace it will be zero padded. >> >> TODO how best to deal with assignment of modifier token values? The >> rough idea was to namespace things with an 8bit vendor-id, and then >> beyond that it is treated as an opaque value. But that was a >> relatively >> arbitrary choice. There are cases where same tiling pattern and/or >> compression is supported by various different vendors. So we should >> standardize to use the vendor-id and value of the first one who >> documents the format? > > > Maybe: > __u64 modifier[4]; > __u64 vendor_modifier[4]; Seems rendundant since the modifier added in this patch is already vendor specific. Or what exactly are you trying to accomplish here? >>> >>> >>> I am trying to avoid packet-in-a-packet (bitmasks) mumbo-jumbo and vendor >>> id >>> on the head followed by maybe standardized or maybe vendor specific tag. >>> Feels funny. Would it not be simpler to put a struct in there? >> >> >> The u64 modifier is just an opaque thing, with 8 bit to identifier the >> vendor (for easier number management really) and the low 56 bits can be >> whatever we want them. On i915 I think we should just enumerate all the >> various tiling modes we have. And if the modifiers aren't set we use the >> tiling mode of the underlying gem bo. We already have code in place to >> guarantee that the underlying bo's tiling can't change as long as there's >> a kms fb around, which means all code which checks for tiling can switch >> over to these flags. >> >> struct won't work since by definition this is vendor-specific, and every >> vendor is slightly insane in a different way. > > > Well 8 + 56 bits is a "struct" already and not fully opaque. Are the bits > expensive? :) That was first part of my point. tbh, we could decide to do something different from 8+56b later if needed.. nothing should really *depend* on the 8+56, since it is intended to be an opaque token. The 8+56 was just intended to make it easier to merge values coming from different driver trees with less conflicts. > Secondly, "vendor who registers first" part of discussion is what came to my > attention as well. > > With this, a hypothetical standard format would be under a vendor id which > looks funny to me. While you could split standard formats/modifiers and > vendor specific modifiers. maybe we should s/vendor/driver/ > I don't care really, it was just an idea, but I don't get this arguments why > it is, not only not better, but worse than a bitfield. :) I guess it gets into bikeshed territory a bit, but I've tried to avoid giving userspace the temptation to assume it is much more than an opaque value. The 8+56 thing was mainly just intended for logistical convenience ;-) BR, -R > Regards, > > Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Subsume intel_ctx_submit_request in to drm_i915_gem_request
On 01/15/2015 01:10 PM, Nick Hoath wrote:
void intel_execlists_retire_requests(struct intel_engine_cs *ring)
{
- struct intel_ctx_submit_request *req, *tmp;
+ struct drm_i915_gem_request *req, *tmp;
struct drm_i915_private *dev_priv = ring->dev->dev_private;
unsigned long flags;
struct list_head retired_list;
@@ -776,7 +771,7 @@ void intel_execlists_retire_requests(struct intel_engine_cs
*ring)
spin_unlock_irqrestore(&ring->execlist_lock, flags);
list_for_each_entry_safe(req, tmp, &retired_list, execlist_link) {
- struct intel_context *ctx = req->request->ctx;
+ struct intel_context *ctx = req->ctx;
struct drm_i915_gem_object *ctx_obj =
ctx->engine[ring->id].state;
@@ -784,9 +779,8 @@ void intel_execlists_retire_requests(struct intel_engine_cs
*ring)
intel_lr_context_unpin(ring, ctx);
intel_runtime_pm_put(dev_priv);
i915_gem_context_unreference(ctx);
- i915_gem_request_unreference(req->request);
+ i915_gem_request_unreference(req);
list_del(&req->execlist_link);
It looks like the this req unreference can be the last one in which case
list_del explodes. I don't know if it was intended that it cannot be the
last unreference, but I have a log which proves it can be. :)
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 12/12] drm/i915/dsi: remove intel_dsi_cmd.c and the unused functions therein
On Fri, Jan 23, 2015 at 05:58:43PM +0530, Shobhit Kumar wrote:
> On 01/16/2015 05:57 PM, Jani Nikula wrote:
> >The removed functions can be resurrected in intel_dsi.c as need arises.
> >
> >Signed-off-by: Jani Nikula
>
> Reviewed-By: Shobhit Kumar
Ok, merged all the remaining patches.
Thanks, Daniel
>
> >---
> > drivers/gpu/drm/i915/Makefile | 1 -
> > drivers/gpu/drm/i915/intel_dsi.c | 1 -
> > drivers/gpu/drm/i915/intel_dsi_cmd.c | 117
> > -
> > drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 1 -
> > 4 files changed, 120 deletions(-)
> > delete mode 100644 drivers/gpu/drm/i915/intel_dsi_cmd.c
> >
> >diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> >index 16e3dc350274..63afe63bf0e4 100644
> >--- a/drivers/gpu/drm/i915/Makefile
> >+++ b/drivers/gpu/drm/i915/Makefile
> >@@ -71,7 +71,6 @@ i915-y += dvo_ch7017.o \
> > intel_ddi.o \
> > intel_dp.o \
> > intel_dp_mst.o \
> >- intel_dsi_cmd.o \
> > intel_dsi.o \
> > intel_dsi_pll.o \
> > intel_dsi_panel_vbt.o \
> >diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> >b/drivers/gpu/drm/i915/intel_dsi.c
> >index 791d90b4c047..02ae5e583b27 100644
> >--- a/drivers/gpu/drm/i915/intel_dsi.c
> >+++ b/drivers/gpu/drm/i915/intel_dsi.c
> >@@ -33,7 +33,6 @@
> > #include "i915_drv.h"
> > #include "intel_drv.h"
> > #include "intel_dsi.h"
> >-#include "intel_dsi_cmd.h"
> >
> > static const struct {
> > u16 panel_id;
> >diff --git a/drivers/gpu/drm/i915/intel_dsi_cmd.c
> >b/drivers/gpu/drm/i915/intel_dsi_cmd.c
> >deleted file mode 100644
> >index acdc5da7b46f..
> >--- a/drivers/gpu/drm/i915/intel_dsi_cmd.c
> >+++ /dev/null
> >@@ -1,117 +0,0 @@
> >-/*
> >- * 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.
> >- *
> >- * Author: Jani Nikula
> >- */
> >-
> >-#include
> >-#include
> >-#include
> >-#include
> >-#include "i915_drv.h"
> >-#include "intel_drv.h"
> >-#include "intel_dsi.h"
> >-#include "intel_dsi_cmd.h"
> >-
> >-/*
> >- * XXX: MIPI_DATA_ADDRESS, MIPI_DATA_LENGTH, MIPI_COMMAND_LENGTH, and
> >- * MIPI_COMMAND_ADDRESS registers.
> >- *
> >- * Apparently these registers provide a MIPI adapter level way to send
> >(lots of)
> >- * commands and data to the receiver, without having to write the commands
> >and
> >- * data to MIPI_{HS,LP}_GEN_{CTRL,DATA} registers word by word.
> >- *
> >- * Presumably for anything other than MIPI_DCS_WRITE_MEMORY_START and
> >- * MIPI_DCS_WRITE_MEMORY_CONTINUE (which are used to update the external
> >- * framebuffer in command mode displays) these are just an optimization
> >that can
> >- * come later.
> >- *
> >- * For memory writes, these should probably be used for performance.
> >- */
> >-
> >-static void print_stat(struct intel_dsi *intel_dsi, enum port port)
> >-{
> >-struct drm_encoder *encoder = &intel_dsi->base.base;
> >-struct drm_device *dev = encoder->dev;
> >-struct drm_i915_private *dev_priv = dev->dev_private;
> >-u32 val;
> >-
> >-val = I915_READ(MIPI_INTR_STAT(port));
> >-
> >-#define STAT_BIT(val, bit) (val) & (bit) ? " " #bit : ""
> >-DRM_DEBUG_KMS("MIPI_INTR_STAT(%c) = %08x"
> >-
> >"%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s"
> >- "\n", port_name(port), val,
> >- STAT_BIT(val, TEARING_EFFECT),
> >- STAT_BIT(val, SPL_PKT_SENT_INTERRUPT),
> >- STAT_BIT(val, GEN_READ_DATA_AVAIL),
> >- STAT_BIT(val, LP_GENERIC_WR_FIFO_FULL),
> >- STAT_BIT(val, HS_GENERIC_WR_FIFO_FULL),
> >- STAT_BIT(val, RX_PROT_VIOLATION),
> >- STAT_BIT(val, RX_INVALID_TX_LENGTH),
> >- STAT_BIT(val, ACK_WITH_NO_ERROR),
>
Re: [Intel-gfx] [PATCH] drm/i915/documentation: Add intel_uncore.c to drm.tmpl
On Wed, Jan 28, 2015 at 05:47:58PM +0200, Mika Kuoppala wrote: > Include intel_uncore.c in template for it to include d > documentation for intel_uncore_forcewake_get and *_put. > > Cc: Daniel Vetter > Signed-off-by: Mika Kuoppala Queued for -next, thanks for the patch. -Daniel > --- > Documentation/DocBook/drm.tmpl | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl > index 77d0455..03f1985 100644 > --- a/Documentation/DocBook/drm.tmpl > +++ b/Documentation/DocBook/drm.tmpl > @@ -3969,6 +3969,7 @@ int num_ioctls; > Runtime Power Management > !Pdrivers/gpu/drm/i915/intel_runtime_pm.c runtime pm > !Idrivers/gpu/drm/i915/intel_runtime_pm.c > +!Idrivers/gpu/drm/i915/intel_uncore.c > > > Interrupt Handling > -- > 1.9.1 > -- 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/bdw: Implement WaForceContextSaveRestoreNonCoherent
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5657 -Summary- Platform Delta drm-intel-nightly Series Applied PNV 353/353 353/353 ILK 353/353 353/353 SNB 400/422 400/422 IVB +2 485/487 487/487 BYT 296/296 296/296 HSW +1 507/508 508/508 BDW 401/402 401/402 -Detailed- Platform Testdrm-intel-nightly Series Applied IVB igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance DMESG_WARN(4, M34M21)PASS(5, M4) PASS(1, M4) IVB igt_gem_storedw_batches_loop_normal DMESG_WARN(4, M34M4)PASS(8, M34M4M21) PASS(1, M4) HSW igt_gem_pwrite_pread_snooped-pwrite-blt-cpu_mmap-performance DMESG_WARN(1, M40)PASS(13, M40M20) PASS(1, M40) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: Enabling PSR on Skylake
On Thu, Jan 29, 2015 at 09:27:14AM +0530, sonika wrote:
>
> On Wednesday 28 January 2015 09:32 PM, Daniel Vetter wrote:
> >On Thu, Jan 22, 2015 at 02:30:54PM +0530, Sonika Jindal wrote:
> >>Mainly taking care of some register offsets, otherwise things are similar to
> >>hsw. Also, programming ddi aux to use hardcoded values for psr data select.
> >>
> >>v2: introduce EDP_PSR_AUX_BASE macro (Chris)
> >>v3: Moving to HW tracking for SKL+ platforms, so activating source psr
> >>during
> >>psr_enabling and then avoiding psr entries and exits for each frontbuffer
> >>updates.
> >>v4: Using SKL DDI AUX regs instead of changing PSR_AUX regs definition
> >>(Rodrigo)
> >>
> >>Signed-off-by: Sonika Jindal
> >>Reviewed-by: Rodrigo Vivi
> >>---
> >> drivers/gpu/drm/i915/i915_drv.h |3 ++-
> >> drivers/gpu/drm/i915/i915_reg.h |5 +
> >> drivers/gpu/drm/i915/intel_frontbuffer.c |7 +--
> >> drivers/gpu/drm/i915/intel_psr.c | 26 --
> >> 4 files changed, 36 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >>b/drivers/gpu/drm/i915/i915_drv.h
> >>index 66f0c60..3d24872 100644
> >>--- a/drivers/gpu/drm/i915/i915_drv.h
> >>+++ b/drivers/gpu/drm/i915/i915_drv.h
> >>@@ -2371,7 +2371,8 @@ struct drm_i915_cmd_table {
> >> #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi)
> >> #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
> >> #define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev)
> >> || \
> >>-IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev))
> >>+IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
> >>+IS_SKYLAKE(dev))
> >> #define HAS_RUNTIME_PM(dev) (IS_GEN6(dev) || IS_HASWELL(dev) || \
> >> IS_BROADWELL(dev) || IS_VALLEYVIEW(dev))
> >> #define HAS_RC6(dev) (INTEL_INFO(dev)->gen >= 6)
> >>diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >>b/drivers/gpu/drm/i915/i915_reg.h
> >>index a39bb03..a6f06fa 100644
> >>--- a/drivers/gpu/drm/i915/i915_reg.h
> >>+++ b/drivers/gpu/drm/i915/i915_reg.h
> >>@@ -3748,6 +3748,11 @@ enum punit_power_well {
> >> #define DP_AUX_CH_CTL_PRECHARGE_TEST(1 << 11)
> >> #define DP_AUX_CH_CTL_BIT_CLOCK_2X_MASK(0x7ff)
> >> #define DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT 0
> >>+#define DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL (1 << 14)
> >>+#define DP_AUX_CH_CTL_FS_DATA_AUX_REG_SKL(1 << 13)
> >>+#define DP_AUX_CH_CTL_GTC_DATA_AUX_REG_SKL (1 << 12)
> >>+#define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL_MASK (1f << 5)
> >>+#define DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
> >> #define DP_AUX_CH_CTL_SYNC_PULSE_SKL(c) ((c) - 1)
> >> /*
> >>diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c
> >>b/drivers/gpu/drm/i915/intel_frontbuffer.c
> >>index 79f6d72..010d550 100644
> >>--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> >>+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> >>@@ -156,7 +156,9 @@ void intel_fb_obj_invalidate(struct drm_i915_gem_object
> >>*obj,
> >>intel_mark_fb_busy(dev, obj->frontbuffer_bits, ring);
> >>- intel_psr_invalidate(dev, obj->frontbuffer_bits);
> >>+
> >>+ if (INTEL_INFO(dev)->gen < 9)
> >>+ intel_psr_invalidate(dev, obj->frontbuffer_bits);
> >> }
> >> /**
> >>@@ -182,7 +184,8 @@ void intel_frontbuffer_flush(struct drm_device *dev,
> >>intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
> >>- intel_psr_flush(dev, frontbuffer_bits);
> >>+ if (INTEL_INFO(dev)->gen < 9)
> >>+ intel_psr_flush(dev, frontbuffer_bits);
> >Again no, not going to take wholesale filtering of the sw invalidate
> >paths. This needs to be properly tested and pushed down into the psr
> >specific invalidate/flush functions on a per-function basis.
> >
> >I've dropped these two hunks and merged the patch.
> >-Daniel
> Hi Daniel,
> Even SW tracking doesn't work in many cases, like I reported earlier in
> ubuntu login screen where we don't get frontbuffer flushes and we don't
> enter PSR at all with SW tracking.
> I see similar behavior even in fbcon mode. So, I am not sure how you can say
> that SW tracking is the only right way.
In some cases the sw tracking isn't especially accurate (cpu based
frontbuffer rendering to the gtt). Paulo has seen a similar issue with
fbc, and since hw tracking works correctly for that case his patches
filter that source of sw invalidates out. Paulo should be back from his
vacation next week, so please ping him when he's back.
> If there are cases where HW tracking fails (and I know a few), we need to
> fix them. I can move this gen check to the intel_psr_* function if that is
> the major concern.
Yeah, the check should be pushed down imo, in the core sw tracking code
it's a bit in the wrong layer.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Re: [Intel-gfx] [RFC v3] drm/i915: Android native sync support
On Wed, Jan 28, 2015 at 04:52:53PM +, Tvrtko Ursulin wrote: > > On 01/28/2015 09:29 AM, Daniel Vetter wrote: > >On Tue, Jan 27, 2015 at 11:29:36AM +, Tvrtko Ursulin wrote: > >>From: Tvrtko Ursulin > >> > >>Add Android native sync support with fences exported as file descriptors via > >>the execbuf ioctl (rsvd2 field is used). > >> > >>This is a continuation of Jesse Barnes's previous work, squashed to arrive > >>at > >>the final destination, cleaned up, with some fixes and preliminary light > >>testing. > >> > >>GEM requests are extended with fence structures which are associated with > >>Android sync fences exported to user space via file descriptors. Fences > >>which > >>are waited upon, and while exported to userspace, are referenced and added > >>to > >>the irq_queue so they are signalled when requests are completed. There is no > >>overhead apart from the where fences are not requested. > >> > >>Based on patches by Jesse Barnes: > >>drm/i915: Android sync points for i915 v3 > >>drm/i915: add fences to the request struct > >>drm/i915: sync fence fixes/updates > >> > >>To do: > >>* Extend driver data with context id / ring id (TBD). > >> > >>v2: > >>* Code review comments. (Chris Wilson) > >>* ring->add_request() was a wrong thing to call - rebase on top of John > >> Harrison's (drm/i915: Early alloc request) to ensure correct request > >> is > >> present before creating a fence. > >>* Take a request reference from signalling path as well to ensure > >> request > >> sticks around while fence is on the request completion wait queue. > >> > >>v3: > >>* Use worker to unreference on completion so it can lock the mutex from > >> interrupt context. > >> > >>Signed-off-by: Tvrtko Ursulin > >>Cc: Jesse Barnes > >>Cc: John Harrison > > > >btw for merging we need to split the conversion to fences out from the > >actual userspace interface. And imo we should replace a lot of our > >existing usage of i915_gem_request with fences within the driver, too. Not > >tacked on the side like in your patch here. All the recent refactoring > >have been aiming to match i915 requests to the internal fence interfaces, > >so we should be pretty much there. > > Ok I did not realize this. It did not make sense to me to split it out if > the only access point to create them is via Android native sync, but from > what you are saying fences should be initialized and active for all requests > all the time. With the native sync part established on demand. > > In what respects has the refactoring been aligning requests and fences? requests are also refcounted, like fences. The actual interface for waiting might still have a slight mismatch. > >We also need this to be able to integrate with the scheduler properly - if > >that needs to deal both with fences and i915 requests separately it'll be > >a bit more messy. If it's all just fences the code should be streamlined a > >lot. > > Requests will remain as main data structure representing the unit of GPU > work? Yes. requests will be just a subclass of fences (through structure embedding). > Is so, it sounds logical that fences are associated (or aggregated) with > requests. I don't see how scheduler would work with fences and not requests. The important part is that the scheduler can work with fences which are _not_ i915 requests (e.g. from the camera pipeline or similar things). -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: Fix a use-after-free in intel_execlists_retire_requests
Remove request from list before unreferencing it, in case it's actually the only reference. (Found by Tvrtko Ursulin) Signed-off-by: Nick Hoath --- drivers/gpu/drm/i915/intel_lrc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 70e449b..a94346f 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -732,8 +732,8 @@ void intel_execlists_retire_requests(struct intel_engine_cs *ring) intel_lr_context_unpin(ring, ctx); intel_runtime_pm_put(dev_priv); i915_gem_context_unreference(ctx); - i915_gem_request_unreference(req); list_del(&req->execlist_link); + i915_gem_request_unreference(req); } } -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] RFC: drm: add support for tiled/compressed/etc modifier in addfb2
From: Rob Clark
In DRM/KMS we are lacking a good way to deal with tiled/compressed
formats. Especially in the case of dmabuf/prime buffer sharing, where
we cannot always rely on under-the-hood flags passed to driver specific
gem-create ioctl to pass around these extra flags.
The proposal is to add a per-plane format modifier. This allows to, if
necessary, use different tiling patters for sub-sampled planes, etc.
The format modifiers are added at the end of the ioctl struct, so for
legacy userspace it will be zero padded.
v1: original
v1.5: increase modifier to 64b
v2: Incorporate review comments from the big thread, plus a few more.
- Add a getcap so that userspace doesn't have to jump through hoops.
- Allow modifiers only when a flag is set. That way drivers know when
they're dealing with old userspace and need to fish out e.g. tiling
from other information.
- After rolling out checks for ->modifier to all drivers I've decided
that this is way too fragile and needs an explicit opt-in flag. So
do that instead.
- Add a define (just for documentation really) for the "NONE"
modifier. Imo we don't need to add mask #defines since drivers
really should only do exact matches against values defined with
fourcc_mod_code.
- Drop the Samsung tiling modifier on Rob's request since he's not yet
sure whether that one is accurate.
v3:
- Also add a new ->modifier[] array to struct drm_framebuffer and fill
it in drm_helper_mode_fill_fb_struct. Requested by Tvrkto Uruslin.
- Remove TODO in comment and add code comment that modifiers should be
properly documented, requested by Rob.
Cc: Rob Clark
Cc: Tvrtko Ursulin
Cc: Laurent Pinchart
Cc: Daniel Stone
Cc: Ville Syrjälä
Cc: Michel Dänzer
Signed-off-by: Rob Clark (v1.5)
Reviewed-by: Rob Clark
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_crtc.c| 14 +-
drivers/gpu/drm/drm_crtc_helper.c | 1 +
drivers/gpu/drm/drm_ioctl.c | 3 +++
include/drm/drm_crtc.h| 4
include/uapi/drm/drm.h| 1 +
include/uapi/drm/drm_fourcc.h | 32
include/uapi/drm/drm_mode.h | 9 +
7 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 419f9d915c78..8090e3d64f67 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3324,6 +3324,12 @@ static int framebuffer_check(const struct
drm_mode_fb_cmd2 *r)
DRM_DEBUG_KMS("bad pitch %u for plane %d\n",
r->pitches[i], i);
return -EINVAL;
}
+
+ if (r->modifier[i] && !(r->flags & DRM_MODE_FB_MODIFIERS)) {
+ DRM_DEBUG_KMS("bad fb modifier %llu for plane %d\n",
+ r->modifier[i], i);
+ return -EINVAL;
+ }
}
return 0;
@@ -3337,7 +3343,7 @@ static struct drm_framebuffer
*add_framebuffer_internal(struct drm_device *dev,
struct drm_framebuffer *fb;
int ret;
- if (r->flags & ~DRM_MODE_FB_INTERLACED) {
+ if (r->flags & ~(DRM_MODE_FB_INTERLACED | DRM_MODE_FB_MODIFIERS)) {
DRM_DEBUG_KMS("bad framebuffer flags 0x%08x\n", r->flags);
return ERR_PTR(-EINVAL);
}
@@ -3353,6 +3359,12 @@ static struct drm_framebuffer
*add_framebuffer_internal(struct drm_device *dev,
return ERR_PTR(-EINVAL);
}
+ if (r->flags & DRM_MODE_FB_MODIFIERS &&
+ !dev->mode_config.allow_fb_modifiers) {
+ DRM_DEBUG_KMS("driver does not support fb modifiers\n");
+ return ERR_PTR(-EINVAL);
+ }
+
ret = framebuffer_check(r);
if (ret)
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/drm_crtc_helper.c
b/drivers/gpu/drm/drm_crtc_helper.c
index b1979e7bdc88..3053aab968f9 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -837,6 +837,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_framebuffer
*fb,
for (i = 0; i < 4; i++) {
fb->pitches[i] = mode_cmd->pitches[i];
fb->offsets[i] = mode_cmd->offsets[i];
+ fb->modifier[i] = mode_cmd->modifier[i];
}
drm_fb_get_bpp_depth(mode_cmd->pixel_format, &fb->depth,
&fb->bits_per_pixel);
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 3785d66721f2..a6d773a61c2d 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -321,6 +321,9 @@ static int drm_getcap(struct drm_device *dev, void *data,
struct drm_file *file_
else
req->value = 64;
break;
+ case DRM_CAP_ADDFB2_MODIFIERS:
+ req->value = dev->mode_config.allow_fb_modifiers;
+ break;
default:
return -EINVAL;
}
dif
Re: [Intel-gfx] [PATCH] drm/i915: Remove nested work in gpu error handling
On Wed, Jan 28, 2015 at 03:30:35PM +, Chris Wilson wrote: > On Wed, Jan 28, 2015 at 05:03:14PM +0200, Mika Kuoppala wrote: > > Now when we declare gpu errors only through our own dedicated > > hangcheck workqueue there is no need to have a separate workqueue > > for handling the resetting and waking up the clients as the deadlock > > concerns are no more. > > > > The only exception is i915_debugfs::i915_set_wedged, which triggers > > error handling through process context. However as this is only used through > > test harness it is responsibility for test harness not to introduce hangs > > through both debug interface and through hangcheck mechanism at the same > > time. > > > > Remove gpu_error.work and let the hangcheck work do the tasks it used to. > > > > v2: Add a big warning sign into i915_debugfs::i915_set_wedged (Chris) > > > > Cc: Chris Wilson > > Signed-off-by: Mika Kuoppala > Reviewed-by: Chris Wilson Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] intel_crtc_wait_for_pending_flips
I have a rather infrequent hang of the graphics system but when it happens the only way out is to pull the power. This has happened on serveral kernel versions but it is infrequent enough so that I now only have the most resent still in the logs. what I remember is that I always see intel_crtc_wait_for_pending_flips on the call stack. here is the tracebacks that I have in the log. I had to pull the power since I could not switch to a VT from X and I could not get it to do a reboot. Jan 29 12:43:22 brix kernel: [77018.170497] [ cut here ] Jan 29 12:43:22 brix kernel: [77018.170597] WARNING: CPU: 0 PID: 2067 at /home/apw/COD/linux/drivers/gpu/drm/i915/intel_display.c:3473 intel_crtc_wait_for_pending_flips+0x157/0x1c0 [i915]() Jan 29 12:43:22 brix kernel: [77018.170601] Modules linked in: nfsv3 autofs4 xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc dm_crypt rfcomm bnep nfsd auth_rpcgss nfs_acl nfs lockd grace sunrpc binfmt_misc fscache pl2303 usbserial cdc_ether usbnet x86_pkg_temp_thermal intel_powerclamp kvm_intel kvm crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd btusb joydev bluetooth r8152 mei_me lpc_ich mei snd_soc_rt5640 snd_soc_rl6231 shpchp snd_soc_core snd_compress snd_pcm_dmaengine i2c_hid dw_dmac spi_pxa2xx_platform dw_dmac_core i2c_designware_platform 8250_dw snd_soc_sst_acpi i2c_designware_core mac_hid snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_intel snd_hda_controller snd_hda_codec snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq snd_seq_device snd_timer snd soundcore parport_pc ppdev it87 hwmon_vid coretemp lp parport nls_iso8859_1 btrfs xor hid_logitech_dj raid6_pq hid_generic usbhid hid i915 i2c_algo_bit drm_kms_helper ahci r8169 drm libahci mii sdhci_acpi video sdhci Jan 29 12:43:22 brix kernel: [77018.170737] CPU: 0 PID: 2067 Comm: Xorg Not tainted 3.18.1-031801-generic #201412170637 Jan 29 12:43:22 brix kernel: [77018.170741] Hardware name: GIGABYTE M4HM87P-00/M4HM87P-00, BIOS F2 12/11/2013 Jan 29 12:43:22 brix kernel: [77018.170744] 0d91 8803f6fa7b38 827a5f19 0007 Jan 29 12:43:22 brix kernel: [77018.170750] 8803f6fa7b78 82074b0c 0202 Jan 29 12:43:22 brix kernel: [77018.170756] 88040829b1a8 8804082a5000 880035d78430 88040829b000 Jan 29 12:43:22 brix kernel: [77018.170762] Call Trace: Jan 29 12:43:22 brix kernel: [77018.170779] [] dump_stack+0x46/0x58 Jan 29 12:43:22 brix kernel: [77018.170792] [] warn_slowpath_common+0x8c/0xc0 Jan 29 12:43:22 brix kernel: [77018.170799] [] warn_slowpath_null+0x1a/0x20 Jan 29 12:43:22 brix kernel: [77018.170851] [] intel_crtc_wait_for_pending_flips+0x157/0x1c0 [i915] Jan 29 12:43:22 brix kernel: [77018.170860] [] ? prepare_to_wait_event+0x100/0x100 Jan 29 12:43:22 brix kernel: [77018.170908] [] intel_crtc_disable_planes+0x34/0x150 [i915] Jan 29 12:43:22 brix kernel: [77018.170952] [] haswell_crtc_disable+0x5d/0x210 [i915] Jan 29 12:43:22 brix kernel: [77018.170996] [] intel_crtc_control+0x57/0x130 [i915] Jan 29 12:43:22 brix kernel: [77018.171038] [] intel_crtc_update_dpms+0x67/0x80 [i915] Jan 29 12:43:22 brix kernel: [77018.171081] [] intel_connector_dpms+0x61/0x70 [i915] Jan 29 12:43:22 brix kernel: [77018.171118] [] drm_mode_connector_set_obj_prop+0xa4/0xb0 [drm] Jan 29 12:43:22 brix kernel: [77018.171155] [] drm_mode_obj_set_property_ioctl+0x134/0x1d0 [drm] Jan 29 12:43:22 brix kernel: [77018.171188] [] drm_mode_connector_property_set_ioctl+0x30/0x40 [drm] Jan 29 12:43:22 brix kernel: [77018.171214] [] drm_ioctl+0x2e6/0x590 [drm] Jan 29 12:43:22 brix kernel: [77018.171261] [] ? btrfs_file_write_iter+0x29d/0x360 [btrfs] Jan 29 12:43:22 brix kernel: [77018.171298] [] ? drm_mode_obj_set_property_ioctl+0x1d0/0x1d0 [drm] Jan 29 12:43:22 brix kernel: [77018.171307] [] do_vfs_ioctl+0x75/0x2c0 Jan 29 12:43:22 brix kernel: [77018.171313] [] ? vfs_write+0x196/0x1f0 Jan 29 12:43:22 brix kernel: [77018.171321] [] ? __fget_light+0x25/0x70 Jan 29 12:43:22 brix kernel: [77018.171328] [] SyS_ioctl+0x91/0xb0 Jan 29 12:43:22 brix kernel: [77018.171335] [] system_call_fastpath+0x16/0x1b Jan 29 12:43:22 brix kernel: [77018.171339] ---[ end trace c0eb6b78177b6051 ]--- Jan 29 12:43:22 brix kernel: [77018.171342] [ cut here ] Jan 29 12:43:22 brix kernel: [77018.171389] WARNING: CPU: 0 PID: 2067 at /home/apw/COD/linux/drivers/gpu/drm/i915/intel_display.c:3479 intel_crtc_wait_for_pending_flips+0x1a8/0x1c0 [i915]() Jan 29 12:43:22 brix kernel: [77018.171391] Removing stuck page flip Jan 29 12:43:22 brix kernel: [77018
Re: [Intel-gfx] i915: WARN_ON(val > dev_priv->rps.max_freq_softlimit)
On Wed, Jan 28, 2015 at 10:36:02PM -0800, O'Rourke, Tom wrote: > On Wed, Jan 28, 2015 at 01:28:58PM +0200, Ville Syrjälä wrote: > > On Wed, Jan 28, 2015 at 09:58:15AM +, Chris Wilson wrote: > > > On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote: > > > > Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) resulted in > > > > this WARN at boot (and pretty frequently afterwards): > > > > > > > > WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377 > > > > gen6_set_rps+0x371/0x3c0() > > > > WARN_ON(val > dev_priv->rps.max_freq_softlimit) > > > > > > [snip] > > > > > > > I'm not at all familiar with this hardware, but I took a quick look into > > > > what changed with this commit for my laptop. Before the commit, > > > > rps.min_freq_softlimit is 4 (from rps.min_freq) and > > > > rps.max_freq_softlimit is 22. > > > > > > > > After the commit, rps.min_freq_softlimit is set to the > > > > rps.efficient_freq value read from pcode, which is 34 on my laptop. > > > > So later when gen6_set_rps() is called with rps.min_freq_softlimit that > > > > warning is hit. > > > > > > > > Any thoughts? It certainly seems fishy that this commit causes > > > > rps.min_freq_softlimit to be greater than rps.max_freq_softlimit. > > > > > > Very fishy indeed. Moral of this story, never trust hw. > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > b/drivers/gpu/drm/i915/intel_pm.c > > > index 3e630feb18e4..bbedd2901c54 100644 > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > @@ -4007,7 +4007,10 @@ static void gen6_init_rps_frequencies(struct > > > drm_device *dev) > > > &ddcc_status); > > > if (0 == ret) > > > dev_priv->rps.efficient_freq = > > > - (ddcc_status >> 8) & 0xff; > > > + clamp_t(u8, > > > + (ddcc_status >> 8) & 0xff, > > > + dev_priv->rps.min_freq, > > > + dev_priv->rps.max_freq); > > > > Maybe better to fall back to rp1_freq if this is bogus? > > > [TOR:] Michael, Thank you for bringing this problem to our attention. > > Yes, this function needs some range checking to maintain > RPn <= RPe <= RP0. > > A value of 34 seems too high for RPe. > What values does the Carbon X1 (Haswell) have for RPn and RP0? 4 & 22, already in Micheal's original bug report. Tom, can you pls polish the clamping into a proper patch with m-l references? Micheal, can you please test the first hunk from Chris (the one that adds the clamp) to make sure it does indeed address the WARN_ON you're seeing? 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] intel_crtc_wait_for_pending_flips
On Thu, Jan 29, 2015 at 06:00:26PM +0100, Kenneth Johansson wrote: > I have a rather infrequent hang of the graphics system but when it happens > the only way out is to pull the power. This has happened on serveral kernel > versions but it is infrequent enough so that I now only have the most resent > still in the logs. > > what I remember is that I always see intel_crtc_wait_for_pending_flips on > the call stack. Yeah, stuck pageflip and then the code falls over waiting for it. A few things to test: - Please retest with latest -rc kernel to make sure the problem is still there. - Please add i915.use_mmio_flip=1 to your kernel cmdline and retest. - Please boot with drm.debug=0xe and grab the boot dmesg (just so we know your hw config). To make sure we don't lose track of your report please file a new bug on bugs.freedesktop.org against DRI -> DRM (Intel). Thanks, Daniel > > > here is the tracebacks that I have in the log. I had to pull the power since > I could not switch to a VT from X and I could not get it to do a reboot. > > Jan 29 12:43:22 brix kernel: [77018.170497] [ cut here > ] > > Jan 29 12:43:22 brix kernel: [77018.170597] WARNING: CPU: 0 PID: 2067 at > /home/apw/COD/linux/drivers/gpu/drm/i915/intel_display.c:3473 > intel_crtc_wait_for_pending_flips+0x157/0x1c0 [i915]() > > Jan 29 12:43:22 brix kernel: [77018.170601] Modules linked in: nfsv3 autofs4 > xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat > nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack > ipt_REJECT nf_reject_ipv4 xt_tcpudp iptable_filter ip_tables x_tables bridge > stp llc dm_crypt rfcomm bnep nfsd auth_rpcgss nfs_acl nfs lockd grace sunrpc > binfmt_misc fscache pl2303 usbserial cdc_ether usbnet x86_pkg_temp_thermal > intel_powerclamp kvm_intel kvm crct10dif_pclmul crc32_pclmul > ghash_clmulni_intel aesni_intel aes_x86_64 lrw gf128mul glue_helper > ablk_helper cryptd btusb joydev bluetooth r8152 mei_me lpc_ich mei > snd_soc_rt5640 snd_soc_rl6231 shpchp snd_soc_core snd_compress > snd_pcm_dmaengine i2c_hid dw_dmac spi_pxa2xx_platform dw_dmac_core > i2c_designware_platform 8250_dw snd_soc_sst_acpi i2c_designware_core mac_hid > snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_intel > snd_hda_controller snd_hda_codec snd_hwdep snd_pcm snd_seq_midi > snd_seq_midi_event snd_rawmidi snd_seq snd_seq_device snd_timer snd soundcore > parport_pc ppdev it87 hwmon_vid coretemp lp parport nls_iso8859_1 btrfs xor > hid_logitech_dj raid6_pq hid_generic usbhid hid i915 i2c_algo_bit > drm_kms_helper ahci r8169 drm libahci mii sdhci_acpi video sdhci > > Jan 29 12:43:22 brix kernel: [77018.170737] CPU: 0 PID: 2067 Comm: Xorg Not > tainted 3.18.1-031801-generic #201412170637 > > Jan 29 12:43:22 brix kernel: [77018.170741] Hardware name: GIGABYTE > M4HM87P-00/M4HM87P-00, BIOS F2 12/11/2013 > > Jan 29 12:43:22 brix kernel: [77018.170744] 0d91 > 8803f6fa7b38 827a5f19 0007 > > Jan 29 12:43:22 brix kernel: [77018.170750] > 8803f6fa7b78 82074b0c 0202 > > Jan 29 12:43:22 brix kernel: [77018.170756] 88040829b1a8 > 8804082a5000 880035d78430 88040829b000 > > Jan 29 12:43:22 brix kernel: [77018.170762] Call Trace: > > Jan 29 12:43:22 brix kernel: [77018.170779] [] > dump_stack+0x46/0x58 > > Jan 29 12:43:22 brix kernel: [77018.170792] [] > warn_slowpath_common+0x8c/0xc0 > > Jan 29 12:43:22 brix kernel: [77018.170799] [] > warn_slowpath_null+0x1a/0x20 > > Jan 29 12:43:22 brix kernel: [77018.170851] [] > intel_crtc_wait_for_pending_flips+0x157/0x1c0 [i915] > > Jan 29 12:43:22 brix kernel: [77018.170860] [] ? > prepare_to_wait_event+0x100/0x100 > > Jan 29 12:43:22 brix kernel: [77018.170908] [] > intel_crtc_disable_planes+0x34/0x150 [i915] > > Jan 29 12:43:22 brix kernel: [77018.170952] [] > haswell_crtc_disable+0x5d/0x210 [i915] > > Jan 29 12:43:22 brix kernel: [77018.170996] [] > intel_crtc_control+0x57/0x130 [i915] > > Jan 29 12:43:22 brix kernel: [77018.171038] [] > intel_crtc_update_dpms+0x67/0x80 [i915] > > Jan 29 12:43:22 brix kernel: [77018.171081] [] > intel_connector_dpms+0x61/0x70 [i915] > > Jan 29 12:43:22 brix kernel: [77018.171118] [] > drm_mode_connector_set_obj_prop+0xa4/0xb0 [drm] > > Jan 29 12:43:22 brix kernel: [77018.171155] [] > drm_mode_obj_set_property_ioctl+0x134/0x1d0 [drm] > > Jan 29 12:43:22 brix kernel: [77018.171188] [] > drm_mode_connector_property_set_ioctl+0x30/0x40 [drm] > > Jan 29 12:43:22 brix kernel: [77018.171214] [] > drm_ioctl+0x2e6/0x590 [drm] > > Jan 29 12:43:22 brix kernel: [77018.171261] [] ? > btrfs_file_write_iter+0x29d/0x360 [btrfs] > > Jan 29 12:43:22 brix kernel: [77018.171298] [] ? > drm_mode_obj_set_property_ioctl+0x1d0/0x1d0 [drm] > > Jan 29 12:43:22 brix kernel: [77018.171307] [] > do_vfs_ioctl+0x75/0x2c0 > > Jan 29 12:43:22
[Intel-gfx] [PATCH] drm/i915: More DPIO magic for CHV HDMI & DP
This patch implements latest changes in Gain, lock threshold and integer
co-efficient values using sideband r/w. Without these changes there will
be signal integrity issues for both HDMI and DP.
Change-Id: I7b7151b5ab3a52c4c912cf10602c69a7d1a70222
Signed-off-by: Vijay Purushothaman
Tested-by: Hong Liu
---
drivers/gpu/drm/i915/i915_reg.h | 31
drivers/gpu/drm/i915/intel_display.c | 67 --
2 files changed, 79 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 137c5e0..2b3f065 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1049,6 +1049,37 @@ enum punit_power_well {
#define DPIO_CHV_PROP_COEFF_SHIFT0
#define CHV_PLL_DW6(ch) _PIPE(ch, _CHV_PLL_DW6_CH0, _CHV_PLL_DW6_CH1)
+#define _CHV_PLL_DW7_CH0 0x801c
+#define _CHV_PLL_DW7_CH1 0x803c
+#define CHV_PLL_DW7(ch) _PIPE(ch, _CHV_PLL_DW7_CH0, _CHV_PLL_DW7_CH1)
+
+#define _CHV_PLL_DW8_CH0 0x8020
+#define _CHV_PLL_DW8_CH1 0x81A0
+#define CHV_PLL_DW8(ch) _PIPE(ch, _CHV_PLL_DW8_CH0, _CHV_PLL_DW8_CH1)
+
+#define _CHV_PLL_DW9_CH0 0x8024
+#define _CHV_PLL_DW9_CH1 0x81A4
+#define DPIO_CHV_INT_LOCK_THRESHOLD_SHIFT 1 /* 3 bits */
+#define DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE1 /* 1: coarse & 0 :
fine */
+#define CHV_PLL_DW9(ch) _PIPE(ch, _CHV_PLL_DW9_CH0, _CHV_PLL_DW9_CH1)
+
+#define _CHV_PLL_DW10_CH0 0x8040
+#define _CHV_PLL_DW10_CH1 0x8060
+#define CHV_PLL_DW10(ch) _PIPE(ch, _CHV_PLL_DW10_CH0, _CHV_PLL_DW10_CH1)
+
+#define _CHV_PLL_DW11_BCAST0xC044
+#define _CHV_PLL_DW11_CH0 0x8044
+#define _CHV_PLL_DW11_CH1 0x8064
+#define CHV_PLL_DW11(ch) _PIPE(ch, _CHV_PLL_DW11_CH0, _CHV_PLL_DW11_CH1)
+
+#define _CHV_PLL_DW12_CH0 0x8048
+#define _CHV_PLL_DW12_CH1 0x8068
+#define CHV_PLL_DW12(ch) _PIPE(ch, _CHV_PLL_DW12_CH0, _CHV_PLL_DW12_CH1)
+
+#define _CHV_PLL_DW13_CH0 0x804C
+#define _CHV_PLL_DW13_CH1 0x806C
+#define CHV_PLL_DW13(ch) _PIPE(ch, _CHV_PLL_DW13_CH0, _CHV_PLL_DW13_CH1)
+
#define _CHV_CMN_DW5_CH0 0x8114
#define CHV_BUFRIGHTENA1_DISABLE (0 << 20)
#define CHV_BUFRIGHTENA1_NORMAL (1 << 20)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index c362d11e..fb27faf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6576,9 +6576,9 @@ static void chv_update_pll(struct intel_crtc *crtc)
int pipe = crtc->pipe;
int dpll_reg = DPLL(crtc->pipe);
enum dpio_channel port = vlv_pipe_to_channel(pipe);
- u32 loopfilter, intcoeff;
+ u32 loopfilter, tribuf_calcntr;
u32 bestn, bestm1, bestm2, bestp1, bestp2, bestm2_frac;
- int refclk;
+ int vco;
crtc->config.dpll_hw_state.dpll = DPLL_SSC_REF_CLOCK_CHV |
DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
@@ -6595,6 +6595,7 @@ static void chv_update_pll(struct intel_crtc *crtc)
bestm2 = crtc->config.dpll.m2 >> 22;
bestp1 = crtc->config.dpll.p1;
bestp2 = crtc->config.dpll.p2;
+ vco = crtc->config.dpll.vco;
/*
* Enable Refclk and SSC
@@ -6619,31 +6620,59 @@ static void chv_update_pll(struct intel_crtc *crtc)
DPIO_CHV_M1_DIV_BY_2 |
1 << DPIO_CHV_N_DIV_SHIFT);
- /* M2 fraction division */
- vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW2(port), bestm2_frac);
+ if (bestm2_frac) {
+ /* M2 fraction division */
+ vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW2(port), bestm2_frac);
+
+ /* M2 fraction division enable */
+ vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW3(port),
+ vlv_dpio_read(dev_priv, pipe,
CHV_PLL_DW3(port)) &
+ DPIO_CHV_FRAC_DIV_EN);
+
+ /* Program digital lock detect threshold */
+ vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW9(port),
+ vlv_dpio_read(dev_priv, pipe,
CHV_PLL_DW9(port)) |
+ (0x5 << DPIO_CHV_INT_LOCK_THRESHOLD_SHIFT));
+ } else {
+ /* M2 fraction division disable */
+ vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW3(port),
+ vlv_dpio_read(dev_priv, pipe,
CHV_PLL_DW3(port)) &
+ ~(DPIO_CHV_FRAC_DIV_EN));
- /* M2 fraction division enable */
- vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW3(port),
- DPIO_CHV_FRAC_DIV_EN |
- (2 << DPIO_CHV_FEEDFWD_GAIN_SHIFT));
+ /* Program digital lock detect threshold */
+ vlv_dpio_write(dev_priv, pipe, CHV_PLL_DW9(port),
+
Re: [Intel-gfx] [PATCH 1/3] drm/i915/skl: Split the SKL PCI ids by GT
On Thu, Jan 29, 2015 at 02:13:38PM +, Damien Lespiau wrote: > We need to have a separate GT3 struct intel_device_info to declare they > have a second VCS. Let's start by splitting the PCI ids per-GT. > Would it be a good idea to do more programmatic population of these fields, rather than creating an entire new instance of the struct just to alter one field? This relates to our other conversation about the memory consumed by the 30+ device infos and the concern when adding new fields. -Jeff ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Keep plane->state updated on pageflip
Until all drivers have transitioned to atomic, the framebuffer associated with a plane is tracked in both plane->fb (for legacy) and plane->state->fb (for all the new atomic codeflow). All of our modeset and plane updates use drm_plane->update_plane(), so in theory plane->fb and plane->state->fb should always stay in sync and point at the same thing for i915. However we forgot about the pageflip ioctl case, which currently only updates plane->fb and leaves plane->state->fb at a stale value. Surprisingly, this doesn't cause any real problems at the moment since internally we use the plane->fb pointer in most of the places that matter, and on the next .update_plane() call, we use plane->fb to figure out which framebuffer to cleanup. However when we switch to the full atomic helpers for update_plane()/disable_plane(), those helpers use plane->state->fb to figure out which framebuffer to cleanup, so not having updated the plane->state->fb pointer causes things to blow up following a pageflip ioctl. The fix here is to just make sure we update plane->state->fb at the same time we update plane->fb in the pageflip ioctl. Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/intel_display.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 423ef95..a7eeed6 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9801,6 +9801,13 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, crtc->primary->fb = fb; + /* Keep state structure in sync */ + if (crtc->primary->state->fb) + drm_framebuffer_unreference(crtc->primary->state->fb); + crtc->primary->state->fb = fb; + if (crtc->primary->state->fb) + drm_framebuffer_reference(crtc->primary->state->fb); + work->pending_flip_obj = obj; atomic_inc(&intel_crtc->unpin_work_count); -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Switch planes from transitional helpers to full atomic helpers
There are two sets of helper functions provided by the DRM core that can
implement the .update_plane() and .disable_plane() hooks in terms of a
driver's atomic entrypoints. The transitional helpers (which we have
been using so far) create a plane state and then use the plane's atomic
entrypoints to perform the atomic begin/check/prepare/commit/finish
sequence on that single plane only. The full atomic helpers create a
top-level atomic state (which is capable of holding multiple object
states for planes, crtc's, and/or connectors) and then passes the
top-level atomic state through the full "atomic modeset" pipeline.
Switching from the transitional to full helpers here shouldn't result in
any functional change, but will enable us to exercise/test more of the
internal atomic pipeline with the legacy API's used by existing
applications.
Signed-off-by: Matt Roper
---
drivers/gpu/drm/i915/intel_display.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index a7eeed6..2e9f321 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12060,8 +12060,8 @@ void intel_plane_destroy(struct drm_plane *plane)
}
const struct drm_plane_funcs intel_plane_funcs = {
- .update_plane = drm_plane_helper_update,
- .disable_plane = drm_plane_helper_disable,
+ .update_plane = drm_atomic_helper_update_plane,
+ .disable_plane = drm_atomic_helper_disable_plane,
.destroy = intel_plane_destroy,
.set_property = drm_atomic_helper_plane_set_property,
.atomic_get_property = intel_plane_atomic_get_property,
--
1.8.5.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Intel HD 4400] strongly irritating artefacts on 2560x1440 laptop display
On 01/28/2015 10:45 AM, Chris Wilson wrote: > From watching the video, and your comments here, it is clear this is not > a driver bug (neither in the ddx nor opengl compositor). That leaves us > with hardware misconfiguration, aka kernel bug. Some of the artefacts > could be a display underrun. It would be good to check a drm.debug=0xe > dmesg for any warnings (and the info itself will be useful). Another > thing worth playing with is i915.enable_fbc=1 (though that is really > just swapping one problem with another). > -Chris Thanks a lot for looking into this. I uploaded the dmesg to the dropbox area on https://www.dropbox.com/sh/9wh0nzvp0w1phxz/AAAsvAqHN8ApXzor58FmGbwLa?dl=0. I will report the results of playing with the various parameters later. The defaults (what I've tested so far) are: enable_cmd_parser: 1 enable_execlists: 0 enable_fbc: -1 enable_hangcheck: Y enable_ips: 1 enable_ppgtt: 1 enable_psr: 0 enable_rc6: 1 2 more questions to the experts: 1. Would it make sense to check out and try the latest drm-intel-fixes or drm-intel-next branch? 2. How likely is it that this phenomenon indicates faulty hardware? Martin ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Intel HD 4400] strongly irritating artefacts on 2560x1440 laptop display
On 01/28/2015 10:45 AM, Jani Nikula wrote:
> Just a quick guess, what does this say:
>
> # cat /sys/module/i915/parameters/enable_{fbc,ips,psr}
fbc: -1, ips: 1, psr: 0
>
> Please try these module parameters for the non-zero ones:
>
> i915.enable_fbc=0
> i915.enable_ips=0
> i915.enable_psr=0
These settings had no effect.
Thanks a lot anyway.
Martin
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Intel HD 4400] strongly irritating artefacts on 2560x1440 laptop display
On 01/28/2015 10:45 AM, Chris Wilson wrote: > From watching the video, and your comments here, it is clear this is not > a driver bug (neither in the ddx nor opengl compositor). That leaves us > with hardware misconfiguration, aka kernel bug. Some of the artefacts > could be a display underrun. It would be good to check a drm.debug=0xe > dmesg for any warnings (and the info itself will be useful). Another > thing worth playing with is i915.enable_fbc=1 (though that is really > just swapping one problem with another). i915.enable_fbc=1 had no effect. I agree with you that this is probably a low-level problem, although the fact that the artefacts strongly depend on Window content doesn't fit that theory. Martin PS: I have just done a thing I thought I'd never do - I am downloading an evaluation copy of Windows in order to check whether my HW is faulty. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] memcontrol.c BUG
On 29 January 2015 at 18:16, Chris Wilson wrote: > On Wed, Jan 28, 2015 at 03:32:43PM +0100, Michal Hocko wrote: >> On Wed 28-01-15 08:48:52, Chris Wilson wrote: >> > On Wed, Jan 28, 2015 at 08:13:06AM +1000, Dave Airlie wrote: >> > > https://bugzilla.redhat.com/show_bug.cgi?id=1165369 >> > > >> > > ov 18 09:23:22 elissa.gathman.org kernel: page:f5e36a40 count:2 >> > > mapcount:0 mapping: (null) index:0x0 >> > > Nov 18 09:23:22 elissa.gathman.org kernel: page flags: >> > > 0x80090029(locked|uptodate|lru|swapcache|swapbacked) >> > > Nov 18 09:23:22 elissa.gathman.org kernel: page dumped because: >> > > VM_BUG_ON_PAGE(!lrucare && PageLRU(oldpage)) >> > > Nov 18 09:23:23 elissa.gathman.org kernel: [ cut here >> > > ] >> > > Nov 18 09:23:23 elissa.gathman.org kernel: kernel BUG at >> > > mm/memcontrol.c:6733! >> >> I guess this matches the following bugon in your kernel: >> VM_BUG_ON_PAGE(!lrucare && PageLRU(oldpage), oldpage); >> >> so the oldpage is on the LRU list already. I am completely unfamiliar >> with 965GM but is the page perhaps shared with somebody with a different >> gfp mask requirement (e.g. userspace accessing the memory via mmap)? So >> the other (racing) caller didn't need to move the page and put it on >> LRU. > > Generally, yes. The shmemfs filp is exported through a vm_mmap() as well > as pinned into the GPU via shmem_read_mapping_page_gfp(). But I would > not expect that to be the case very often, if at all, on 965GM as the > two access paths are incoherent. Still it sounds promising, hopefully > Dave can put it into a fedora kernel for testing? http://kojipkgs.fedoraproject.org/scratch/airlied/task_8760024/ done, also asked on the bug for testers. Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i915: WARN_ON(val > dev_priv->rps.max_freq_softlimit)
On Thu, Jan 29, 2015 at 06:12:31PM +0100, Daniel Vetter wrote: > On Wed, Jan 28, 2015 at 10:36:02PM -0800, O'Rourke, Tom wrote: > > On Wed, Jan 28, 2015 at 01:28:58PM +0200, Ville Syrjälä wrote: > > > On Wed, Jan 28, 2015 at 09:58:15AM +, Chris Wilson wrote: > > > > On Wed, Jan 28, 2015 at 12:43:21AM -0500, Michael Auchter wrote: > > > > > Testing out 3.19-rc6 on my 2014 Thinkpad X1 Carbon (Haswell) resulted > > > > > in > > > > > this WARN at boot (and pretty frequently afterwards): > > > > > > > > > > WARNING: CPU: 0 PID: 989 at drivers/gpu/drm/i915/intel_pm.c:4377 > > > > > gen6_set_rps+0x371/0x3c0() > > > > > WARN_ON(val > dev_priv->rps.max_freq_softlimit) > > > > > > > > [snip] > > > > > > > > > I'm not at all familiar with this hardware, but I took a quick look > > > > > into > > > > > what changed with this commit for my laptop. Before the commit, > > > > > rps.min_freq_softlimit is 4 (from rps.min_freq) and > > > > > rps.max_freq_softlimit is 22. > > > > > > > > > > After the commit, rps.min_freq_softlimit is set to the > > > > > rps.efficient_freq value read from pcode, which is 34 on my laptop. > > > > > So later when gen6_set_rps() is called with rps.min_freq_softlimit > > > > > that > > > > > warning is hit. > > > > > > > > > > Any thoughts? It certainly seems fishy that this commit causes > > > > > rps.min_freq_softlimit to be greater than rps.max_freq_softlimit. > > > > > > > > Very fishy indeed. Moral of this story, never trust hw. > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > > > > b/drivers/gpu/drm/i915/intel_pm.c > > > > index 3e630feb18e4..bbedd2901c54 100644 > > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > > @@ -4007,7 +4007,10 @@ static void gen6_init_rps_frequencies(struct > > > > drm_device *dev) > > > > &ddcc_status); > > > > if (0 == ret) > > > > dev_priv->rps.efficient_freq = > > > > - (ddcc_status >> 8) & 0xff; > > > > + clamp_t(u8, > > > > + (ddcc_status >> 8) & 0xff, > > > > + dev_priv->rps.min_freq, > > > > + dev_priv->rps.max_freq); > > > > > > Maybe better to fall back to rp1_freq if this is bogus? > > > > > [TOR:] Michael, Thank you for bringing this problem to our attention. > > > > Yes, this function needs some range checking to maintain > > RPn <= RPe <= RP0. > > > > A value of 34 seems too high for RPe. > > What values does the Carbon X1 (Haswell) have for RPn and RP0? > > 4 & 22, already in Micheal's original bug report. > > Tom, can you pls polish the clamping into a proper patch with m-l > references? > > Micheal, can you please test the first hunk from Chris (the one that adds > the clamp) to make sure it does indeed address the WARN_ON you're seeing? The clamp suggested by Chris does indeed fix the WARN_ON. In the case where RPe is greater than RP0, RPe will now be clamped to RP0. Is this likely to result in increased power consumption? At a quick glance on my laptop it does not (idling around 5W before and after) but Ville had suggested earlier to fall back to RP1, which would be more consistent with previous kernels. Thanks again for the quick responses, Michael ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] memcontrol.c BUG
On Wed, 28 Jan 2015, Michal Hocko wrote:
> On Wed 28-01-15 08:48:52, Chris Wilson wrote:
> > On Wed, Jan 28, 2015 at 08:13:06AM +1000, Dave Airlie wrote:
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1165369
> > >
> > > ov 18 09:23:22 elissa.gathman.org kernel: page:f5e36a40 count:2
> > > mapcount:0 mapping: (null) index:0x0
> > > Nov 18 09:23:22 elissa.gathman.org kernel: page flags:
> > > 0x80090029(locked|uptodate|lru|swapcache|swapbacked)
> > > Nov 18 09:23:22 elissa.gathman.org kernel: page dumped because:
> > > VM_BUG_ON_PAGE(!lrucare && PageLRU(oldpage))
> > > Nov 18 09:23:23 elissa.gathman.org kernel: [ cut here
> > > ]
> > > Nov 18 09:23:23 elissa.gathman.org kernel: kernel BUG at
> > > mm/memcontrol.c:6733!
>
> I guess this matches the following bugon in your kernel:
> VM_BUG_ON_PAGE(!lrucare && PageLRU(oldpage), oldpage);
>
> so the oldpage is on the LRU list already. I am completely unfamiliar
> with 965GM but is the page perhaps shared with somebody with a different
> gfp mask requirement (e.g. userspace accessing the memory via mmap)? So
> the other (racing) caller didn't need to move the page and put it on
> LRU.
It would be surprising (but not impossible) for oldpage not to be on
the LRU already: it's a swapin readahead page that has every right to
be on LRU, but turns out to have been allocated from an unsuitable zone,
once we discover that it's needed in one of these odd hardware-limited
mappings. (Whereas newpage is newly allocated and not yet on LRU.)
>
> If yes we need to tell shmem_replace_page to do the lrucare handling.
Absolutely, thanks Michal. It would also be good to change the comment
on mem_cgroup_migrate() in mm/memcontrol.c, from "@lrucare: both pages..."
to "@lrucare: either or both pages..." - though I certainly won't pretend
that the corrected wording would have prevented this bug creeping in!
>
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 339e06639956..e3cdc1a16c0f 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1013,7 +1013,7 @@ static int shmem_replace_page(struct page **pagep,
> gfp_t gfp,
>*/
> oldpage = newpage;
> } else {
> - mem_cgroup_migrate(oldpage, newpage, false);
> + mem_cgroup_migrate(oldpage, newpage, true);
> lru_cache_add_anon(newpage);
> *pagep = newpage;
> }
Acked-by: Hugh Dickins
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] intel_crtc_wait_for_pending_flips
On 2015-01-29 18:15, Daniel Vetter wrote: On Thu, Jan 29, 2015 at 06:00:26PM +0100, Kenneth Johansson wrote: I have a rather infrequent hang of the graphics system but when it happens the only way out is to pull the power. This has happened on serveral kernel versions but it is infrequent enough so that I now only have the most resent still in the logs. what I remember is that I always see intel_crtc_wait_for_pending_flips on the call stack. Yeah, stuck pageflip and then the code falls over waiting for it. A few things to test: - Please retest with latest -rc kernel to make sure the problem is still there. - Please add i915.use_mmio_flip=1 to your kernel cmdline and retest. - Please boot with drm.debug=0xe and grab the boot dmesg (just so we know your hw config). To make sure we don't lose track of your report please file a new bug on bugs.freedesktop.org against DRI -> DRM (Intel). Thanks, Daniel I create a bug report on https://bugs.freedesktop.org/show_bug.cgi?id=88872 Installed the 3.19,rc6 and booted up with drm.debug=0xe. [1.723323] [drm:i915_dump_device_info] i915 device info: gen=7, pciid=0x0d22 rev=0x08 flags=need_gfx_hws,is_haswell,has_fbc,has_hotplug,has_llc,has_ddi,has_fpga_dbg, It can take many days for the issue to actually happen. here is the tracebacks that I have in the log. I had to pull the power since I could not switch to a VT from X and I could not get it to do a reboot. Jan 29 12:43:22 brix kernel: [77018.170497] [ cut here ] Jan 29 12:43:22 brix kernel: [77018.170597] WARNING: CPU: 0 PID: 2067 at /home/apw/COD/linux/drivers/gpu/drm/i915/intel_display.c:3473 intel_crtc_wait_for_pending_flips+0x157/0x1c0 [i915]() Jan 29 12:43:22 brix kernel: [77018.170601] Modules linked in: nfsv3 autofs4 xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp iptable_filter ip_tables x_tables bridge stp llc dm_crypt rfcomm bnep nfsd auth_rpcgss nfs_acl nfs lockd grace sunrpc binfmt_misc fscache pl2303 usbserial cdc_ether usbnet x86_pkg_temp_thermal intel_powerclamp kvm_intel kvm crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd btusb joydev bluetooth r8152 mei_me lpc_ich mei snd_soc_rt5640 snd_soc_rl6231 shpchp snd_soc_core snd_compress snd_pcm_dmaengine i2c_hid dw_dmac spi_pxa2xx_platform dw_dmac_core i2c_designware_platform 8250_dw snd_soc_sst_acpi i2c_designware_core mac_hid snd_hda_codec_realtek snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_intel snd_hda_controller snd_hda_codec snd_hwdep snd_pcm snd_seq_midi snd_seq_midi_event snd_rawmidi snd_seq snd_seq_device snd_timer snd soundcore parport_pc ppdev it87 hwmon_vid coretemp lp parport nls_iso8859_1 btrfs xor hid_logitech_dj raid6_pq hid_generic usbhid hid i915 i2c_algo_bit drm_kms_helper ahci r8169 drm libahci mii sdhci_acpi video sdhci Jan 29 12:43:22 brix kernel: [77018.170737] CPU: 0 PID: 2067 Comm: Xorg Not tainted 3.18.1-031801-generic #201412170637 Jan 29 12:43:22 brix kernel: [77018.170741] Hardware name: GIGABYTE M4HM87P-00/M4HM87P-00, BIOS F2 12/11/2013 Jan 29 12:43:22 brix kernel: [77018.170744] 0d91 8803f6fa7b38 827a5f19 0007 Jan 29 12:43:22 brix kernel: [77018.170750] 8803f6fa7b78 82074b0c 0202 Jan 29 12:43:22 brix kernel: [77018.170756] 88040829b1a8 8804082a5000 880035d78430 88040829b000 Jan 29 12:43:22 brix kernel: [77018.170762] Call Trace: Jan 29 12:43:22 brix kernel: [77018.170779] [] dump_stack+0x46/0x58 Jan 29 12:43:22 brix kernel: [77018.170792] [] warn_slowpath_common+0x8c/0xc0 Jan 29 12:43:22 brix kernel: [77018.170799] [] warn_slowpath_null+0x1a/0x20 Jan 29 12:43:22 brix kernel: [77018.170851] [] intel_crtc_wait_for_pending_flips+0x157/0x1c0 [i915] Jan 29 12:43:22 brix kernel: [77018.170860] [] ? prepare_to_wait_event+0x100/0x100 Jan 29 12:43:22 brix kernel: [77018.170908] [] intel_crtc_disable_planes+0x34/0x150 [i915] Jan 29 12:43:22 brix kernel: [77018.170952] [] haswell_crtc_disable+0x5d/0x210 [i915] Jan 29 12:43:22 brix kernel: [77018.170996] [] intel_crtc_control+0x57/0x130 [i915] Jan 29 12:43:22 brix kernel: [77018.171038] [] intel_crtc_update_dpms+0x67/0x80 [i915] Jan 29 12:43:22 brix kernel: [77018.171081] [] intel_connector_dpms+0x61/0x70 [i915] Jan 29 12:43:22 brix kernel: [77018.171118] [] drm_mode_connector_set_obj_prop+0xa4/0xb0 [drm] Jan 29 12:43:22 brix kernel: [77018.171155] [] drm_mode_obj_set_property_ioctl+0x134/0x1d0 [drm] Jan 29 12:43:22 brix kernel: [77018.171188] [] drm_mode_connector_property_set_ioctl+0x30/0x40 [drm] Jan 29 12:43:22 brix kernel: [77018.171214] [] drm_ioctl+0x2e6/0x590 [drm] Jan 29 12:43:22 brix kernel: [77018.171261] [] ? btrfs_file_write_iter+0x29d/0x360 [bt
Re: [Intel-gfx] [PATCH 1/3] drm/i915/skl: Split the SKL PCI ids by GT
On Thu, 29 Jan 2015, Jeff McGee wrote: > On Thu, Jan 29, 2015 at 02:13:38PM +, Damien Lespiau wrote: >> We need to have a separate GT3 struct intel_device_info to declare they >> have a second VCS. Let's start by splitting the PCI ids per-GT. >> > Would it be a good idea to do more programmatic population of > these fields, rather than creating an entire new instance of the > struct just to alter one field? This relates to our other > conversation about the memory consumed by the 30+ device infos > and the concern when adding new fields. From a debugging perspective, I do like the way it is. You can look at or search the info structs and you know which platforms have what, no thinking involved. On a related note, I'm contemplating sending a patch to obliterate the _INTEL_BDW_M and _INTEL_BDW_D macros from i915_pciids.h because it hides the IDs from a simple grep. See how I try to optimize space and time resources - of my brain! BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
