[Bug 66888] New: [radeonsi] Need support GL_EXT_framebuffer_multisample
https://bugs.freedesktop.org/show_bug.cgi?id=66888 Priority: medium Bug ID: 66888 Assignee: dri-devel@lists.freedesktop.org Summary: [radeonsi] Need support GL_EXT_framebuffer_multisample Severity: enhancement Classification: Unclassified OS: Linux (All) Reporter: granti...@gmail.com Hardware: x86 (IA32) Status: NEW Version: git Component: Drivers/Gallium/radeonsi Product: Mesa Created attachment 82401 --> https://bugs.freedesktop.org/attachment.cgi?id=82401&action=edit graphical glitch unigine-tropics and unigine-sanctuary don't runs. Required extension GL_EXT_framebuffer_multisample is not supported. ArchLinux x86; kernel 3.10; mesa - git; xf86-video-ati - git; llvm - 3.3 Radeon HD 7950 OpenGL renderer string: Gallium 0.4 on AMD TAHITI OpenGL version string: 2.1 Mesa 9.2.0-devel (git-796b73d) OpenGL shading language version string: 1.30 If run MESA_EXTENSION_OVERRIDE="GL_EXT_framebuffer_multisample" unigine-tropics, test runs but have graphical glitch. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 66888] [radeonsi] Need support GL_EXT_framebuffer_multisample
https://bugs.freedesktop.org/show_bug.cgi?id=66888 Vladimir Ysikov changed: What|Removed |Added Attachment #82401|text/plain |image/png mime type|| -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 66425] "failed testing IB on ring 5" when suspending to disk
https://bugs.freedesktop.org/show_bug.cgi?id=66425 --- Comment #24 from Austin Lund --- Patch tested and works on my machine. I now have problems for "processors" when doing pm_test, so I still cannot actually test this on a full resume from disk, but at least pm_test with "devices" works. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 66425] "failed testing IB on ring 5" when suspending to disk
https://bugs.freedesktop.org/show_bug.cgi?id=66425 Christian König changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #25 from Christian König --- Sounds like we can merk this as resolved now. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 66714] Mobility Radeon HD 5650 doesn't boot with kernel 3.10 (and newer) when using radeon.audio=1
https://bugs.freedesktop.org/show_bug.cgi?id=66714 Marco Trevisan (Treviño) changed: What|Removed |Added Status|NEW |ASSIGNED --- Comment #4 from Marco Trevisan (Treviño) --- So, I've tried the patch on a 3.10 kernel that was affected and... Great, it Works! :) FYI, I've also added a couple of error logs in the code to see where exactly the failure was and this was the result: [2.073137] [drm] fb mappable at 0xC035F000 [2.073139] [drm] vram apper at 0xC000 [2.073140] [drm] size 8294400 [2.073141] [drm] fb depth is 24 [2.073142] [drm]pitch is 7680 [2.073252] fbcon: radeondrmfb (fb0) is primary device [2.073323] [drm:evergreen_hdmi_enable] *ERROR* Invalid DIG (dig: 880229d0fc00, dig->afmt: (null)) /home/marco/Dev/debs/linux-3.10.0/drivers/gpu/drm/radeon/evergreen_hdmi.c:298 [2.573589] [drm:evergreen_hdmi_enable] *ERROR* Invalid DIG (dig: 880229d0fc00, dig->afmt: (null)) /home/marco/Dev/debs/linux-3.10.0/drivers/gpu/drm/radeon/evergreen_hdmi.c:298 [2.607449] Console: switching to colour frame buffer device 170x48 [2.610160] radeon :01:00.0: fb0: radeondrmfb frame buffer device [2.610162] radeon :01:00.0: registered panic notifier [2.610237] [drm] Initialized radeon 2.33.0 20080528 for :01:00.0 on minor 0 Full dmesg at http://pastebin.ubuntu.com/5873667/ I don't know if this may be only the side effect of another issue (wrong initialization order?). One more thing I discovered in the past days, is that the kernel crash was not happening in case that the HDMI was plugged after that ligthtdm was running (attaching it anytime before was leading to a freeze). -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 66714] Mobility Radeon HD 5650 doesn't boot with kernel 3.10 (and newer) when using radeon.audio=1
https://bugs.freedesktop.org/show_bug.cgi?id=66714 --- Comment #5 from Marco Trevisan (Treviño) --- Comment on attachment 82200 --> https://bugs.freedesktop.org/attachment.cgi?id=82200 possible fix Review of attachment 82200: - Why not including some DRM_ERROR logs as well? -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] radeon kms: do not flush uninitialized hotplug work
Fix a warning from lockdep caused by calling flush_work() for uninitialized hotplug work. Initialize hotplug_work, audio_work and reset_work upon successful radeon_irq_kms_init() completion and thus perform hotplug flush_work only when rdev->irq.installed is true. [4.790019] [drm] Loading CEDAR Microcode [4.790943] r600_cp: Failed to load firmware "radeon/CEDAR_smc.bin" [4.791152] [drm:evergreen_startup] *ERROR* Failed to load firmware! [4.791330] radeon :01:00.0: disabling GPU acceleration [4.792633] INFO: trying to register non-static key. [4.792792] the code is fine but needs lockdep annotation. [4.792953] turning off the locking correctness validator. [4.793114] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 3.11.0-rc0-dbg-10676-gfe56456-dirty #1816 [4.793314] Hardware name: Acer Aspire 5741G/Aspire 5741G , BIOS V1.20 02/08/2011 [4.793507] 821fd810 8801530b9a18 8160434e 0002 [4.794155] 8801530b9ad8 810b8404 8801530b0798 8801530b [4.794789] 8801530b9b00 0046 04c0 [4.795418] Call Trace: [4.795573] [] dump_stack+0x4e/0x82 [4.795731] [] __lock_acquire+0x1a64/0x1d30 [4.795893] [] ? dev_vprintk_emit+0x50/0x60 [4.796034] [] lock_acquire+0xa4/0x200 [4.796216] [] ? flush_work+0x5/0x280 [4.796375] [] flush_work+0x3d/0x280 [4.796520] [] ? flush_work+0x5/0x280 [4.796682] [] ? trace_hardirqs_on_caller+0xfd/0x1c0 [4.796862] [] ? delay_tsc+0x95/0xf0 [4.797024] [] radeon_irq_kms_fini+0x2b/0x70 [4.797186] [] evergreen_init+0x2a9/0x2e0 [4.797347] [] radeon_device_init+0x5ef/0x700 [4.797511] [] ? pci_find_capability+0x47/0x50 [4.797672] [] radeon_driver_load_kms+0x8d/0x150 [4.797843] [] drm_get_pci_dev+0x166/0x280 [4.798007] [] ? kfree+0xf5/0x2e0 [4.798168] [] ? radeon_pci_probe+0x98/0xd0 [4.798329] [] radeon_pci_probe+0xaa/0xd0 [4.798489] [] pci_device_probe+0x84/0xe0 [4.798644] [] driver_probe_device+0x76/0x240 [4.798805] [] __driver_attach+0x93/0xa0 [4.798948] [] ? __device_attach+0x40/0x40 [4.799126] [] bus_for_each_dev+0x6b/0xb0 [4.799272] [] driver_attach+0x1e/0x20 [4.799434] [] bus_add_driver+0x1f0/0x280 [4.799596] [] driver_register+0x74/0x150 [4.799758] [] __pci_register_driver+0x5d/0x60 [4.799936] [] ? ttm_init+0x67/0x67 [4.800081] [] drm_pci_init+0x115/0x130 [4.800243] [] ? ttm_init+0x67/0x67 [4.800405] [] radeon_init+0x9c/0xba [4.800586] [] do_one_initcall+0xfa/0x150 [4.800746] [] ? parse_args+0x120/0x330 [4.800909] [] kernel_init_freeable+0x111/0x191 [4.801052] [] ? do_early_param+0x88/0x88 [4.801233] [] ? rest_init+0x140/0x140 [4.801393] [] kernel_init+0xe/0x180 [4.801556] [] ret_from_fork+0x7c/0xb0 [4.801718] [] ? rest_init+0x140/0x140 Signed-off-by: Sergey Senozhatsky --- drivers/gpu/drm/radeon/radeon_irq_kms.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c index bcdefd1..081886b 100644 --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c @@ -260,10 +260,6 @@ int radeon_irq_kms_init(struct radeon_device *rdev) { int r = 0; - INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func); - INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi); - INIT_WORK(&rdev->reset_work, radeon_irq_reset_work_func); - spin_lock_init(&rdev->irq.lock); r = drm_vblank_init(rdev->ddev, rdev->num_crtc); if (r) { @@ -285,6 +281,11 @@ int radeon_irq_kms_init(struct radeon_device *rdev) rdev->irq.installed = false; return r; } + + INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func); + INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi); + INIT_WORK(&rdev->reset_work, radeon_irq_reset_work_func); + DRM_INFO("radeon: irq initialized.\n"); return 0; } @@ -304,8 +305,8 @@ void radeon_irq_kms_fini(struct radeon_device *rdev) rdev->irq.installed = false; if (rdev->msi_enabled) pci_disable_msi(rdev->pdev); + flush_work(&rdev->hotplug_work); } - flush_work(&rdev->hotplug_work); } /** ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 63935] TURKS [drm:r600_uvd_init] *ERROR* UVD not responding, trying to reset the VCPU!!!
https://bugs.freedesktop.org/show_bug.cgi?id=63935 Christian König changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |FIXED --- Comment #57 from Christian König --- The suspend/resume issue was something completely different, so we probably can now close this bugreport. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 66714] Mobility Radeon HD 5650 doesn't boot with kernel 3.10 (and newer) when using radeon.audio=1
https://bugs.freedesktop.org/show_bug.cgi?id=66714 Christian König changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #6 from Christian König --- (In reply to comment #5) > Comment on attachment 82200 [details] [review] > possible fix > > Review of attachment 82200 [details] [review]: > - > > Why not including some DRM_ERROR logs as well? Because that isn't a bug. You just have an LVDS pannel connected to a DIG encoder and so this encoder doesn't have an audio block. Alex patch is already quite right. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] radeon kms: do not flush uninitialized hotplug work
On Sun, Jul 14, 2013 at 7:03 AM, Sergey Senozhatsky wrote: > Fix a warning from lockdep caused by calling flush_work() for > uninitialized hotplug work. Initialize hotplug_work, audio_work > and reset_work upon successful radeon_irq_kms_init() completion > and thus perform hotplug flush_work only when rdev->irq.installed > is true. > > [4.790019] [drm] Loading CEDAR Microcode > [4.790943] r600_cp: Failed to load firmware "radeon/CEDAR_smc.bin" > [4.791152] [drm:evergreen_startup] *ERROR* Failed to load firmware! > [4.791330] radeon :01:00.0: disabling GPU acceleration > > [4.792633] INFO: trying to register non-static key. > [4.792792] the code is fine but needs lockdep annotation. > [4.792953] turning off the locking correctness validator. > > [4.793114] CPU: 2 PID: 1 Comm: swapper/0 Not tainted > 3.11.0-rc0-dbg-10676-gfe56456-dirty #1816 > [4.793314] Hardware name: Acer Aspire 5741G/Aspire 5741G > , BIOS V1.20 02/08/2011 > [4.793507] 821fd810 8801530b9a18 8160434e > 0002 > [4.794155] 8801530b9ad8 810b8404 8801530b0798 > 8801530b > [4.794789] 8801530b9b00 0046 04c0 > > [4.795418] Call Trace: > [4.795573] [] dump_stack+0x4e/0x82 > [4.795731] [] __lock_acquire+0x1a64/0x1d30 > [4.795893] [] ? dev_vprintk_emit+0x50/0x60 > [4.796034] [] lock_acquire+0xa4/0x200 > [4.796216] [] ? flush_work+0x5/0x280 > [4.796375] [] flush_work+0x3d/0x280 > [4.796520] [] ? flush_work+0x5/0x280 > [4.796682] [] ? trace_hardirqs_on_caller+0xfd/0x1c0 > [4.796862] [] ? delay_tsc+0x95/0xf0 > [4.797024] [] radeon_irq_kms_fini+0x2b/0x70 > [4.797186] [] evergreen_init+0x2a9/0x2e0 > [4.797347] [] radeon_device_init+0x5ef/0x700 > [4.797511] [] ? pci_find_capability+0x47/0x50 > [4.797672] [] radeon_driver_load_kms+0x8d/0x150 > [4.797843] [] drm_get_pci_dev+0x166/0x280 > [4.798007] [] ? kfree+0xf5/0x2e0 > [4.798168] [] ? radeon_pci_probe+0x98/0xd0 > [4.798329] [] radeon_pci_probe+0xaa/0xd0 > [4.798489] [] pci_device_probe+0x84/0xe0 > [4.798644] [] driver_probe_device+0x76/0x240 > [4.798805] [] __driver_attach+0x93/0xa0 > [4.798948] [] ? __device_attach+0x40/0x40 > [4.799126] [] bus_for_each_dev+0x6b/0xb0 > [4.799272] [] driver_attach+0x1e/0x20 > [4.799434] [] bus_add_driver+0x1f0/0x280 > [4.799596] [] driver_register+0x74/0x150 > [4.799758] [] __pci_register_driver+0x5d/0x60 > [4.799936] [] ? ttm_init+0x67/0x67 > [4.800081] [] drm_pci_init+0x115/0x130 > [4.800243] [] ? ttm_init+0x67/0x67 > [4.800405] [] radeon_init+0x9c/0xba > [4.800586] [] do_one_initcall+0xfa/0x150 > [4.800746] [] ? parse_args+0x120/0x330 > [4.800909] [] kernel_init_freeable+0x111/0x191 > [4.801052] [] ? do_early_param+0x88/0x88 > [4.801233] [] ? rest_init+0x140/0x140 > [4.801393] [] kernel_init+0xe/0x180 > [4.801556] [] ret_from_fork+0x7c/0xb0 > [4.801718] [] ? rest_init+0x140/0x140 > > > Signed-off-by: Sergey Senozhatsky > Applied to by fixes tree. thanks! Alex > --- > > drivers/gpu/drm/radeon/radeon_irq_kms.c | 11 ++- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c > b/drivers/gpu/drm/radeon/radeon_irq_kms.c > index bcdefd1..081886b 100644 > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c > @@ -260,10 +260,6 @@ int radeon_irq_kms_init(struct radeon_device *rdev) > { > int r = 0; > > - INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func); > - INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi); > - INIT_WORK(&rdev->reset_work, radeon_irq_reset_work_func); > - > spin_lock_init(&rdev->irq.lock); > r = drm_vblank_init(rdev->ddev, rdev->num_crtc); > if (r) { > @@ -285,6 +281,11 @@ int radeon_irq_kms_init(struct radeon_device *rdev) > rdev->irq.installed = false; > return r; > } > + > + INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func); > + INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi); > + INIT_WORK(&rdev->reset_work, radeon_irq_reset_work_func); > + > DRM_INFO("radeon: irq initialized.\n"); > return 0; > } > @@ -304,8 +305,8 @@ void radeon_irq_kms_fini(struct radeon_device *rdev) > rdev->irq.installed = false; > if (rdev->msi_enabled) > pci_disable_msi(rdev->pdev); > + flush_work(&rdev->hotplug_work); > } > - flush_work(&rdev->hotplug_work); > } > > /** > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel __
[PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
This patch fixes regression in power consumtion of sandy bridge gpu, which exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking that it's extremely busy. After that it never reaches rc6 state. Bug was introduce by commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 ("drm/i915: load boot context at driver init time"). Without documentation it's not clear what is happening here, probably this breaks internal state of hardware ring buffers and confuses RPS engine. Fortunately keeping forcewake during whole initialization sequence in gen6_init_clock_gating() fixes this bug. References: https://bugs.freedesktop.org/show_bug.cgi?id=54089 Signed-off-by: Konstantin Khlebnikov --- 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 aa01128..839a43f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3640,6 +3640,8 @@ static void gen6_init_clock_gating(struct drm_device *dev) int pipe; uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE; + gen6_gt_force_wake_get(dev_priv); + I915_WRITE(ILK_DSPCLK_GATE_D, dspclk_gate); I915_WRITE(ILK_DISPLAY_CHICKEN2, @@ -3728,6 +3730,8 @@ static void gen6_init_clock_gating(struct drm_device *dev) cpt_init_clock_gating(dev); gen6_check_mch_setup(dev); + + gen6_gt_force_wake_put(dev_priv); } static void gen7_setup_fixed_func_scheduler(struct drm_i915_private *dev_priv) ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Sun, Jul 14, 2013 at 6:30 PM, Konstantin Khlebnikov wrote: > This patch fixes regression in power consumtion of sandy bridge gpu, which > exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking that > it's extremely busy. After that it never reaches rc6 state. > > Bug was introduce by commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 > ("drm/i915: load boot context at driver init time"). Without documentation > it's not clear what is happening here, probably this breaks internal state of > hardware ring buffers and confuses RPS engine. Fortunately keeping forcewake > during whole initialization sequence in gen6_init_clock_gating() fixes this > bug. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=54089 > Signed-off-by: Konstantin Khlebnikov We already hold an forcewake reference while setting up the rps stuff, should we maybe hold the forcewake for the entire duration, i.e. grab it here in clock_gating and release it only in gen6/vlv_enable_rps? Can you please test that version, too? In any case the forcewake grabbing here in the clock gating function needs a big comment that otherwise setting the MCTL register might break rc6 entry. -Daniel > --- > 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 aa01128..839a43f 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3640,6 +3640,8 @@ static void gen6_init_clock_gating(struct drm_device > *dev) > int pipe; > uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE; > > + gen6_gt_force_wake_get(dev_priv); > + > I915_WRITE(ILK_DSPCLK_GATE_D, dspclk_gate); > > I915_WRITE(ILK_DISPLAY_CHICKEN2, > @@ -3728,6 +3730,8 @@ static void gen6_init_clock_gating(struct drm_device > *dev) > cpt_init_clock_gating(dev); > > gen6_check_mch_setup(dev); > + > + gen6_gt_force_wake_put(dev_priv); > } > > static void gen7_setup_fixed_func_scheduler(struct drm_i915_private > *dev_priv) > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 64850] Second screen black on Pitcairn PRO
https://bugs.freedesktop.org/show_bug.cgi?id=64850 --- Comment #23 from Jakob Nixdorf --- Just tested the first kernel with RadeonSI support (3.4.0 to my knowledge) with the same result. Second screen stays black in TTY and in Xorg. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
Daniel Vetter wrote: On Sun, Jul 14, 2013 at 6:30 PM, Konstantin Khlebnikov wrote: This patch fixes regression in power consumtion of sandy bridge gpu, which exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking that it's extremely busy. After that it never reaches rc6 state. Bug was introduce by commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 ("drm/i915: load boot context at driver init time"). Without documentation it's not clear what is happening here, probably this breaks internal state of hardware ring buffers and confuses RPS engine. Fortunately keeping forcewake during whole initialization sequence in gen6_init_clock_gating() fixes this bug. References: https://bugs.freedesktop.org/show_bug.cgi?id=54089 Signed-off-by: Konstantin Khlebnikov We already hold an forcewake reference while setting up the rps stuff, should we maybe hold the forcewake for the entire duration, i.e. grab it here in clock_gating and release it only in gen6/vlv_enable_rps? Can you please test that version, too? This will be racy because rps stuff is done in separate work which might be canceled if intel_disable_gt_powersave() happens before its completion. In any case the forcewake grabbing here in the clock gating function needs a big comment that otherwise setting the MCTL register might break rc6 entry. -Daniel --- 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 aa01128..839a43f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3640,6 +3640,8 @@ static void gen6_init_clock_gating(struct drm_device *dev) int pipe; uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE; + gen6_gt_force_wake_get(dev_priv); + I915_WRITE(ILK_DSPCLK_GATE_D, dspclk_gate); I915_WRITE(ILK_DISPLAY_CHICKEN2, @@ -3728,6 +3730,8 @@ static void gen6_init_clock_gating(struct drm_device *dev) cpt_init_clock_gating(dev); gen6_check_mch_setup(dev); + + gen6_gt_force_wake_put(dev_priv); } static void gen7_setup_fixed_func_scheduler(struct drm_i915_private *dev_priv) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Sun, Jul 14, 2013 at 06:52:39PM +0200, Daniel Vetter wrote: > On Sun, Jul 14, 2013 at 6:30 PM, Konstantin Khlebnikov > wrote: > > This patch fixes regression in power consumtion of sandy bridge gpu, which > > exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking > > that > > it's extremely busy. After that it never reaches rc6 state. > > > > Bug was introduce by commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 > > ("drm/i915: load boot context at driver init time"). Without documentation > > it's not clear what is happening here, probably this breaks internal state > > of > > hardware ring buffers and confuses RPS engine. Fortunately keeping forcewake > > during whole initialization sequence in gen6_init_clock_gating() fixes this > > bug. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=54089 > > Signed-off-by: Konstantin Khlebnikov > > We already hold an forcewake reference while setting up the rps stuff, > should we maybe hold the forcewake for the entire duration, i.e. grab > it here in clock_gating and release it only in gen6/vlv_enable_rps? > Can you please test that version, too? > > In any case the forcewake grabbing here in the clock gating function > needs a big comment that otherwise setting the MCTL register might > break rc6 entry. It is not clear why the forcewake works, but is easy to imagine one of the operations in that sequence requires the GPU to be awake at the time of programming for it to succeed. MBCTL:EnableBootFetch does seem the most suspicious from its wording in the bspec. I guess all instances of poking this bit should be protected similary (snb, ivb, vlv, hsw). Based on that reasoning and that waking the GPU up here has no negative consequences, and so long as all paths are fixed, I am happy to give this an Acked-by: Chris Wilson Also, we need to reapply the w/a after a Function Level Reset, in other words we do need to repeat the init_clock_gating after intel_gpu_reset(). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DT binding review for Armada display subsystem
On Sat, Jul 13, 2013 at 08:25:15AM -0600, Daniel Drake wrote: > I guess the IRE falls into the same category as the DCON - we won't > implement it for now, but knowing where it might fit in is useful. I don't see much need at the moment for IRE. IRE isn't going to be useful for general graphics rotation as it only supports up to 16bpp RGB. It looks to me like this module was designed to rotate YUV/camera images. If we wanted to rotate a video image, then it would probably be more efficient to use this, but it will cause a conversion of the image format in doing so. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DT binding review for Armada display subsystem
On 07/13/2013 04:25 PM, Daniel Drake wrote: On Sat, Jul 13, 2013 at 2:35 AM, Jean-Francois Moine wrote: I use my Cubox for daily jobs as a desktop computer. My kernel is a DT driven 3.10.0. The dove-drm, tda998x and si5351 (clock) are kernel modules. I set 3 clocks in the DT for the LCD0: lcdclk, axi and extclk0 (si5351). Normally, the external clock is used, but, sometimes, the si5351 module is not yet initialized when the drm driver starts. So, for 1920x1080p, it uses the lcdclk which sets the LCD clock to 13 (40/3) instead of 148500. As a result, display and sound still work correctly on my TV set thru HDMI. So, it would be nice to have 2 usable clocks per LCD, until we find a good way to initialize the modules in the right order at startup time. Having multiple usable clocks is implemented in the patch I referred you to. However it doesn't solve the "better clock turns up after initializing the driver" problem, which seems like a tricky one. Maybe the best solution is to defer probe until all DT-defined clocks become available. That means that whoever compiles the kernel must take care to not forget to build the clock drivers for all the clocks referenced in this part of the DT, but perhaps that is a reasonable thing to require. On the other hand, this problem acts in support of a simpler approach mentioned by Sebastian: developers figure out what the best clock is for each CRTC on each board, and the DT only specifies that one clock. The driver uses that clock if it is available and defers probe if it is not. Daniel, sorry for the late response, I just came back from a boat trip around Capri :D About the clocks and deferred probing, the issue here is that you might have trouble to find out if that clock will ever come up. Therefore, I suggested the easiest heuristic which is "let the DT author decide". I am fine with allowing more than one clock from DT and get or wait for all/one clock. Back to the specific case of the Cubox with new ideas. The Cubox is based on the Armada 510 (Dove), so, all the hardware must be described in dove.dtsi. This includes the LCDs and DCON/IRE, the possible clocks and the (static) v4l2 links: As a sidenote, I think we have concluded that we are not going to interact with the armada 510 DCON in any way at the moment. The driver will not have code that touches it, and the DT will not mention it. The first person who actually needs to use the DCON will have the responsibility for doing that work. Nevertheless it makes sense for us to pick a DT design where we know that the DCON could be slotted in later. True, do not link dcon node with any of lcd, tda998x or anything else. If there is support for it in the driver we just use of_find_compatible_node and let the driver do the magic. From a physical POV, lcd0 is directly connected to tda998x through dumb RGB pins. DCON can control data flow to those pins but should not be DT linked. lcd0: lcd-controller@82 { compatible = "marvell,dove-lcd0"; reg = <0x82 0x1c8>; interrupts = <47>; clocks = <&core_clk 3>, <&lcdclk>; clock-names = "axi", "lcdclk"; About clock names for Armada 510 LCD: I suggest "axiclk", "lcdpll", "extclk0", "extclk1". IIRC axiclk is 333MHz (or maybe 2xTCLK), lcdpll can be derived from 2GHz core PLL with min integer divider of 5. extclk0/1 are dedicated pins and can be used from both lcd0 and lcd1. marvell-output = <&dcon 0>; status = "disabled"; }; lcd1: lcd-controller@81 { compatible = "marvell,dove-lcd1"; reg = <0x81 0x1c8>; interrupts = <46>; clocks = <&core_clk 3>, <&lcdclk>; clock-names = "axi", "lcdclk"; marvell-output = <&dcon 1>; status = "disabled"; }; /* display controller and image rotation engine */ dcon: display-controller@83 { compatible = "marvell,dove-dcon"; reg = <0x83 0xc4>, /* DCON */ <0x831000 0x8c>; /* IRE */ interrupts = <45>; marvell-input = <&lcd0>, <&lcd1>; status = "disabled"; }; I guess the IRE falls into the same category as the DCON - we won't implement it for now, but knowing where it might fit in is useful. Why would you put it in the same node as the DCON though? It has its own separate register space and additionally it is a component shared with the MMP boards (whereas the DCON is not). DCON and IRE are summarized in the same register description in Dove FS. Maybe we can split them up and have two nodes or just one if registers overlap. Anyway, not that important for now. The specific Cubox hardware (tda998x and si5351) is described in dove-cubox.dts, with the new clocks and
Re: DT binding review for Armada display subsystem
On 07/13/2013 01:12 PM, Russell King - ARM Linux wrote: On Sat, Jul 13, 2013 at 12:56:50PM +0200, Sylwester Nawrocki wrote: On 07/13/2013 10:35 AM, Jean-Francois Moine wrote: On Fri, 12 Jul 2013 13:00:23 -0600 Daniel Drake wrote: On Fri, Jul 12, 2013 at 12:39 PM, Jean-Francois Moine wrote: - the phandles to the clocks does not tell how the clock may be set by the driver (it is an array index in the 510). In the other threads on clock selection, we decided that that exact information on how to select a clock (i.e. register bits/values) must be in the driver, not in the DT. What was suggested there is a well-documented scheme for clock naming, so the driver knows which clock is which. That is defined in the proposed DT binding though the "valid clock names" part. For an example driver implementation of this you can see my patch "armada_drm clock selection - try 2". OK. Sorry to go back to this thread. I use my Cubox for daily jobs as a desktop computer. My kernel is a DT driven 3.10.0. The dove-drm, tda998x and si5351 (clock) are kernel modules. I set 3 clocks in the DT for the LCD0: lcdclk, axi and extclk0 Hmm, a bit off topic question, does it work when the si5351 module gets removed ? I can't see anything preventing clock provider module from being removed why any of its clocks is used and clk_unregister() function is currently unimplemented. When I designed the clk API, I arranged for things like clk_get() to take a reference on the module if the clock was supplied by a module. You'll see some evidence of that by looking back in the git history for arch/arm/mach-integrator/include/mach/clkdev.h. Last time I checked, clkdev API neither implements referencing nor unregister. This is on Mike's list and IIRC there already has been a proposal for unregister. Si5351 was the first clkdev driver ever that could possibly be unloaded, so there may be still some features missing. If the common clk API has been designed without any thought to clocks supplied by modules and making sure that in-use clocks don't go away, then it's going to be a real pain to sort that out. I don't think refcounting clocks makes sense (what do you do with an enabled clock that you then remove the module for but it's still being used? Do you just shut it down anyway? Do you leave it running? What about subsequent calls?). Good point, first I thought always disable on last user dropping it but that may most likely break some platforms. Second thought, get back to the state when the driver was loaded. I think this is one case where taking a reference on the module supplying it makes total sense. (si5351). Normally, the external clock is used, but, sometimes, the si5351 module is not yet initialized when the drm driver starts. So, for 1920x1080p, it uses the lcdclk which sets the LCD clock to 13 (40/3) instead of 148500. As a result, display and sound still work correctly on my TV set thru HDMI. So, it would be nice to have 2 usable clocks per LCD, until we find a good way to initialize the modules in the right order at startup time. Doesn't deferred probing help here ? Indeed it does. Just because one TV works with such a wrong clock does not mean that all TVs and HDMI compatible monitors will do. It's 11% out. The reason that audio works is because of the way the HDMI transmitter works - it can calculate the CTS value to send (by measuring it) and it sends that value over the link so the TV can regenerate the correct audio clock from the HDMI clock. True, wrong clock will most likely not work on all monitors or TV. My impression for TVs is that the the "cheaper" the brand is, the more likely they accept any mode/clock combination ;) Whether that's going to be universally true, I don't know - it depends on how much audio data gets sent via each frame. As the HDMI clock is slower, there would need to be more audio data sent. Also: audio/video clock relation is sent in AVI packets over HDMI. Picking a clock with 11% off may not even allow you to send (valid) CTS information. lcd0: lcd-controller@82 { compatible = "marvell,dove-lcd0"; [...] }; lcd1: lcd-controller@81 { compatible = "marvell,dove-lcd1"; [...] }; Using different compatible strings to indicate multiple instances of same hardware doesn't seem right. Unless LCD0, LCD1 are really different pieces They aren't. They're 100% identical in the Armada 510. of hardware incompatible with each other I think it would be more correct to use same compatible property and DT node aliases, similarly as is now done with e.g. I2C busses: aliases { lcd0 = &lcd_0; lcd1 = &lcd_1; }; lcd_0: lcd-controller@82 { compatible = "marvell,dove-lcd"; I'd much prefer marvell,armada-510-lcd rather than using the codenames for the devices. Otherwise we're going to run into total
Re: DT binding review for Armada display subsystem
On 07/13/2013 07:44 PM, Sebastian Hesselbarth wrote: On 07/13/2013 01:12 PM, Russell King - ARM Linux wrote: On Sat, Jul 13, 2013 at 12:56:50PM +0200, Sylwester Nawrocki wrote: On 07/13/2013 10:35 AM, Jean-Francois Moine wrote: On Fri, 12 Jul 2013 13:00:23 -0600 Daniel Drake wrote: On Fri, Jul 12, 2013 at 12:39 PM, Jean-Francois Moine wrote: [...] I use my Cubox for daily jobs as a desktop computer. My kernel is a DT driven 3.10.0. The dove-drm, tda998x and si5351 (clock) are kernel modules. I set 3 clocks in the DT for the LCD0: lcdclk, axi and extclk0 Hmm, a bit off topic question, does it work when the si5351 module gets removed ? I can't see anything preventing clock provider module from being removed why any of its clocks is used and clk_unregister() function is currently unimplemented. When I designed the clk API, I arranged for things like clk_get() to take a reference on the module if the clock was supplied by a module. You'll see some evidence of that by looking back in the git history for arch/arm/mach-integrator/include/mach/clkdev.h. Last time I checked, clkdev API neither implements referencing nor unregister. This is on Mike's list and IIRC there already has been a proposal for unregister. Si5351 was the first clkdev driver ever I wasn't aware of it, thanks. I've seen a patch from Jiada Wang, it seems they're working on v4 with clock object reference counting. Presumably we need both clk_get() to be taking reference on the module and reference counted clk free, e.g. in cases where clock provider is a hot-pluggable device. It might be too paranoid though, I haven't seen hardware configurations where a clock source could be unplugged safely when whole system is running. that could possibly be unloaded, so there may be still some features missing. I guess there should be a warning in e.g. Kconfig saying that it is safe to use this driver only with CONFIG_MODULE_UNLOAD or something similar. Otherwise users might not be happy and might start filling bug reports. :) If the common clk API has been designed without any thought to clocks supplied by modules and making sure that in-use clocks don't go away, then it's going to be a real pain to sort that out. I don't think refcounting clocks makes sense (what do you do with an enabled clock that you then remove the module for but it's still being used? Do you just shut it down anyway? Do you leave it running? What about subsequent calls?). Good point, first I thought always disable on last user dropping it but that may most likely break some platforms. Second thought, get back to the state when the driver was loaded. When last user drops the reference it could be already too late to change anything in hardware, since the clock provider module is already unloaded. Anyway I suppose it's best to take reference on the clk provider module in clk_get(). This can easily lead to circular dependencies though, when e.g. a module A is a clock consumer of a clock provided by module B, and module B calls back into module A, so it also takes reference on A. The only solution I could think of is to call clk_get() only before a clock is enabled and clk_put() right after it is disabled and stops being used. I think this is one case where taking a reference on the module supplying it makes total sense. (si5351). Normally, the external clock is used, but, sometimes, the si5351 module is not yet initialized when the drm driver starts. So, for 1920x1080p, it uses the lcdclk which sets the LCD clock to 13 (40/3) instead of 148500. As a result, display and sound still work correctly on my TV set thru HDMI. So, it would be nice to have 2 usable clocks per LCD, until we find a good way to initialize the modules in the right order at startup time. Doesn't deferred probing help here ? Indeed it does. Just because one TV works with such a wrong clock does not mean that all TVs and HDMI compatible monitors will do. It's 11% out. The reason that audio works is because of the way the HDMI transmitter works - it can calculate the CTS value to send (by measuring it) and it sends that value over the link so the TV can regenerate the correct audio clock from the HDMI clock. True, wrong clock will most likely not work on all monitors or TV. My impression for TVs is that the the "cheaper" the brand is, the more likely they accept any mode/clock combination ;) Whether that's going to be universally true, I don't know - it depends on how much audio data gets sent via each frame. As the HDMI clock is slower, there would need to be more audio data sent. Also: audio/video clock relation is sent in AVI packets over HDMI. Picking a clock with 11% off may not even allow you to send (valid) CTS information. lcd0: lcd-controller@82 { compatible = "marvell,dove-lcd0"; [...] }; lcd1: lcd-controller@81 { compatible = "marvell,dove-lcd1"; [...] }; Using different compatible strings to indicate multiple instances of same hardware doesn't
Re: DT binding review for Armada display subsystem
On Sat, Jul 13, 2013 at 07:44:58PM +0200, Sebastian Hesselbarth wrote: > On 07/13/2013 01:12 PM, Russell King - ARM Linux wrote: >> When I designed the clk API, I arranged for things like clk_get() to >> take a reference on the module if the clock was supplied by a module. >> You'll see some evidence of that by looking back in the git history >> for arch/arm/mach-integrator/include/mach/clkdev.h. > > Last time I checked, clkdev API neither implements referencing nor > unregister. *Sigh*. a613163dff04cbfcb7d66b06ef4a5f65498ee59b: arch/arm/mach-integrator/include/mach/clkdev.h: -struct clk { - unsigned long rate; - const struct clk_ops*ops; - struct module *owner; - const struct icst_params *params; - void __iomem*vcoreg; - void*data; -}; - -static inline int __clk_get(struct clk *clk) -{ - return try_module_get(clk->owner); -} - -static inline void __clk_put(struct clk *clk) -{ - module_put(clk->owner); -} 70ee65771424829fd092a1df9afcc7e24c94004b: arch/arm/mach-integrator/impd1.c: static int impd1_probe(struct lm_device *dev) ... - for (i = 0; i < ARRAY_SIZE(impd1->vcos); i++) { - impd1->vcos[i].ops = &impd1_clk_ops, - impd1->vcos[i].owner = THIS_MODULE, - impd1->vcos[i].params = &impd1_vco_params, - impd1->vcos[i].data = impd1; - } - impd1->vcos[0].vcoreg = impd1->base + IMPD1_OSC1; - impd1->vcos[1].vcoreg = impd1->base + IMPD1_OSC2; - - impd1->clks[0] = clkdev_alloc(&impd1->vcos[0], NULL, "lm%x:01000", - dev->id); - impd1->clks[1] = clkdev_alloc(&fixed_14745600, NULL, "lm%x:00100", - dev->id); - impd1->clks[2] = clkdev_alloc(&fixed_14745600, NULL, "lm%x:00200", - dev->id); - for (i = 0; i < ARRAY_SIZE(impd1->clks); i++) - clkdev_add(impd1->clks[i]); ... static void impd1_remove(struct lm_device *dev) ... - for (i = 0; i < ARRAY_SIZE(impd1->clks); i++) - clkdev_drop(impd1->clks[i]); drivers/clk/clkdev.c: /* * clkdev_drop - remove a clock dynamically allocated */ void clkdev_drop(struct clk_lookup *cl) { mutex_lock(&clocks_mutex); list_del(&cl->node); mutex_unlock(&clocks_mutex); kfree(cl); } EXPORT_SYMBOL(clkdev_drop); No, of course, I'm imagining all the above! Now, today, the IM-PD/1 support got broken in respect of ensuring that things are properly refcounted in the rush to convert things to this wonderful new common clk API - because it's oh soo much better. Yes, right, I'd forgotten the number one rule of kernel programming - always sacrifice correctness when we get a new fantasic hip idea! Silly me. > This is on Mike's list and IIRC there already has been > a proposal for unregister. Si5351 was the first clkdev driver ever > that could possibly be unloaded, so there may be still some features > missing. Utter rubbish - it's not the first as you can see from the above. Integrator had this support since the clk and clkdev APIs came along, because the IM-PD/1 module was implemented as a module, and it has to create and register clocks for its peripherals. What you've found is a backwards step with the common clk API, nothing more. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DT binding review for Armada display subsystem
On Sat, Jul 13, 2013 at 10:43:29PM +0200, Sylwester Nawrocki wrote: > I wasn't aware of it, thanks. I've seen a patch from Jiada Wang, it seems > they're working on v4 with clock object reference counting. Presumably we > need both clk_get() to be taking reference on the module and reference > counted clk free, e.g. in cases where clock provider is a hot-pluggable > device. It might be too paranoid though, I haven't seen hardware > configurations where a clock source could be unplugged safely when whole > system is running. I'm not going to accept refcounting being thrown into clk_get(). The clkdev API already has refcounting, as much as it needs to. It just needs people to use the hooks that I provided back in 2008 when I created the clkdev API for doing _precisely_ this job. Have a read through these commits, which backup my statement above: 0318e693d3a56836632bf1a2cfdafb7f34bcc703 - initial commit of the clkdev API d72fbdf01fc77628c0b837d0dd2fd564fa26ede6 - converting Integrator to clkdev API and it will show you how to do refcounting. The common clk API just needs to stop defining __clk_get() and __clk_put() to be an empty function and implement them appropriately for it's clk implementation, like they were always meant to be. __clk_get() and __clk_put() are the clk-implementation specific parts of the clkdev API, because the clkdev API is utterly divorsed from the internals of what a 'struct clk' actually is. clkdev just treats a 'struct clk' as a completely opaque type and never bothers poking about inside it. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DT binding review for Armada display subsystem
On 07/13/2013 11:02 PM, Russell King - ARM Linux wrote: On Sat, Jul 13, 2013 at 10:43:29PM +0200, Sylwester Nawrocki wrote: I wasn't aware of it, thanks. I've seen a patch from Jiada Wang, it seems they're working on v4 with clock object reference counting. Presumably we need both clk_get() to be taking reference on the module and reference counted clk free, e.g. in cases where clock provider is a hot-pluggable device. It might be too paranoid though, I haven't seen hardware configurations where a clock source could be unplugged safely when whole system is running. I'm not going to accept refcounting being thrown into clk_get(). The clkdev API already has refcounting, as much as it needs to. It just needs people to use the hooks that I provided back in 2008 when I created the clkdev API for doing _precisely_ this job. Have a read through these commits, which backup my statement above: 0318e693d3a56836632bf1a2cfdafb7f34bcc703 - initial commit of the clkdev API d72fbdf01fc77628c0b837d0dd2fd564fa26ede6 - converting Integrator to clkdev API and it will show you how to do refcounting. The common clk API just needs to stop defining __clk_get() and __clk_put() to be an empty function and implement them appropriately for it's clk implementation, like they were always meant to be. Sure, I've already seen the above commits. This is how I understood it as well - __clk_get(), __clk_put() need to be defined by the common clk API, since it is going to replace all the arch specific implementations. __clk_get() and __clk_put() are the clk-implementation specific parts of the clkdev API, because the clkdev API is utterly divorsed from the internals of what a 'struct clk' actually is. clkdev just treats a 'struct clk' as a completely opaque type and never bothers poking about inside it. OK, but at the clock's implementation side we may need, e.g. to use kref to keep track of the clock's state, and free it only when it is unprepared, all its children are unregistered, etc. ? Is it not what you stated in your comment to patch [1] ? [1] https://patchwork.kernel.org/patch/2651171/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DT binding review for Armada display subsystem
On Sun, Jul 14, 2013 at 12:16:58AM +0200, Sylwester Nawrocki wrote: > On 07/13/2013 11:02 PM, Russell King - ARM Linux wrote: >> On Sat, Jul 13, 2013 at 10:43:29PM +0200, Sylwester Nawrocki wrote: >>> I wasn't aware of it, thanks. I've seen a patch from Jiada Wang, it seems >>> they're working on v4 with clock object reference counting. Presumably we >>> need both clk_get() to be taking reference on the module and reference >>> counted clk free, e.g. in cases where clock provider is a hot-pluggable >>> device. It might be too paranoid though, I haven't seen hardware >>> configurations where a clock source could be unplugged safely when whole >>> system is running. >> >> I'm not going to accept refcounting being thrown into clk_get(). The >> clkdev API already has refcounting, as much as it needs to. It just >> needs people to use the hooks that I provided back in 2008 when I >> created the clkdev API for doing _precisely_ this job. >> >> Have a read through these commits, which backup my statement above: >> >> 0318e693d3a56836632bf1a2cfdafb7f34bcc703 - initial commit of the clkdev API >> d72fbdf01fc77628c0b837d0dd2fd564fa26ede6 - converting Integrator to clkdev >> API >> >> and it will show you how to do refcounting. The common clk API just >> needs to stop defining __clk_get() and __clk_put() to be an empty >> function and implement them appropriately for it's clk implementation, >> like they were always meant to be. > > Sure, I've already seen the above commits. This is how I understood it > as well - __clk_get(), __clk_put() need to be defined by the common clk > API, since it is going to replace all the arch specific implementations. > >> __clk_get() and __clk_put() are the clk-implementation specific parts >> of the clkdev API, because the clkdev API is utterly divorsed from the >> internals of what a 'struct clk' actually is. clkdev just treats a >> 'struct clk' as a completely opaque type and never bothers poking >> about inside it. > > OK, but at the clock's implementation side we may need, e.g. to use kref > to keep track of the clock's state, and free it only when it is unprepared, > all its children are unregistered, etc. ? Is it not what you stated in > your comment to patch [1] ? If you want to do refcounting on a clock (which you run into problems as I highlighted earlier in this thread) then yes, you need to use a kref, and take a reference in __clk_get() and drop it in __clk_put() to make things safe. Whether you also take a reference on the module supplying the clock or not depends whether you need to keep the code around to manipulate that clock while there are users of it. As I've already said - consider the possibilities with this scenario. Here's one: A clock consumer is using a clock, but the clock supplier has been removed. The clock consumer wants to change the state of the clock in some way - eg, system is suspending. clk_disable() doesn't fail, but on resume, clk_enable() does... (and how many people assume that clk_enable() never fails?) And who knows what rate the clock is now producing or whether it's even producing anything... I'm not saying don't do the refcounting thing I mentioned back in June. I'm merely pointing out the issues that there are. There isn't a one right answer here because each has their own advantages and disadvantages (and problems.) ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 59322] r300g MSAA breaks Half-Life 2 in Wine
https://bugs.freedesktop.org/show_bug.cgi?id=59322 --- Comment #8 from Marek Olšák --- I have found the problem. Wine is trying to use the GL_SRGB8_ALPHA8 format, which is not supported for rendering by the driver internally, but I agree the format should be allowed in the API. The fix is on the way. Is it a big deal that ARB_framebuffer_sRGB is unsupported? In such a case, the renderbuffer format GL_SRGB8_ALPHA8 is equivalent to GL_RGBA8. -- You are receiving this mail because: You are the assignee for the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Update][PATCH] ACPI / video / i915: Remove ACPI backlight if firmware expects Windows 8
On 07/13/2013 08:46 AM, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki > > According to Matthew Garrett, "Windows 8 leaves backlight control up > to individual graphics drivers rather than making ACPI calls itself. > There's plenty of evidence to suggest that the Intel driver for > Windows [8] doesn't use the ACPI interface, including the fact that > it's broken on a bunch of machines when the OS claims to support > Windows 8. The simplest thing to do appears to be to disable the > ACPI backlight interface on these systems". > > There's a problem with that approach, however, because simply > avoiding to register the ACPI backlight interface if the firmware > calls _OSI for Windows 8 may not work in the following situations: > (1) The ACPI backlight interface actually works on the given system > and the i915 driver is not loaded (e.g. another graphics driver > is used). > (2) The ACPI backlight interface doesn't work on the given system, > but there is a vendor platform driver that will register its > own, equally broken, backlight interface if not prevented from > doing so by the ACPI subsystem. > Therefore we need to allow the ACPI backlight interface to be > registered until the i915 driver is loaded which then will unregister > it if the firmware has called _OSI for Windows 8 (or will register > the ACPI video driver without backlight support if not already > present). > > For this reason, introduce an alternative function for registering > ACPI video, acpi_video_register_with_quirks(), that will check > whether or not the ACPI video driver has already been registered > and whether or not the backlight Windows 8 quirk has to be applied. > If the quirk has to be applied, it will block the ACPI backlight > support and either unregister the backlight interface if the ACPI > video driver has already been registered, or register the ACPI > video driver without the backlight interface otherwise. Make > the i915 driver use acpi_video_register_with_quirks() instead of > acpi_video_register() in i915_driver_load(). > > This change is based on earlier patches from Matthew Garrett, > Chun-Yi Lee and Seth Forshee and Aaron Lu's comments. > > Signed-off-by: Rafael J. Wysocki Reviewed-by: Aaron Lu BTW, I also tested on a Toshiba laptop Z830 where its AML code claims support of win8, the result is as expected: ACPI video interface is removed, i915 Xorg driver picks intel_backlight. Thanks for the fix. -Aaron > --- > drivers/acpi/internal.h | 11 ++ > drivers/acpi/video.c| 65 > > drivers/acpi/video_detect.c | 21 > drivers/gpu/drm/i915/i915_dma.c |2 - > include/acpi/video.h| 11 ++ > include/linux/acpi.h|1 > 6 files changed, 103 insertions(+), 8 deletions(-) > > Index: linux-pm/drivers/acpi/video.c > === > --- linux-pm.orig/drivers/acpi/video.c > +++ linux-pm/drivers/acpi/video.c > @@ -44,6 +44,8 @@ > #include > #include > > +#include "internal.h" > + > #define PREFIX "ACPI: " > > #define ACPI_VIDEO_BUS_NAME "Video Bus" > @@ -898,7 +900,7 @@ static void acpi_video_device_find_cap(s > device->cap._DDC = 1; > } > > - if (acpi_video_backlight_support()) { > + if (acpi_video_verify_backlight_support()) { > struct backlight_properties props; > struct pci_dev *pdev; > acpi_handle acpi_parent; > @@ -1854,6 +1856,46 @@ static int acpi_video_bus_remove(struct > return 0; > } > > +static acpi_status video_unregister_backlight(acpi_handle handle, u32 lvl, > + void *context, void **rv) > +{ > + struct acpi_device *acpi_dev; > + struct acpi_video_bus *video; > + struct acpi_video_device *dev, *next; > + > + if (acpi_bus_get_device(handle, &acpi_dev)) > + return AE_OK; > + > + if (acpi_match_device_ids(acpi_dev, video_device_ids)) > + return AE_OK; > + > + video = acpi_driver_data(acpi_dev); > + if (!video) > + return AE_OK; > + > + acpi_video_bus_stop_devices(video); > + mutex_lock(&video->device_list_lock); > + list_for_each_entry_safe(dev, next, &video->video_device_list, entry) { > + if (dev->backlight) { > + backlight_device_unregister(dev->backlight); > + dev->backlight = NULL; > + kfree(dev->brightness->levels); > + kfree(dev->brightness); > + } > + if (dev->cooling_dev) { > + sysfs_remove_link(&dev->dev->dev.kobj, > + "thermal_cooling"); > + sysfs_remove_link(&dev->cooling_dev->device.kobj, > + "device"); > + thermal_cooling_
Re: [PATCH] drm/nouveau: kill nouveau_ttm_fault_reserve_notify handler to prevent useless buffer moves
On Fri, Jul 12, 2013 at 10:45 PM, Maarten Lankhorst wrote: > I have no idea what this bogus restriction on placement is, but it breaks > decoding 1080p > VDPAU at boot speed. With this patch applied I only need to bump the vdec > clock to > get real-time 1080p decoding. It prevents a lot of VRAM <-> VRAM buffer moves. It's not bogus, and is required for pre-GF8 boards with VRAM > BAR size. What configuration does the buffer that's getting moved here have exactly? The placement restriction isn't necessary on GF8, the rest of the restrictions may currently be required still however. Ben. > > Signed-off-by: Maarten Lankhorst > --- > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c > b/drivers/gpu/drm/nouveau/nouveau_bo.c > index d506da5..86eb321 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c > @@ -1339,34 +1339,6 @@ nouveau_ttm_io_mem_free(struct ttm_bo_device *bdev, > struct ttm_mem_reg *mem) > } > > static int > -nouveau_ttm_fault_reserve_notify(struct ttm_buffer_object *bo) > -{ > - struct nouveau_drm *drm = nouveau_bdev(bo->bdev); > - struct nouveau_bo *nvbo = nouveau_bo(bo); > - struct nouveau_device *device = nv_device(drm->device); > - u32 mappable = pci_resource_len(device->pdev, 1) >> PAGE_SHIFT; > - > - /* as long as the bo isn't in vram, and isn't tiled, we've got > -* nothing to do here. > -*/ > - if (bo->mem.mem_type != TTM_PL_VRAM) { > - if (nv_device(drm->device)->card_type < NV_50 || > - !nouveau_bo_tile_layout(nvbo)) > - return 0; > - } > - > - /* make sure bo is in mappable vram */ > - if (bo->mem.start + bo->mem.num_pages < mappable) > - return 0; > - > - > - nvbo->placement.fpfn = 0; > - nvbo->placement.lpfn = mappable; > - nouveau_bo_placement_set(nvbo, TTM_PL_FLAG_VRAM, 0); > - return nouveau_bo_validate(nvbo, false, false); > -} > - > -static int > nouveau_ttm_tt_populate(struct ttm_tt *ttm) > { > struct ttm_dma_tt *ttm_dma = (void *)ttm; > @@ -1524,7 +1496,6 @@ struct ttm_bo_driver nouveau_bo_driver = { > .sync_obj_flush = nouveau_bo_fence_flush, > .sync_obj_unref = nouveau_bo_fence_unref, > .sync_obj_ref = nouveau_bo_fence_ref, > - .fault_reserve_notify = &nouveau_ttm_fault_reserve_notify, > .io_mem_reserve = &nouveau_ttm_io_mem_reserve, > .io_mem_free = &nouveau_ttm_io_mem_free, > }; > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 66883] New: [r600 llvm backend] Assert with 32-bit lightsmark, TF2 when shader dump is enabled
https://bugs.freedesktop.org/show_bug.cgi?id=66883 Priority: medium Bug ID: 66883 Assignee: dri-devel at lists.freedesktop.org Summary: [r600 llvm backend] Assert with 32-bit lightsmark, TF2 when shader dump is enabled Severity: normal Classification: Unclassified OS: All Reporter: ptpzz at yandex.ru Hardware: Other Status: NEW Version: git Component: Drivers/Gallium/r600 Product: Mesa I have asserts in the middle of shader dump in llvm backend with 32-bit Team Fortress 2 and Lightsmark, R600_DEBUG=ps,vs. Here is message with Lightsmark: ... ALU_CLAUSE 8; dbg:backend: DebugLoc.cpp:27: llvm::MDNode* llvm::DebugLoc::getScope(const llvm::LLVMContext&) const: Assertion `unsigned(ScopeIdx) <= Ctx.pImpl->ScopeRecords.size() && "Invalid ScopeIdx!"' failed. TF2 message: ... ALU_CLAUSE 32; dbg:hl2_linux: DebugLoc.cpp:33: llvm::MDNode* llvm::DebugLoc::getScope(const llvm::LLVMContext&) const: Assertion `unsigned(-ScopeIdx) <= Ctx.pImpl->ScopeInlinedAtRecords.size() && "Invalid ScopeIdx"' failed. It seems in both cases it happens on ALU clause start in the dump. So far I can't reproduce this issue with 64-bit Lightsmark/mesa/llvm. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130714/96f6e511/attachment.html>
[Bug 62959] r600g (HD 6950 Cayman) fails piglit tests and hangs system
https://bugs.freedesktop.org/show_bug.cgi?id=62959 --- Comment #66 from Alexandre Demers --- While I'm the one who opened this bug, on my side I'm able to run all piglit tests without any hangs since awhile now. I don't even need to run one test at a time anymore. But I'm using latest git versions of mesa, drm, ddx with a 3.10 kernel if this can be of any help to those who are adding themselves to the CC list. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130714/b256d6fc/attachment.html>
[Bug 66713] Team Fortress 2 crashes with r600-sb on HD4850
https://bugs.freedesktop.org/show_bug.cgi?id=66713 --- Comment #6 from Vadim Girlin --- Created attachment 82400 --> https://bugs.freedesktop.org/attachment.cgi?id=82400&action=edit [PATCH 1/2] r600g/sb: fix register allocation Does this patch help? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130714/b0fb9f09/attachment.html>
[Bug 66888] New: [radeonsi] Need support GL_EXT_framebuffer_multisample
https://bugs.freedesktop.org/show_bug.cgi?id=66888 Priority: medium Bug ID: 66888 Assignee: dri-devel at lists.freedesktop.org Summary: [radeonsi] Need support GL_EXT_framebuffer_multisample Severity: enhancement Classification: Unclassified OS: Linux (All) Reporter: grantipak at gmail.com Hardware: x86 (IA32) Status: NEW Version: git Component: Drivers/Gallium/radeonsi Product: Mesa Created attachment 82401 --> https://bugs.freedesktop.org/attachment.cgi?id=82401&action=edit graphical glitch unigine-tropics and unigine-sanctuary don't runs. Required extension GL_EXT_framebuffer_multisample is not supported. ArchLinux x86; kernel 3.10; mesa - git; xf86-video-ati - git; llvm - 3.3 Radeon HD 7950 OpenGL renderer string: Gallium 0.4 on AMD TAHITI OpenGL version string: 2.1 Mesa 9.2.0-devel (git-796b73d) OpenGL shading language version string: 1.30 If run MESA_EXTENSION_OVERRIDE="GL_EXT_framebuffer_multisample" unigine-tropics, test runs but have graphical glitch. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130714/34c86900/attachment.html>
[Bug 66888] [radeonsi] Need support GL_EXT_framebuffer_multisample
https://bugs.freedesktop.org/show_bug.cgi?id=66888 Vladimir Ysikov changed: What|Removed |Added Attachment #82401|text/plain |image/png mime type|| -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130714/2856b6a4/attachment.html>
[Bug 66425] "failed testing IB on ring 5" when suspending to disk
https://bugs.freedesktop.org/show_bug.cgi?id=66425 --- Comment #24 from Austin Lund --- Patch tested and works on my machine. I now have problems for "processors" when doing pm_test, so I still cannot actually test this on a full resume from disk, but at least pm_test with "devices" works. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130714/eb046c6d/attachment-0001.html>
[Bug 66425] "failed testing IB on ring 5" when suspending to disk
https://bugs.freedesktop.org/show_bug.cgi?id=66425 Christian K?nig changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #25 from Christian K?nig --- Sounds like we can merk this as resolved now. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130714/a6e50f35/attachment.html>
[Bug 66714] Mobility Radeon HD 5650 doesn't boot with kernel 3.10 (and newer) when using radeon.audio=1
https://bugs.freedesktop.org/show_bug.cgi?id=66714 Marco Trevisan (Trevi?o) changed: What|Removed |Added Status|NEW |ASSIGNED --- Comment #4 from Marco Trevisan (Trevi?o) --- So, I've tried the patch on a 3.10 kernel that was affected and... Great, it Works! :) FYI, I've also added a couple of error logs in the code to see where exactly the failure was and this was the result: [2.073137] [drm] fb mappable at 0xC035F000 [2.073139] [drm] vram apper at 0xC000 [2.073140] [drm] size 8294400 [2.073141] [drm] fb depth is 24 [2.073142] [drm]pitch is 7680 [2.073252] fbcon: radeondrmfb (fb0) is primary device [2.073323] [drm:evergreen_hdmi_enable] *ERROR* Invalid DIG (dig: 880229d0fc00, dig->afmt: (null)) /home/marco/Dev/debs/linux-3.10.0/drivers/gpu/drm/radeon/evergreen_hdmi.c:298 [2.573589] [drm:evergreen_hdmi_enable] *ERROR* Invalid DIG (dig: 880229d0fc00, dig->afmt: (null)) /home/marco/Dev/debs/linux-3.10.0/drivers/gpu/drm/radeon/evergreen_hdmi.c:298 [2.607449] Console: switching to colour frame buffer device 170x48 [2.610160] radeon :01:00.0: fb0: radeondrmfb frame buffer device [2.610162] radeon :01:00.0: registered panic notifier [2.610237] [drm] Initialized radeon 2.33.0 20080528 for :01:00.0 on minor 0 Full dmesg at http://pastebin.ubuntu.com/5873667/ I don't know if this may be only the side effect of another issue (wrong initialization order?). One more thing I discovered in the past days, is that the kernel crash was not happening in case that the HDMI was plugged after that ligthtdm was running (attaching it anytime before was leading to a freeze). -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130714/89fb758e/attachment.html>
[Bug 66714] Mobility Radeon HD 5650 doesn't boot with kernel 3.10 (and newer) when using radeon.audio=1
https://bugs.freedesktop.org/show_bug.cgi?id=66714 --- Comment #5 from Marco Trevisan (Trevi?o) --- Comment on attachment 82200 --> https://bugs.freedesktop.org/attachment.cgi?id=82200 possible fix Review of attachment 82200: - Why not including some DRM_ERROR logs as well? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130714/24e4a281/attachment.html>
[PATCH] radeon kms: do not flush uninitialized hotplug work
Fix a warning from lockdep caused by calling flush_work() for uninitialized hotplug work. Initialize hotplug_work, audio_work and reset_work upon successful radeon_irq_kms_init() completion and thus perform hotplug flush_work only when rdev->irq.installed is true. [4.790019] [drm] Loading CEDAR Microcode [4.790943] r600_cp: Failed to load firmware "radeon/CEDAR_smc.bin" [4.791152] [drm:evergreen_startup] *ERROR* Failed to load firmware! [4.791330] radeon :01:00.0: disabling GPU acceleration [4.792633] INFO: trying to register non-static key. [4.792792] the code is fine but needs lockdep annotation. [4.792953] turning off the locking correctness validator. [4.793114] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 3.11.0-rc0-dbg-10676-gfe56456-dirty #1816 [4.793314] Hardware name: Acer Aspire 5741G/Aspire 5741G , BIOS V1.20 02/08/2011 [4.793507] 821fd810 8801530b9a18 8160434e 0002 [4.794155] 8801530b9ad8 810b8404 8801530b0798 8801530b [4.794789] 8801530b9b00 0046 04c0 [4.795418] Call Trace: [4.795573] [] dump_stack+0x4e/0x82 [4.795731] [] __lock_acquire+0x1a64/0x1d30 [4.795893] [] ? dev_vprintk_emit+0x50/0x60 [4.796034] [] lock_acquire+0xa4/0x200 [4.796216] [] ? flush_work+0x5/0x280 [4.796375] [] flush_work+0x3d/0x280 [4.796520] [] ? flush_work+0x5/0x280 [4.796682] [] ? trace_hardirqs_on_caller+0xfd/0x1c0 [4.796862] [] ? delay_tsc+0x95/0xf0 [4.797024] [] radeon_irq_kms_fini+0x2b/0x70 [4.797186] [] evergreen_init+0x2a9/0x2e0 [4.797347] [] radeon_device_init+0x5ef/0x700 [4.797511] [] ? pci_find_capability+0x47/0x50 [4.797672] [] radeon_driver_load_kms+0x8d/0x150 [4.797843] [] drm_get_pci_dev+0x166/0x280 [4.798007] [] ? kfree+0xf5/0x2e0 [4.798168] [] ? radeon_pci_probe+0x98/0xd0 [4.798329] [] radeon_pci_probe+0xaa/0xd0 [4.798489] [] pci_device_probe+0x84/0xe0 [4.798644] [] driver_probe_device+0x76/0x240 [4.798805] [] __driver_attach+0x93/0xa0 [4.798948] [] ? __device_attach+0x40/0x40 [4.799126] [] bus_for_each_dev+0x6b/0xb0 [4.799272] [] driver_attach+0x1e/0x20 [4.799434] [] bus_add_driver+0x1f0/0x280 [4.799596] [] driver_register+0x74/0x150 [4.799758] [] __pci_register_driver+0x5d/0x60 [4.799936] [] ? ttm_init+0x67/0x67 [4.800081] [] drm_pci_init+0x115/0x130 [4.800243] [] ? ttm_init+0x67/0x67 [4.800405] [] radeon_init+0x9c/0xba [4.800586] [] do_one_initcall+0xfa/0x150 [4.800746] [] ? parse_args+0x120/0x330 [4.800909] [] kernel_init_freeable+0x111/0x191 [4.801052] [] ? do_early_param+0x88/0x88 [4.801233] [] ? rest_init+0x140/0x140 [4.801393] [] kernel_init+0xe/0x180 [4.801556] [] ret_from_fork+0x7c/0xb0 [4.801718] [] ? rest_init+0x140/0x140 Signed-off-by: Sergey Senozhatsky --- drivers/gpu/drm/radeon/radeon_irq_kms.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c index bcdefd1..081886b 100644 --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c @@ -260,10 +260,6 @@ int radeon_irq_kms_init(struct radeon_device *rdev) { int r = 0; - INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func); - INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi); - INIT_WORK(&rdev->reset_work, radeon_irq_reset_work_func); - spin_lock_init(&rdev->irq.lock); r = drm_vblank_init(rdev->ddev, rdev->num_crtc); if (r) { @@ -285,6 +281,11 @@ int radeon_irq_kms_init(struct radeon_device *rdev) rdev->irq.installed = false; return r; } + + INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func); + INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi); + INIT_WORK(&rdev->reset_work, radeon_irq_reset_work_func); + DRM_INFO("radeon: irq initialized.\n"); return 0; } @@ -304,8 +305,8 @@ void radeon_irq_kms_fini(struct radeon_device *rdev) rdev->irq.installed = false; if (rdev->msi_enabled) pci_disable_msi(rdev->pdev); + flush_work(&rdev->hotplug_work); } - flush_work(&rdev->hotplug_work); } /**
[Bug 63935] TURKS [drm:r600_uvd_init] *ERROR* UVD not responding, trying to reset the VCPU!!!
https://bugs.freedesktop.org/show_bug.cgi?id=63935 Christian K?nig changed: What|Removed |Added Status|REOPENED|RESOLVED Resolution|--- |FIXED --- Comment #57 from Christian K?nig --- The suspend/resume issue was something completely different, so we probably can now close this bugreport. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130714/d2ca9c28/attachment-0001.html>
[Bug 66714] Mobility Radeon HD 5650 doesn't boot with kernel 3.10 (and newer) when using radeon.audio=1
https://bugs.freedesktop.org/show_bug.cgi?id=66714 Christian K?nig changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #6 from Christian K?nig --- (In reply to comment #5) > Comment on attachment 82200 [details] [review] > possible fix > > Review of attachment 82200 [details] [review]: > - > > Why not including some DRM_ERROR logs as well? Because that isn't a bug. You just have an LVDS pannel connected to a DIG encoder and so this encoder doesn't have an audio block. Alex patch is already quite right. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130714/6f7a56b3/attachment.html>
[PATCH] radeon kms: do not flush uninitialized hotplug work
On Sun, Jul 14, 2013 at 7:03 AM, Sergey Senozhatsky wrote: > Fix a warning from lockdep caused by calling flush_work() for > uninitialized hotplug work. Initialize hotplug_work, audio_work > and reset_work upon successful radeon_irq_kms_init() completion > and thus perform hotplug flush_work only when rdev->irq.installed > is true. > > [4.790019] [drm] Loading CEDAR Microcode > [4.790943] r600_cp: Failed to load firmware "radeon/CEDAR_smc.bin" > [4.791152] [drm:evergreen_startup] *ERROR* Failed to load firmware! > [4.791330] radeon :01:00.0: disabling GPU acceleration > > [4.792633] INFO: trying to register non-static key. > [4.792792] the code is fine but needs lockdep annotation. > [4.792953] turning off the locking correctness validator. > > [4.793114] CPU: 2 PID: 1 Comm: swapper/0 Not tainted > 3.11.0-rc0-dbg-10676-gfe56456-dirty #1816 > [4.793314] Hardware name: Acer Aspire 5741G/Aspire 5741G > , BIOS V1.20 02/08/2011 > [4.793507] 821fd810 8801530b9a18 8160434e > 0002 > [4.794155] 8801530b9ad8 810b8404 8801530b0798 > 8801530b > [4.794789] 8801530b9b00 0046 04c0 > > [4.795418] Call Trace: > [4.795573] [] dump_stack+0x4e/0x82 > [4.795731] [] __lock_acquire+0x1a64/0x1d30 > [4.795893] [] ? dev_vprintk_emit+0x50/0x60 > [4.796034] [] lock_acquire+0xa4/0x200 > [4.796216] [] ? flush_work+0x5/0x280 > [4.796375] [] flush_work+0x3d/0x280 > [4.796520] [] ? flush_work+0x5/0x280 > [4.796682] [] ? trace_hardirqs_on_caller+0xfd/0x1c0 > [4.796862] [] ? delay_tsc+0x95/0xf0 > [4.797024] [] radeon_irq_kms_fini+0x2b/0x70 > [4.797186] [] evergreen_init+0x2a9/0x2e0 > [4.797347] [] radeon_device_init+0x5ef/0x700 > [4.797511] [] ? pci_find_capability+0x47/0x50 > [4.797672] [] radeon_driver_load_kms+0x8d/0x150 > [4.797843] [] drm_get_pci_dev+0x166/0x280 > [4.798007] [] ? kfree+0xf5/0x2e0 > [4.798168] [] ? radeon_pci_probe+0x98/0xd0 > [4.798329] [] radeon_pci_probe+0xaa/0xd0 > [4.798489] [] pci_device_probe+0x84/0xe0 > [4.798644] [] driver_probe_device+0x76/0x240 > [4.798805] [] __driver_attach+0x93/0xa0 > [4.798948] [] ? __device_attach+0x40/0x40 > [4.799126] [] bus_for_each_dev+0x6b/0xb0 > [4.799272] [] driver_attach+0x1e/0x20 > [4.799434] [] bus_add_driver+0x1f0/0x280 > [4.799596] [] driver_register+0x74/0x150 > [4.799758] [] __pci_register_driver+0x5d/0x60 > [4.799936] [] ? ttm_init+0x67/0x67 > [4.800081] [] drm_pci_init+0x115/0x130 > [4.800243] [] ? ttm_init+0x67/0x67 > [4.800405] [] radeon_init+0x9c/0xba > [4.800586] [] do_one_initcall+0xfa/0x150 > [4.800746] [] ? parse_args+0x120/0x330 > [4.800909] [] kernel_init_freeable+0x111/0x191 > [4.801052] [] ? do_early_param+0x88/0x88 > [4.801233] [] ? rest_init+0x140/0x140 > [4.801393] [] kernel_init+0xe/0x180 > [4.801556] [] ret_from_fork+0x7c/0xb0 > [4.801718] [] ? rest_init+0x140/0x140 > > > Signed-off-by: Sergey Senozhatsky > Applied to by fixes tree. thanks! Alex > --- > > drivers/gpu/drm/radeon/radeon_irq_kms.c | 11 ++- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c > b/drivers/gpu/drm/radeon/radeon_irq_kms.c > index bcdefd1..081886b 100644 > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c > @@ -260,10 +260,6 @@ int radeon_irq_kms_init(struct radeon_device *rdev) > { > int r = 0; > > - INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func); > - INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi); > - INIT_WORK(&rdev->reset_work, radeon_irq_reset_work_func); > - > spin_lock_init(&rdev->irq.lock); > r = drm_vblank_init(rdev->ddev, rdev->num_crtc); > if (r) { > @@ -285,6 +281,11 @@ int radeon_irq_kms_init(struct radeon_device *rdev) > rdev->irq.installed = false; > return r; > } > + > + INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func); > + INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi); > + INIT_WORK(&rdev->reset_work, radeon_irq_reset_work_func); > + > DRM_INFO("radeon: irq initialized.\n"); > return 0; > } > @@ -304,8 +305,8 @@ void radeon_irq_kms_fini(struct radeon_device *rdev) > rdev->irq.installed = false; > if (rdev->msi_enabled) > pci_disable_msi(rdev->pdev); > + flush_work(&rdev->hotplug_work); > } > - flush_work(&rdev->hotplug_work); > } > > /** > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
This patch fixes regression in power consumtion of sandy bridge gpu, which exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking that it's extremely busy. After that it never reaches rc6 state. Bug was introduce by commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 ("drm/i915: load boot context at driver init time"). Without documentation it's not clear what is happening here, probably this breaks internal state of hardware ring buffers and confuses RPS engine. Fortunately keeping forcewake during whole initialization sequence in gen6_init_clock_gating() fixes this bug. References: https://bugs.freedesktop.org/show_bug.cgi?id=54089 Signed-off-by: Konstantin Khlebnikov --- 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 aa01128..839a43f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3640,6 +3640,8 @@ static void gen6_init_clock_gating(struct drm_device *dev) int pipe; uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE; + gen6_gt_force_wake_get(dev_priv); + I915_WRITE(ILK_DSPCLK_GATE_D, dspclk_gate); I915_WRITE(ILK_DISPLAY_CHICKEN2, @@ -3728,6 +3730,8 @@ static void gen6_init_clock_gating(struct drm_device *dev) cpt_init_clock_gating(dev); gen6_check_mch_setup(dev); + + gen6_gt_force_wake_put(dev_priv); } static void gen7_setup_fixed_func_scheduler(struct drm_i915_private *dev_priv)
[PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Sun, Jul 14, 2013 at 6:30 PM, Konstantin Khlebnikov wrote: > This patch fixes regression in power consumtion of sandy bridge gpu, which > exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking that > it's extremely busy. After that it never reaches rc6 state. > > Bug was introduce by commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 > ("drm/i915: load boot context at driver init time"). Without documentation > it's not clear what is happening here, probably this breaks internal state of > hardware ring buffers and confuses RPS engine. Fortunately keeping forcewake > during whole initialization sequence in gen6_init_clock_gating() fixes this > bug. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=54089 > Signed-off-by: Konstantin Khlebnikov We already hold an forcewake reference while setting up the rps stuff, should we maybe hold the forcewake for the entire duration, i.e. grab it here in clock_gating and release it only in gen6/vlv_enable_rps? Can you please test that version, too? In any case the forcewake grabbing here in the clock gating function needs a big comment that otherwise setting the MCTL register might break rc6 entry. -Daniel > --- > 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 aa01128..839a43f 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3640,6 +3640,8 @@ static void gen6_init_clock_gating(struct drm_device > *dev) > int pipe; > uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE; > > + gen6_gt_force_wake_get(dev_priv); > + > I915_WRITE(ILK_DSPCLK_GATE_D, dspclk_gate); > > I915_WRITE(ILK_DISPLAY_CHICKEN2, > @@ -3728,6 +3730,8 @@ static void gen6_init_clock_gating(struct drm_device > *dev) > cpt_init_clock_gating(dev); > > gen6_check_mch_setup(dev); > + > + gen6_gt_force_wake_put(dev_priv); > } > > static void gen7_setup_fixed_func_scheduler(struct drm_i915_private > *dev_priv) > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 64850] Second screen black on Pitcairn PRO
https://bugs.freedesktop.org/show_bug.cgi?id=64850 --- Comment #23 from Jakob Nixdorf --- Just tested the first kernel with RadeonSI support (3.4.0 to my knowledge) with the same result. Second screen stays black in TTY and in Xorg. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130714/c550d24a/attachment.html>
[PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
Daniel Vetter wrote: > On Sun, Jul 14, 2013 at 6:30 PM, Konstantin Khlebnikov > wrote: >> This patch fixes regression in power consumtion of sandy bridge gpu, which >> exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking >> that >> it's extremely busy. After that it never reaches rc6 state. >> >> Bug was introduce by commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 >> ("drm/i915: load boot context at driver init time"). Without documentation >> it's not clear what is happening here, probably this breaks internal state of >> hardware ring buffers and confuses RPS engine. Fortunately keeping forcewake >> during whole initialization sequence in gen6_init_clock_gating() fixes this >> bug. >> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=54089 >> Signed-off-by: Konstantin Khlebnikov > > We already hold an forcewake reference while setting up the rps stuff, > should we maybe hold the forcewake for the entire duration, i.e. grab > it here in clock_gating and release it only in gen6/vlv_enable_rps? > Can you please test that version, too? This will be racy because rps stuff is done in separate work which might be canceled if intel_disable_gt_powersave() happens before its completion. > > In any case the forcewake grabbing here in the clock gating function > needs a big comment that otherwise setting the MCTL register might > break rc6 entry. > -Daniel > >> --- >> 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 aa01128..839a43f 100644 >> --- a/drivers/gpu/drm/i915/intel_pm.c >> +++ b/drivers/gpu/drm/i915/intel_pm.c >> @@ -3640,6 +3640,8 @@ static void gen6_init_clock_gating(struct drm_device >> *dev) >> int pipe; >> uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE; >> >> + gen6_gt_force_wake_get(dev_priv); >> + >> I915_WRITE(ILK_DSPCLK_GATE_D, dspclk_gate); >> >> I915_WRITE(ILK_DISPLAY_CHICKEN2, >> @@ -3728,6 +3730,8 @@ static void gen6_init_clock_gating(struct drm_device >> *dev) >> cpt_init_clock_gating(dev); >> >> gen6_check_mch_setup(dev); >> + >> + gen6_gt_force_wake_put(dev_priv); >> } >> >> static void gen7_setup_fixed_func_scheduler(struct drm_i915_private >> *dev_priv) >> > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm/i915: fix long-standing SNB regression in power consumption after resume
On Sun, Jul 14, 2013 at 06:52:39PM +0200, Daniel Vetter wrote: > On Sun, Jul 14, 2013 at 6:30 PM, Konstantin Khlebnikov > wrote: > > This patch fixes regression in power consumtion of sandy bridge gpu, which > > exists since v3.6 Sometimes after resuming from s2ram gpu starts thinking > > that > > it's extremely busy. After that it never reaches rc6 state. > > > > Bug was introduce by commit b4ae3f22d238617ca11610b29fde16cf8c0bc6e0 > > ("drm/i915: load boot context at driver init time"). Without documentation > > it's not clear what is happening here, probably this breaks internal state > > of > > hardware ring buffers and confuses RPS engine. Fortunately keeping forcewake > > during whole initialization sequence in gen6_init_clock_gating() fixes this > > bug. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=54089 > > Signed-off-by: Konstantin Khlebnikov > > We already hold an forcewake reference while setting up the rps stuff, > should we maybe hold the forcewake for the entire duration, i.e. grab > it here in clock_gating and release it only in gen6/vlv_enable_rps? > Can you please test that version, too? > > In any case the forcewake grabbing here in the clock gating function > needs a big comment that otherwise setting the MCTL register might > break rc6 entry. It is not clear why the forcewake works, but is easy to imagine one of the operations in that sequence requires the GPU to be awake at the time of programming for it to succeed. MBCTL:EnableBootFetch does seem the most suspicious from its wording in the bspec. I guess all instances of poking this bit should be protected similary (snb, ivb, vlv, hsw). Based on that reasoning and that waking the GPU up here has no negative consequences, and so long as all paths are fixed, I am happy to give this an Acked-by: Chris Wilson Also, we need to reapply the w/a after a Function Level Reset, in other words we do need to repeat the init_clock_gating after intel_gpu_reset(). -Chris -- Chris Wilson, Intel Open Source Technology Centre
DT binding review for Armada display subsystem
On 07/13/2013 11:02 PM, Russell King - ARM Linux wrote: > On Sat, Jul 13, 2013 at 10:43:29PM +0200, Sylwester Nawrocki wrote: >> I wasn't aware of it, thanks. I've seen a patch from Jiada Wang, it seems >> they're working on v4 with clock object reference counting. Presumably we >> need both clk_get() to be taking reference on the module and reference >> counted clk free, e.g. in cases where clock provider is a hot-pluggable >> device. It might be too paranoid though, I haven't seen hardware >> configurations where a clock source could be unplugged safely when whole >> system is running. > > I'm not going to accept refcounting being thrown into clk_get(). The > clkdev API already has refcounting, as much as it needs to. It just > needs people to use the hooks that I provided back in 2008 when I > created the clkdev API for doing _precisely_ this job. > > Have a read through these commits, which backup my statement above: > > 0318e693d3a56836632bf1a2cfdafb7f34bcc703 - initial commit of the clkdev API > d72fbdf01fc77628c0b837d0dd2fd564fa26ede6 - converting Integrator to clkdev API > > and it will show you how to do refcounting. The common clk API just > needs to stop defining __clk_get() and __clk_put() to be an empty > function and implement them appropriately for it's clk implementation, > like they were always meant to be. Sure, I've already seen the above commits. This is how I understood it as well - __clk_get(), __clk_put() need to be defined by the common clk API, since it is going to replace all the arch specific implementations. > __clk_get() and __clk_put() are the clk-implementation specific parts > of the clkdev API, because the clkdev API is utterly divorsed from the > internals of what a 'struct clk' actually is. clkdev just treats a > 'struct clk' as a completely opaque type and never bothers poking > about inside it. OK, but at the clock's implementation side we may need, e.g. to use kref to keep track of the clock's state, and free it only when it is unprepared, all its children are unregistered, etc. ? Is it not what you stated in your comment to patch [1] ? [1] https://patchwork.kernel.org/patch/2651171/
DT binding review for Armada display subsystem
On Sun, Jul 14, 2013 at 12:16:58AM +0200, Sylwester Nawrocki wrote: > On 07/13/2013 11:02 PM, Russell King - ARM Linux wrote: >> On Sat, Jul 13, 2013 at 10:43:29PM +0200, Sylwester Nawrocki wrote: >>> I wasn't aware of it, thanks. I've seen a patch from Jiada Wang, it seems >>> they're working on v4 with clock object reference counting. Presumably we >>> need both clk_get() to be taking reference on the module and reference >>> counted clk free, e.g. in cases where clock provider is a hot-pluggable >>> device. It might be too paranoid though, I haven't seen hardware >>> configurations where a clock source could be unplugged safely when whole >>> system is running. >> >> I'm not going to accept refcounting being thrown into clk_get(). The >> clkdev API already has refcounting, as much as it needs to. It just >> needs people to use the hooks that I provided back in 2008 when I >> created the clkdev API for doing _precisely_ this job. >> >> Have a read through these commits, which backup my statement above: >> >> 0318e693d3a56836632bf1a2cfdafb7f34bcc703 - initial commit of the clkdev API >> d72fbdf01fc77628c0b837d0dd2fd564fa26ede6 - converting Integrator to clkdev >> API >> >> and it will show you how to do refcounting. The common clk API just >> needs to stop defining __clk_get() and __clk_put() to be an empty >> function and implement them appropriately for it's clk implementation, >> like they were always meant to be. > > Sure, I've already seen the above commits. This is how I understood it > as well - __clk_get(), __clk_put() need to be defined by the common clk > API, since it is going to replace all the arch specific implementations. > >> __clk_get() and __clk_put() are the clk-implementation specific parts >> of the clkdev API, because the clkdev API is utterly divorsed from the >> internals of what a 'struct clk' actually is. clkdev just treats a >> 'struct clk' as a completely opaque type and never bothers poking >> about inside it. > > OK, but at the clock's implementation side we may need, e.g. to use kref > to keep track of the clock's state, and free it only when it is unprepared, > all its children are unregistered, etc. ? Is it not what you stated in > your comment to patch [1] ? If you want to do refcounting on a clock (which you run into problems as I highlighted earlier in this thread) then yes, you need to use a kref, and take a reference in __clk_get() and drop it in __clk_put() to make things safe. Whether you also take a reference on the module supplying the clock or not depends whether you need to keep the code around to manipulate that clock while there are users of it. As I've already said - consider the possibilities with this scenario. Here's one: A clock consumer is using a clock, but the clock supplier has been removed. The clock consumer wants to change the state of the clock in some way - eg, system is suspending. clk_disable() doesn't fail, but on resume, clk_enable() does... (and how many people assume that clk_enable() never fails?) And who knows what rate the clock is now producing or whether it's even producing anything... I'm not saying don't do the refcounting thing I mentioned back in June. I'm merely pointing out the issues that there are. There isn't a one right answer here because each has their own advantages and disadvantages (and problems.)