[Intel-gfx] [PATCH i-g-t] tests: Fix compilation when some gcc configurations
Depending how the gcc was compiled it may be necessary to enable SSE4 instructions explicitly. Otherwise: CC gem_gtt_speed.o In file included from gem_gtt_speed.c:54:0: /usr/lib/gcc/x86_64-linux-gnu/4.8/include/smmintrin.h:31:3: error: #error "SSE4.1 instruction set not enabled" # error "SSE4.1 instruction set not enabled" ^ gem_gtt_speed.c: In function ‘streaming_load’: gem_gtt_speed.c:59:2: error: unknown type name ‘__m128i’ __m128i tmp, *s = src; ^ gem_gtt_speed.c:65:3: error: implicit declaration of function ‘_mm_stream_load_si128’ [-Werror=implicit-function-declaration] tmp += _mm_stream_load_si128(s++); ^ gem_gtt_speed.c:65:3: warning: nested extern declaration of ‘_mm_stream_load_si128’ [-Wnested-externs] gem_gtt_speed.c:70:2: error: unknown type name ‘__m128i’ *(volatile __m128i *)src = tmp; ^ CC: Chris Wilson Signed-off-by: Damien Lespiau --- tests/Makefile.am | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Makefile.am b/tests/Makefile.am index 102b8a6..8c6b0a3 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -76,6 +76,7 @@ gem_ctx_basic_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS) gem_ctx_basic_LDADD = $(LDADD) -lpthread gem_ctx_thrash_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS) gem_ctx_thrash_LDADD = $(LDADD) -lpthread +gem_exec_flush_CFLAGS = $(AM_CFLAGS) -msse4 gem_exec_parallel_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS) gem_exec_parallel_LDADD = $(LDADD) -lpthread gem_fence_thrash_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS) @@ -84,6 +85,7 @@ gem_fence_upload_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS) gem_fence_upload_LDADD = $(LDADD) -lpthread gem_flink_race_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS) gem_flink_race_LDADD = $(LDADD) -lpthread +gem_gtt_speed_CFLAGS = $(AM_CFLAGS) -msse4 gem_mmap_gtt_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS) gem_mmap_gtt_LDADD = $(LDADD) -lpthread gem_mmap_wc_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS) -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests: Fix compilation when some gcc configurations
Depending how the gcc was compiled it may be necessary to enable SSE4 instructions explicitly. Otherwise: CC gem_gtt_speed.o In file included from gem_gtt_speed.c:54:0: /usr/lib/gcc/x86_64-linux-gnu/4.8/include/smmintrin.h:31:3: error: #error "SSE4.1 instruction set not enabled" # error "SSE4.1 instruction set not enabled" ^ v2: Use a pragma directive (Chris) Cc: Chris Wilson Signed-off-by: Damien Lespiau --- tests/gem_exec_flush.c | 1 + tests/gem_gtt_speed.c | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/gem_exec_flush.c b/tests/gem_exec_flush.c index c81a977..b43d511 100644 --- a/tests/gem_exec_flush.c +++ b/tests/gem_exec_flush.c @@ -41,6 +41,7 @@ IGT_TEST_DESCRIPTION("Basic check of flushing after batches"); #define MOVNT 512 #if defined(__x86_64__) +#pragma GCC target ("sse4.1") #include __attribute__((noinline)) __attribute__((target("sse4.1"))) diff --git a/tests/gem_gtt_speed.c b/tests/gem_gtt_speed.c index 94b3de3..ed8cfda 100644 --- a/tests/gem_gtt_speed.c +++ b/tests/gem_gtt_speed.c @@ -51,6 +51,7 @@ static double elapsed(const struct timeval *start, } #if defined(__x86_64__) +#pragma GCC target ("sse4.1") #include __attribute__((noinline)) __attribute__((target("sse4.1"))) -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tests: Fix compilation when some gcc configurations
On Thu, Jul 14, 2016 at 06:48:26PM +0100, Chris Wilson wrote: > On Thu, Jul 14, 2016 at 06:44:59PM +0100, Chris Wilson wrote: > > On Thu, Jul 14, 2016 at 06:31:37PM +0100, Damien Lespiau wrote: > > > Depending how the gcc was compiled it may be necessary to enable SSE4 > > > instructions explicitly. Otherwise: > > > > > > CC gem_gtt_speed.o > > > In file included from gem_gtt_speed.c:54:0: > > > /usr/lib/gcc/x86_64-linux-gnu/4.8/include/smmintrin.h:31:3: error: #error > > > "SSE4.1 instruction set not enabled" > > > # error "SSE4.1 instruction set not enabled" > > > > So the challenge is getting the function attribute applied to the > > include. > > > > Ah, can you try > > #pragma GCC target ("sse4.1") > > in those blocks instead? Yup, that also seems to enable sse4.1 for the rest of the compilation unit. > Oh, and we probably need an #if GCC > 4.y to be fully backwards > compatible. Couldn't find in less than 5 mins this information, gave up. Someone exhibiting the problem will have to fix it :) -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Eliminate dead code in intel_sanitize_enable_ppgtt()
We exit early if has_aliasing_ppgtt is 0, so towards the end of the function has_aliasing_ppgtt can only be 1. Also: if (foo) return 1; else return 0; when foo is already a bool is really just: return foo; Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 4668477..6aae4b8 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -161,7 +161,7 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, if (INTEL_GEN(dev_priv) >= 8 && i915.enable_execlists) return has_full_48bit_ppgtt ? 3 : 2; else - return has_aliasing_ppgtt ? 1 : 0; + return has_aliasing_ppgtt; } static int ppgtt_bind_vma(struct i915_vma *vma, -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Restore skl_gt3 device info
On Fri, Dec 04, 2015 at 04:19:49PM +0100, Daniel Vetter wrote: > This was broken in > > commit 6a8beeffed3b2d33151150e3a03696e697f16d46 > Author: Wayne Boyer > Date: Wed Dec 2 13:28:14 2015 -0800 > > drm/i915: Clean up device info structure definitions > > and I didn't spot this while reviewing. We really need that CI farm up > asap! We had a BAT run on this one: http://patchwork.freedesktop.org/series/1360/ But no SKL GT3 coverage. Also, what seems to be flaky tests. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: Add SKL GT4 PCI IDs
On Fri, Nov 06, 2015 at 02:11:16PM +0200, Mika Kuoppala wrote: > From: Mika Kuoppala > > Add Skylake Intel Graphics GT4 PCI IDs > > v2: Rebase > > Signed-off-by: Mika Kuoppala > --- Reviewed-by: Damien Lespiau > drivers/gpu/drm/i915/i915_drv.c | 1 + > include/drm/i915_pciids.h | 13 ++--- > 2 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 9f55209..59810b6 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -465,6 +465,7 @@ static const struct pci_device_id pciidlist[] = { > INTEL_SKL_GT1_IDS(&intel_skylake_info), > INTEL_SKL_GT2_IDS(&intel_skylake_info), > INTEL_SKL_GT3_IDS(&intel_skylake_gt3_info), > + INTEL_SKL_GT4_IDS(&intel_skylake_gt3_info), > INTEL_BXT_IDS(&intel_broxton_info), > INTEL_KBL_GT1_IDS(&intel_kabylake_info), > INTEL_KBL_GT2_IDS(&intel_kabylake_info), > diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h > index f1a113e..f970209 100644 > --- a/include/drm/i915_pciids.h > +++ b/include/drm/i915_pciids.h > @@ -279,12 +279,19 @@ > #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 */ \ > + INTEL_VGA_DEVICE(0x192A, info) /* SRV GT3 */ > > -#define INTEL_SKL_IDS(info) \ > +#define INTEL_SKL_GT4_IDS(info) \ > + INTEL_VGA_DEVICE(0x1932, info), /* DT GT4 */ \ > + INTEL_VGA_DEVICE(0x193B, info), /* Halo GT4 */ \ > + INTEL_VGA_DEVICE(0x193D, info), /* WKS GT4 */ \ > + INTEL_VGA_DEVICE(0x193A, info) /* SRV GT4 */ > + > +#define INTEL_SKL_IDS(info) \ > INTEL_SKL_GT1_IDS(info), \ > INTEL_SKL_GT2_IDS(info), \ > - INTEL_SKL_GT3_IDS(info) > + INTEL_SKL_GT3_IDS(info), \ > + INTEL_SKL_GT4_IDS(info) > > #define INTEL_BXT_IDS(info) \ > INTEL_VGA_DEVICE(0x0A84, info), \ > -- > 2.5.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: Add SKL GT4 PCI IDs
On Fri, Dec 04, 2015 at 07:00:36PM +, Damien Lespiau wrote: > On Fri, Nov 06, 2015 at 02:11:16PM +0200, Mika Kuoppala wrote: > > From: Mika Kuoppala > > > > Add Skylake Intel Graphics GT4 PCI IDs > > > > v2: Rebase > > > > Signed-off-by: Mika Kuoppala > > --- > > Reviewed-by: Damien Lespiau And picked up for dinq. Thanks! I do wonder if it's a canditate for stable though. user land drivers will lag behind... -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH xf86-video-intel] Sync PCI ids with latest kernel, adding more BXT devices
commit 985dd4360fdf2533fe48a33a4a2094f2e4718dc0 Author: Imre Deak Date: Thu Jan 28 16:04:12 2016 +0200 drm/i915/bxt: update list of PCIIDs Signed-off-by: Damien Lespiau --- src/i915_pciids.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/i915_pciids.h b/src/i915_pciids.h index f970209..9b48ac1 100644 --- a/src/i915_pciids.h +++ b/src/i915_pciids.h @@ -296,7 +296,9 @@ #define INTEL_BXT_IDS(info) \ INTEL_VGA_DEVICE(0x0A84, info), \ INTEL_VGA_DEVICE(0x1A84, info), \ - INTEL_VGA_DEVICE(0x5A84, info) + INTEL_VGA_DEVICE(0x1A85, info), \ + INTEL_VGA_DEVICE(0x5A84, info), /* APL HD Graphics 505 */ \ + INTEL_VGA_DEVICE(0x5A85, info) /* APL HD Graphics 500 */ #define INTEL_KBL_GT1_IDS(info)\ INTEL_VGA_DEVICE(0x5913, info), /* ULT GT1.5 */ \ -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] list-workarounds: Extend the script to Mesa
On Thu, Feb 04, 2016 at 06:14:02PM +, Kibey, Sameer wrote: > Updated the list-workarounds script so that it > can parse Mesa directory if provided. Moved the > common code to a separate function to allow > reuse for both kernel and mesa. > > The new command line is: > Usage: list-workarounds [options] path-to-kernel >-k path-to-kernel -m path-to-mesa > > The legacy usage is retained to avoid breaking > backwards compatibility. New parameters -k and > -m are added for the new behavior. > > Either kernel or mesa or both paths can be specified. > If path-to-mesa is invalid, error is reported. > > Signed-off-by: Sameer Kibey Out of curiosity, how did you send the email? It doesn't seem to have been sent with git send-email and so the patch isn't picked up by our patchwork instance. Out of the comments below, I guess the only serious one is allowing both byt/vlv, but maybe mesa only uses one of the two? I wouldn't mind landing the patch with that answered. > --- > scripts/list-workarounds | 75 > ++-- > 1 file changed, 54 insertions(+), 21 deletions(-) > > diff --git a/scripts/list-workarounds b/scripts/list-workarounds > index d11b6a9..0b63541 100755 > --- a/scripts/list-workarounds > +++ b/scripts/list-workarounds > @@ -18,7 +18,7 @@ def find_nth(haystack, needle, n): > return start > > valid_platforms = ('ctg', 'elk', 'ilk', 'snb', 'ivb', 'vlv', 'hsw', 'bdw', > -'chv', 'skl', 'bxt') > +'chv', 'skl', 'bxt', 'kbl', 'byt') Do we really need both byt and vlv? that creates two different names for the same platform, which sounds like a recipe to have the actual set of W/As for this platform be the union of vlv and byt ones. > def parse_platforms(line, p): > l = p.split(',') > for p in l: > @@ -65,9 +65,15 @@ def execute(cmd): > return out, err > > def parse_options(args): > - usage = "Usage: list-workarounds [options] path-to-kernel" > + usage = "Usage: list-workarounds [options] path-to-kernel -k > path-to-kernel -m path-to-mesa" > parser = optparse.OptionParser(usage, version=1.0) Quite frankly, I'd just remove the old behaviour. > + parser.add_option("-k", "--kernel-path", dest="kernel_path", > default=None, > + help="path to kernel") > + > + parser.add_option("-m", "--mesa-path", dest="mesa_path", default=None, > + help="path to mesa") > + > parser.add_option("-v", "--verbose", action="store_true", > dest="verbose", default=False, > help="be more verbose") > @@ -76,30 +82,14 @@ def parse_options(args): > help="List workarounds for the specified platform") > > (options, args) = parser.parse_args() > - > return (options, args) > > -if __name__ == '__main__': > - (options, args) = parse_options(sys.argv[1:]) > - verbose = options.verbose > - > - if not len(args): > - sys.stderr.write("error: A path to a kernel tree is required\n") > - sys.exit(1) > - > - kernel_path = args[0] > - kconfig = os.path.join(kernel_path, 'Kconfig') > - if not os.path.isfile(kconfig): > - sys.stderr.write("error: %s does not point to a kernel tree \n" > - % kernel_path) > - sys.exit(1) > - > - i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915') > +def print_workarounds(code_path, driver_dir): > olddir = os.getcwd() > - os.chdir(kernel_path) > + os.chdir(code_path) project_root? > work_arounds, err = execute(['git', 'grep', '-n', >'-e', 'W[aA][A-Z0-9][a-zA-Z0-9_]\+', > - i915_dir]) > + driver_dir]) > os.chdir(olddir) > if err: > print(err) > @@ -111,3 +101,46 @@ if __name__ == '__main__': > print("%s: %s" % (wa, ', '.join(workarounds[wa]))) > elif options.platform in workarounds[wa]: > print(wa) > + > + > +if __name__ == '__main__': > + (options, args) = parse_options(sys.argv) > + verbose = options.verbose > + kernel_path = None > + > + if not len(args) and options.kernel_path == None and options.mesa_path > == None: > + sys.stderr.write("error: A path to either a kernel tree or Mesa > is required\n") > + sys.exit(1) > + > + if len(args): > + kernel_path = args[0] > + elif options.kernel_path != None: > + kernel_path = options.kernel_path > + > + if kernel_path != None: > + # --- list Kernel workarounds if path is provided --- > + kconfig = os.path.join(kernel_path, 'Kconfig') > + if not os.path.isfile(kconfig): > + sys.stderr.write("error: %s does not point to a kernel > tree \n" > +
Re: [Intel-gfx] [PATCH v2 i-g-t] igt/list-workarounds: Extend the script to Mesa
On Fri, Feb 05, 2016 at 01:55:19PM -0800, Sameer Kibey wrote: > Updated the list-workarounds script so that it > can parse Mesa directory if provided. Moved the > common code to a separate function to allow > reuse for both kernel and mesa. > > The new command line is: > Usage: list-workarounds [options] path-to-kernel >-k path-to-kernel -m path-to-mesa > > The legacy usage is retained to avoid breaking > backwards compatibility. New parameters -k and > -m are added for the new behavior. > > Either kernel or mesa or both paths can be specified. > If path-to-mesa is invalid, error is reported. > > Signed-off-by: Sameer Kibey Pushed thanks for the patch. -- Damien > --- > scripts/list-workarounds | 74 > ++-- > 1 file changed, 53 insertions(+), 21 deletions(-) > > diff --git a/scripts/list-workarounds b/scripts/list-workarounds > index d11b6a9..8b41ae5 100755 > --- a/scripts/list-workarounds > +++ b/scripts/list-workarounds > @@ -18,7 +18,7 @@ def find_nth(haystack, needle, n): > return start > > valid_platforms = ('ctg', 'elk', 'ilk', 'snb', 'ivb', 'vlv', 'hsw', 'bdw', > -'chv', 'skl', 'bxt') > +'chv', 'skl', 'bxt', 'kbl') > def parse_platforms(line, p): > l = p.split(',') > for p in l: > @@ -65,9 +65,15 @@ def execute(cmd): > return out, err > > def parse_options(args): > - usage = "Usage: list-workarounds [options] path-to-kernel" > + usage = "Usage: list-workarounds [options] path-to-kernel -k > path-to-kernel -m path-to-mesa" > parser = optparse.OptionParser(usage, version=1.0) > > + parser.add_option("-k", "--kernel-path", dest="kernel_path", > default=None, > + help="path to kernel") > + > + parser.add_option("-m", "--mesa-path", dest="mesa_path", default=None, > + help="path to mesa") > + > parser.add_option("-v", "--verbose", action="store_true", > dest="verbose", default=False, > help="be more verbose") > @@ -76,38 +82,64 @@ def parse_options(args): > help="List workarounds for the specified platform") > > (options, args) = parser.parse_args() > - > return (options, args) > > -if __name__ == '__main__': > - (options, args) = parse_options(sys.argv[1:]) > - verbose = options.verbose > - > - if not len(args): > - sys.stderr.write("error: A path to a kernel tree is required\n") > - sys.exit(1) > - > - kernel_path = args[0] > - kconfig = os.path.join(kernel_path, 'Kconfig') > - if not os.path.isfile(kconfig): > - sys.stderr.write("error: %s does not point to a kernel tree \n" > - % kernel_path) > - sys.exit(1) > - > - i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915') > +def print_workarounds(project_root, driver_dir, project): > olddir = os.getcwd() > - os.chdir(kernel_path) > + os.chdir(project_root) > work_arounds, err = execute(['git', 'grep', '-n', >'-e', 'W[aA][A-Z0-9][a-zA-Z0-9_]\+', > - i915_dir]) > + driver_dir]) > os.chdir(olddir) > if err: > print(err) > sys.exit(1) > > parse(work_arounds) > + print "\nList of workarounds found in %s:" % project > for wa in sorted(workarounds.keys()): > if not options.platform: > print("%s: %s" % (wa, ', '.join(workarounds[wa]))) > elif options.platform in workarounds[wa]: > print(wa) > + > + > +if __name__ == '__main__': > + (options, args) = parse_options(sys.argv) > + verbose = options.verbose > + kernel_path = None > + > + if not len(args) and options.kernel_path == None and options.mesa_path > == None: > + sys.stderr.write("error: A path to either a kernel tree or Mesa > is required\n") > + sys.exit(1) > + > + if len(args): > + kernel_path = args[0] > + elif options.kernel_path != None: > + kernel_path = options.kernel_path > + > + if kernel_path != None: > + # --- list Kernel workarounds if path is provided --- > + kconfig = os.path.join(kernel_path, 'Kconfig') > + if not os.path.isfile(kconfig): > + sys.stderr.write("error: %s does not point to a kernel > tree \n" > + % kernel_path) > + sys.exit(1) > + > + i915_dir = os.path.join('drivers', 'gpu', 'drm', 'i915') > + print_workarounds(kernel_path, i915_dir, "kernel") > + > + # --- list mesa workarounds if path is provided --- > + if options.mesa_path != None: > + # reset workarounds array > + workarounds = {}
[Intel-gfx] [PATCH i-g-t] list-workarounds: Fix python 2 print statement
That script is a python 3 script, so we can't use the python 2 print statement, it's a function now. I missed it in the review because reviewing a diff without additional context gives you a partial story. Cc: Sameer Kibey Cc: Dylan Baker Signed-off-by: Damien Lespiau --- scripts/list-workarounds | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/list-workarounds b/scripts/list-workarounds index 8b41ae5..70c026d 100755 --- a/scripts/list-workarounds +++ b/scripts/list-workarounds @@ -96,7 +96,7 @@ def print_workarounds(project_root, driver_dir, project): sys.exit(1) parse(work_arounds) - print "\nList of workarounds found in %s:" % project + print("\nList of workarounds found in %s:" % project) for wa in sorted(workarounds.keys()): if not options.platform: print("%s: %s" % (wa, ', '.join(workarounds[wa]))) -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Mesa-dev] [PATCH v2 i-g-t] igt/list-workarounds: Extend the script to Mesa
On Fri, Feb 05, 2016 at 04:12:08PM -0800, Dylan Baker wrote: > > > parse(work_arounds) > > > + print "\nList of workarounds found in %s:" % project > > Hey Damien, the script says it's python 3, and this ^^^ is broken syntax > in python 3 (but not in 2). :( I did notice the python2 construct, but then, of course, the diff is missing the full context so didn't realize it was a python3 script. Sent and pushed the obvious fix. I really want to trust that developers run the code at least once before submitting, even if it's a rework of the original patch. Even better would be a simple unit test, and hook make distcheck to patchwork. I'll look into that at some point. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH RESEND FOR CI] drm/i915: Fix races on fbdev
Hi Lukas, I'm sorry I haven't reacted sooner. I've enabled the option to only consider git send-email patches on intel-gfx as we were having a lot of false positives (ie patchwork considering emails were people were making suggestions with diffs as patches to test). I'm not sure how you send your emails, but I can tell why that it didn't pass the "is this a git send-email patch?" question. We're looking at two things: - The X-Mailer header - The Message-Id (git send-email has a special way to craft its message IDs) I've replayed your email by hand with an extra X-Mailer header and patchwork picked it up (and hopefully we should have a run of our basic acceptance tests done on it): https://patchwork.freedesktop.org/series/4068/ You can keep whatever you're doing now and trick patchwork to take your patches by added this extra header to your emails: X-Mailer: git-send-email 2.4.3 Or use git send-email. HTH, -- Damien On Thu, Mar 03, 2016 at 07:30:33PM +0100, Lukas Wunner wrote: > Hi Damien, Hi Daniel, > > I've submitted the patch below for the third time now in an attempt > to get CI to test it, again to no avail. This time I didn't set the > In-Reply-To header and only submitted it as a single patch instead > of as a series because I expected this might confuse CI. > > Nevertheless CI misclassified it under "Series" instead of "Patches", > reports that the series consists of 0 patches and doesn't test it: > https://patchwork.freedesktop.org/series/4068/ > > Earlier attempts: > https://patchwork.freedesktop.org/series/3453/ > https://patchwork.freedesktop.org/series/3126/ > > Sorry, I give up. > > Best regards, > > Lukas > > On Thu, Mar 03, 2016 at 06:02:53PM +0100, Lukas Wunner wrote: > > The ->lastclose callback invokes intel_fbdev_restore_mode() and has > > been witnessed to run before intel_fbdev_initial_config_async() > > has finished. > > > > We might likewise receive hotplug events or be suspended before > > we've had a chance to fully set up the fbdev. > > > > Fix by waiting for the asynchronous thread to finish. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93580 > > Reported-by: Gustav Fägerlind > > Reported-by: "Li, Weinan Z" > > Cc: Chris Wilson > > Cc: sta...@vger.kernel.org > > Signed-off-by: Lukas Wunner > > --- > > drivers/gpu/drm/i915/i915_dma.c| 8 +++- > > drivers/gpu/drm/i915/intel_fbdev.c | 4 > > 2 files changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c > > b/drivers/gpu/drm/i915/i915_dma.c > > index 4aa3db6..9d76dfb 100644 > > --- a/drivers/gpu/drm/i915/i915_dma.c > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > @@ -430,11 +430,9 @@ static int i915_load_modeset_init(struct drm_device > > *dev) > > * Some ports require correctly set-up hpd registers for detection to > > * work properly (leading to ghost connected connector status), e.g. VGA > > * on gm45. Hence we can only set up the initial fbdev config after hpd > > -* irqs are fully enabled. Now we should scan for the initial config > > -* only once hotplug handling is enabled, but due to screwed-up locking > > -* around kms/fbdev init we can't protect the fdbev initial config > > -* scanning against hotplug events. Hence do this first and ignore the > > -* tiny window where we will loose hotplug notifactions. > > +* irqs are fully enabled. We protect the fbdev initial config scanning > > +* against hotplug events by waiting in intel_fbdev_output_poll_changed > > +* until the asynchronous thread has finished. > > */ > > intel_fbdev_initial_config_async(dev); > > > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c > > b/drivers/gpu/drm/i915/intel_fbdev.c > > index ae9cf6f..936c3d7 100644 > > --- a/drivers/gpu/drm/i915/intel_fbdev.c > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > > @@ -754,6 +754,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, > > int state, bool synchronous > > struct intel_fbdev *ifbdev = dev_priv->fbdev; > > struct fb_info *info; > > > > + async_synchronize_full(); > > if (!ifbdev) > > return; > > > > @@ -800,6 +801,8 @@ void intel_fbdev_set_suspend(struct drm_device *dev, > > int state, bool synchronous > > void intel_fbdev_output_poll_changed(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = dev->dev_private; > > + > > + async_synchronize_full(); > > if (dev_priv->fbdev) > > drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper); > > } > > @@ -811,6 +814,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev) > > struct intel_fbdev *ifbdev = dev_priv->fbdev; > > struct drm_fb_helper *fb_helper; > > > > + async_synchronize_full(); > > if (!ifbdev) > > return; > > > > -- > > 1.8.5.2 (Apple Git-48) > > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.
Re: [Intel-gfx] [PATCH i-g-t] benchmarks/, overlay/, demos/, tools/, tests/: Add Werror by default.
On Mon, May 09, 2016 at 04:23:44PM +0300, Marius Vlad wrote: > Easier to catch compilation errors. Having -Werror by default is a no go as you cannot control/predict the set of warnings (and the quality of those) of all previous and future gcc/clang versions. Always using this flag will cause distributions to hate us. Adding a test (with patchwork integration!) that ensures each commit posted on this mailing-list compiles without new warning with a chosen toolchain (and even passes distcheck!) would be nice. -- Damien > Signed-off-by: Marius Vlad > --- > benchmarks/Makefile.am | 2 +- > demos/Makefile.am | 3 ++- > overlay/Makefile.am| 3 ++- > tests/Makefile.am | 2 +- > tools/Makefile.am | 4 +++- > 5 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/benchmarks/Makefile.am b/benchmarks/Makefile.am > index 2c2d100..46992f8 100644 > --- a/benchmarks/Makefile.am > +++ b/benchmarks/Makefile.am > @@ -2,7 +2,7 @@ > include Makefile.sources > > AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib > -AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) $(LIBUNWIND_CFLAGS) > +AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) $(LIBUNWIND_CFLAGS) > -Werror > LDADD = $(top_builddir)/lib/libintel_tools.la > > benchmarks_LTLIBRARIES = gem_exec_tracer.la > diff --git a/demos/Makefile.am b/demos/Makefile.am > index e6fbb3b..9eacd16 100644 > --- a/demos/Makefile.am > +++ b/demos/Makefile.am > @@ -3,5 +3,6 @@ bin_PROGRAMS =\ > $(NULL) > > AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib > -AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) > $(LIBUNWIND_CFLAGS) > +AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) \ > + $(LIBUNWIND_CFLAGS) -Werror > LDADD = $(top_builddir)/lib/libintel_tools.la > diff --git a/overlay/Makefile.am b/overlay/Makefile.am > index c648875..ec68489 100644 > --- a/overlay/Makefile.am > +++ b/overlay/Makefile.am > @@ -3,7 +3,8 @@ bin_PROGRAMS = intel-gpu-overlay > endif > > AM_CPPFLAGS = -I. > -AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) > $(OVERLAY_CFLAGS) > +AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) \ > + $(OVERLAY_CFLAGS) -Werror > LDADD = $(DRM_LIBS) $(PCIACCESS_LIBS) $(CAIRO_LIBS) $(OVERLAY_LIBS) > > intel_gpu_overlay_SOURCES = \ > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 45e3359..22256ce 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -59,7 +59,7 @@ AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(DEBUG_CFLAGS)\ > -include "$(srcdir)/../lib/check-ndebug.h" \ > -DIGT_SRCDIR=\""$(abs_srcdir)"\" \ > -DIGT_DATADIR=\""$(pkgdatadir)"\" \ > - $(LIBUNWIND_CFLAGS) \ > + $(LIBUNWIND_CFLAGS) -Werror \ > $(NULL) > > LDADD = ../lib/libintel_tools.la $(GLIB_LIBS) > diff --git a/tools/Makefile.am b/tools/Makefile.am > index df48d94..0ba1ff7 100644 > --- a/tools/Makefile.am > +++ b/tools/Makefile.am > @@ -3,7 +3,9 @@ include Makefile.sources > SUBDIRS = null_state_gen registers > > AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib > -AM_CFLAGS = $(DEBUG_CFLAGS) $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) > $(CAIRO_CFLAGS) $(LIBUNWIND_CFLAGS) -DPKGDATADIR=\"$(pkgdatadir)\" > +AM_CFLAGS = $(DEBUG_CFLAGS) $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) \ > + $(CWARNFLAGS) $(CAIRO_CFLAGS) $(LIBUNWIND_CFLAGS) \ > + -DPKGDATADIR=\"$(pkgdatadir)\" -Werror > LDADD = $(top_builddir)/lib/libintel_tools.la > AM_LDFLAGS = -Wl,--as-needed > > -- > 2.8.0.rc3 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] benchmarks/, overlay/, demos/, tools/, tests/: Add Werror by default.
On Mon, May 09, 2016 at 06:55:12PM +0300, Marius Vlad wrote: > > Adding a test (with patchwork integration!) that ensures each commit > > posted on this mailing-list compiles without new warning with a chosen > > toolchain (and even passes distcheck!) would be nice. > We have this for check and distcheck internally. The whole point of > Werror was to catch warnings as well when building, and letting us know > so we can fix it. The problem is (unfortunately) that not all patches > arrive thru m-l. Don't really know how much traction some CI/buildbot > for i-g-t will have. Oh, CI for i-g-t patches is a must have and on the roadmap. Can't really keep on testing kernel patches if we can just regress everything with random i-g-t patches. Doing the distcheck for every patch on the ml would already be a nice thing. Alternatively, you could have an --enable-werror configure flag developers may wish to use. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] benchmarks/, overlay/, demos/, tools/, tests/: Add optional Werror.
On Tue, May 10, 2016 at 05:32:15PM +0300, Marius Vlad wrote: > v2: Initially added Werror by default. Make it optional so it doesn't > break android build and (potential) distros maintaing the package > (Hinted by Damien Lespiau). > > --enable-werror will enable -Werror compiler flag. > > Signed-off-by: Marius Vlad Looks like some people might want to use this: Acked-by: Damien Lespiau -- Damien > --- > benchmarks/Makefile.am | 3 ++- > configure.ac | 10 ++ > demos/Makefile.am | 3 ++- > overlay/Makefile.am| 3 ++- > tests/Makefile.am | 2 +- > tools/Makefile.am | 4 +++- > 6 files changed, 20 insertions(+), 5 deletions(-) > > diff --git a/benchmarks/Makefile.am b/benchmarks/Makefile.am > index 2c2d100..49d2f64 100644 > --- a/benchmarks/Makefile.am > +++ b/benchmarks/Makefile.am > @@ -2,7 +2,8 @@ > include Makefile.sources > > AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib > -AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) $(LIBUNWIND_CFLAGS) > +AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) $(LIBUNWIND_CFLAGS) \ > + $(WERROR_CFLAGS) > LDADD = $(top_builddir)/lib/libintel_tools.la > > benchmarks_LTLIBRARIES = gem_exec_tracer.la > diff --git a/configure.ac b/configure.ac > index 0589782..11b1d46 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -229,6 +229,11 @@ AC_ARG_ENABLE(debug, >[Build tests without debug symbols]), > [], [enable_debug=yes]) > > +AC_ARG_ENABLE(werror, > + AS_HELP_STRING([--enable-werror], > + [Fail on warnings]), > + [], [enable_werror=no]) > + > if test "x$enable_debug" = xyes; then > AS_COMPILER_FLAG([-g3], [DEBUG_CFLAGS="-g3"], [DEBUG_CFLAGS="-g"]) > AS_COMPILER_FLAG([-Og], [DEBUG_CFLAGS+=" -Og > -Wno-maybe-uninitialized"], # disable maybe-uninitialized due to false > positives > @@ -236,6 +241,10 @@ if test "x$enable_debug" = xyes; then > AC_SUBST([DEBUG_CFLAGS]) > fi > > +if test "x$enable_werror" = xyes; then > + AS_COMPILER_FLAG([-Werror], [WERROR_CFLAGS="-Werror"]) > +fi > + > # prevent relinking the world on every commit for developers > AC_ARG_ENABLE(git-hash, > AS_HELP_STRING([--disable-git-hash], > @@ -313,6 +322,7 @@ echo " Overlay: X: > ${enable_overlay_xlib}, Xv: ${enable_overla > echo " x86-specific tools : ${build_x86}" > echo "" > echo " • API-Documentation : ${enable_gtk_doc}" > +echo " • Fail on warnings: : ${enable_werror}" > echo "" > > # vim: set ft=config ts=8 sw=8 tw=0 noet : > diff --git a/demos/Makefile.am b/demos/Makefile.am > index e6fbb3b..f5725f4 100644 > --- a/demos/Makefile.am > +++ b/demos/Makefile.am > @@ -3,5 +3,6 @@ bin_PROGRAMS =\ > $(NULL) > > AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/lib > -AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) > $(LIBUNWIND_CFLAGS) > +AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) \ > + $(CAIRO_CFLAGS) $(LIBUNWIND_CFLAGS) $(WERROR_CFLAGS) > LDADD = $(top_builddir)/lib/libintel_tools.la > diff --git a/overlay/Makefile.am b/overlay/Makefile.am > index c648875..c926557 100644 > --- a/overlay/Makefile.am > +++ b/overlay/Makefile.am > @@ -3,7 +3,8 @@ bin_PROGRAMS = intel-gpu-overlay > endif > > AM_CPPFLAGS = -I. > -AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) $(CAIRO_CFLAGS) > $(OVERLAY_CFLAGS) > +AM_CFLAGS = $(DRM_CFLAGS) $(PCIACCESS_CFLAGS) $(CWARNFLAGS) \ > + $(CAIRO_CFLAGS) $(OVERLAY_CFLAGS) $(WERROR_CLFAGS) > LDADD = $(DRM_LIBS) $(PCIACCESS_LIBS) $(CAIRO_LIBS) $(OVERLAY_LIBS) > > intel_gpu_overlay_SOURCES = \ > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 45e3359..32b9115 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -59,7 +59,7 @@ AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) $(DEBUG_CFLAGS)\ > -include "$(srcdir)/../lib/check-ndebug.h" \ > -DIGT_SRCDIR=\""$(abs_srcdir)"\" \ > -DIGT_DATADIR=\""$(pkgdatadir)"\" \ > - $(LIBUNWIND_CFLAGS) \ > + $(LIBUNWIND_CFLAGS) $(WERROR_CFLAGS) \ > $(NULL) > > LDADD = ../lib/libintel_tools.la $(GLIB_LIBS) > diff --git a/tools/Makefile.am b/tools/Makefile.am > index df48d94..5f45144 100644 > --- a/tools/Makefile.am > +++ b/tools/Makefile.am > @@ -3,7 +3,9 @@ include Makefile.sources > SUBDIRS =
Re: [Intel-gfx] [PATCH i-g-t] assembler/: Fix lex warnings for %empty and %nonassoc.
On Mon, May 16, 2016 at 01:39:10PM +0300, Marius Vlad wrote: > Signed-off-by: Marius Vlad > --- > assembler/gram.y | 74 > > 1 file changed, 37 insertions(+), 37 deletions(-) The only way to test the change is to regenerate the vaapi shaders from source and check for differences in the generated opcodes. https://cgit.freedesktop.org/vaapi/intel-driver/ If all the shaders do compile and there's no difference in the generated code, this is: Acked-by: Damien Lespiau -- Damien > > diff --git a/assembler/gram.y b/assembler/gram.y > index 23e1a57..15b8b64 100644 > --- a/assembler/gram.y > +++ b/assembler/gram.y > @@ -509,15 +509,15 @@ static void resolve_subnr(struct brw_reg *reg) > %token BASE ELEMENTSIZE SRCREGION DSTREGION TYPE > > %token DEFAULT_EXEC_SIZE_PRAGMA DEFAULT_REG_TYPE_PRAGMA > -%nonassoc SUBREGNUM > -%nonassoc SNDOPR > +%precedence SUBREGNUM > +%precedence SNDOPR > %left PLUS MINUS > %left MULTIPLY DIVIDE > -%right UMINUS > -%nonassoc DOT > -%nonassoc STR_SYMBOL_REG > -%nonassoc EMPTEXECSIZE > -%nonassoc LPAREN > +%precedence UMINUS > +%precedence DOT > +%precedence STR_SYMBOL_REG > +%precedence EMPTEXECSIZE > +%precedence LPAREN > > %type exp sndopr > %type simple_int > @@ -655,7 +655,7 @@ declare_elementsize: ELEMENTSIZE EQ exp > $$ = $3; > } > ; > -declare_srcregion: /* empty */ > +declare_srcregion: %empty /* empty */ > { > /* XXX is this default correct?*/ > memset (&$$, '\0', sizeof ($$)); > @@ -668,7 +668,7 @@ declare_srcregion: /* empty */ > $$ = $3; > } > ; > -declare_dstregion: /* empty */ > +declare_dstregion: %empty /* empty */ > { > $$ = 1; > } > @@ -1962,20 +1962,20 @@ msgtarget:NULL_TOKEN > ; > > urb_allocate:ALLOCATE { $$ = 1; } > - | /* empty */ { $$ = 0; } > + | %empty /* empty */ { $$ = 0; } > ; > > urb_used:USED { $$ = 1; } > - | /* empty */ { $$ = 0; } > + | %empty /* empty */ { $$ = 0; } > ; > > urb_complete:COMPLETE { $$ = 1; } > - | /* empty */ { $$ = 0; } > + | %empty /* empty */ { $$ = 0; } > ; > > urb_swizzle: TRANSPOSE { $$ = BRW_URB_SWIZZLE_TRANSPOSE; } > | INTERLEAVE { $$ = BRW_URB_SWIZZLE_INTERLEAVE; } > - | /* empty */ { $$ = BRW_URB_SWIZZLE_NONE; } > + | %empty /* empty */ { $$ = BRW_URB_SWIZZLE_NONE; } > ; > > sampler_datatype: > @@ -1988,11 +1988,11 @@ math_function:INV | LOG | EXP | SQRT | POW | > SIN | COS | SINCOS | INTDIV > | INTMOD | INTDIVMOD > ; > > -math_signed: /* empty */ { $$ = 0; } > +math_signed: %empty /* empty */ { $$ = 0; } > | SIGNED { $$ = 1; } > ; > > -math_scalar: /* empty */ { $$ = 0; } > +math_scalar: %empty /* empty */ { $$ = 0; } > | SCALAR { $$ = 1; } > ; > > @@ -2374,7 +2374,7 @@ addrparam: addrreg COMMA immaddroffset > /* The immaddroffset provides an immediate offset value added to the > addresses > * from the address register in register-indirect register access. > */ > -immaddroffset: /* empty */ { $$ = 0; } > +immaddroffset: %empty /* empty */ { $$ = 0; } > | exp > ; > > @@ -2384,7 +2384,7 @@ subregnum: DOT exp > { > $$ = $2; > } > - | %prec SUBREGNUM > + | %empty %prec SUBREGNUM > { > /* Default to subreg 0 if unspecified. */ > $$ = 0; > @@ -2687,7 +2687,7 @@ relativelocation2: > ; > > /* 1.4.7: Regions */ > -dstregion: /* empty */ > +dstregion: %empty /* empty */ > { > $$ = DEFAULT_DSTREGION; > } > @@ -2703,7 +2703,7 @@ dstregion: /* empty */ > } > ; > > -region: /* empty */ > +region: %empty /* empty */ > { > /* XXX is this default value correct?*/ > memset (&$$, '\0', sizeof ($$)); > @@ -2757,7 +2757,7 @@ indirectregion: region | region_wh > /* regtype returns an integer register type suitable for inserting into an > * instruction. > */ > -regtype: /* empty */ > +regtype: %empty /* empty */ > { $$.type = program_defaults.register_type;$$.is_default = 1;} > | TYPE_F { $$.type = BRW_REGISTER_TYPE
Re: [Intel-gfx] [PATCH i-g-t] assembler/: Fix lex warnings for %empty and %nonassoc.
On Thu, May 19, 2016 at 07:02:40AM -0700, Ben Widawsky wrote: > On Thu, May 19, 2016 at 12:28:10PM +0100, Damien Lespiau wrote: > > On Mon, May 16, 2016 at 01:39:10PM +0300, Marius Vlad wrote: > > > Signed-off-by: Marius Vlad > > > --- > > > assembler/gram.y | 74 > > > > > > 1 file changed, 37 insertions(+), 37 deletions(-) > > > > The only way to test the change is to regenerate the vaapi shaders from > > source and check for differences in the generated opcodes. > > Didn't we have something like this in the test/ directory? We have just a few unmaintained tests that don't pass (or do they?) and they barely scratch the surface of what would be needed to decent coverage. vaapi-driver is a much better test suite. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Store and print dmc firmware version
On Fri, Sep 18, 2015 at 06:17:05PM +0300, Mika Kuoppala wrote: > Parse csr/dmc firmware version and augment debug message > by printing it. > > Cc: Animesh Manna > Signed-off-by: Mika Kuoppala FWIW I did something similar in the past, but that contribution was denied. I also had the DC states entry counts, which sounds like something super useful to have: http://lists.freedesktop.org/archives/intel-gfx/2015-June/thread.html The DMC firmware version decoding was different in my patch and I'm sure it worked then. Maye the header has changed :( Feel free to use any part of that series with your authorship. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Store and print dmc firmware version
On Thu, Oct 08, 2015 at 12:03:30PM +0100, Damien Lespiau wrote: > The DMC firmware version decoding was different in my patch and I'm sure > it worked then. Maye the header has changed :( By the way, if this is indeed the case, could you fix intel_firmware_decode as well? http://cgit.freedesktop.org/xorg/app/intel-gpu-tools/tree/tools/intel_firmware_decode.c Thanks, -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use dpcd read wake for sink crc calls.
On Thu, Oct 22, 2015 at 12:01:21AM +0530, Thulasimani, Sivakumar wrote: > > > On 8/25/2015 2:50 AM, Vivi, Rodrigo wrote: > >On Mon, 2015-08-24 at 19:54 +, Zanoni, Paulo R wrote: > >>Em Qui, 2015-08-20 às 16:23 -0700, Rodrigo Vivi escreveu: > >>>Let's use a native read with retry as suggested per spec to > >>>fix Sink CRC on SKL when PSR is enabled. > >>> > >>>With PSR enabled panel is probably taking more time to wake > >>>and dpcd read is faling. > >>Does this commit actually fix any known problem with Sink CRC? Or is > >>it > >>just a try? It would be nice to have this clarified in the commit > >>message. > >It was just a try but that made sink crc working on my SKL when PSR is > >enabled. nothing much to add... > SKL has new register AUX_MUTEX which should be used when accessing dpcd > on edp. just searched the nightly code and could not find it. it might be > the reason > for random dpcd failures reported in the other thread. We had patches for that back in December 2013 :) The feedback from Art was: The non-software aux users are PSR/SRD and GTC. Better leave out the mutex for now. Hardware is going to try do the arbitration itself. I expect you will then need to increase any software timeout you may have. Do you know if anything has changed since then? -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] HDMI 4k modes VIC
On Wed, Oct 28, 2015 at 01:58:55PM +, Sharma, Shashank wrote: >Hi Damien > >This is regarding one of the patches: >drm: Add support for alternate clocks of 4k modes >(3f2f653378112c1453c0d83c81746a9225e4bc75) > >I am seeing that from function drm_match_hdmi_mode we are not returning >correct VIC's for 4k modes (listed in edid_4k_modes[]) >Looks like we are returning VIC=(1, 2, 3, 4) for 4K modes (3840x2160@30Hz, >3840x2160@25Hz, 3840x2160@24Hz and 4096x2160@24Hz) drm_match_hdmi_mode() doesn't return the VIC for the AVI infoframe, but the VIC for the HDMI vendor specific infoframe. See section 8.2.3 of the HDMI 1.4 spec. >But as per the CEA-861-F specs, the respective VIC should be (95, 94 and >93). The VIC returned is being used for writing AVI IF in >drm_hdmi_vendor_infoframe_from_display_mode. That's not the AVI infoframe, that's the HDMI vendor specific infoframe. >Please help me to understand if there any specific reason for this, or am >I missing something in interpretation ? Are you seeing a bug? it's totally possible, I've never used an actual conformance tool when I wrote that code, so it's likely buggy and the VIC in the AVI infoframe may well be wrong. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] HDMI 4k modes VIC
On Wed, Oct 28, 2015 at 06:38:21PM +0200, Jani Nikula wrote: > > Are you seeing a bug? it's totally possible, I've never used an actual > > conformance tool when I wrote that code, so it's likely buggy and the > > VIC in the AVI infoframe may well be wrong. > > Possibly relevant > https://bugs.freedesktop.org/show_bug.cgi?id=92217 That one looks different, we don't do aspect ratio on modes very well, not even exposing them to user space. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] i-g-t/libdrm email tagging & patchwork
Hi all, I've added a feature to sort the patches sent to intel-gfx into 3 buckets: i915, intel-gpu-tools and libdrm. This sorting relies on tagging patches, using the subject prefixes (which is what most people do already anyway). - i915 (intel-gfx): catchall project, all mails not matching any of the other 2 projects will end up there - intel-gpu-tools: mails need to be tagged with i-g-t, igt or intel-gpu-tools - libdrm-intel: mails need to be tagged with libdrm This tagging can be set up per git repository with: $ git config format.subjectprefix "PATCH i-g-t" And use git send-email as usual. A note of caution though, using the command line argument --subject-prefix will override the one configured, so the tag will have to be repeated. To limit the number of things one needs to think about I'd suggest to use --reroll-count to tag patches with the v2/v3/... tags. I'm more and more thinking that wrapping the sending logic for developers into the git-pw command would be a good thing (especially for --in-reply-to) but that'd be for another time. There are two new patchwork projects then: http://patchwork.freedesktop.org/project/intel-gpu-tools/ http://patchwork.freedesktop.org/project/libdrm-intel/ I've also run the sorting on all the existing patches so the entries that were historically in the intel-gfx project are now in those new projects. There is still some work left to limit the noise of those lists of patches, eg. some patches are still marked as New but, in reality, they have been merged. Solving that is quite important and high-ish the TODO list. HTH, -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i-g-t/libdrm email tagging & patchwork
On Mon, Nov 09, 2015 at 10:45:14AM +0200, Jani Nikula wrote: > On Sun, 08 Nov 2015, Damien Lespiau wrote: > > There are two new patchwork projects then: > > > > http://patchwork.freedesktop.org/project/intel-gpu-tools/ > > http://patchwork.freedesktop.org/project/libdrm-intel/ > > > > I've also run the sorting on all the existing patches so the entries > > that were historically in the intel-gfx project are now in those new > > projects. > > Is it possible to manually move patches between projects? Damn, you noticed! No, it's not (yet?) possible. > > There is still some work left to limit the noise of those lists of > > patches, eg. some patches are still marked as New but, in reality, they > > have been merged. Solving that is quite important and high-ish the TODO > > list. > > This may be due to git am -3 subtly changing the patch, or the committer > not-so-subtly changing the patch [*], while applying, and the git hook > on fdo doesn't recognize the patch. Since we've started to add the Link: > tag to patches, we could use that extra bit of info in the hook to link > commits to patchwork. That would work for us, but not in the general case (for other projects). I was thinking of using some kind of other heuristic, eg. (subject, commit message, files touched) with a levenshtein distance on text to allow typo correction. Just for us, we could take a shortcut and make dim do something always correct based on the message-id, I'll have a think. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i-g-t/libdrm email tagging & patchwork
On Mon, Nov 09, 2015 at 01:21:33PM +0200, Jani Nikula wrote: > On Mon, 09 Nov 2015, Damien Lespiau wrote: > > That would work for us, but not in the general case (for other > > projects). I was thinking of using some kind of other heuristic, eg. > > (subject, commit message, files touched) with a levenshtein distance on > > text to allow typo correction. > > > > Just for us, we could take a shortcut and make dim do something always > > correct based on the message-id, I'll have a think. > > Do you mean we'd move the patchwork update to client side rather than > server side? We could do it either way, have dim use git-pw to mark the patch as accepted or have the post commit hook look at the brand new tag and infer the id of the patch to close. I'm guessing you'd rather have the second option. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] lib: Add Skylake Intel Graphics GT4 PCI IDs
On Fri, Nov 06, 2015 at 04:42:10PM +0200, Mika Kuoppala wrote: > Add Skylake Intel Graphics GT4 PCI IDs. > > Signed-off-by: Mika Kuoppala Reviewed-by: Damien Lespiau -- Damien > --- > lib/intel_chipset.h | 12 +++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/lib/intel_chipset.h b/lib/intel_chipset.h > index 6fcc244..93bf565 100644 > --- a/lib/intel_chipset.h > +++ b/lib/intel_chipset.h > @@ -198,13 +198,17 @@ void intel_check_pch(void); > #define PCI_CHIP_SKYLAKE_ULX_GT2 0x191E > #define PCI_CHIP_SKYLAKE_DT_GT2 0x1912 > #define PCI_CHIP_SKYLAKE_DT_GT1 0x1902 > +#define PCI_CHIP_SKYLAKE_DT_GT4 0x1932 > #define PCI_CHIP_SKYLAKE_HALO_GT20x191B > #define PCI_CHIP_SKYLAKE_HALO_GT30x192B > #define PCI_CHIP_SKYLAKE_HALO_GT10x190B > +#define PCI_CHIP_SKYLAKE_HALO_GT40x193B > #define PCI_CHIP_SKYLAKE_SRV_GT2 0x191A > #define PCI_CHIP_SKYLAKE_SRV_GT3 0x192A > #define PCI_CHIP_SKYLAKE_SRV_GT1 0x190A > +#define PCI_CHIP_SKYLAKE_SRV_GT4 0x193A > #define PCI_CHIP_SKYLAKE_WKS_GT2 0x191D > +#define PCI_CHIP_SKYLAKE_WKS_GT4 0x193D > > #define PCI_CHIP_KABYLAKE_ULT_GT2 0x5916 > #define PCI_CHIP_KABYLAKE_ULT_GT1_50x5913 > @@ -409,6 +413,11 @@ void intel_check_pch(void); >(devid) == PCI_CHIP_SKYLAKE_HALO_GT3 || \ >(devid) == PCI_CHIP_SKYLAKE_SRV_GT3) > > +#define IS_SKL_GT4(devid)((devid) == PCI_CHIP_SKYLAKE_DT_GT4 || \ > + (devid) == PCI_CHIP_SKYLAKE_HALO_GT4 || \ > + (devid) == PCI_CHIP_SKYLAKE_WKS_GT4|| \ > + (devid) == PCI_CHIP_SKYLAKE_SRV_GT4) > + > #define IS_KBL_GT1(devid)((devid) == PCI_CHIP_KABYLAKE_ULT_GT1_5|| \ >(devid) == PCI_CHIP_KABYLAKE_ULX_GT1_5|| \ >(devid) == PCI_CHIP_KABYLAKE_DT_GT1_5|| \ > @@ -437,7 +446,8 @@ void intel_check_pch(void); > #define IS_SKYLAKE(devid)(IS_KABYLAKE(devid) || \ >IS_SKL_GT1(devid) || \ >IS_SKL_GT2(devid) || \ > - IS_SKL_GT3(devid)) > + IS_SKL_GT3(devid) || \ > + IS_SKL_GT4(devid)) > > #define IS_BROXTON(devid)((devid) == PCI_CHIP_BROXTON_0 || \ >(devid) == PCI_CHIP_BROXTON_1 || \ > -- > 2.5.0 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: PSR: Let's rely more on frontbuffer tracking.
On Thu, Nov 19, 2015 at 11:24:22AM +, Zanoni, Paulo R wrote: > Em Qui, 2015-11-19 às 13:16 +0200, Jani Nikula escreveu: > > On Wed, 18 Nov 2015, "Zanoni, Paulo R" > > wrote: > > > Em Qua, 2015-11-18 às 11:21 -0800, Rodrigo Vivi escreveu: > > > > Cc: Paulo Zanoni > > > s/Cc/Reviewed-by/ > > > > I don't think our patchwork is quite smart enough for that yet, so > > for > > future reference, please be write the whole thing. > > I completely forgot about this. Sorry. > > I guess that also means that if I say "If you do this and this and > that, then I'll give you Reviewed-by: Paulo Zanoni l.com> it will mark the patch as reviewed even though it needs rework, > right? I guess we just can't trust the PW automatic tag parsing since > it can have either false positives or false negatives. Not until some > major breakthroughs in AI. The Reviewed-by parsing is quite simple: it will match '^Reviewed-by:'. That means indenting the tag should be enough to bypass patchwork's accounting. That said, I think we need to extend the language we use for review and make patchwork understand it. For instance, being able to review several patches in one go could be done with: Patches 1-3,6: Reviewed-by: Damien Lespiau damien.lesp...@intel.com (See: https://github.com/dlespiau/patchwork/issues/27) > Is suggesting deprecating the use of emails as a way to handle patches > still considered trolling? No :) The way I see it is that it's easier to progressively enhance what we have to do what we want -- I'll be the first to say patchwork is very fragile today -- rather than switching to something totally different, probably closing the door to non-intel contributors and suddenly having two different systems for core DRM patch handling, among other things. Either way, we have to try to improve what we have. I took one path but it doesn't mean that was the best, someone else can always try to take another set of tools and convince/force people to use that. The goals are the important bit: test every patch before it's merged into -nightly, improve the developer experience and patch flow/tracking along the way. For the first goal, we are almost there (in terms of patchwork, the CI system, piglit, intel-gpu-tools, ... still need quite a bit for work): For instance on series #890, I've uploaded checkpatch.pl test results: http://patchwork.freedesktop.org/series/890/ I'll be deploying that checkpatch.pl test in the next week or so as an example of patchwork/test integration. QA is looking into that integration with BATs at the moment. Of course, email/notifications are part of the plan once happy enough with the tests. For the second goal, it's a long process of small improvements. We're really not there, but interesting things have been created already: I'm quite a fan of git-pw to retrieve series from the ml, for those series correctly parsed that is... Which leads me to the last thing: parsing things from the ml is fragile. The main problems are both people and to a lesser extend mailing-lists: using mails to carry patches does have problems, the vast majority of problems come from people though. People will get stuff wrong: attach patches to the wrong message-id, do things that are plain weird like suddenly attaching patch "2.4/10" as a way to say "oh actually insert a 11th patch in the middle there", typo the review tags, ... I think the last part is solvable, by having a tool wrapping git send-email to do the right things, or at least deterministic things, when sending patches and reviews. That's a bit blue sky stuff, I certainly would love to get there eventually though. Thinking about it, it's wouldn't actually be that complicated to have a start of such a tool, I've captured the first simple thoughts here: https://github.com/dlespiau/patchwork/issues/81 Oh well, it's a way longer email than the one I wanted to write. HTH, -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] i-g-t/libdrm email tagging & patchwork
On Thu, Nov 19, 2015 at 12:44:07PM +0200, Jani Nikula wrote: > On Wed, 18 Nov 2015, Daniel Vetter wrote: > > On Sun, Nov 08, 2015 at 12:31:36AM +0000, Damien Lespiau wrote: > >> Hi all, > >> > >> I've added a feature to sort the patches sent to intel-gfx into 3 > >> buckets: i915, intel-gpu-tools and libdrm. This sorting relies on > >> tagging patches, using the subject prefixes (which is what most people > >> do already anyway). > >> > >> - i915 (intel-gfx): catchall project, all mails not matching any of > >> the other 2 projects will end up there > >> > >> - intel-gpu-tools: mails need to be tagged with i-g-t, igt or > >> intel-gpu-tools > >> > >> - libdrm-intel: mails need to be tagged with libdrm > >> > >> This tagging can be set up per git repository with: > >> > >> $ git config format.subjectprefix "PATCH i-g-t" > > > > Is there any way we could push this out to users somehow? I have bazillion > > of machines, I'll get this wrong eventually ... So will everyone else I > > guess. > > Googling around, I don't think we can automatically force this on > people. We could add a script to make it easier for people to set this > up. Either a setup that needs to be re-run every time there are changes, > or a setup that symlinks a git hook back into a file stored in the > repository so changes are deployed automatically. The latter has > security implications, so I'd go for the former. So, we could have: $ git pw init https://patchwork.freedesktop.org/ intel-gpu-tools which would retrieve some server side config and shove it into .gitconfig. That does require a step anyway though, not sure how ideal this is or what else could be interesting to do with such a thing. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/kbl: Enable PW1 and Misc I/O power wells
On Wed, Jan 06, 2016 at 12:08:36PM +, Michel Thierry wrote: > My kbl stopped working because of this. > > Fixes regression from > commit 2f693e28b8df69f67beced5e18bb2b91c2bfcec2 > Author: Damien Lespiau > Date: Wed Nov 4 19:24:12 2015 +0200 > drm/i915: Make turning on/off PW1 and Misc I/O part of the init/fini > sequences > > Cc: Damien Lespiau > Cc: Paulo Zanoni > Cc: Patrik Jakobsson > Cc: Imre Deak > Signed-off-by: Michel Thierry Part of a time where IS_SKYLAKE() would have been true for KBL... Reviewed-by: Damien Lespiau -- Damien > --- > drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > b/drivers/gpu/drm/i915/intel_runtime_pm.c > index ddbdbff..4b44e68 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -1851,7 +1851,7 @@ void skl_pw1_misc_io_init(struct drm_i915_private > *dev_priv) > { > struct i915_power_well *well; > > - if (!IS_SKYLAKE(dev_priv)) > + if (!(IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))) > return; > > well = lookup_power_well(dev_priv, SKL_DISP_PW_1); > @@ -1865,7 +1865,7 @@ void skl_pw1_misc_io_fini(struct drm_i915_private > *dev_priv) > { > struct i915_power_well *well; > > - if (!IS_SKYLAKE(dev_priv)) > + if (!(IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))) > return; > > well = lookup_power_well(dev_priv, SKL_DISP_PW_1); > -- > 2.6.4 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH xf86-video-intel] Sync PCI ids with latest kernel, adding SKL GT4
Syncs with: commit 15620206ae87ba9643ffa6f5ddb5471be7192006 Author: Mika Kuoppala Date: Fri Nov 6 14:11:16 2015 +0200 drm/i915/skl: Add SKL GT4 PCI IDs Signed-off-by: Damien Lespiau --- src/i915_pciids.h | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/i915_pciids.h b/src/i915_pciids.h index f1a113e..f970209 100644 --- a/src/i915_pciids.h +++ b/src/i915_pciids.h @@ -279,12 +279,19 @@ #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 */ \ + INTEL_VGA_DEVICE(0x192A, info) /* SRV GT3 */ -#define INTEL_SKL_IDS(info) \ +#define INTEL_SKL_GT4_IDS(info) \ + INTEL_VGA_DEVICE(0x1932, info), /* DT GT4 */ \ + INTEL_VGA_DEVICE(0x193B, info), /* Halo GT4 */ \ + INTEL_VGA_DEVICE(0x193D, info), /* WKS GT4 */ \ + INTEL_VGA_DEVICE(0x193A, info) /* SRV GT4 */ + +#define INTEL_SKL_IDS(info) \ INTEL_SKL_GT1_IDS(info), \ INTEL_SKL_GT2_IDS(info), \ - INTEL_SKL_GT3_IDS(info) + INTEL_SKL_GT3_IDS(info), \ + INTEL_SKL_GT4_IDS(info) #define INTEL_BXT_IDS(info) \ INTEL_VGA_DEVICE(0x0A84, info), \ -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Handle PipeC fused off on GEN7+
On Wed, Jan 13, 2016 at 04:46:43PM +0200, Gabriel Feceoru wrote: > Starting with Gen7 (IVB) Display PipeC can be fused off on some production > parts. When disabled, display hardware will prevent the pipe C register bit > from being set to 1. > > Fixed by adjusting pipe_count to reflect this. The title is misleading, it's really just IVB/HSW/BDW not gen7+ You want to add the changelog of the patch in the commit message as well, eg. v2: Rename HSW_PIPE_C_DISABLE to IVB_PIPE_C_DISABLE as it already exists on ivybridge (Ville) also, we can get rid of the MMIO access (see below) -- Damien > Signed-off-by: Gabriel Feceoru > --- > drivers/gpu/drm/i915/i915_dma.c | 3 +++ > drivers/gpu/drm/i915/i915_reg.h | 1 + > 2 files changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 44a896c..c3b93e7 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -813,6 +813,9 @@ static void intel_device_info_runtime_init(struct > drm_device *dev) >!(sfuse_strap & SFUSE_STRAP_FUSE_LOCK))) { > DRM_INFO("Display fused off, disabling\n"); > info->num_pipes = 0; > + } else if (I915_READ(FUSE_STRAP) & IVB_PIPE_C_DISABLE) { > + DRM_INFO("PipeC fused off\n"); > + info->num_pipes -= 1; FUSE_STRAP is alreay read above, it's in the fuse_trap variable. HTH, -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Demote user facing DMC firmware load failure message
On Wed, Jan 13, 2016 at 05:38:15PM +, Chris Wilson wrote: > This is an expected error given the lack of the firmware so emit it at > KERN_NOTICE and not KERN_ERROR. Also include the firmware URL in the > user facing message so that the user can investigate and fix the issue > on their own, and also explain the consequence in plain language. > > The complete failure message, including the first line from the firmware > loader, becomes > > i915 :00:02.0: Direct firmware load for i915/skl_dmc_ver1.bin failed with > error -2 > i915 :00:02.0: Failed to load DMC firmware > [https://01.org/linuxgraphics/intel-linux-graphics-firmwares], disabling > runtime power management. > > Signed-off-by: Chris Wilson > Cc: Damien Lespiau > Cc: Imre Deak > Cc: Sunil Kamath > Cc: Daniel Vetter > Cc: Animesh Manna > Cc: Jani Nikula Reviewed-by: Damien Lespiau -- Damien > --- > drivers/gpu/drm/i915/intel_csr.c | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_csr.c > b/drivers/gpu/drm/i915/intel_csr.c > index 3f2850029c17..5c2f9a40c81b 100644 > --- a/drivers/gpu/drm/i915/intel_csr.c > +++ b/drivers/gpu/drm/i915/intel_csr.c > @@ -44,6 +44,8 @@ > #define I915_CSR_SKL "i915/skl_dmc_ver1.bin" > #define I915_CSR_BXT "i915/bxt_dmc_ver1.bin" > > +#define FIRMWARE_URL > "https://01.org/linuxgraphics/intel-linux-graphics-firmwares"; > + > MODULE_FIRMWARE(I915_CSR_SKL); > MODULE_FIRMWARE(I915_CSR_BXT); > > @@ -282,7 +284,7 @@ static uint32_t *parse_csr_fw(struct drm_i915_private > *dev_priv, > csr->version < SKL_CSR_VERSION_REQUIRED) { > DRM_INFO("Refusing to load old Skylake DMC firmware v%u.%u," >" please upgrade to v%u.%u or later" > - " > [https://01.org/linuxgraphics/intel-linux-graphics-firmwares].\n";, > +" [" FIRMWARE_URL "].\n", >CSR_VERSION_MAJOR(csr->version), >CSR_VERSION_MINOR(csr->version), >CSR_VERSION_MAJOR(SKL_CSR_VERSION_REQUIRED), > @@ -400,7 +402,10 @@ out: >CSR_VERSION_MAJOR(csr->version), >CSR_VERSION_MINOR(csr->version)); > } else { > - DRM_ERROR("Failed to load DMC firmware, disabling rpm\n"); > + dev_notice(dev_priv->dev->dev, > +"Failed to load DMC firmware" > +" [" FIRMWARE_URL "]," > +" disabling runtime power management.\n"); > } > > release_firmware(fw); > -- > 2.7.0.rc3 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/7] drm/i915/skl: WA for watermark calculation based on Arbitrated Display BW
On Thu, Jan 14, 2016 at 11:30:31PM +0800, kbuild test robot wrote: > Hi Mahesh, > > [auto build test ERROR on drm-intel/for-linux-next] > [also build test ERROR on next-20160114] > [cannot apply to v4.4] > [if your patch is applied to the wrong git tree, please drop us a note to > help improving the system] > > url: > https://github.com/0day-ci/linux/commits/Shobhit-Kumar/Misc-WM-fixes-and-Arbitrated-Display-Bandwidth-WA-for-SKL/20160114-200444 > base: git://anongit.freedesktop.org/drm-intel for-linux-next > config: i386-defconfig (attached as .config) > reproduce: > # save the attached .config to linux build tree > make ARCH=i386 > > All errors (new ones prefixed by >>): > >drivers/built-in.o: In function `skl_update_wm': > >> intel_pm.c:(.text+0xdcbfb): undefined reference to `__udivdi3' >intel_pm.c:(.text+0xdccb7): undefined reference to `__udivdi3' In case you wonder, compiling for x86 32 bits, this is most likely because DIV_ROUND_UP() uses a stray '/' operator and you use it with 64 bit values, which will make gcc use a run-time helper function that isn't part of the kernel. You need to use DIV_ROUND_UP_ULL(), making sure the second parameter is 32 bits only. HTH, -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] tools/Android.mk: Add zlib support
On Fri, Jan 15, 2016 at 03:49:00PM +, Morton, Derek J wrote: > Can this be merged so IGT on Android builds? No one has raise any > objections since Monday about this patch. Merged, thanks for the patch. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: warning for FBC crtc/fb locking + smaller fixes
On Thu, Jan 21, 2016 at 08:14:34PM +, Zanoni, Paulo R wrote: > Em Ter, 2016-01-19 às 14:50 +, Patchwork escreveu: > > == Summary == > > > > Built on 20c388faff9d8c41ab27e825c685526561b892a2 drm-intel-nightly: > > 2016y-01m-19d-13h-31m-46s UTC integration manifest > > > > Test kms_flip: > > Subgroup basic-flip-vs-modeset: > > pass -> DMESG-WARN (skl-i5k-2) > > The dmesg warn is the following: > > [ 273.515394] [drm:gen8_irq_handler [i915]] *ERROR* The master control > interrupt lied (DE PIPE)! > > I coulnd't find anything showing that this error was present before the > patch series, but I also can't reproduce it on my SKL machine and I > find it hard to believe that this error was introduced by any of the > patches in question. > > So, with the new rules, how do I proceed here? I'm sure we can be pragmatic here. We've seen that one before, at least on BDW. > If the series was shorter I'd just resubmit and see if the RNG allows > me to pass, but sending 25 patches again won't be cool. It would be > good to have a way to ask the CI to run the failed subtests again on > the same patch series in order to confirm things. Right, that's indeed a feature that is planned, while I'd like to finish other things first, I guess it's simple enough that I can bump the priority a bit. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH libdrm] xf86drm: Bound strstr() to the allocated data
We are reading at most sizeof(data) bytes, but then data may not contain a terminating '\0', at least in theory, so strstr() may overflow the stack allocated array. Make sure that data always contains at least one '\0'. Signed-off-by: Damien Lespiau --- xf86drm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xf86drm.c b/xf86drm.c index 7e28b4f..5f587d9 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -2863,7 +2863,7 @@ static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info) { #ifdef __linux__ char path[PATH_MAX + 1]; -char data[128]; +char data[128 + 1]; char *str; int domain, bus, dev, func; int fd, ret; @@ -2874,6 +2874,7 @@ static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info) return -errno; ret = read(fd, data, sizeof(data)); +data[128] = '\0'; close(fd); if (ret < 0) return -errno; -- 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH libdrm] xf86drm: Bound strstr() to the allocated data
On Fri, Jan 22, 2016 at 04:48:05PM +0200, Ville Syrjälä wrote: > On Fri, Jan 22, 2016 at 12:51:23PM +0000, Damien Lespiau wrote: > > We are reading at most sizeof(data) bytes, but then data may not contain > > a terminating '\0', at least in theory, so strstr() may overflow the > > stack allocated array. > > > > Make sure that data always contains at least one '\0'. > > > > Signed-off-by: Damien Lespiau > > --- > > xf86drm.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/xf86drm.c b/xf86drm.c > > index 7e28b4f..5f587d9 100644 > > --- a/xf86drm.c > > +++ b/xf86drm.c > > @@ -2863,7 +2863,7 @@ static int drmParsePciBusInfo(int maj, int min, > > drmPciBusInfoPtr info) > > { > > #ifdef __linux__ > > char path[PATH_MAX + 1]; > > -char data[128]; > > +char data[128 + 1]; > > char *str; > > int domain, bus, dev, func; > > int fd, ret; > > @@ -2874,6 +2874,7 @@ static int drmParsePciBusInfo(int maj, int min, > > drmPciBusInfoPtr info) > > return -errno; > > > > ret = read(fd, data, sizeof(data)); > > +data[128] = '\0'; > > Slightly more paranoid would be something along the lines of > if (ret >= 0) > data[ret] = '\0'; > > But this should be good enough I think so > Reviewed-by: Ville Syrjälä Thanks for the review, pushed! > The other thing I spotted while looking at the code is the fact that it > doesn't check the snprint() return value. But I guess PATH_MAX is big > enough that even if you somehow make maj and min INT_MIN it'll still > fit. Right, doesn't seem we can overflow path[]. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for MAINTAINERS: Add "B:" preferred bug reporting method (rev3)
On Wed, Jan 27, 2016 at 09:16:24AM -0800, Joe Perches wrote: > On Wed, 2016-01-27 at 16:47 +, Patchwork wrote: > > == Summary == > > > > Built on 5ae916607e3e12ba18c848dff42baaad5b118c4b drm-intel-nightly: > > 2016y-01m-27d-12h-48m-36s UTC integration manifest > > I've no idea what this means. > I'm not sure I care. > Why send this to me? We're experimenting with patchwork and trying to teach it about series of patches and hooking automatic test systems to it. Unfortunately, you've been the victim of a number of problems: - A mail of yours was parsed as a patch because it had a patch suggestion in it, which made that diff a candidate for automatic testing. http://lists.freedesktop.org/archives/intel-gfx/2016-January/085483.html I just fixed it manually, you shouldn't receive any more email. A better fix will land soon, your shouldn't have been a candidate for testing in the first place as we're supposed to only consider git send-email patches (an configuration option). - We still have a few flaky tests, so the test result was incoherent with just changing a text file - We may be to refine the system to not bother testing patches that don't touch code. In case you are curious: http://patchwork.freedesktop.org/project/intel-gfx/series/ http://patchwork.freedesktop.org/series/2539/ Sorry for the trouble, -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915/skl/kbl: Add support for pipe fusing
On Wed, Jan 20, 2016 at 03:31:20PM +0100, Patrik Jakobsson wrote: > On SKL and KBL we can have pipe A/B/C disabled by fuse settings. The > pipes must be fused in descending order (e.g. C, B+C, A+B+C). We simply > decrease info->num_pipes if we find a valid fused out config. > > v2: Don't store the pipe disabled mask in device info (Damien) > > v3: Don't check FUSE_STRAP register for pipe c disabled > > Cc: Damien Lespiau > Signed-off-by: Patrik Jakobsson Reviewed-by: Damien Lespiau (listing the valid cases would have made things simpler?) -- Damien > --- > drivers/gpu/drm/i915/i915_dma.c | 31 +++ > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > 2 files changed, 34 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 44a896c..daaa67f 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -814,6 +814,37 @@ static void intel_device_info_runtime_init(struct > drm_device *dev) > DRM_INFO("Display fused off, disabling\n"); > info->num_pipes = 0; > } > + } else if (info->num_pipes > 0 && INTEL_INFO(dev)->gen == 9) { > + u32 dfsm = I915_READ(SKL_DFSM); > + u8 disabled_mask = 0; > + bool invalid; > + int num_bits; > + > + if (dfsm & SKL_DFSM_PIPE_A_DISABLE) > + disabled_mask |= BIT(PIPE_A); > + if (dfsm & SKL_DFSM_PIPE_B_DISABLE) > + disabled_mask |= BIT(PIPE_B); > + if (dfsm & SKL_DFSM_PIPE_C_DISABLE) > + disabled_mask |= BIT(PIPE_C); > + > + num_bits = hweight8(disabled_mask); > + > + switch (disabled_mask) { > + case BIT(PIPE_A): > + case BIT(PIPE_B): > + case BIT(PIPE_A) | BIT(PIPE_B): > + case BIT(PIPE_A) | BIT(PIPE_C): > + invalid = true; > + break; > + default: > + invalid = false; > + } > + > + if (num_bits > info->num_pipes || invalid) > + DRM_ERROR("invalid pipe fuse configuration: 0x%x\n", > + disabled_mask); > + else > + info->num_pipes -= num_bits; > } > > /* Initialize slice/subslice/EU info */ > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 556a458..c6e6a24 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -5991,6 +5991,9 @@ enum skl_disp_power_wells { > #define SKL_DFSM_CDCLK_LIMIT_540 (1 << 23) > #define SKL_DFSM_CDCLK_LIMIT_450 (2 << 23) > #define SKL_DFSM_CDCLK_LIMIT_337_5 (3 << 23) > +#define SKL_DFSM_PIPE_A_DISABLE (1 << 30) > +#define SKL_DFSM_PIPE_B_DISABLE (1 << 21) > +#define SKL_DFSM_PIPE_C_DISABLE (1 << 28) > > #define FF_SLICE_CS_CHICKEN2 _MMIO(0x20e4) > #define GEN9_TSG_BARRIER_ACK_DISABLE(1<<8) > -- > 2.5.0 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/18] drm/i915: Create a more generic pm handler for hsw+
On Sat, Apr 27, 2013 at 05:59:19PM -0700, Ben Widawsky wrote: > HSW has some special requirements for the VEBOX. Splitting out the > interrupt handler will make the code a bit nicer and less error prone > when we begin to handle those. > > The slight functional change in this patch (queueing work while holding > the spinlock) is intentional as it makes a subsequent patch a bit nicer. > The change should also only effect HSW platforms. > > Based on patches from: > CC: Haihao Xiang > Signed-off-by: Ben Widawsky Reviewed-by: Damien Lespiau -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/18] drm/i915: make PM interrupt writes non-destructive
On Sat, Apr 27, 2013 at 05:59:20PM -0700, Ben Widawsky wrote: > @@ -2720,12 +2720,12 @@ static void gen6_enable_rps(struct drm_device *dev) > gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8); > > /* requires MSI enabled */ > - I915_WRITE(GEN6_PMIER, GEN6_PM_DEFERRED_EVENTS); > + I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) | GEN6_PM_RPS_EVENTS); > spin_lock_irq(&dev_priv->rps.lock); > WARN_ON(dev_priv->rps.pm_iir != 0); > - I915_WRITE(GEN6_PMIMR, 0); > + I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS); You're not unmasking the RPS interrupts in PMIMR here now. I'm missing how they are enabled now. > spin_unlock_irq(&dev_priv->rps.lock); > - /* enable all PM interrupts */ > + /* unmask all PM interrupts */ > I915_WRITE(GEN6_PMINTRMSK, 0); > > rc6vids = 0; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/18] drm/i915: Create an ivybridge_irq_preinstall
On Sat, Apr 27, 2013 at 05:59:21PM -0700, Ben Widawsky wrote: > Signed-off-by: Ben Widawsky > --- > drivers/gpu/drm/i915/i915_irq.c | 27 ++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 13ea6c2..21b09cd 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2502,6 +2502,31 @@ static void ironlake_irq_preinstall(struct drm_device > *dev) > POSTING_READ(SDEIER); > } > > +static void ivybridge_irq_preinstall(struct drm_device *dev) > +{ > + drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; > + > + atomic_set(&dev_priv->irq_received, 0); > + > + I915_WRITE(HWSTAM, 0xeffe); > + > + /* XXX hotplug from PCH */ > + > + I915_WRITE(DEIMR, 0x); > + I915_WRITE(DEIER, 0x0); > + POSTING_READ(DEIER); > + > + /* and GT */ > + I915_WRITE(GTIMR, 0x); > + I915_WRITE(GTIER, 0x0); > + POSTING_READ(GTIER); you're missing a: if (HAS_PCH_NOP(dev)) return; that you've added in the ironlake path since that patch. > + > + /* south display irq */ > + I915_WRITE(SDEIMR, 0x); > + I915_WRITE(SDEIER, 0x0); > + POSTING_READ(SDEIER); > +} SDEIER is set to 0x now with a big comment explaining why. > static void valleyview_irq_preinstall(struct drm_device *dev) > { > drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private; > @@ -3500,7 +3525,7 @@ void intel_irq_init(struct drm_device *dev) > } else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) { > /* Share pre & uninstall handlers with ILK/SNB */ > dev->driver->irq_handler = ivybridge_irq_handler; > - dev->driver->irq_preinstall = ironlake_irq_preinstall; > + dev->driver->irq_preinstall = ivybridge_irq_preinstall; > dev->driver->irq_postinstall = ivybridge_irq_postinstall; > dev->driver->irq_uninstall = ironlake_irq_uninstall; > dev->driver->enable_vblank = ivybridge_enable_vblank; > -- > 1.8.2.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] [PATCH 11/18] drm/i915: Add PM regs to pre install
On Sat, Apr 27, 2013 at 05:59:22PM -0700, Ben Widawsky wrote: > Signed-off-by: Ben Widawsky > --- > drivers/gpu/drm/i915/i915_irq.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 21b09cd..4a1b7f5 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2525,6 +2525,11 @@ static void ivybridge_irq_preinstall(struct drm_device > *dev) > I915_WRITE(SDEIMR, 0x); > I915_WRITE(SDEIER, 0x0); > POSTING_READ(SDEIER); > + > + /* Power management */ > + I915_WRITE(GEN6_PMIMR, 0x); > + I915_WRITE(GEN6_PMIER, 0x0); > + POSTING_READ(GEN6_PMIER); > } With the comments on the previous patch, I guess this shoulde be put before the if (HAS_PCH_NOP) return; block and thus before the DE init. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/18] drm/i915: Convert irq_refounct to struct
On Sat, Apr 27, 2013 at 05:59:23PM -0700, Ben Widawsky wrote: > It's overkill on older gens, but it's useful for newer gens. > > Signed-off-by: Ben Widawsky Reviewed-by: Damien Lespiau -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/18] drm/i915: consolidate interrupt naming scheme
On Sat, Apr 27, 2013 at 05:59:24PM -0700, Ben Widawsky wrote: > The motivation here is we're going to add some new interrupt definitions > and handling outside of the GT interrupts which is all we've managed so > far (with some RPS exceptions). By consolidating the names in the future > we can make thing a bit cleaner as we don't need to define register > names twice, and we can leverage pretty decent overlap in HW registers > since ILK. > > To explain briefly what is in the comments: there are two sets of > interrupt masking/enabling registers. At least so far, the definitions > of the two sets overlap. The old code setup distinct names for > interrupts in each set, ie. one for global, and one for ring. This made > things confusing when using the wrong defines in the wrong places. > > rebase: Modified VLV bits > > Signed-off-by: Ben Widawsky With or without the naming biskeshed below: Reviewed-by: Damien Lespiau -- Damien > --- > drivers/gpu/drm/i915/i915_irq.c | 58 +- > drivers/gpu/drm/i915/i915_reg.h | 101 > ++-- > drivers/gpu/drm/i915/intel_ringbuffer.c | 17 +++--- > 3 files changed, 79 insertions(+), 97 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 4a1b7f5..06e254a 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -781,7 +781,7 @@ static void ivybridge_parity_work(struct work_struct > *work) > I915_WRITE(GEN7_MISCCPCTL, misccpctl); > > spin_lock_irqsave(&dev_priv->irq_lock, flags); > - dev_priv->gt_irq_mask &= ~GT_GEN7_L3_PARITY_ERROR_INTERRUPT; > + dev_priv->gt_irq_mask &= ~GT_RENDER_L3_PARITY_ERROR_INTERRUPT; > I915_WRITE(GTIMR, dev_priv->gt_irq_mask); > spin_unlock_irqrestore(&dev_priv->irq_lock, flags); > > @@ -813,7 +813,7 @@ static void ivybridge_handle_parity_error(struct > drm_device *dev) > return; > > spin_lock_irqsave(&dev_priv->irq_lock, flags); > - dev_priv->gt_irq_mask |= GT_GEN7_L3_PARITY_ERROR_INTERRUPT; > + dev_priv->gt_irq_mask |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT; > I915_WRITE(GTIMR, dev_priv->gt_irq_mask); > spin_unlock_irqrestore(&dev_priv->irq_lock, flags); > > @@ -825,22 +825,21 @@ static void snb_gt_irq_handler(struct drm_device *dev, > u32 gt_iir) > { > > - if (gt_iir & (GEN6_RENDER_USER_INTERRUPT | > - GEN6_RENDER_PIPE_CONTROL_NOTIFY_INTERRUPT)) > + if (gt_iir & (GT_RENDER_USER_INTERRUPT | > GT_RENDER_PIPECTL_NOTIFY_INTERRUPT)) > notify_ring(dev, &dev_priv->ring[RCS]); > - if (gt_iir & GEN6_BSD_USER_INTERRUPT) > + if (gt_iir & GT_BSD_USER_INTERRUPT) > notify_ring(dev, &dev_priv->ring[VCS]); > - if (gt_iir & GEN6_BLITTER_USER_INTERRUPT) > + if (gt_iir & GT_BLT_USER_INTERRUPT) > notify_ring(dev, &dev_priv->ring[BCS]); > > - if (gt_iir & (GT_GEN6_BLT_CS_ERROR_INTERRUPT | > - GT_GEN6_BSD_CS_ERROR_INTERRUPT | > - GT_RENDER_CS_ERROR_INTERRUPT)) { > + if (gt_iir & (GT_BLT_CS_ERROR_INTERRUPT | > + GT_BSD_CS_ERROR_INTERRUPT | > + GT_RENDER_MASTER_ERROR_INTERRUPT)) { If we ware in the naming domain here, not sure why the CS master error for render would have a different name than the others, GT_RENDER_CS_ERROR_INTERRUPT looked good to me. > DRM_ERROR("GT error interrupt 0x%08x\n", gt_iir); > i915_handle_error(dev, false); > } > > - if (gt_iir & GT_GEN7_L3_PARITY_ERROR_INTERRUPT) > + if (gt_iir & GT_RENDER_L3_PARITY_ERROR_INTERRUPT) > ivybridge_handle_parity_error(dev); > } > > @@ -1284,9 +1283,9 @@ static void ilk_gt_irq_handler(struct drm_device *dev, > struct drm_i915_private *dev_priv, > u32 gt_iir) > { > - if (gt_iir & (GT_USER_INTERRUPT | GT_PIPE_NOTIFY)) > + if (gt_iir & (GT_RENDER_USER_INTERRUPT | > GT_RENDER_PIPECTL_NOTIFY_INTERRUPT)) > notify_ring(dev, &dev_priv->ring[RCS]); > - if (gt_iir & GT_BSD_USER_INTERRUPT) > + if (gt_iir & ILK_BSD_USER_INTERRUPT) > notify_ring(dev, &dev_priv->ring[VCS]); > } > > @@ -2629,7 +2628,7 @@ static int ironlake_irq_postinstall(struct drm_device > *dev) > DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE | > DE_AUX_CH
Re: [Intel-gfx] [PATCH 14/18] drm/i915: vebox interrupt get/put
On Sat, Apr 27, 2013 at 05:59:25PM -0700, Ben Widawsky wrote: > v2: Use the correct lock to protect PM interrupt regs, this was > accidentally lost from earlier (Haihao) > Fix return types (Ben) > > CC: Xiang, Haihao > Signed-off-by: Ben Widawsky > --- Reviewed-by: Damien Lespiau -- Damien > drivers/gpu/drm/i915/intel_ringbuffer.c | 46 > +++-- > drivers/gpu/drm/i915/intel_ringbuffer.h | 5 ++-- > 2 files changed, 47 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > b/drivers/gpu/drm/i915/intel_ringbuffer.c > index ccfa1f9..93a3128 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1005,6 +1005,48 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring) > gen6_gt_force_wake_put(dev_priv); > } > > +static bool > +hsw_vebox_get_irq(struct intel_ring_buffer *ring) > +{ > + struct drm_device *dev = ring->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + unsigned long flags; > + > + if (!dev->irq_enabled) > + return false; > + > + spin_lock_irqsave(&dev_priv->rps.lock, flags); > + if (ring->irq_refcount.pm++ == 0) { > + u32 pm_imr = I915_READ(GEN6_PMIMR); > + I915_WRITE_IMR(ring, ~ring->irq_enable_mask); > + I915_WRITE(GEN6_PMIMR, pm_imr & ~ring->irq_enable_mask); > + POSTING_READ(GEN6_PMIMR); > + } > + spin_unlock_irqrestore(&dev_priv->rps.lock, flags); > + > + return true; > +} > + > +static void > +hsw_vebox_put_irq(struct intel_ring_buffer *ring) > +{ > + struct drm_device *dev = ring->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + unsigned long flags; > + > + if (!dev->irq_enabled) > + return; > + > + spin_lock_irqsave(&dev_priv->rps.lock, flags); > + if (--ring->irq_refcount.pm == 0) { > + u32 pm_imr = I915_READ(GEN6_PMIMR); > + I915_WRITE_IMR(ring, ~0); > + I915_WRITE(GEN6_PMIMR, pm_imr | ring->irq_enable_mask); > + POSTING_READ(GEN6_PMIMR); > + } > + spin_unlock_irqrestore(&dev_priv->rps.lock, flags); > +} > + > static int > i965_dispatch_execbuffer(struct intel_ring_buffer *ring, >u32 offset, u32 length, > @@ -1913,8 +1955,8 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev) > ring->get_seqno = gen6_ring_get_seqno; > ring->set_seqno = ring_set_seqno; > ring->irq_enable_mask = 0; > - ring->irq_get = NULL; > - ring->irq_put = NULL; > + ring->irq_get = hsw_vebox_get_irq; > + ring->irq_put = hsw_vebox_put_irq; > ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer; > ring->sync_to = gen6_ring_sync; > ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_VER; > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h > b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 24b4413..d040dae 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -69,8 +69,9 @@ struct intel_ring_buffer { > u32 last_retired_head; > > struct { > - u32 gt; > - } irq_refcount; /* protected by dev_priv->irq_lock */ > + u32 gt; /* protected by dev_priv->irq_lock */ > + u32 pm; /* protected by dev_priv->rps.lock (sucks) */ > + } irq_refcount; > u32 irq_enable_mask;/* bitmask to enable ring > interrupt */ > u32 trace_irq_seqno; > u32 sync_seqno[I915_NUM_RINGS-1]; > -- > 1.8.2.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] [PATCH 15/18] drm/i915: Enable vebox interrupts
On Sat, Apr 27, 2013 at 05:59:26PM -0700, Ben Widawsky wrote: > Similar to a patch originally written by: > > v2: Reversed the meanings of masked and enabled (Haihao) > Made non-destructive writes in case enable/disabler rps runs first > (Haihao) > > CC: Xiang, Haihao > Signed-off-by: Ben Widawsky With or without the bikeshed below: Reviewed-by: Damien Lespiau -- Damien > --- > drivers/gpu/drm/i915/i915_irq.c | 26 -- > drivers/gpu/drm/i915/i915_reg.h | 3 +++ > drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +- > 3 files changed, 28 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 06e254a..ae2ee9d 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -944,8 +944,15 @@ static void hsw_pm_irq_handler(struct drm_i915_private > *dev_priv, > } > spin_unlock_irqrestore(&dev_priv->rps.lock, flags); > > - if (pm_iir & ~GEN6_PM_RPS_EVENTS) > - DRM_ERROR("Unexpected PM interrupted\n"); > + if (pm_iir & ~GEN6_PM_RPS_EVENTS) { > + if (pm_iir & PM_VEBOX_USER_INTERRUPT) > + notify_ring(dev_priv->dev, &dev_priv->ring[VECS]); > + > + if (pm_iir & PM_VEBOX_CS_ERROR_INTERRUPT) { > + DRM_ERROR("PM error interrupt 0x%08x\n", pm_iir); Maybe we could even be a bit more explicit here, saying it's a command stream/parser error. > + i915_handle_error(dev_priv->dev, false); > + } > + } > } > > static irqreturn_t valleyview_irq_handler(int irq, void *arg) > @@ -2701,6 +2708,21 @@ static int ivybridge_irq_postinstall(struct drm_device > *dev) > I915_WRITE(GTIER, gt_irqs); > POSTING_READ(GTIER); > > + I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR)); > + if (HAS_VEBOX(dev)) { > + u32 pm_irqs, pmier, pmimr; > + pm_irqs = PM_VEBOX_USER_INTERRUPT | PM_VEBOX_CS_ERROR_INTERRUPT; > + > + /* Our enable/disable rps functions may touch these registers so > + * make sure to set a known state for only the non-RPS bits. */ > + pmier = (I915_READ(GEN6_PMIER) & GEN6_PM_RPS_EVENTS) | pm_irqs; > + pmimr = (I915_READ(GEN6_PMIMR) | ~GEN6_PM_RPS_EVENTS) & > ~pm_irqs; > + I915_WRITE(GEN6_PMIMR, pmimr); > + I915_WRITE(GEN6_PMIER, pmier); > + } > + > + POSTING_READ(GEN6_PMIER); > + > ibx_irq_postinstall(dev); > > return 0; > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 399d041..9e8b8b4 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -860,6 +860,9 @@ > #define GT_RENDER_DEBUG_INTERRUPT(1 << 1) > #define GT_RENDER_USER_INTERRUPT (1 << 0) > > +#define PM_VEBOX_CS_ERROR_INTERRUPT (1 << 12) /* hsw+ */ > +#define PM_VEBOX_USER_INTERRUPT (1 << 10) /* hsw+ */ > + > /* These are all the "old" interrupts */ > #define ILK_BSD_USER_INTERRUPT (1<<5) > #define I915_PIPE_CONTROL_NOTIFY_INTERRUPT (1<<18) > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 93a3128..30f22e1 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1954,7 +1954,7 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev) > ring->add_request = gen6_add_request; > ring->get_seqno = gen6_ring_get_seqno; > ring->set_seqno = ring_set_seqno; > - ring->irq_enable_mask = 0; > + ring->irq_enable_mask = PM_VEBOX_USER_INTERRUPT | > PM_VEBOX_CS_ERROR_INTERRUPT; > ring->irq_get = hsw_vebox_get_irq; > ring->irq_put = hsw_vebox_put_irq; > ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer; > -- > 1.8.2.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] [PATCH 16/18] drm/i915: add VEBOX into debugfs
On Sat, Apr 27, 2013 at 05:59:27PM -0700, Ben Widawsky wrote: > From: "Xiang, Haihao" > > v2 (Ben): s/hsw/hws > > Signed-off-by: Xiang, Haihao > [Order changed, and modified by] > Signed-off-by: Ben Widawsky > --- > drivers/gpu/drm/i915/i915_debugfs.c | 13 + > 1 file changed, 13 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index a55630a..71fb7aa 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -379,6 +379,17 @@ static int i915_gem_request_info(struct seq_file *m, > void *data) > } > count++; > } > + if (!list_empty(&dev_priv->ring[VECS].request_list)) { > + seq_printf(m, "VEBOX requests:\n"); > + list_for_each_entry(gem_request, > + &dev_priv->ring[VECS].request_list, > + list) { > + seq_printf(m, "%d @ %d\n", > +gem_request->seqno, > +(int) (jiffies - > gem_request->emitted_jiffies)); > + } > + count++; > + } > mutex_unlock(&dev->struct_mutex); That block doesn't seem necessary as the code above this chuck cycles over all the rings? > > if (count == 0) > @@ -570,6 +581,7 @@ static const char *ring_str(int ring) > case RCS: return "render"; > case VCS: return "bsd"; > case BCS: return "blt"; > + case VECS: return "vebox"; > default: return ""; > } > } > @@ -2099,6 +2111,7 @@ static struct drm_info_list i915_debugfs_list[] = { > {"i915_gem_hws", i915_hws_info, 0, (void *)RCS}, > {"i915_gem_hws_blt", i915_hws_info, 0, (void *)BCS}, > {"i915_gem_hws_bsd", i915_hws_info, 0, (void *)VCS}, > + {"i915_gem_hws_vebox", i915_hws_info, 0, (void *)VECS}, > {"i915_rstdby_delays", i915_rstdby_delays, 0}, > {"i915_cur_delayinfo", i915_cur_delayinfo, 0}, > {"i915_delayfreq_table", i915_delayfreq_table, 0}, > -- > 1.8.2.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] [PATCH 17/18] drm/i915: add I915_EXEC_VEBOX to i915_gem_do_execbuffer()
On Sat, Apr 27, 2013 at 05:59:28PM -0700, Ben Widawsky wrote: > From: "Xiang, Haihao" > > A user can run batchbuffer via VEBOX ring. > > Signed-off-by: Xiang, Haihao > [Order changed by] > Signed-off-by: Ben Widawsky Reviewed-by: Damien Lespiau -- Damien > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 9 + > include/uapi/drm/i915_drm.h| 1 + > 2 files changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 117ce38..a8bb62c 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -885,6 +885,15 @@ i915_gem_do_execbuffer(struct drm_device *dev, void > *data, > return -EPERM; > } > break; > + case I915_EXEC_VEBOX: > + ring = &dev_priv->ring[VECS]; > + if (ctx_id != 0) { > + DRM_DEBUG("Ring %s doesn't support contexts\n", > + ring->name); > + return -EPERM; > + } > + break; > + > default: > DRM_DEBUG("execbuf with unknown ring: %d\n", > (int)(args->flags & I915_EXEC_RING_MASK)); > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 07d5941..81b9981 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -660,6 +660,7 @@ struct drm_i915_gem_execbuffer2 { > #define I915_EXEC_RENDER (1<<0) > #define I915_EXEC_BSD(2<<0) > #define I915_EXEC_BLT(3<<0) > +#define I915_EXEC_VEBOX (4<<0) > > /* Used for switching the constants addressing mode on gen4+ RENDER ring. > * Gen6+ only supports relative addressing to dynamic state (default) and > -- > 1.8.2.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] [PATCH 18/18] drm/i915: add I915_PARAM_HAS_VEBOX to i915_getparam
On Sat, Apr 27, 2013 at 05:59:29PM -0700, Ben Widawsky wrote: > From: "Xiang, Haihao" > > This will let userland only try to use the new ring > when the appropriate kernel is present > > Signed-off-by: Xiang, Haihao > [Order changed, and merge conflict resolved by] > Signed-off-by: Ben Widawsky Reviewed-by: Damien Lespiau -- Damien > --- > drivers/gpu/drm/i915/i915_dma.c | 3 +++ > include/uapi/drm/i915_drm.h | 2 +- > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index a1648eb..66a1e39 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -956,6 +956,9 @@ static int i915_getparam(struct drm_device *dev, void > *data, > case I915_PARAM_HAS_BLT: > value = intel_ring_initialized(&dev_priv->ring[BCS]); > break; > + case I915_PARAM_HAS_VEBOX: > + value = intel_ring_initialized(&dev_priv->ring[VECS]); > + break; > case I915_PARAM_HAS_RELAXED_FENCING: > value = 1; > break; > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h > index 81b9981..923ed7f 100644 > --- a/include/uapi/drm/i915_drm.h > +++ b/include/uapi/drm/i915_drm.h > @@ -305,7 +305,7 @@ typedef struct drm_i915_irq_wait { > #define I915_PARAM_HAS_WAIT_TIMEOUT 19 > #define I915_PARAM_HAS_SEMAPHORES 20 > #define I915_PARAM_HAS_PRIME_VMAP_FLUSH 21 > -#define I915_PARAM_RSVD_FOR_FUTURE_USE22 > +#define I915_PARAM_HAS_VEBOX 22 > #define I915_PARAM_HAS_SECURE_BATCHES 23 > #define I915_PARAM_HAS_PINNED_BATCHES 24 > #define I915_PARAM_HAS_EXEC_NO_RELOC 25 > -- > 1.8.2.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] [PATCH 13/18] drm/i915: consolidate interrupt naming scheme
On Tue, May 28, 2013 at 11:50:46AM -0700, Ben Widawsky wrote: > > > - if (gt_iir & (GT_GEN6_BLT_CS_ERROR_INTERRUPT | > > > - GT_GEN6_BSD_CS_ERROR_INTERRUPT | > > > - GT_RENDER_CS_ERROR_INTERRUPT)) { > > > + if (gt_iir & (GT_BLT_CS_ERROR_INTERRUPT | > > > + GT_BSD_CS_ERROR_INTERRUPT | > > > + GT_RENDER_MASTER_ERROR_INTERRUPT)) { > > > > If we ware in the naming domain here, not sure why the CS master error > > for render would have a different name than the others, > > GT_RENDER_CS_ERROR_INTERRUPT looked good to me. > > I was just copying the docs. I presume on earlier gens, maybe it meant > something else? It seems I accidently dropped the "CS" part though. I've > added that back, and left the MASTER. > > "Render Command Parser Master Error" They are all called $engine Command Parser Master Error, my point was they should look alike (and I'd just drop the master, I think it's just because it means an aggregate of the possible CS errors (2 at most as far as I can see, privilege error and bad command). -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/18] [v2] drm/i915: Comments for semaphore clarification
On Tue, May 28, 2013 at 07:22:17PM -0700, Ben Widawsky wrote: > Semaphores are tied very closely to the rings in the GPU. Trivial patch > adds comments to the existing code so that when we add new rings we can > include comments there as well. It also helps distinguish the ring to > semaphore mailbox interactions by using the ringname in the semaphore > data structures. > > This patch should have no functional impact. > > v2: The English parts (as opposed to register names) of the comments > were reversed. (Damien) > > Signed-off-by: Ben Widawsky Reviewed-by: Damien Lespiau -- Damien > --- > drivers/gpu/drm/i915/i915_reg.h | 12 ++-- > drivers/gpu/drm/i915/intel_ringbuffer.c | 18 +- > drivers/gpu/drm/i915/intel_ringbuffer.h | 4 ++-- > 3 files changed, 17 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index dbd9de5..6579d0c 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -265,12 +265,12 @@ > #define MI_SEMAPHORE_UPDATE (1<<21) > #define MI_SEMAPHORE_COMPARE(1<<20) > #define MI_SEMAPHORE_REGISTER (1<<18) > -#define MI_SEMAPHORE_SYNC_RV(2<<16) > -#define MI_SEMAPHORE_SYNC_RB(0<<16) > -#define MI_SEMAPHORE_SYNC_VR(0<<16) > -#define MI_SEMAPHORE_SYNC_VB(2<<16) > -#define MI_SEMAPHORE_SYNC_BR(2<<16) > -#define MI_SEMAPHORE_SYNC_BV(0<<16) > +#define MI_SEMAPHORE_SYNC_RB(0<<16) /* BCS wait for RCS > (BRSYNC) */ > +#define MI_SEMAPHORE_SYNC_RV(2<<16) /* VCS wait for RCS > (VRSYNC) */ > +#define MI_SEMAPHORE_SYNC_VR(0<<16) /* RCS wait for VCS > (RVSYNC) */ > +#define MI_SEMAPHORE_SYNC_VB(2<<16) /* BCS wait for VCS > (BVSYNC) */ > +#define MI_SEMAPHORE_SYNC_BV(0<<16) /* VCS wait for BCS > (VBSYNC) */ > +#define MI_SEMAPHORE_SYNC_BR(2<<16) /* RCS wait for BCS > (RBSYNC) */ > #define MI_SEMAPHORE_SYNC_INVALID (1<<0) > /* > * 3D instructions used by the kernel > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 9b97cf6..2d2a362 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1671,9 +1671,9 @@ int intel_init_render_ring_buffer(struct drm_device > *dev) > ring->get_seqno = gen6_ring_get_seqno; > ring->set_seqno = ring_set_seqno; > ring->sync_to = gen6_ring_sync; > - ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_INVALID; > - ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_RV; > - ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_RB; > + ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID; > + ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV; > + ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB; > ring->signal_mbox[0] = GEN6_VRSYNC; > ring->signal_mbox[1] = GEN6_BRSYNC; > } else if (IS_GEN5(dev)) { > @@ -1830,9 +1830,9 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev) > ring->irq_put = gen6_ring_put_irq; > ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer; > ring->sync_to = gen6_ring_sync; > - ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_VR; > - ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_INVALID; > - ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_VB; > + ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_VR; > + ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID; > + ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VB; > ring->signal_mbox[0] = GEN6_RVSYNC; > ring->signal_mbox[1] = GEN6_BVSYNC; > } else { > @@ -1876,9 +1876,9 @@ int intel_init_blt_ring_buffer(struct drm_device *dev) > ring->irq_put = gen6_ring_put_irq; > ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer; > ring->sync_to = gen6_ring_sync; > - ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_BR; > - ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_BV; > - ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_INVALID; > + ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_BR; > + ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_BV; > + ring->semaphore_reg
Re: [Intel-gfx] [PATCH 02/18] drm/i915: Semaphore MBOX update generalization
On Tue, May 28, 2013 at 07:22:18PM -0700, Ben Widawsky wrote: > This replaces the existing MBOX update code with a more generalized > calculation for emitting mbox updates. We also create a sentinel for > doing the updates so we can more abstractly deal with the rings. > > When doing MBOX updates the code must be aware of the /other/ rings. > Until now the platforms which supported semaphores had a fixed number of > rings and so it made sense for the code to be very specialized > (hardcoded). > > The patch does contain a functional change, but should have no > behavioral changes. > > Signed-off-by: Ben Widawsky Reviewed-by: Damien Lespiau -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/18] [v2] drm/i915: Add VECS semaphore bits
On Tue, May 28, 2013 at 07:22:20PM -0700, Ben Widawsky wrote: > Like the other rings, the VECS supports semaphores. The semaphore stuff > is a bit wonky so this patch on it's own should be nice for review. > > This patch should have no functional impact. > > v2: Fix the English parts of clarification (again, register names were > right, text was reversed) (Damien) > Restore the still valid invariant. (Damien) > The bsd semaphore register should be MI_SEMAPHORE_SYNC_VVE (Damien) > > Signed-off-by: Ben Widawsky Reviewed-by: Damien Lespiau -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/18] [v2] drm/i915: Create an ivybridge_irq_preinstall
On Tue, May 28, 2013 at 07:22:25PM -0700, Ben Widawsky wrote: > v2: Add new PCH_NOP check (Damien) > Add SDEIMR comment (Damien) > > Signed-off-by: Ben Widawsky Reviewed-by: Damien Lespiau -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 16/18] [v3] drm/i915: add VEBOX into debugfs
On Wed, May 29, 2013 at 09:22:36AM -0700, Ben Widawsky wrote: > From: "Xiang, Haihao" > > v2: Removed rebase relic VECS ring from i915_gem_request_info (Damien) > > v3: s/hsw/hws in debugfs which I introduced in v2 (Jon) > > Signed-off-by: Xiang, Haihao > [Order changed, and modified by] > CC: "Bloomfield, Jon" > Signed-off-by: Ben Widawsky > --- Reviewed-by: Damien Lespiau -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/18] [v5] drm/i915: make PM interrupt writes non-destructive
On Tue, May 28, 2013 at 07:22:27PM -0700, Ben Widawsky wrote: > PM interrupts have an expanded role on HSW. It helps route the EBOX > interrupts. This patch is necessary to make the existing code which > touches the mask, and enable registers more friendly to other code paths > that also will need these registers. > > To be more explicit: > At preinstall all interrupts are masked and disabled. This implies that > preinstall should always happen before any enabling/disabling of RPS or > other interrupts. > > The PMIMR is touched by the workqueue, so enable/disable touch IER and > IIR. Similarly, the code currently expects IMR has no use outside of the > RPS related interrupts so they unconditionally set 0, or ~0. We could > use IER in the workqueue, and IMR elsewhere, but since the workqueue > use-case is more transient the existing usage makes sense. > > Disable RPS events: > IER := IER & ~GEN6_PM_RPS_EVENTS // Disable RPS related interrupts > IIR := GEN6_PM_RPS_EVENTS // Disable any outstanding interrupts > > Enable RPS events: > IER := IER | GEN6_PM_RPS_EVENTS // Enable the RPS related interrupts > IIR := GEN6_PM_RPS_EVENTS // Make sure there were no leftover events > (really shouldn't happen) > > v2: Shouldn't destroy PMIIR or PMIMR VEBOX interrupt state in > enable/disable rps functions (Haihao) > > v3: Bug found by Chris where we were clearing the wrong bits at rps > disable. > expanded commit message > > v4: v3 was based off the wrong branch > > v5: Added the setting of PMIMR because of previous patch update > > CC: Chris Wilson > Signed-off-by: Ben Widawsky > --- Reviewed-by: Damien Lespiau -- Damien > drivers/gpu/drm/i915/i915_irq.c | 21 +++-- > drivers/gpu/drm/i915/i915_reg.h | 2 +- > drivers/gpu/drm/i915/intel_pm.c | 13 +++-- > 3 files changed, 19 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 9c66fcf..8da936d 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -700,10 +700,11 @@ static void gen6_pm_rps_work(struct work_struct *work) > pm_iir = dev_priv->rps.pm_iir; > dev_priv->rps.pm_iir = 0; > pm_imr = I915_READ(GEN6_PMIMR); > - I915_WRITE(GEN6_PMIMR, 0); > + /* Make sure not to corrupt PMIMR state used by ringbuffer code */ > + I915_WRITE(GEN6_PMIMR, pm_imr & ~GEN6_PM_RPS_EVENTS); > spin_unlock_irq(&dev_priv->rps.lock); > > - if ((pm_iir & GEN6_PM_DEFERRED_EVENTS) == 0) > + if ((pm_iir & GEN6_PM_RPS_EVENTS) == 0) > return; > > mutex_lock(&dev_priv->rps.hw_lock); > @@ -933,17 +934,17 @@ static void hsw_pm_irq_handler(struct drm_i915_private > *dev_priv, > unsigned long flags; > > spin_lock_irqsave(&dev_priv->rps.lock, flags); > - dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_DEFERRED_EVENTS; > + dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_RPS_EVENTS; > if (dev_priv->rps.pm_iir) { > I915_WRITE(GEN6_PMIMR, dev_priv->rps.pm_iir); > /* never want to mask useful interrupts. (also posting read) */ > - WARN_ON(I915_READ_NOTRACE(GEN6_PMIMR) & > ~GEN6_PM_DEFERRED_EVENTS); > + WARN_ON(I915_READ_NOTRACE(GEN6_PMIMR) & ~GEN6_PM_RPS_EVENTS); > /* TODO: if queue_work is slow, move it out of the spinlock */ > queue_work(dev_priv->wq, &dev_priv->rps.work); > } > spin_unlock_irqrestore(&dev_priv->rps.lock, flags); > > - if (pm_iir & ~GEN6_PM_DEFERRED_EVENTS) > + if (pm_iir & ~GEN6_PM_RPS_EVENTS) > DRM_ERROR("Unexpected PM interrupted\n"); > } > > @@ -1018,7 +1019,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void > *arg) > if (pipe_stats[0] & PIPE_GMBUS_INTERRUPT_STATUS) > gmbus_irq_handler(dev); > > - if (pm_iir & GEN6_PM_DEFERRED_EVENTS) > + if (pm_iir & GEN6_PM_RPS_EVENTS) > gen6_queue_rps_work(dev_priv, pm_iir); > > I915_WRITE(GTIIR, gt_iir); > @@ -1259,7 +1260,7 @@ static irqreturn_t ivybridge_irq_handler(int irq, void > *arg) > if (pm_iir) { > if (IS_HASWELL(dev)) > hsw_pm_irq_handler(dev_priv, pm_iir); > - else if (pm_iir & GEN6_PM_DEFERRED_EVENTS) > + else if (pm_iir & GEN6_PM_RPS_EVENTS) > gen6_queue_rps_work(dev_priv, pm_iir); > I915_WRITE(GEN6_PMIIR, pm_iir); > ret
Re: [Intel-gfx] [PATCH 10/18] [v2] drm/i915: Add PM regs to pre/post install
On Tue, May 28, 2013 at 07:22:26PM -0700, Ben Widawsky wrote: > At the moment, these values are wiped out anyway by the rps > enable/disable. That will be changed in the next patch though. > > v2: Add post install setup to address issue found by Damien in the next > patch. > replaced > WARN_ON(dev_priv->rps.pm_iir != 0); > with rps.pm_iir = 0; > > With the v2 of this patch and the deferred pm enabling (which changed > since the original patches) we're now able to get PM interrupts before > we've brought up enabled rps. At this point in boot, we don't want to do > anything about it, so we simply ignore it. Since writing the original > assertion, the code has changed quite a bit, and I believe removing this > assertion is perfectly safe. > > Signed-off-by: Ben Widawsky Reviewed-by: Damien Lespiau -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4 V6] i915/drm: Add private api for power well usage
On Mon, May 27, 2013 at 05:15:16PM +0800, Wang Xingchao wrote: > Haswell Display audio depends on power well in graphic side, it should > request power well before use it and release power well after use. > I915 will not shutdown power well if it detects audio is using. > This patch protects display audio crash for Intel Haswell C3 stepping board. > > Signed-off-by: Wang Xingchao Daniel, you probably want to give your opinion on the "2 APIs to manage the power well" discussion below. In case, the logic seems correct so: with or without the two nitpicks below (that we can address later if deemed necessary): Reviewed-by: Damien Lespiau -- Damien > --- > drivers/gpu/drm/i915/i915_dma.c |6 +++ > drivers/gpu/drm/i915/i915_drv.h | 12 + > drivers/gpu/drm/i915/intel_drv.h |4 ++ > drivers/gpu/drm/i915/intel_pm.c | 92 > +++--- > include/drm/i915_powerwell.h | 36 +++ > 5 files changed, 143 insertions(+), 7 deletions(-) > create mode 100644 include/drm/i915_powerwell.h > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index f5addac..b702915 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1652,6 +1652,9 @@ int i915_driver_load(struct drm_device *dev, unsigned > long flags) > /* Start out suspended */ > dev_priv->mm.suspended = 1; > > + if (IS_HASWELL(dev)) > + i915_init_power_well(dev); > + > if (drm_core_check_feature(dev, DRIVER_MODESET)) { > ret = i915_load_modeset_init(dev); > if (ret < 0) { > @@ -1708,6 +1711,9 @@ int i915_driver_unload(struct drm_device *dev) > > intel_gpu_ips_teardown(); > > + if (IS_HASWELL(dev)) > + i915_remove_power_well(dev); > + > i915_teardown_sysfs(dev); > > if (dev_priv->mm.inactive_shrinker.shrink) > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 14817de..ea94e5e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -720,6 +720,15 @@ struct intel_ilk_power_mgmt { > struct drm_i915_gem_object *renderctx; > }; > > +/* Power well structure for haswell */ > +struct i915_power_well { > + struct drm_device *device; > + spinlock_t lock; > + /* power well enable/disable usage count */ > + int count; > + int i915_request; > +}; > + > struct i915_dri1_state { > unsigned allow_batchbuffer : 1; > u32 __iomem *gfx_hws_cpu_addr; > @@ -1073,6 +1082,9 @@ typedef struct drm_i915_private { >* mchdev_lock in intel_pm.c */ > struct intel_ilk_power_mgmt ips; > > + /* Haswell power well */ > + struct i915_power_well *hsw_pwr; > + We usually just put the structure inside struct drm_i915_private and pass it's pointer around instead of an allocation for a few bytes. We also try quite hard to not be platform specific but feature specific in our code (eg. HAS_POWER_WELL() and not IS_HASWELL()). hsw_pwr should probably be renamed to power_well. > enum no_fbc_reason no_fbc_reason; > > struct drm_mm_node *compressed_fb; > diff --git a/drivers/gpu/drm/i915/intel_drv.h > b/drivers/gpu/drm/i915/intel_drv.h > index 0f35545..8b5017d 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -754,6 +754,10 @@ extern void intel_update_fbc(struct drm_device *dev); > extern void intel_gpu_ips_init(struct drm_i915_private *dev_priv); > extern void intel_gpu_ips_teardown(void); > > +/* Power well */ > +extern int i915_init_power_well(struct drm_device *dev); > +extern void i915_remove_power_well(struct drm_device *dev); > + > extern bool intel_display_power_enabled(struct drm_device *dev, > enum intel_display_power_domain domain); > extern void intel_init_power_well(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 1a76572..f429347 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -4502,18 +4502,12 @@ bool intel_display_power_enabled(struct drm_device > *dev, > } > } > > -void intel_set_power_well(struct drm_device *dev, bool enable) > +static void __intel_set_power_well(struct drm_device *dev, bool enable) > { > struct drm_i915_private *dev_priv = dev->dev_private; > bool is_enabled, enable_requested; > uint32_t tmp; > > - if (!HAS_POWER_WELL(dev)) > - return; > - > - if (!i915_disable_power_well && !enable) > -
Re: [Intel-gfx] [PATCH] drm/i915: rip out the PM_IIR WARN, again
On Fri, May 31, 2013 at 09:12:19AM +0200, Daniel Vetter wrote: > This has accidentally been reintroduced with > > commit 22aae764a3fa21ee502b99e8986cb4e49ec14cfe > Author: Ben Widawsky > Date: Tue May 28 19:22:24 2013 -0700 > > drm/i915: Create a more generic pm handler for hsw+ > > See > > commit 58bf8062d0b293b8e1028e5b0342082002886bd4 > Author: Daniel Vetter > Date: Thu Jun 21 14:55:22 2012 +0200 > > drm/i915: rip out the PM_IIR WARN > > for the extensive reasoning why the WARN is bogus. > > Cc: Damien Lespiau > Cc: Ben Widawsky > Cc: Chris Wilson > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65197 > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_irq.c |7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) Reviewed-by: Damien Lespiau There's a conflict with the rest of the VECS series, but fortunately seems like an easy one. -- Damien > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index da5c9ab..df85abc 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -934,11 +934,8 @@ static void hsw_pm_irq_handler(struct drm_i915_private > *dev_priv, > > spin_lock_irqsave(&dev_priv->rps.lock, flags); > dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_DEFERRED_EVENTS; > - if (dev_priv->rps.pm_iir) { > - I915_WRITE(GEN6_PMIMR, dev_priv->rps.pm_iir); > - /* never want to mask useful interrupts. (also posting read) */ > - WARN_ON(I915_READ_NOTRACE(GEN6_PMIMR) & > ~GEN6_PM_DEFERRED_EVENTS); > - } > + I915_WRITE(GEN6_PMIMR, dev_priv->rps.pm_iir); > + POSTING_READ(GEN6_PMIMR); > spin_unlock_irqrestore(&dev_priv->rps.lock, flags); > > queue_work(dev_priv->wq, &dev_priv->rps.work); > -- > 1.7.10.4 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/3] tools: Remove unused tools/intel_iosf_read.c
Also intel_iosf_read() does not exist, and would need a bit more arguments. Signed-off-by: Damien Lespiau --- tools/intel_iosf_read.c | 70 - 1 file changed, 70 deletions(-) delete mode 100644 tools/intel_iosf_read.c diff --git a/tools/intel_iosf_read.c b/tools/intel_iosf_read.c deleted file mode 100644 index 15cd82c..000 --- a/tools/intel_iosf_read.c +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Copyright © 2012 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: - * Vijay Purushothaman - * - */ - -#include -#include -#include -#include -#include -#include "intel_gpu_tools.h" - -static void usage(char *cmdname) -{ - printf("Warning : This program will work only on Valleyview\n"); - printf("Usage: %s [addr]\n", cmdname); - printf("\t addr : in 0x format\n"); -} - -int main(int argc, char** argv) -{ - int ret = 0; - uint32_t reg, val; - char *cmdname = strdup(argv[0]); - struct pci_device *dev = intel_get_pci_device(); - - if (argc != 2 || !IS_VALLEYVIEW(dev->device_id)) { - usage(cmdname); - ret = 1; - goto out; - } - - sscanf(argv[1], "0x%x", ®); - - intel_register_access_init(dev, 0); - - ret = intel_iosf_read(reg, &val); - if (ret) - fprintf(stderr, "iosf read failed: %d\n", ret); - - printf("Read IOSF register: 0x%x - Value : 0x%x\n", reg, val); - - intel_register_access_fini(); - -out: - free(cmdname); - return ret; -} -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/3] tools: Remove intel_disable_clock_gating
This tool only supports ILK. I take the fact that nobody has felt the need to update it for later platforms as a sign it's not very useful. Signed-off-by: Damien Lespiau --- Android.mk | 33 -- tools/.gitignore | 1 - tools/Makefile.am | 1 - tools/intel_disable_clock_gating.c | 71 -- 4 files changed, 106 deletions(-) delete mode 100644 tools/intel_disable_clock_gating.c diff --git a/Android.mk b/Android.mk index 3be3462..a44740c 100644 --- a/Android.mk +++ b/Android.mk @@ -68,39 +68,6 @@ include $(BUILD_EXECUTABLE) # include $(CLEAR_VARS) -LOCAL_SRC_FILES := \ - tools/intel_disable_clock_gating.c \ - lib/intel_pci.c \ - lib/intel_gpu_tools.h \ - tools/intel_reg.h \ - lib/intel_batchbuffer.h \ - lib/intel_batchbuffer.c \ - lib/intel_mmio.c\ - tools/intel_chipset.h - - -LOCAL_C_INCLUDES +=\ - $(LOCAL_PATH)/lib \ - $(TOPDIR)hardware/intel/libdrm/include/drm \ - $(TOPDIR)hardware/intel/libdrm/intel\ - $(LOCAL_PATH)/../libpciaccess/include/ - -LOCAL_CFLAGS += -DHAVE_LIBDRM_ATOMIC_PRIMITIVES=1 -LOCAL_CFLAGS += -DANDROID - - -LOCAL_MODULE := intel_disable_clock_gating -LOCAL_MODULE_TAGS := optional - -LOCAL_SHARED_LIBRARIES := libpciaccess \ - libdrm \ - libdrm_intel - -include $(BUILD_EXECUTABLE) - -# -include $(CLEAR_VARS) - LOCAL_SRC_FILES := \ tools/intel_audio_dump.c \ lib/intel_pci.c \ diff --git a/tools/.gitignore b/tools/.gitignore index 23ac628..8c6622d 100644 --- a/tools/.gitignore +++ b/tools/.gitignore @@ -3,7 +3,6 @@ intel_audio_dump intel_backlight intel_bios_dumper intel_bios_reader -intel_disable_clock_gating intel_dpio_read intel_dpio_write intel_dump_decode diff --git a/tools/Makefile.am b/tools/Makefile.am index 2519169..1686b9c 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -3,7 +3,6 @@ SUBDIRS = quick_dump endif bin_PROGRAMS = \ - intel_disable_clock_gating \ intel_audio_dump\ intel_backlight \ intel_bios_dumper \ diff --git a/tools/intel_disable_clock_gating.c b/tools/intel_disable_clock_gating.c deleted file mode 100644 index 8dde3e1..000 --- a/tools/intel_disable_clock_gating.c +++ /dev/null @@ -1,71 +0,0 @@ -/* - * Copyright © 2010 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: - * Zhenyu Wang - * - */ - -#include -#include -#include -#include -#include -#include "intel_gpu_tools.h" - -int main(int argc, char** argv) -{ - struct pci_device *pci_dev; - - pci_dev = intel_get_pci_device(); - intel_get_mmio(pci_dev); - - if (IS_GEN5(pci_dev->device_id)) { - printf("Restore method:\n"); - - printf("intel_reg_write 0x%x 0x%08x\n", - PCH_3DCGDIS0, INREG(PCH_3DCGDIS0)); - OUTREG(PCH_3DCGDIS0, 0x); - - printf("intel_reg_write 0x%x 0x%08x\n", - PCH_3DCGDIS1, INREG(PCH_3DCGDIS1)); - OUTREG(PCH_3DCGDIS1, 0x); - - printf("intel_reg_write 0x%x 0x%08x\n", - PCH_3DRAMCGDIS0, INREG(PCH_3DRAMCGDIS0)); - OUTREG(PCH_3DRAMCGDIS0, 0x); - - printf(&q
[Intel-gfx] [PATCH i-g-t 1/3] instdone: Add an assert to make sure we never overflow instdone_bits
Signed-off-by: Damien Lespiau --- lib/instdone.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/instdone.c b/lib/instdone.c index 4679a9c..ad5c62f 100644 --- a/lib/instdone.c +++ b/lib/instdone.c @@ -37,6 +37,7 @@ int num_instdone_bits = 0; static void add_instdone_bit(uint32_t reg, uint32_t bit, const char *name) { + assert(num_instdone_bits < MAX_INSTDONE_BITS); instdone_bits[num_instdone_bits].reg = reg; instdone_bits[num_instdone_bits].bit = bit; instdone_bits[num_instdone_bits].name = name; -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC] lib: Import intel_sideband.c from the kernel
XXX: more detailed commit message --- lib/Makefile.am| 9 +- lib/intel_dpio.c | 94 - lib/intel_gpu_tools.h | 15 ++ lib/intel_iosf.c | 85 lib/intel_reg.h| 221 +++-- lib/intel_sideband.c | 191 ++ tools/intel_dpio_read.c => lib/intel_sideband.h| 64 ++ .../kernel-shim/drm-shim.h | 62 +++--- .../kernel-shim/i915_drv.h | 53 ++--- .../kernel-shim/intel_drv.h| 69 +++ .../intel_dpio_read.c => lib/kernel-shim/kernel.h | 76 +++ scripts/sync-from-kernel | 30 +++ tools/intel_dpio_read.c| 4 +- tools/intel_dpio_write.c | 4 +- tools/intel_nc_read.c | 6 +- tools/intel_nc_write.c | 10 +- tools/intel_punit_read.c | 6 +- tools/intel_punit_write.c | 10 +- 18 files changed, 592 insertions(+), 417 deletions(-) delete mode 100644 lib/intel_dpio.c delete mode 100644 lib/intel_iosf.c create mode 100644 lib/intel_sideband.c copy tools/intel_dpio_read.c => lib/intel_sideband.h (52%) copy tools/intel_dpio_write.c => lib/kernel-shim/drm-shim.h (55%) copy tools/intel_dpio_write.c => lib/kernel-shim/i915_drv.h (55%) copy tools/intel_dpio_write.c => lib/kernel-shim/intel_drv.h (55%) copy tools/intel_dpio_read.c => lib/kernel-shim/kernel.h (52%) create mode 100755 scripts/sync-from-kernel diff --git a/lib/Makefile.am b/lib/Makefile.am index 387141b..2cafe22 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -1,10 +1,14 @@ noinst_LTLIBRARIES = libintel_tools.la -AM_CPPFLAGS = -I$(top_srcdir) +AM_CPPFLAGS = -I$(top_srcdir) -I$(srcdir)/kernel-shim AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) libintel_tools_la_SOURCES =\ + kernel-shim/kernel.h\ + kernel-shim/i915_drv.h \ + kernel-shim/intel_drv.h \ + kernel-shim/drm-shim.h \ debug.h \ drmtest.c \ drmtest.h \ @@ -29,8 +33,7 @@ libintel_tools_la_SOURCES = \ rendercopy_gen7.c \ rendercopy.h\ intel_reg_map.c \ - intel_dpio.c\ - intel_iosf.c\ + intel_sideband.c\ $(NULL) LDADD = $(CAIRO_LIBS) diff --git a/lib/intel_dpio.c b/lib/intel_dpio.c deleted file mode 100644 index acfd201..000 --- a/lib/intel_dpio.c +++ /dev/null @@ -1,94 +0,0 @@ -/* - * Copyright © 2008 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: - *Vijay Purushothaman - * - */ -#include -#include -#include -#include -#include "intel_gpu_tools.h" - -static uint32_t intel_display_reg_read(uint32_t reg) -{ - struct pci_device *dev = intel_get_pci_device(); - - if (IS_VALLEYVIEW(dev->device_id)) - reg += VLV_DISPLAY_BASE; - return (*(volatile uint32_t*)((volatile char*)mmio + reg)); -} - -static void intel_display_reg_write(uint32_t reg, uint32_t val) -{ - volatile uint32_t *ptr; - struct pci_device *dev = intel_get_pci_device(); - - if (IS_VALLEYVIEW(dev->device_id)) - reg += VLV_DISPLAY_BASE; - ptr = (volatile uint32_t*)((volatile char*)mmio + reg); - *ptr = val; -} - -/* - * In SoCs like Valleyview some of the PLL & Lane control registers - * can be accessed only through IO side band fabric called DPIO - */ -uint32_t -intel_dpio_reg_read(uint32_t reg) -{ - /* Check whether the side band fabric is ready to accept commands */ - do { - usleep(1); -
[Intel-gfx] Playing with the idea of pulling kernel code into i-g-t
I played a bit with this as that's something thas has been floating around for a bit and could be used for: - unit testing kernel functions (eg. PLL computations) - reuse off some code (intel_reg.h, sideband, device_info struct, ...) - Ville wanted to fuzzy test the EDID code (and has a branch somewhere IIRC) - ... To make it maintainable we could have a script that, when pointed to a kernel, pulls in the needed pieces. A simple form of this is scripts/sync-from-kernel in the following patch. $ ./scripts/sync-from-kernel ../linux-2.6/ Using v3.9-rc1-15371-g393f670 Importing intel_sideband.c You then need a shim layer that replicates what we use from the kernel. A lot can probably be faked without too much trouble. A good use case for that would be to be able to have one master copy of i915_reg.h (the kernel's) and sync it into i-g-t. In any case, the following patch pulls in intel_sideband.c and replaces what we currenlty have. It's aboslutely not tested and might just end up being a pointless exercise... -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Move default: to the end of the switch
As it's usual to do, don't leave it in the middle of valid cases. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_irq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 63996aa..a8ef086 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1484,14 +1484,14 @@ static void i915_get_extra_instdone(struct drm_device *dev, instdone[0] = I915_READ(INSTDONE_I965); instdone[1] = I915_READ(INSTDONE1); break; - default: - WARN_ONCE(1, "Unsupported platform\n"); case 7: instdone[0] = I915_READ(GEN7_INSTDONE_1); instdone[1] = I915_READ(GEN7_SC_INSTDONE); instdone[2] = I915_READ(GEN7_SAMPLER_INSTDONE); instdone[3] = I915_READ(GEN7_ROW_INSTDONE); break; + default: + WARN_ONCE(1, "Unsupported platform\n"); } } -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Fix 'port' typo in comment
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_reg.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 47a9de0..d146993 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -390,7 +390,7 @@ * * DPIO is VLV only. * - * Note: digital port B is DDI0, digital pot C is DDI1 + * Note: digital port B is DDI0, digital port C is DDI1 */ #define DPIO_DEVFN 0 #define DPIO_OPCODE_REG_WRITE 1 -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Move default: to the end of the switch
On Wed, Jun 05, 2013 at 02:28:50PM +0300, Jani Nikula wrote: > On Wed, 05 Jun 2013, Damien Lespiau wrote: > > As it's usual to do, don't leave it in the middle of valid cases. > > > > Signed-off-by: Damien Lespiau > > --- > > drivers/gpu/drm/i915/i915_irq.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > b/drivers/gpu/drm/i915/i915_irq.c > > index 63996aa..a8ef086 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -1484,14 +1484,14 @@ static void i915_get_extra_instdone(struct > > drm_device *dev, > > instdone[0] = I915_READ(INSTDONE_I965); > > instdone[1] = I915_READ(INSTDONE1); > > break; > > - default: > > - WARN_ONCE(1, "Unsupported platform\n"); > > This is a functional change because of the follow through here. The > commit message should reflect this. Hum indeed, it was probably intentional, trying to provide a default behaviour? never mind then (it could do with a comment I guess). -- Damien > > BR, > Jani. > > > case 7: > > instdone[0] = I915_READ(GEN7_INSTDONE_1); > > instdone[1] = I915_READ(GEN7_SC_INSTDONE); > > instdone[2] = I915_READ(GEN7_SAMPLER_INSTDONE); > > instdone[3] = I915_READ(GEN7_ROW_INSTDONE); > > break; > > + default: > > + WARN_ONCE(1, "Unsupported platform\n"); > > } > > } > > > > -- > > 1.8.1.4 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Another pass on Workarounds, with a focus on HSW
Hi, Patches 1-7: Some comments to signal the Wa we're implementing for the next person that goes through the list. Patch 8: Random detail when reading that code Patch 9-10: Two new workarounds. I don't remember at all the discussions around RC6+ on GEN6+, but there's definitely a workaround that tells us to disable RC6+. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 02/10] drm/i915: We implement WaFbcAsynchFlipDisableFbcQueue on ilk and snb
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_pm.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 57e99b1..eb3c2c4 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4420,6 +4420,8 @@ static void ironlake_init_clock_gating(struct drm_device *dev) * The bit 22 of 0x42000 * The bit 22 of 0x42004 * The bit 7,8,9 of 0x42020. +* +* WaFbcAsynchFlipDisableFbcQueue:ilk */ if (IS_IRONLAKE_M(dev)) { I915_WRITE(ILK_DISPLAY_CHICKEN1, @@ -4557,6 +4559,8 @@ static void gen6_init_clock_gating(struct drm_device *dev) * The bit5 and bit7 of 0x42020 * The bit14 of 0x70180 * The bit14 of 0x71180 +* +* WaFbcAsynchFlipDisableFbcQueue:snb */ I915_WRITE(ILK_DISPLAY_CHICKEN1, I915_READ(ILK_DISPLAY_CHICKEN1) | -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 01/10] drm/i915: We implement WaFbcWaitForVBlankBeforeEnable for ilk and snb
We also wait for that blank on other platforms but the w/a doesn't apply there. Not an issue at all. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_pm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 50fe3d7..57e99b1 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -404,6 +404,8 @@ void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval) * following the termination of the page-flipping sequence * and indeed performing the enable as a co-routine and not * waiting synchronously upon the vblank. +* +* WaFbcWaitForVBlankBeforeEnable:ilk,snb */ schedule_delayed_work(&work->work, msecs_to_jiffies(50)); } -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 03/10] drm/i915: We implement WaFbcDisableDpfcClockGating on ilk
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_pm.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index eb3c2c4..2eb1846 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4385,7 +4385,10 @@ static void ironlake_init_clock_gating(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE; - /* Required for FBC */ + /* +* Required for FBC +* WaFbcDisableDpfcClockGating:ilk +*/ dspclk_gate |= ILK_DPFCRUNIT_CLOCK_GATE_DISABLE | ILK_DPFCUNIT_CLOCK_GATE_DISABLE | ILK_DPFDUNIT_CLOCK_GATE_ENABLE; -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 05/10] drm/i915: We implement the TLB invalidation mode workaounds
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_ringbuffer.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 0e72da6..ce14594 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -523,11 +523,14 @@ static int init_render_ring(struct intel_ring_buffer *ring) if (INTEL_INFO(dev)->gen >= 6) I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(ASYNC_FLIP_PERF_DISABLE)); - /* Required for the hardware to program scanline values for waiting */ + /* Required for the hardware to program scanline values for waiting +* WaEnableFlushTlbInvalidationMode:snb +*/ if (INTEL_INFO(dev)->gen == 6) I915_WRITE(GFX_MODE, _MASKED_BIT_ENABLE(GFX_TLB_INVALIDATE_ALWAYS)); + /* WaBCSVCSTlbInvalidationMode:ivb,hsw,vlv */ if (IS_GEN7(dev)) I915_WRITE(GFX_MODE_GEN7, _MASKED_BIT_DISABLE(GFX_TLB_INVALIDATE_ALWAYS) | -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/10] drm/i915: We implement WaMPhyProgramming on Haswell
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_display.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 827d7ca..9e0d69c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5122,7 +5122,10 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) BUG_ON(val != final); } -/* Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. */ +/* + * Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. + * WaMPhyProgramming:hsw + */ static void lpt_init_pch_refclk(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 07/10] drm/i915: We implement WaFlushOpOnCSStall
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index ce14594..009e616 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -171,6 +171,8 @@ gen4_render_ring_flush(struct intel_ring_buffer *ring, * Post-sync nonzero is what triggered this second workaround, so we * can't use that one either. Notify enable is IRQs, which aren't * really our business. That leaves only stall at scoreboard. + * + * WaFlushOpOnCSStall:ivb */ static int intel_emit_post_sync_nonzero_flush(struct intel_ring_buffer *ring) @@ -296,6 +298,7 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring, * Workaround: 4th PIPE_CONTROL command (except the ones with only * read-cache invalidate bits set) must have the CS_STALL bit set. We * don't try to be clever and just set it unconditionally. +* WaFlushOpOnCSStall:ivb,hsw,vlv */ flags |= PIPE_CONTROL_CS_STALL; -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 06/10] drm/i915: We implement WaEDPModeSetSequenceChange
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_ddi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 486c46b..bc7dd9a 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1050,6 +1050,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder) intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); intel_dp_start_link_train(intel_dp); intel_dp_complete_link_train(intel_dp); + /* WaEDPModeSetSequenceChange:hsw */ if (port != PORT_A) intel_dp_stop_link_train(intel_dp); } @@ -1113,6 +1114,7 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder) } else if (type == INTEL_OUTPUT_EDP) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); + /* WaEDPModeSetSequenceChange:hsw */ if (port == PORT_A) intel_dp_stop_link_train(intel_dp); -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 08/10] drm/i915: Don't try to calculate RC6 residency on GEN4 and before
intel_enable_rc6() is used to check if we can compute the RC6 residency in the sysfs code. Disable this for platforms older than Ironlake. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_pm.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2eb1846..0465cd6 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3154,6 +3154,10 @@ static void valleyview_disable_rps(struct drm_device *dev) int intel_enable_rc6(const struct drm_device *dev) { + /* No RC6 before Ironlake */ + if (INTEL_INFO(dev)->gen < 5) + return 0; + /* Respect the kernel parameter if it is set */ if (i915_enable_rc6 >= 0) return i915_enable_rc6; -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/10] drm/i915: Implement WaRsDisableRC6Plus
Let's disable RC6+ by default. We still leave the possibility to override this with the module parameter. I've also shuffled the code around so we always log if we have RC6 enabled or disabled. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_pm.c | 25 + 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 0465cd6..e19952f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3158,27 +3158,20 @@ int intel_enable_rc6(const struct drm_device *dev) if (INTEL_INFO(dev)->gen < 5) return 0; - /* Respect the kernel parameter if it is set */ - if (i915_enable_rc6 >= 0) - return i915_enable_rc6; - /* Disable RC6 on Ironlake */ - if (INTEL_INFO(dev)->gen == 5) + if (i915_enable_rc6 == 0 || INTEL_INFO(dev)->gen == 5) { + DRM_DEBUG_DRIVER("RC6 disabled\n"); return 0; - - if (IS_HASWELL(dev)) { - DRM_DEBUG_DRIVER("Haswell: only RC6 available\n"); - return INTEL_RC6_ENABLE; } - /* snb/ivb have more than one rc6 state. */ - if (INTEL_INFO(dev)->gen == 6) { - DRM_DEBUG_DRIVER("Sandybridge: deep RC6 disabled\n"); - return INTEL_RC6_ENABLE; - } + DRM_DEBUG_DRIVER("RC6 enabled\n"); + + if (i915_enable_rc6 > 0) + return i915_enable_rc6; - DRM_DEBUG_DRIVER("RC6 and deep RC6 enabled\n"); - return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE); + /* snb/ivb have more than one rc6 state. However we only allow RC6 +* WaRsDisableRC6Plus:snb,ivb,vlv */ + return INTEL_RC6_ENABLE; } static void gen6_enable_rps(struct drm_device *dev) -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 10/10] drm/i915: Implement WaBlockMsgChannelDuringGfxReset
Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_gem.c | 9 + drivers/gpu/drm/i915/i915_reg.h | 9 +++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 58048d4..187a9a4 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4063,6 +4063,15 @@ i915_gem_init_hw(struct drm_device *dev) I915_WRITE(GEN7_MSG_CTL, temp); } + /* WaBlockMsgChannelDuringGfxReset:hsw */ + if (IS_HASWELL(dev)) { + u32 temp = I915_READ(GEN7_MISCCPCTL); + temp &= MISCCPCTL_BLOCK_MC_GFX_FULL_SR | + MISCCPCTL_BLOCK_MC_RENDER_SR | + MISCCPCTL_BLOCK_MC_FLR; + I915_WRITE(GEN7_MISCCPCTL, temp); + } + i915_gem_l3_remap(dev); i915_gem_init_swizzling(dev); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 47a9de0..ff27c73 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4595,8 +4595,13 @@ #define GEN6_RC6 3 #define GEN6_RC7 4 -#define GEN7_MISCCPCTL (0x9424) -#define GEN7_DOP_CLOCK_GATE_ENABLE (1<<0) +#define GEN7_MISCCPCTL (0x9424) +#define MISCCPCTL_BLOCK_MC_GFX_FULL_SR (1<<6) +#define MISCCPCTL_BLOCK_MC_BLITTER_SR(1<<5) +#define MISCCPCTL_BLOCK_MC_MEDIA_SR (1<<4) +#define MISCCPCTL_BLOCK_MC_RENDER_SR (1<<3) +#define MISCCPCTL_BLOCK_MC_FLR (1<<2) +#define GEN7_DOP_CLOCK_GATE_ENABLE (1<<0) /* IVYBRIDGE DPF */ #define GEN7_L3CDERRST10xB008 /* L3CD Error Status 1 */ -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/10] drm/i915: Implement WaBlockMsgChannelDuringGfxReset
On Fri, Jun 07, 2013 at 05:51:57PM +0100, Chris Wilson wrote: > On Fri, Jun 07, 2013 at 05:41:16PM +0100, Damien Lespiau wrote: > > During GFX reset? This patch looks a little permanent to me, so please > explain. The description of the bits is "Enable msg channel blockreq/ack mechanism for [GFX full SR|FLR|Render SR]", so I'm assuming that it's only needed to program it once and it's taken into account when doing the reset. I could also test it I guess... -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/10] drm/i915: Implement WaBlockMsgChannelDuringGfxReset
On Fri, Jun 07, 2013 at 05:59:45PM +0100, Damien Lespiau wrote: > I could also test it I guess... Well, it does seem to behave. The confusion may be from the WA name, the field is called "Block Msg channel during resets". This is still mostly guess work though. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix old reference to i830_sdvo_get_capabilities()
It's now intel_sdvo_get_capabilities(). Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_sdvo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 7068195..221d415 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -80,7 +80,7 @@ struct intel_sdvo { /* * Capabilities of the SDVO device returned by -* i830_sdvo_get_capabilities() +* intel_sdvo_get_capabilities() */ struct intel_sdvo_caps caps; -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Initialize active_outputs to never read unitialized values
In case of intel_sdvo_get_active_outputs() failing, we end up reading a value from the stack. Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/intel_sdvo.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index ac923b7..040a259 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1277,7 +1277,7 @@ static bool intel_sdvo_connector_get_hw_state(struct intel_connector *connector) struct intel_sdvo_connector *intel_sdvo_connector = to_intel_sdvo_connector(&connector->base); struct intel_sdvo *intel_sdvo = intel_attached_sdvo(&connector->base); - u16 active_outputs; + u16 active_outputs = 0; intel_sdvo_get_active_outputs(intel_sdvo, &active_outputs); @@ -1293,7 +1293,7 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder, struct drm_device *dev = encoder->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base); - u16 active_outputs; + u16 active_outputs = 0; u32 tmp; tmp = I915_READ(intel_sdvo->sdvo_reg); -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Use FBINFO_STATE defines instead of 0 and 1
This makes, arguably, the condition on state easier to read. Suggested-by: Chris Wilson Signed-off-by: Damien Lespiau --- drivers/gpu/drm/i915/i915_drv.c | 6 +++--- drivers/gpu/drm/i915/intel_fb.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index b23cd63..381c9dd 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -546,7 +546,7 @@ static int i915_drm_freeze(struct drm_device *dev) intel_opregion_fini(dev); console_lock(); - intel_fbdev_set_suspend(dev, 1); + intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED); console_unlock(); return 0; @@ -590,7 +590,7 @@ void intel_console_resume(struct work_struct *work) struct drm_device *dev = dev_priv->dev; console_lock(); - intel_fbdev_set_suspend(dev, 0); + intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING); console_unlock(); } @@ -659,7 +659,7 @@ static int __i915_drm_thaw(struct drm_device *dev) * path of resume if possible. */ if (console_trylock()) { - intel_fbdev_set_suspend(dev, 0); + intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING); console_unlock(); } else { schedule_work(&dev_priv->console_resume_work); diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c index 7a77d4c..244060a 100644 --- a/drivers/gpu/drm/i915/intel_fb.c +++ b/drivers/gpu/drm/i915/intel_fb.c @@ -275,7 +275,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state) * been restored from swap. If the object is stolen however, it will be * full of whatever garbage was left in there. */ - if (!state && ifbdev->ifb.obj->stolen) + if (state == FBINFO_STATE_RUNNING && ifbdev->ifb.obj->stolen) memset_io(info->screen_base, 0, info->screen_size); fb_set_suspend(info, state); -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/9] drm/i915: fixup i8xx pll limits
On Tue, May 21, 2013 at 09:54:53PM +0200, Daniel Vetter wrote: > So apparently the pll limits in the docs are the real values, but the > formula actually seems to consistenly use register values. See > > commit 4f7dfb6788dd022446847fbbfbe45e13bedb5be2 > Author: Patrik Jakobsson > Date: Wed Feb 13 22:20:22 2013 +0100 > > drm/i915: Set i9xx sdvo clock limits according to specifications > > On gen2 the situation is a bit more messy: We reuse the code for > m1/m2/n computation and register frobbing from gen3, so those limits > need to be in register values (and so need to substract 2 from the doc > limits). > > But gen2 also stores p1/p2 with 2/1 substracted in the register! For > those though i8xx_update_pll does the adjustments, but the clock > computation code assumes it works like on gen3. So for divisor limits > we must _not_ adjust them to the register values. I think this deserves a comment for the next one looking at that code. Also the documented computations for p are: gen2: p = (p1 + 2) * 2^(p2 + 1) gen3: p = p1 * p2 which lead me to believe we weren't computing the dot clock correctly on gen2 (but in fact it's fine as p1/p2 are never expressed as register values, which are not a direct deduction (say on gen 3, p2 = 10 is coded with 0, p2 = 5 is coded by 1)) So IIUC, we have n, m1, m2 that we express in the "register domain" while m, p1, p2, p are in the "clock formula domain", for lack of a better wording. And so the limits follow that. Otherwise, the patch looks good: Reviewed-by: Damien Lespiau -- Damien > > v2: Extract the common i8xx m/n limits into a single define. This was > motivated by the mess for the g4x limits where they've all been off in > different directions, apparently to fix specific bugs ... > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_display.c | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 959c2ae..ea8eb0c 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -84,13 +84,16 @@ intel_fdi_link_freq(struct drm_device *dev) > return 27; > } > > +#define I8XX_DPLL_M_N_LIMITS \ > + .n = { .min = 1, .max = 14 }, \ > + .m = { .min = 96, .max = 140 }, \ > + .m1 = { .min = 16, .max = 24 }, \ > + .m2 = { .min = 4, .max = 14 }, > + > static const intel_limit_t intel_limits_i8xx_dvo = { > .dot = { .min = 25000, .max = 35 }, > .vco = { .min = 93, .max = 140 }, > - .n = { .min = 3, .max = 16 }, > - .m = { .min = 96, .max = 140 }, > - .m1 = { .min = 18, .max = 26 }, > - .m2 = { .min = 6, .max = 16 }, > + I8XX_DPLL_M_N_LIMITS > .p = { .min = 4, .max = 128 }, > .p1 = { .min = 2, .max = 33 }, > .p2 = { .dot_limit = 165000, > @@ -100,10 +103,7 @@ static const intel_limit_t intel_limits_i8xx_dvo = { > static const intel_limit_t intel_limits_i8xx_lvds = { > .dot = { .min = 25000, .max = 35 }, > .vco = { .min = 93, .max = 140 }, > - .n = { .min = 3, .max = 16 }, > - .m = { .min = 96, .max = 140 }, > - .m1 = { .min = 18, .max = 26 }, > - .m2 = { .min = 6, .max = 16 }, > + I8XX_DPLL_M_N_LIMITS > .p = { .min = 4, .max = 128 }, > .p1 = { .min = 1, .max = 6 }, > .p2 = { .dot_limit = 165000, > -- > 1.7.11.7 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/9] drm/i915: fixup g4x pll limits
On Tue, May 21, 2013 at 09:54:54PM +0200, Daniel Vetter wrote: > Again the same confusion that our code expects m1/m2 in register values. > This time around with the added fun that many of the existing values > have been all off by a bit in different directions. Hence extract a > common #define. > > Note that n limits differ between lvds and other outputs. Strangely they've > all been correct already. > > v2: Rebased on top of the DP pll rework, which makes it even more > obvious that we can do this ... I'm a bit confused by this one: - I don't seem to find the special LVDS limits for n - Those limits are gated by a IS_G4X(). IS_G4X() is true for eaglelake and cantiga. The docs single out Cantiga and we seem to need a different set of limits for that platform (compared to the rest of gen4) platforms? - The limits for n are 3-6 or 3-5 in this code but I read 3-8 and 5-6 (cantiga) - m doesn't seem to match what I have, it seems like it could be the cantiga values (except that m is in "formula space" so we don't substract 2) - m1 and m2 seem to match what I have for cantiga, are we supposed to have those limits for eaglelake as well? Well, the only conclusion is that I must be reading the wrong docs, or? > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_display.c | 20 > 1 file changed, 8 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index ea8eb0c..cb54131 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -136,14 +136,16 @@ static const intel_limit_t intel_limits_i9xx_lvds = { > .p2_slow = 14, .p2_fast = 7 }, > }; > > +#define G4X_DPLL_M_LIMITS \ > + .m = { .min = 104, .max = 138 },\ > + .m1 = { .min = 15, .max = 21 },\ > + .m2 = { .min = 3, .max = 11 }, > > static const intel_limit_t intel_limits_g4x_sdvo = { > .dot = { .min = 25000, .max = 27 }, > .vco = { .min = 175, .max = 350}, > .n = { .min = 1, .max = 4 }, > - .m = { .min = 104, .max = 138 }, > - .m1 = { .min = 17, .max = 23 }, > - .m2 = { .min = 5, .max = 11 }, > + G4X_DPLL_M_LIMITS > .p = { .min = 10, .max = 30 }, > .p1 = { .min = 1, .max = 3}, > .p2 = { .dot_limit = 27, > @@ -156,9 +158,7 @@ static const intel_limit_t intel_limits_g4x_hdmi = { > .dot = { .min = 22000, .max = 40 }, > .vco = { .min = 175, .max = 350}, > .n = { .min = 1, .max = 4 }, > - .m = { .min = 104, .max = 138 }, > - .m1 = { .min = 16, .max = 23 }, > - .m2 = { .min = 5, .max = 11 }, > + G4X_DPLL_M_LIMITS > .p = { .min = 5, .max = 80 }, > .p1 = { .min = 1, .max = 8}, > .p2 = { .dot_limit = 165000, > @@ -169,9 +169,7 @@ static const intel_limit_t > intel_limits_g4x_single_channel_lvds = { > .dot = { .min = 2, .max = 115000 }, > .vco = { .min = 175, .max = 350 }, > .n = { .min = 1, .max = 3 }, > - .m = { .min = 104, .max = 138 }, > - .m1 = { .min = 17, .max = 23 }, > - .m2 = { .min = 5, .max = 11 }, > + G4X_DPLL_M_LIMITS > .p = { .min = 28, .max = 112 }, > .p1 = { .min = 2, .max = 8 }, > .p2 = { .dot_limit = 0, > @@ -183,9 +181,7 @@ static const intel_limit_t > intel_limits_g4x_dual_channel_lvds = { > .dot = { .min = 8, .max = 224000 }, > .vco = { .min = 175, .max = 350 }, > .n = { .min = 1, .max = 3 }, > - .m = { .min = 104, .max = 138 }, > - .m1 = { .min = 17, .max = 23 }, > - .m2 = { .min = 5, .max = 11 }, > + G4X_DPLL_M_LIMITS > .p = { .min = 14, .max = 42 }, > .p1 = { .min = 2, .max = 6 }, > .p2 = { .dot_limit = 0, > -- > 1.7.11.7 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/9] drm/i915: pnv dpll doesn't use m1!
On Tue, May 21, 2013 at 09:54:55PM +0200, Daniel Vetter wrote: > So don't try to store it in the DPLL_FP register. > > Otherwise it looks like the limits for pineview are correct: It has > it's own clock computation code, which doesn't use an offset for n > divisors, and the register value based m limits look sane enough. - n can vary between 2 and 6, but we declare the 3-6 as limits. - p1 seems to be able to go up to 9 - the m upper limit seems a bit big, but the docs are a bit shy on that values for pnv. Otherwise, the change itself seems good: Reviewed-by: Damien Lespiau > > v2: Rebase on top of the pineview clock refactor and fixup up the > commit message: It's m1 pnv doens't care about, not m2! > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_display.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index cb54131..520e340 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4300,7 +4300,7 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int > num_connectors) > > static uint32_t pnv_dpll_compute_fp(struct dpll *dpll) > { > - return (1 << dpll->n) << 16 | dpll->m1 << 8 | dpll->m2; > + return (1 << dpll->n) << 16 | dpll->m2; > } > > static uint32_t i9xx_dpll_compute_fp(struct dpll *dpll) > -- > 1.7.11.7 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/9] drm/i915: fixup g4x pll limits
On Wed, Jun 12, 2013 at 09:39:54AM +0200, Daniel Vetter wrote: > Afaik eaglelake as the desktop version of cantiga has all the same stuff > (minus a few mobile-only power saving features). The spreadsheet I have > here is even called eaglelake_cantiga_something.xls. Ah right, so let's believe this spreadsheet, it's more convincing that the pure hw limits. With that: Reviewed-by: Damien Lespiau -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/9] drm/i915: fix ibx/cpt/ppt dpll limits
On Tue, May 21, 2013 at 09:54:56PM +0200, Daniel Vetter wrote: > Now this was broken in pretty fundamental ways: > - M1/M2 have been consistently off by 2 and used doc values instead of > the two less registers values our code expects. > - M/N limits often were too small by seemingly arbitrary amounts. I > suspect this started to work around issues due to the wrong M1/M2 > limits. > > Rectify this all and consolidate the limits a bit with a #define where > the docs say that they should be equal. So this time I'm reading ibexpeak_xxx.xls > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_display.c | 36 > +++- > 1 file changed, 11 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 520e340..80698ac 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -217,18 +217,16 @@ static const intel_limit_t intel_limits_pineview_lvds = > { > .p2_slow = 14, .p2_fast = 14 }, > }; > > -/* Ironlake / Sandybridge > - * > - * We calculate clock using (register_value + 2) for N/M1/M2, so here > - * the range value for them is (actual_value - 2). > - */ > +#define ILK_DPLL_M_N_LIMITS \ > + .n = { .min = 1, .max = 6 }, \ > + .m = { .min = 79, .max = 127 }, \ > + .m1 = { .min = 10, .max = 20 }, \ > + .m2 = { .min = 3, .max = 7 }, \ > + > static const intel_limit_t intel_limits_ironlake_dac = { > .dot = { .min = 25000, .max = 35 }, > .vco = { .min = 176, .max = 351 }, > - .n = { .min = 1, .max = 5 }, > - .m = { .min = 79, .max = 127 }, > - .m1 = { .min = 12, .max = 22 }, > - .m2 = { .min = 5, .max = 9 }, > + ILK_DPLL_M_N_LIMITS > .p = { .min = 5, .max = 80 }, > .p1 = { .min = 1, .max = 8 }, > .p2 = { .dot_limit = 225000, In ibexpeak_.xsl I have n between 3 and 7 (so 1,5 seemed correct) for DAC. It also restrics M between 79 and 118. > @@ -238,10 +236,7 @@ static const intel_limit_t intel_limits_ironlake_dac = { > static const intel_limit_t intel_limits_ironlake_single_lvds = { > .dot = { .min = 25000, .max = 35 }, > .vco = { .min = 176, .max = 351 }, > - .n = { .min = 1, .max = 3 }, > - .m = { .min = 79, .max = 118 }, > - .m1 = { .min = 12, .max = 22 }, > - .m2 = { .min = 5, .max = 9 }, > + ILK_DPLL_M_N_LIMITS > .p = { .min = 28, .max = 112 }, > .p1 = { .min = 2, .max = 8 }, > .p2 = { .dot_limit = 225000, > @@ -251,10 +246,7 @@ static const intel_limit_t > intel_limits_ironlake_single_lvds = { > static const intel_limit_t intel_limits_ironlake_dual_lvds = { > .dot = { .min = 25000, .max = 35 }, > .vco = { .min = 176, .max = 351 }, > - .n = { .min = 1, .max = 3 }, > - .m = { .min = 79, .max = 127 }, > - .m1 = { .min = 12, .max = 22 }, > - .m2 = { .min = 5, .max = 9 }, > + ILK_DPLL_M_N_LIMITS > .p = { .min = 14, .max = 56 }, > .p1 = { .min = 2, .max = 8 }, > .p2 = { .dot_limit = 225000, For single channel LVDS with ref clock at 120MHz I have n between 3 and 5 (so 1,3 seemed correct). M is restricted to 79,118 here as well > @@ -265,10 +257,7 @@ static const intel_limit_t > intel_limits_ironlake_dual_lvds = { > static const intel_limit_t intel_limits_ironlake_single_lvds_100m = { > .dot = { .min = 25000, .max = 35 }, > .vco = { .min = 176, .max = 351 }, > - .n = { .min = 1, .max = 2 }, > - .m = { .min = 79, .max = 126 }, > - .m1 = { .min = 12, .max = 22 }, > - .m2 = { .min = 5, .max = 9 }, > + ILK_DPLL_M_N_LIMITS > .p = { .min = 28, .max = 112 }, > .p1 = { .min = 2, .max = 8 }, > .p2 = { .dot_limit = 225000, For dual channel LVDS, I have n between 3 and 5, so 1,3? M between 79,127 > @@ -278,10 +267,7 @@ static const intel_limit_t > intel_limits_ironlake_single_lvds_100m = { > static const intel_limit_t intel_limits_ironlake_dual_lvds_100m = { > .dot = { .min = 25000, .max = 35 }, > .vco = { .min = 176, .max = 351 }, > - .n = { .min = 1, .max = 3 }, > - .m = { .min = 79, .max = 126 }, > - .m1 = { .min = 12, .max = 22 }, > - .m2 = { .min = 5, .max = 9 }, > + ILK_DPLL_M_N_LIMITS > .p = { .min = 14, .max = 42 }, > .p1 = { .min = 2, .max = 6 }, > .p2 = { .dot_limit = 225000, And there n between 3,4 so (1,2), M was indeed between 79 and 126 -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Drop some no longer required mode/adjusted_mode parameters
On Sun, May 26, 2013 at 05:48:15PM +0200, Daniel Vetter wrote: > We can get at this easily through intel_crtc->config now. > > v2: Drop more stuff gcc spotted. > > v3: Drop even more stuff gcc spotted. > > Signed-off-by: Daniel Vetter Reviewed-by: Damien Lespiau ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 9/9] drm/i915: check for strange pfit pipe assignemnt on ivb/hsw
On Tue, May 21, 2013 at 09:54:59PM +0200, Daniel Vetter wrote: > Panel fitters on ivb/hsw are not created equal since not all of them > support the new high-quality upscaling mode. To offset this the hw > allows us to freely assign the pfits to pipes. > > Since our code currently doesn't support this we might fall over when > taking over firmware state. So check for this case and WARN about it. > We can then improve the code once we've hit this in the wild. Or once > we decide to support the improved upscale modes, though that requires > global arbitrage of modeset resources across crtcs. > > Suggested-by: Mika Kuoppala > Cc: Mika Kuoppala > Signed-off-by: Daniel Vetter This is already in with a IS_GEN7() -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 14/31] drm/i915: display pll hw state readout and checking
On Wed, Jun 05, 2013 at 01:34:16PM +0200, Daniel Vetter wrote: > Currently still with an empty register state, this will follow in a > next step. This one here just creates the new vfunc and uses it for > cross-checking, initial state takeover and the dpll assert function. > > And add a FIXME for the ddi pll readout code, which still needs to be > converted over. > > v2: > - Add some hw state readout debug output. > - Also cross check the enabled crtc counting. > > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_drv.h | 7 > drivers/gpu/drm/i915/intel_display.c | 77 > +--- > 2 files changed, 79 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 8e61128..f23b033 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -143,6 +143,9 @@ enum intel_dpll_id { > }; > #define I915_NUM_PLLS 2 > > +struct intel_dpll_hw_state { > +}; > + > struct intel_shared_dpll { > int refcount; /* count of number of CRTCs sharing this PLL */ > int active; /* count of number of active CRTCs (i.e. DPMS on) */ > @@ -150,10 +153,14 @@ struct intel_shared_dpll { > const char *name; > /* should match the index in the dev_priv->shared_dplls array */ > enum intel_dpll_id id; > + struct intel_dpll_hw_state hw_state; > void (*enable)(struct drm_i915_private *dev_priv, > struct intel_shared_dpll *pll); > void (*disable)(struct drm_i915_private *dev_priv, > struct intel_shared_dpll *pll); > + bool (*get_hw_state)(struct drm_i915_private *dev_priv, > + struct intel_shared_dpll *pll, > + struct intel_dpll_hw_state *hw_state); > }; > > /* Used by dp and fdi links */ > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index ea4b7a6..998ba5c 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -907,8 +907,8 @@ static void assert_shared_dpll(struct drm_i915_private > *dev_priv, > struct intel_shared_dpll *pll, > bool state) > { > - u32 val; > bool cur_state; > + struct intel_dpll_hw_state hw_state; > > if (HAS_PCH_LPT(dev_priv->dev)) { > DRM_DEBUG_DRIVER("LPT detected: skipping PCH PLL test\n"); > @@ -919,11 +919,10 @@ static void assert_shared_dpll(struct drm_i915_private > *dev_priv, > "asserting DPLL %s with no DPLL\n", state_string(state))) > return; > > - val = I915_READ(PCH_DPLL(pll->id)); > - cur_state = !!(val & DPLL_VCO_ENABLE); > + cur_state = pll->get_hw_state(dev_priv, pll, &hw_state); > WARN(cur_state != state, > - "%s assertion failure (expected %s, current %s), val=%08x\n", > - pll->name, state_string(state), state_string(cur_state), val); > + "%s assertion failure (expected %s, current %s)\n", > + pll->name, state_string(state), state_string(cur_state)); > } > #define assert_shared_dpll_enabled(d, p) assert_shared_dpll(d, p, true) > #define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false) > @@ -8071,6 +8070,8 @@ intel_modeset_check_state(struct drm_device *dev) > struct intel_encoder *encoder; > struct intel_connector *connector; > struct intel_crtc_config pipe_config; > + struct intel_dpll_hw_state dpll_hw_state; > + int i; > > list_for_each_entry(connector, &dev->mode_config.connector_list, > base.head) { > @@ -8183,6 +8184,41 @@ intel_modeset_check_state(struct drm_device *dev) > "[sw state]"); > } > } > + > + for (i = 0; i < dev_priv->num_shared_dpll; i++) { > + struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i]; > + int enabled_crtcs = 0, active_crtcs = 0; > + bool active; > + > + memset(&dpll_hw_state, 0, sizeof(dpll_hw_state)); > + > + DRM_DEBUG_KMS("%s\n", pll->name); > + > + active = pll->get_hw_state(dev_priv, pll, &dpll_hw_state); > + > + WARN(pll->active > pll->refcount, > + "more active pll users than references: %i vs %i\n", > + pll->active, pll->refcount); > + WARN(pll->active && !pll->on, > + "pll in active use but not on in sw tracking\n"); > + WARN(pll->on != active, > + "pll on state mismatch (expected %i, found %i)\n", > + pll->on, active); > + > + list_for_each_entry(crtc, &dev->mode_config.crtc_list, > + base.head) { > + if (crtc->base.enabled && > intel_crtc_to_shared_dpll(crtc) == pll) > +
Re: [Intel-gfx] [PATCH 15/31] drm/i915: extract readout_hw_state from setup_hw_state
On Wed, Jun 05, 2013 at 01:34:17PM +0200, Daniel Vetter wrote: > Simply grew too big. This also makes the fixup and restore logic in > setup_hw_state stand out a bit more clearly. > > Signed-off-by: Daniel Vetter Reviewed-by: Damien Lespiau -- Damien > --- > drivers/gpu/drm/i915/intel_display.c | 20 +++- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 998ba5c..95ed27b 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -9630,14 +9630,10 @@ void i915_redisable_vga(struct drm_device *dev) > } > } > > -/* Scan out the current hw modeset state, sanitizes it and maps it into the > drm > - * and i915 state tracking structures. */ > -void intel_modeset_setup_hw_state(struct drm_device *dev, > - bool force_restore) > +static void intel_modeset_readout_hw_state(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > enum pipe pipe; > - struct drm_plane *plane; > struct intel_crtc *crtc; > struct intel_encoder *encoder; > struct intel_connector *connector; > @@ -9713,6 +9709,20 @@ void intel_modeset_setup_hw_state(struct drm_device > *dev, > drm_get_connector_name(&connector->base), > connector->base.encoder ? "enabled" : "disabled"); > } > +} > + > +/* Scan out the current hw modeset state, sanitizes it and maps it into the > drm > + * and i915 state tracking structures. */ > +void intel_modeset_setup_hw_state(struct drm_device *dev, > + bool force_restore) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + enum pipe pipe; > + struct drm_plane *plane; > + struct intel_crtc *crtc; > + struct intel_encoder *encoder; > + > + intel_modeset_readout_hw_state(dev); > > /* HW state is read out, now we need to sanitize this mess. */ > list_for_each_entry(encoder, &dev->mode_config.encoder_list, > -- > 1.7.11.7 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 16/31] drm/i915: split up intel_modeset_check_state
On Wed, Jun 05, 2013 at 01:34:18PM +0200, Daniel Vetter wrote: > Simply grew too large and neede to be split up into parts. > > Signed-off-by: Daniel Vetter Reviewed-by: Damien Lespiau -- Damien > --- > drivers/gpu/drm/i915/intel_display.c | 44 > +--- > 1 file changed, 36 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 95ed27b..c42b87b 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -8062,16 +8062,10 @@ intel_pipe_config_compare(struct drm_device *dev, > return true; > } > > -void > -intel_modeset_check_state(struct drm_device *dev) > +static void > +check_connector_state(struct drm_device *dev) > { > - drm_i915_private_t *dev_priv = dev->dev_private; > - struct intel_crtc *crtc; > - struct intel_encoder *encoder; > struct intel_connector *connector; > - struct intel_crtc_config pipe_config; > - struct intel_dpll_hw_state dpll_hw_state; > - int i; > > list_for_each_entry(connector, &dev->mode_config.connector_list, > base.head) { > @@ -8082,6 +8076,13 @@ intel_modeset_check_state(struct drm_device *dev) > WARN(&connector->new_encoder->base != connector->base.encoder, >"connector's staged encoder doesn't match current > encoder\n"); > } > +} > + > +static void > +check_encoder_state(struct drm_device *dev) > +{ > + struct intel_encoder *encoder; > + struct intel_connector *connector; > > list_for_each_entry(encoder, &dev->mode_config.encoder_list, > base.head) { > @@ -8133,6 +8134,15 @@ intel_modeset_check_state(struct drm_device *dev) >tracked_pipe, pipe); > > } > +} > + > +static void > +check_crtc_state(struct drm_device *dev) > +{ > + drm_i915_private_t *dev_priv = dev->dev_private; > + struct intel_crtc *crtc; > + struct intel_encoder *encoder; > + struct intel_crtc_config pipe_config; > > list_for_each_entry(crtc, &dev->mode_config.crtc_list, > base.head) { > @@ -8184,6 +8194,15 @@ intel_modeset_check_state(struct drm_device *dev) > "[sw state]"); > } > } > +} > + > +static void > +check_shared_dpll_state(struct drm_device *dev) > +{ > + drm_i915_private_t *dev_priv = dev->dev_private; > + struct intel_crtc *crtc; > + struct intel_dpll_hw_state dpll_hw_state; > + int i; > > for (i = 0; i < dev_priv->num_shared_dpll; i++) { > struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i]; > @@ -8221,6 +8240,15 @@ intel_modeset_check_state(struct drm_device *dev) > } > } > > +void > +intel_modeset_check_state(struct drm_device *dev) > +{ > + check_connector_state(dev); > + check_encoder_state(dev); > + check_crtc_state(dev); > + check_shared_dpll_state(dev); > +} > + > static int __intel_set_mode(struct drm_crtc *crtc, > struct drm_display_mode *mode, > int x, int y, struct drm_framebuffer *fb) > -- > 1.7.11.7 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 17/31] drm/i915: WARN on lack of shared dpll
On Wed, Jun 05, 2013 at 01:34:19PM +0200, Daniel Vetter wrote: > Now that we have proper hw state reconstruction we should never have a > case where we don't have the software dpll state properly set up. So > add WARNs to the respective !pll cases in enable/disabel_shared_dpll. > > Signed-off-by: Daniel Vetter Reviewed-by: Damien Lespiau -- Damien > --- > 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 c42b87b..388ac54 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1371,7 +1371,7 @@ static void ironlake_enable_shared_dpll(struct > intel_crtc *crtc) > > /* PCH PLLs only available on ILK, SNB and IVB */ > BUG_ON(dev_priv->info->gen < 5); > - if (pll == NULL) > + if (WARN_ON(pll == NULL)) > return; > > if (WARN_ON(pll->refcount == 0)) > @@ -1399,7 +1399,7 @@ static void intel_disable_shared_dpll(struct intel_crtc > *crtc) > > /* PCH only available on ILK+ */ > BUG_ON(dev_priv->info->gen < 5); > - if (pll == NULL) > + if (WARN_ON(pll == NULL)) > return; > > if (WARN_ON(pll->refcount == 0)) > -- > 1.7.11.7 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 14/31] drm/i915: display pll hw state readout and checking
On Wed, Jun 12, 2013 at 04:39:14PM +0300, Ville Syrjälä wrote: > On Wed, Jun 12, 2013 at 02:31:23PM +0100, Damien Lespiau wrote: > > On Wed, Jun 05, 2013 at 01:34:16PM +0200, Daniel Vetter wrote: > > > @@ -8621,6 +8657,17 @@ static void intel_cpu_pll_init(struct drm_device > > > *dev) > > > intel_ddi_pll_init(dev); > > > } > > > > > > +static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv, > > > + struct intel_shared_dpll *pll, > > > + struct intel_dpll_hw_state *hw_state) > > > +{ > > > + uint32_t val; > > > + > > > + val = I915_READ(PCH_DPLL(pll->id)); > > > + > > > + return val & DPLL_VCO_ENABLE; > > > +} > > > + > > > > Don't we want !!(val & DPLL_VCO_ENABLE) here? we're comparing this to 0 > > and 1. > > bool is always 0 or 1. Oh, of course! Reviewed-by: Damien Lespiau then. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 18/31] drm/i915: hw state readout and cross-checking for shared dplls
On Wed, Jun 05, 2013 at 01:34:20PM +0200, Daniel Vetter wrote: > Just the plumbing, all the modeset and enable code has not yet been > switched over to use the new state. It seems to be decently broken > anyway, at least wrt to handling of the special pixel mutliplier > enabling sequence. Follow-up patches will clean up that mess. > > Another missing piece is more careful handling (and fixup) of the fp1 > alternate divisor state. The BIOS most likely doesn't bother to > program that one to what we expect. So we need to be more careful with > comparing that state, both for cross checking but also when checking > for dpll sharing when acquiring shared dpll. Otherwise fastboot will > deny a few shared dpll configurations which would otherwise work. > > v2: We need to memcpy the pipe config dpll hw state into the pll, for > otherwise the cross-check code will get angry. > > v3: Don't forget to read the pch pll state in the crtc get_pipe_config > function for ibx/ilk platforms. > > Signed-off-by: Daniel Vetter Reviewed-by: Damien Lespiau -- Damien > --- > drivers/gpu/drm/i915/i915_drv.h | 3 +++ > drivers/gpu/drm/i915/intel_display.c | 38 > > drivers/gpu/drm/i915/intel_drv.h | 3 +++ > 3 files changed, 44 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index f23b033..4dc94ed 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -144,6 +144,9 @@ enum intel_dpll_id { > #define I915_NUM_PLLS 2 > > struct intel_dpll_hw_state { > + uint32_t dpll; > + uint32_t fp0; > + uint32_t fp1; > }; > > struct intel_shared_dpll { > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 388ac54..a30e27a 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3055,7 +3055,11 @@ found: > crtc->config.shared_dpll = i; > DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->name, >pipe_name(crtc->pipe)); > + > if (pll->active == 0) { > + memcpy(&pll->hw_state, &crtc->config.dpll_hw_state, > +sizeof(pll->hw_state)); > + > DRM_DEBUG_DRIVER("setting up %s\n", pll->name); > WARN_ON(pll->on); > assert_shared_dpll_disabled(dev_priv, pll); > @@ -5659,6 +5663,13 @@ static int ironlake_crtc_mode_set(struct drm_crtc > *crtc, >&fp, &reduced_clock, >has_reduced_clock ? &fp2 : NULL); > > + intel_crtc->config.dpll_hw_state.dpll = dpll | DPLL_VCO_ENABLE; > + intel_crtc->config.dpll_hw_state.fp0 = fp; > + if (has_reduced_clock) > + intel_crtc->config.dpll_hw_state.fp1 = fp2; > + else > + intel_crtc->config.dpll_hw_state.fp1 = fp; > + > pll = intel_get_shared_dpll(intel_crtc, dpll, fp); > if (pll == NULL) { > DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", > @@ -5778,6 +5789,8 @@ static bool ironlake_get_pipe_config(struct intel_crtc > *crtc, > return false; > > if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) { > + struct intel_shared_dpll *pll; > + > pipe_config->has_pch_encoder = true; > > tmp = I915_READ(FDI_RX_CTL(crtc->pipe)); > @@ -5799,6 +5812,11 @@ static bool ironlake_get_pipe_config(struct intel_crtc > *crtc, > else > pipe_config->shared_dpll = DPLL_ID_PCH_PLL_A; > } > + > + pll = &dev_priv->shared_dplls[pipe_config->shared_dpll]; > + > + WARN_ON(!pll->get_hw_state(dev_priv, pll, > +&pipe_config->dpll_hw_state)); > } else { > pipe_config->pixel_multiplier = 1; > } > @@ -7987,6 +8005,15 @@ intel_pipe_config_compare(struct drm_device *dev, > struct intel_crtc_config *current_config, > struct intel_crtc_config *pipe_config) > { > +#define PIPE_CONF_CHECK_X(name) \ > + if (current_config->name != pipe_config->name) { \ > + DRM_ERROR("mismatch in " #name " " \ > + "(expected 0x%08x, found 0x%08x)\n", \ > + current_config-&g
Re: [Intel-gfx] [PATCH 19/31] drm/i915: fix up pch pll enabling for pixel multipliers
On Wed, Jun 05, 2013 at 01:34:21PM +0200, Daniel Vetter wrote: > We have a nice comment saying that the pixel multiplier only sticks > once the vco is on and stable. The only problem is that the enable bit > wasn't set at all. This patch fixes this and so brings the ilk+ pch > pll code in line with the i8xx/i9xx pll code. Or at least improves > matters a lot. > > This should fix sdvo on ilk-ivb for low-res modes. > > Signed-off-by: Daniel Vetter Reviewed-by: Damien Lespiau -- Damien > --- > 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 a30e27a..ecf0b1e 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5601,7 +5601,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc > *intel_crtc, > else > dpll |= PLL_REF_INPUT_DREFCLK; > > - return dpll; > + return dpll | DPLL_VCO_ENABLE; > } > > static int ironlake_crtc_mode_set(struct drm_crtc *crtc, > @@ -5663,7 +5663,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, >&fp, &reduced_clock, >has_reduced_clock ? &fp2 : NULL); > > - intel_crtc->config.dpll_hw_state.dpll = dpll | DPLL_VCO_ENABLE; > + intel_crtc->config.dpll_hw_state.dpll = dpll; > intel_crtc->config.dpll_hw_state.fp0 = fp; > if (has_reduced_clock) > intel_crtc->config.dpll_hw_state.fp1 = fp2; > -- > 1.7.11.7 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 22/31] drm/i915: use sw tracked state to select shared dplls
On Wed, Jun 05, 2013 at 01:34:24PM +0200, Daniel Vetter wrote: > Just yet another prep step to be able to do all this up-front, before > we've set up any of the shared dplls in the new state. This will > eventually be useful for atomic modesetting. > > Signed-off-by: Daniel Vetter Reviewed-by: Damien Lespiau -- Damien > --- > drivers/gpu/drm/i915/intel_display.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 334f86a..4d2284e 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2999,7 +2999,7 @@ static void intel_put_shared_dpll(struct intel_crtc > *crtc) > crtc->config.shared_dpll = DPLL_ID_NONE; > } > > -static struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc > *crtc, u32 dpll, u32 fp) > +static struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc > *crtc) > { > struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; > struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc); > @@ -3029,8 +3029,8 @@ static struct intel_shared_dpll > *intel_get_shared_dpll(struct intel_crtc *crtc, > if (pll->refcount == 0) > continue; > > - if (dpll == (I915_READ(PCH_DPLL(pll->id)) & 0x7fff) && > - fp == I915_READ(PCH_FP0(pll->id))) { > + if (memcmp(&crtc->config.dpll_hw_state, &pll->hw_state, > +sizeof(pll->hw_state)) == 0) { > DRM_DEBUG_KMS("CRTC:%d sharing existing %s (refcount > %d, ative %d)\n", > crtc->base.base.id, > pll->name, pll->refcount, pll->active); > @@ -5660,7 +5660,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, > else > intel_crtc->config.dpll_hw_state.fp1 = fp; > > - pll = intel_get_shared_dpll(intel_crtc, dpll, fp); > + pll = intel_get_shared_dpll(intel_crtc); > if (pll == NULL) { > DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n", >pipe_name(pipe)); > -- > 1.7.11.7 > > ___ > 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