Re: [PATCH libdrm 1/2] xf86drm: inline drmIoctl()
On 23 June 2017 03:21:11 BST, "Michel Dänzer" wrote: > On 22/06/17 11:45 PM, Eric Engestrom wrote:> > > On Thursday, 2017-06-22 10:42:21 +0900, Michel Dänzer wrote:> > >>> > >> Also, there are downsides to exposing library API calls as inline> > >> functions: E.g. if drmIoctl(2) ever needs a change (worst case a> > >> security bug fix), every user has to be recompiled to get the fix. > It's> > >> kind of like static linking through the back door. Is it really > worth it> > >> for drmIoctl(2)? Are they ever an actual bottleneck?> > > > > > The start of the conversation [1] was that the profiler was > spending> > > more time going through drmIoctl() than the actual syscall,> > > > Is that an actual problem, as opposed to a cosmetic issue? I.e. is > there> > any scenario where a metric is bounded by drmIoctl, and would > otherwise> > only be bounded by the actual syscall?> I think this is a question for Chris (cc'ed) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/7] drm/bridge: Support hotplugging panel-bridge.
On 22.06.2017 15:34, Boris Brezillon wrote: > On Thu, 22 Jun 2017 15:16:47 +0200 > Andrzej Hajda wrote: > >> On 22.06.2017 14:41, Boris Brezillon wrote: >>> On Thu, 22 Jun 2017 14:29:07 +0200 >>> Andrzej Hajda wrote: >>> On 22.06.2017 11:23, Boris Brezillon wrote: > On Thu, 22 Jun 2017 13:47:43 +0530 > Archit Taneja wrote: > >> On 06/22/2017 01:20 PM, Benjamin Gaignard wrote: >>> 2017-06-20 19:31 GMT+02:00 Eric Anholt : Archit Taneja writes: > On 06/16/2017 08:13 PM, Eric Anholt wrote: >> Archit Taneja writes: >> >>> On 06/16/2017 02:11 AM, Eric Anholt wrote: If the panel-bridge is being set up after the drm_mode_config_reset(), then the connector's state would never get initialized, and we'd dereference the NULL in the hotplug path. We also need to register the connector, so that userspace can get at it. >>> Shouldn't the KMS driver make sure the panel-bridge is set up before >>> drm_mode_config_reset? Is it the case when we're inserting the >>> panel-bridge driver as a module? >>> >>> >>> All the connectors that have been added are registered automatically >>> when drm_dev_register() is called by the KMS driver. Registering a >>> connector in the middle of setting up our driver is prone to race >>> conditions if the userspace decides to use them immediately. >> Yeah, this is fixing initializing panel_bridge at DSI host_attach >> time, >> which in the case of a panel module that creates the DSI device >> (adv7533-style, like you said I should use as a reference) will be >> after >> drm_mode_config_reset() and drm_dev_register(). > Okay. In the case of the msm kms driver, we defer probe until the > adv7533 module is inserted, only then we proceed to > drm_mode_config_reset() > and drm_dev_register(). I assumed this was the general practice > followed by > most kms drivers. I.,e the kms driver defers probe until all connector > related modules are inserted, and only then proceed to create a drm > device. The problem, though, is the panel driver needs the MIPI DSI host to exist to call mipi_dsi_device_register_full() during the probe process. The adv7533 driver gets around this by registering the DSI device in the bridge attach step, but drm_panel doesn't have an attach step. >> I'm not sure how we can get around this. We had discussion about this on >> irc >> recently, but couldn't come up with a good conclusion. We could come up >> with a >> panel_attach() callback to make it similar to bridges, but that's just >> us avoiding >> the real issue. > How about making DSI dev registration fully asynchronous, that is, DSI > devs declared in the DT under the DSI host node will be > registered/attached at probe time, and devs using another control bus > (like the adv7533 controller over i2c) will be registered afterwards. > > That implies moving the drm_brige registration logic outside of the DSI > host ->probe() path. The idea would be to check if all devs connected > to the DSI bus are ready at dsi_host->attach() time. If they are, we > can finally register the XXX -> DSI bridge. If they're not (because > some devs connected to the DSI bus have not been probed yet), then we > do not register the drm_bridge and wait for the next dsi_host->attach() > event. I guess you assumes that dsi-host knows all devs connected to it, thanks to: - subnodes of the host - ie. devices controlled via dsi bus, - graph links from host ports/endpoints - ie. devices controlled by other buses, for example adv7533. >>> Yep, but I think that's already a requirement when populating devices >>> with the OF graph method (if one of the DSI output endpoint does not >>> have a drm_bridge/panel attached to it, the DSI host driver returns >>> -EPROBE_DEFER). >>> I would separate both abstractions to make it more clear: 1. MIPI bus should be registered early - to allow create/bind devices on it, >>> Exactly. >>> 2. drm_bridge should be registered only if all required sinks (bridges/panels) are registered. >>> That's true, until we find a solution to support add DRM bridge hotplug. >>> First point seems OK, I am not sure about the 2nd one - if used consistently, it would require building pipeline from sink to source. >>> Yes. >> Since drm_bridge_attach requires encoder to be not null pipeline >> creation would be painful: >> 1. Every driver must call drm_of_find_panel_or_bridge on sink(s) before >> registering bridge and cache
Re: [PATCH v2 03/11] drm: sun4i: ignore swapped mixer<->tcon connection for DE2
Hi, On Tue, Jun 13, 2017 at 06:05:57PM +0800, Chen-Yu Tsai wrote: > > That's not true. DE1's can output to several TCONs (or rather, TCONs > > can select multiple engines as their input). The A31 for example is in > > this case. > > Actually that's not true. The TCON is bound to the backend. I don't see > any controls for muxing that. So the TCON-backend search routine is very > simple for DE1. The frontends are free to feed either backend though. I think it does, look at the lower bits of TCON0_CTL_REG and TCON1_CTL_REG in the A31 datasheet. It clearly seems used to control from which DE you fetch your data from, and you have both options. > >> For example, if we enabled mixer0, tcon0 and tcon1, tcon1 shouldn't > >> be bound at all. However in BFS situation tcon1 will also be bound > >> and then fail to be bound if the backward engine searching is fixed. > > > > Short term view: we shouldn't be in that case in the first place. > > Long term view: there's no reason it shouldn't work. > > Maybe I missed something? TCONs and everything before them should always > be enabled. There's no reason not to. This is especially true for TCON0 > which holds the mux register on some SoCs. If we're not able to use anything connected to TCON1, disabling it still seems like a good stop-gap measure. > About Maxime's long term view: there's no reason we can't just silently > ignore a component if its supposed companion is missing, like a TCON > missing its backend, or the other way around. It's a bit more complicated than that. TCON0 is probably mandatory, and even if TCON1 is missing, we can entirely route around it in hardware, and I think it's the case for all the stages in our pipelines. There's no reason we could not operate that way. But this is clearly something that we should aim for, not something that needs to be done today. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/7] drm/bridge: Support hotplugging panel-bridge.
On Fri, 23 Jun 2017 09:22:15 +0200 Andrzej Hajda wrote: > On 22.06.2017 15:34, Boris Brezillon wrote: > > On Thu, 22 Jun 2017 15:16:47 +0200 > > Andrzej Hajda wrote: > > > >> On 22.06.2017 14:41, Boris Brezillon wrote: > >>> On Thu, 22 Jun 2017 14:29:07 +0200 > >>> Andrzej Hajda wrote: > >>> > On 22.06.2017 11:23, Boris Brezillon wrote: > > On Thu, 22 Jun 2017 13:47:43 +0530 > > Archit Taneja wrote: > > > >> On 06/22/2017 01:20 PM, Benjamin Gaignard wrote: > >>> 2017-06-20 19:31 GMT+02:00 Eric Anholt : > Archit Taneja writes: > > > On 06/16/2017 08:13 PM, Eric Anholt wrote: > >> Archit Taneja writes: > >> > >>> On 06/16/2017 02:11 AM, Eric Anholt wrote: > If the panel-bridge is being set up after the > drm_mode_config_reset(), > then the connector's state would never get initialized, and we'd > dereference the NULL in the hotplug path. We also need to > register > the connector, so that userspace can get at it. > > >>> Shouldn't the KMS driver make sure the panel-bridge is set up > >>> before > >>> drm_mode_config_reset? Is it the case when we're inserting the > >>> panel-bridge driver as a module? > >>> > >>> > >>> All the connectors that have been added are registered > >>> automatically > >>> when drm_dev_register() is called by the KMS driver. Registering a > >>> connector in the middle of setting up our driver is prone to race > >>> conditions if the userspace decides to use them immediately. > >>> > >> Yeah, this is fixing initializing panel_bridge at DSI host_attach > >> time, > >> which in the case of a panel module that creates the DSI device > >> (adv7533-style, like you said I should use as a reference) will be > >> after > >> drm_mode_config_reset() and drm_dev_register(). > > Okay. In the case of the msm kms driver, we defer probe until the > > adv7533 module is inserted, only then we proceed to > > drm_mode_config_reset() > > and drm_dev_register(). I assumed this was the general practice > > followed by > > most kms drivers. I.,e the kms driver defers probe until all > > connector > > related modules are inserted, and only then proceed to create a drm > > device. > The problem, though, is the panel driver needs the MIPI DSI host to > exist to call mipi_dsi_device_register_full() during the probe > process. > The adv7533 driver gets around this by registering the DSI device in > the > bridge attach step, but drm_panel doesn't have an attach step. > > >> I'm not sure how we can get around this. We had discussion about this > >> on irc > >> recently, but couldn't come up with a good conclusion. We could come > >> up with a > >> panel_attach() callback to make it similar to bridges, but that's just > >> us avoiding > >> the real issue. > > How about making DSI dev registration fully asynchronous, that is, DSI > > devs declared in the DT under the DSI host node will be > > registered/attached at probe time, and devs using another control bus > > (like the adv7533 controller over i2c) will be registered afterwards. > > > > That implies moving the drm_brige registration logic outside of the DSI > > host ->probe() path. The idea would be to check if all devs connected > > to the DSI bus are ready at dsi_host->attach() time. If they are, we > > can finally register the XXX -> DSI bridge. If they're not (because > > some devs connected to the DSI bus have not been probed yet), then we > > do not register the drm_bridge and wait for the next dsi_host->attach() > > event. > I guess you assumes that dsi-host knows all devs connected to it, thanks > to: > - subnodes of the host - ie. devices controlled via dsi bus, > - graph links from host ports/endpoints - ie. devices controlled by > other buses, for example adv7533. > >>> Yep, but I think that's already a requirement when populating devices > >>> with the OF graph method (if one of the DSI output endpoint does not > >>> have a drm_bridge/panel attached to it, the DSI host driver returns > >>> -EPROBE_DEFER). > >>> > I would separate both abstractions to make it more clear: > 1. MIPI bus should be registered early - to allow create/bind devices on > it, > >>> Exactly. > >>> > 2. drm_bridge should be registered only if all required sinks > (bridges/panels) are registered. > >>> That's true, until we find a solution to support add DRM
Re: [PATCH 00/12] fbdev helper locking rework and deferred setup
On Thu, Jun 22, 2017 at 4:54 PM, Liviu Dudau wrote: > On Wed, Jun 21, 2017 at 08:28:03PM +0200, Daniel Vetter wrote: >> Hi all, >> >> This is Thierry's deferred fbdev setup series, with the locking rework almost >> entirely redone. The much wider scope is to get rid of drm_modeset_lock_all >> calls for atomic drivers and remove users of the fairly nasty >> mode_config->acquire_ctx hack, which breaks when doing multiple atomic >> commits. >> >> Testing&review very much appreciated, especially from people who care about >> the >> various fbdev emulation things and the deferred setup stuff. > > Tested on my Juno dev board on a bit of a convoluted setup: the dev board has > built into the SoC 2x HDLCD instances and I also have an FPGA daughter board > with Mali DP 650 running again on a 2x configuration. On boot I'm getting this > warning: > > juno-r0-ld login: [ 241.986785] INFO: task kworker/3:1:80 blocked for more > than 120 seconds. > [ 241.993652] Not tainted 4.12.0-rc5-01275-g1e2237a19156 #2 > [ 241.999689] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables > this message. > [ 242.007660] kworker/3:1 D080 2 0x > [ 242.013340] Workqueue: events output_poll_execute [drm_kms_helper] > [ 242.019637] Call trace: > [ 242.022132] [] __switch_to+0x98/0xb0 > [ 242.027373] [] __schedule+0x19c/0x5e8 > [ 242.032699] [] schedule+0x38/0xa0 > [ 242.037672] [] schedule_preempt_disabled+0x20/0x38 > [ 242.044144] [] __mutex_lock.isra.0+0x140/0x530 > [ 242.050262] [] __mutex_lock_slowpath+0x10/0x18 > [ 242.056379] [] mutex_lock+0x30/0x38 > [ 242.061597] [] > drm_fb_helper_hotplug_event.part.22+0x20/0x100 [drm_kms_helper] > [ 242.070611] [] drm_fb_helper_hotplug_event+0x24/0x38 > [drm_kms_helper] > [ 242.078828] [] drm_fbdev_cma_hotplug_event+0x10/0x20 > [drm_kms_helper] > [ 242.086984] [] hdlcd_fb_output_poll_changed+0x14/0x20 > [hdlcd] > [ 242.094495] [] drm_kms_helper_hotplug_event+0x28/0x38 > [drm_kms_helper] > [ 242.102801] [] output_poll_execute+0x1a0/0x1f0 > [drm_kms_helper] > [ 242.110424] [] process_one_work+0x1d4/0x330 > [ 242.116280] [] worker_thread+0x48/0x468 > [ 242.121781] [] kthread+0x12c/0x130 > [ 242.126839] [] ret_from_fork+0x10/0x50 > [ 242.132252] INFO: task kworker/5:1:82 blocked for more than 120 seconds. > [ 242.139074] Not tainted 4.12.0-rc5-01275-g1e2237a19156 #2 > [ 242.145099] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables > this message. > [ 242.153064] kworker/5:1 D082 2 0x > [ 242.158724] Workqueue: events output_poll_execute [drm_kms_helper] > [ 242.165019] Call trace: > [ 242.167519] [] __switch_to+0x98/0xb0 > [ 242.172755] [] __schedule+0x19c/0x5e8 > [ 242.178079] [] schedule+0x38/0xa0 > [ 242.183051] [] schedule_preempt_disabled+0x20/0x38 > [ 242.189520] [] __mutex_lock.isra.0+0x140/0x530 > [ 242.195639] [] __mutex_lock_slowpath+0x10/0x18 > [ 242.201756] [] mutex_lock+0x30/0x38 > [ 242.206972] [] > drm_fb_helper_hotplug_event.part.22+0x20/0x100 [drm_kms_helper] > [ 242.215984] [] drm_fb_helper_hotplug_event+0x24/0x38 > [drm_kms_helper] > [ 242.224201] [] drm_fbdev_cma_hotplug_event+0x10/0x20 > [drm_kms_helper] > [ 242.232367] [] malidp_output_poll_changed+0x14/0x20 > [mali_dp] > [ 242.239880] [] drm_kms_helper_hotplug_event+0x28/0x38 > [drm_kms_helper] > [ 242.248188] [] output_poll_execute+0x1a0/0x1f0 > [drm_kms_helper] > [ 242.255808] [] process_one_work+0x1d4/0x330 > [ 242.261664] [] worker_thread+0x48/0x468 > [ 242.267196] [] kthread+0x12c/0x130 > [ 242.272312] [] ret_from_fork+0x10/0x50 > > Each hardware type has only one instance of each driver being connected to an > output, HDLCD is connected to an HDMI monitor that is not "active" (i.e. > input is switched to DP connection), while the Mali DP instance is connected > to > a monitor. > > Suggestions on where to look next are welcome. I'm betting I've fumbled an unlock path somewhere and now your stuck trying to get a lock you can't get. Can you pls recompile with lockdep enabled and repro? That will directly tell us where and what went wrong. Thanks for testing. -Daniel > > Best regards, > Liviu > >> >> Thanks, Daniel >> >> Daniel Vetter (7): >> drm/i915: Drop FBDEV #ifdev in mst code >> drm/fb-helper: Push locking in fb_is_bound >> drm/fb-helper: Drop locking from the vsync wait ioctl code >> drm/fb-helper: Push locking into pan_display_atomic|legacy >> drm/fb-helper: Push locking into restore_fbdev_mode_atomic|legacy >> drm/fb-helper: Stop using mode_config.mutex for internals >> drm/fb-helper: Split dpms handling into legacy and atomic paths >> >> Thierry Reding (5): >> drm/fb-helper: Push down modeset lock into FB helpers >> drm/fb-helper: Add top-level lock >> drm/fb-helper: Support deferred setup >> drm/exynos: Remove custom FB helper deferred setup >> drm/hisilicon: Remove custom FB helper deferred setup >> >> drivers/gpu/drm/drm_fb_helper.c
[PATCH] drm/i915: Print pwm device-name when using pwm for backlight control
On Bay Trail devices either the Crystal Cove PMIC's pwm or the LPSS' pwm is used to control the backlight. Other drivers take core of providing a backlight_pwm alias through pwm_add_table, but it is still useful to know which pwm device actually ends up being used for debugging backlight issues, so add a DRM_INFO logging the pwm device-name. Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/intel_panel.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index cb50c527401f..7936a64af393 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1670,6 +1670,9 @@ static int pwm_setup_backlight(struct intel_connector *connector, CRC_PMIC_PWM_PERIOD_NS); panel->backlight.enabled = panel->backlight.level != 0; + DRM_INFO("Using %s pwm-device for backlight control\n", +dev_name(panel->backlight.pwm->chip->dev)); + return 0; } -- 2.13.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2nd resend 1/4] drm/i915: Fix false-positive assert_rpm_wakelock_held in i915_pmic_bus_access_notifier
assert_rpm_wakelock_held is triggered from i915_pmic_bus_access_notifier even though it gets unregistered on (runtime) suspend, this is caused by a race happening under the following circumstances: intel_runtime_pm_put does: atomic_dec(&dev_priv->pm.wakeref_count); pm_runtime_mark_last_busy(kdev); pm_runtime_put_autosuspend(kdev); And pm_runtime_put_autosuspend calls intel_runtime_suspend from a workqueue, so there is ample of time between the atomic_dec() and intel_runtime_suspend() unregistering the notifier. If the notifier gets called in this windowd assert_rpm_wakelock_held falsely triggers (at this point we're not runtime-suspended yet). This commit adds disable_rpm_wakeref_asserts and enable_rpm_wakeref_asserts calls around the intel_uncore_forcewake_get(FORCEWAKE_ALL) call in i915_pmic_bus_access_notifier fixing the false-positive WARN_ON. Reported-by: FKr Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/intel_uncore.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 6d1ea26b2493..dc5e4f3e5a43 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1286,8 +1286,15 @@ static int i915_pmic_bus_access_notifier(struct notifier_block *nb, * bus, which will be busy after this notification, leading to: * "render: timed out waiting for forcewake ack request." * errors. +* +* This notifier may get called between intel_runtime_pm_put() +* doing atomic_dec(wakeref_count) and intel_runtime_resume() +* unregistering this notifier, which leads to false-positive +* assert_rpm_wakelock_held() triggering. */ + disable_rpm_wakeref_asserts(dev_priv); intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); + enable_rpm_wakeref_asserts(dev_priv); break; case MBI_PMIC_BUS_ACCESS_END: intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); -- 2.13.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2nd resend 4/4] drm/i915: Acquire PUNIT->PMIC bus for intel_uncore_forcewake_reset()
intel_uncore_forcewake_reset() does forcewake puts and gets as such we need to make sure that no-one tries to access the PUNIT->PMIC bus (on systems where this bus is shared) while it runs, otherwise bad things happen. Normally this is taken care of by the i915_pmic_bus_access_notifier() which does an intel_uncore_forcewake_get(FORCEWAKE_ALL) when some other driver tries to access the PMIC bus, so that later forcewake gets are no-ops (for the duration of the bus access). But intel_uncore_forcewake_reset gets called in 3 cases: 1) Before registering the pmic_bus_access_notifier 2) After unregistering the pmic_bus_access_notifier 3) To reset forcewake state on a GPU reset In all 3 cases the i915_pmic_bus_access_notifier() protection is insufficient. This commit fixes the pmic bus access race this causes by making intel_uncore_forcewake_reset() call iosf_mbi_punit_acquire() (and iosf_mbi_punit_release() when done). Note that iosf_mbi_punit_acquire() locks a mutex and thus intel_uncore_forcewake_reset() may sleep after this commit. I've checked all callers and they all already take other mutexes, so this is not a problem. Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/intel_uncore.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 882cf7187a57..0fb829cc1cbd 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -252,6 +252,9 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv, int retry_count = 100; enum forcewake_domains fw, active_domains; + /* Acquire the PUNIT->PMIC bus before modifying forcewake settings */ + iosf_mbi_punit_acquire(); + /* Hold uncore.lock across reset to prevent any register access * with forcewake not set correctly. Wait until all pending * timers are run before holding. @@ -308,6 +311,7 @@ static void intel_uncore_forcewake_reset(struct drm_i915_private *dev_priv, assert_forcewakes_inactive(dev_priv); spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); + iosf_mbi_punit_release(); } static u64 gen9_edram_size(struct drm_i915_private *dev_priv) -- 2.13.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2nd resend 0/4] Various PMIC BUS access arbritration fixes
Hi All, These 4 patches fix a number of real issues, yet they have seen 0 feedback so far, please review (and merge). Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2nd resend 2/4] drm/i915: Re-register PMIC bus access notifier on runtime resume
intel_uncore_suspend() unregisters the uncore code's PMIC bus access notifier and gets called on both normal and runtime suspend. intel_uncore_resume_early() re-registers the notifier, but only on normal resume. Add a new intel_uncore_runtime_resume() function which only re-registers the notifier and call that on runtime resume. Reported-by: Imre Deak Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/i915_drv.c | 2 ++ drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_uncore.c | 6 ++ 3 files changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 48428672fc6e..591436fbd62c 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2469,6 +2469,8 @@ static int intel_runtime_resume(struct device *kdev) ret = vlv_resume_prepare(dev_priv, true); } + intel_uncore_runtime_resume(dev_priv); + /* * No point of rolling back things in case of an error, as the best * we can do is to hope that things will still work (and disable RPM). diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2c453a4e97d5..81e0c17c012a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3084,6 +3084,7 @@ extern bool intel_uncore_arm_unclaimed_mmio_detection(struct drm_i915_private *d extern void intel_uncore_fini(struct drm_i915_private *dev_priv); extern void intel_uncore_suspend(struct drm_i915_private *dev_priv); extern void intel_uncore_resume_early(struct drm_i915_private *dev_priv); +extern void intel_uncore_runtime_resume(struct drm_i915_private *dev_priv); const char *intel_uncore_forcewake_domain_to_str(const enum forcewake_domain_id id); void intel_uncore_forcewake_get(struct drm_i915_private *dev_priv, enum forcewake_domains domains); diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index dc5e4f3e5a43..882cf7187a57 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -438,6 +438,12 @@ void intel_uncore_resume_early(struct drm_i915_private *dev_priv) i915_check_and_clear_faults(dev_priv); } +void intel_uncore_runtime_resume(struct drm_i915_private *dev_priv) +{ + iosf_mbi_register_pmic_bus_access_notifier( + &dev_priv->uncore.pmic_bus_access_nb); +} + void intel_uncore_sanitize(struct drm_i915_private *dev_priv) { i915.enable_rc6 = sanitize_rc6_option(dev_priv, i915.enable_rc6); -- 2.13.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2nd resend 3/4] drm/i915: Call uncore_suspend before platform suspend handlers
Quoting Ville: "the forcewake timer might still be active until the uncore suspend, and having active forcewakes while we've already told the GT wake stuff to stop acting normally doesn't seem quite right to me." Reported-by: Ville Syrjälä Suggested-by: Imre Deak Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/i915_drv.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 591436fbd62c..9f744065c502 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -2371,6 +2371,8 @@ static int intel_runtime_suspend(struct device *kdev) intel_runtime_pm_disable_interrupts(dev_priv); + intel_uncore_suspend(dev_priv); + ret = 0; if (IS_GEN9_LP(dev_priv)) { bxt_display_core_uninit(dev_priv); @@ -2383,6 +2385,8 @@ static int intel_runtime_suspend(struct device *kdev) if (ret) { DRM_ERROR("Runtime suspend failed, disabling it (%d)\n", ret); + intel_uncore_runtime_resume(dev_priv); + intel_runtime_pm_enable_interrupts(dev_priv); enable_rpm_wakeref_asserts(dev_priv); @@ -2390,8 +2394,6 @@ static int intel_runtime_suspend(struct device *kdev) return ret; } - intel_uncore_suspend(dev_priv); - enable_rpm_wakeref_asserts(dev_priv); WARN_ON_ONCE(atomic_read(&dev_priv->pm.wakeref_count)); -- 2.13.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
On Thu, Jun 22, 2017 at 10:48:10AM +0200, Peter Rosin wrote: > On 2017-06-22 08:36, Daniel Vetter wrote: > > On Wed, Jun 21, 2017 at 11:40:52AM +0200, Peter Rosin wrote: > >> On 2017-06-21 09:38, Daniel Vetter wrote: > >>> On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote: > This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get > totally obsolete. > > I think the gamma_store can end up invalid on error. But the way I read > it, that can happen in drm_mode_gamma_set_ioctl as well, so why should > this pesky legacy fbdev stuff be any better? > > drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. > However, > it saves it to the gamma_store which should already be up to date with > what > .gamma_get would return and is thus a nop. So, zap it. > >>> > >>> Removing drm_fb_helper_save_lut_atomic should be a separate patch I > >>> think. > >> > >> Then 3 patches would be needed, first some hybrid thing that does it the > >> old way, but also stores the lut in .gamma_store, then the split-out that > >> removes drm_fb_helper_save_lut_atomic, then whatever is needed to get > >> to the desired code. I can certainly do that, but do you want me to? > > > > Explain that in the commit message and it's fine. > > I did the split in v2, I assume that's ok too. Better in case anyone ever > needs to run a bisect on this... > > >>> It's a pre-existing bug, but should we also try to restore the fbdev lut > >>> in drm_fb_helper_restore_fbdev_mode_unlocked()? Would be yet another bug, > >>> but might be relevant for your use-case. Just try to run both an fbdev > >>> application and some kms-native thing, and then SIGKILL the native kms > >>> app. > >>> > >>> But since pre-existing not really required, and probably too much effort. > >> > >> Good thing too, because I don't really know my way around this code... > > > > Btw I cc'ed you on one of my patches in the fbdev locking series, we might > > need to do the same legacy vs. atomic split for the new lut code as I did > > for dpms. The rule with atomic is that you can't do multiple commits under > > drm_modeset_lock_all, you either have to do one overall atomic commit > > (preferred) or drop&reacquire locks again. This matters for LUT since > > you're updating the LUT on all CRTCs, which when using the gamma_set > > atomic helper would be multiple commits :-/ > > Ahh, ok, I see the problem. > > > Using the dpms patch as template it shouldn't be too hard to address that > > for your patch here too. > > Hmm, in that patch you handle the legacy case in a separate function, and > doing that for the lut case looks difficult when the atomic commit happens > inside the helper (typically drm_atomic_helper_legacy_gamma_set which > could perhaps be handled, but a real drag to handle for drivers that have > a custom crtc .gamma_set). Yeah, that's essentially why you need 2 functions. Legacy one calls the legacy interfaces, the atomic one calls the new fancy atomic property stuff directly (i.e. it open-codes the property setting dance from the helper, but with one atomic commit across all crtc). The reason that the legacy callback functions are helpers and not just default behavior is that I expected some drivers to need special hacks to keep their existing legacy kms userspace working. Atomic helpers unified behaviour a lot that beforehand was slightly inconsistent. I somewhat expect that long-term we'll unexport all the compat helpers and just make them the default for all atomic drivers. > So, I'm aiming for the drop&reacquire approach... Hm, I prefer the open-coding, that's at least what we do everywhere else. Has the benefit that it's more atomic (one commit for everything). > However, I don't have all of that series, and I suspect that is why I do > not have any fb_helper->lock. Oh sorry, entire series is on dri-devel. I can do a git branch too if you need one, atm it's in my general pile: https://cgit.freedesktop.org/~danvet/drm/log/ Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/13] drm/amd|radeon: Drop drm_vblank_cleanup
On Thu, Jun 22, 2017 at 08:54:51AM -0400, Alex Deucher wrote: > On Wed, Jun 21, 2017 at 4:28 AM, Daniel Vetter wrote: > > Both drivers shut down all crtc beforehand already, which will shut up > > any pending vblank (the only thing vblank_cleanup really does is > > disable the disable timer). Hence we don't need this here and can > > remove it. > > > > Cc: Alex Deucher > > Cc: Christian König > > Signed-off-by: Daniel Vetter > > Acked-by: Alex Deucher > > Do you want me to pick this up or are you trying to land the whole > series as one? I'd like to get the final patch in soonish, so all going through drm-misc would be less coordination. Thanks for taking a look. -Daniel > > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 1 - > > drivers/gpu/drm/radeon/radeon_irq_kms.c | 1 - > > 2 files changed, 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > > index 62da6c5c6095..2480273c1dca 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c > > @@ -263,7 +263,6 @@ void amdgpu_irq_fini(struct amdgpu_device *adev) > > { > > unsigned i, j; > > > > - drm_vblank_cleanup(adev->ddev); > > if (adev->irq.installed) { > > drm_irq_uninstall(adev->ddev); > > adev->irq.installed = false; > > diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c > > b/drivers/gpu/drm/radeon/radeon_irq_kms.c > > index 7aacb44df201..fff0d11b0600 100644 > > --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c > > +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c > > @@ -324,7 +324,6 @@ int radeon_irq_kms_init(struct radeon_device *rdev) > > */ > > void radeon_irq_kms_fini(struct radeon_device *rdev) > > { > > - drm_vblank_cleanup(rdev->ddev); > > if (rdev->irq.installed) { > > drm_irq_uninstall(rdev->ddev); > > rdev->irq.installed = false; > > -- > > 2.11.0 > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/7] drm/bridge: Support hotplugging panel-bridge.
On Thu, Jun 15, 2017 at 01:41:24PM -0700, Eric Anholt wrote: > If the panel-bridge is being set up after the drm_mode_config_reset(), > then the connector's state would never get initialized, and we'd > dereference the NULL in the hotplug path. We also need to register > the connector, so that userspace can get at it. > > Signed-off-by: Eric Anholt I've not read all the details of the discussion here, just a comment on what this implies if we go with this. > --- > drivers/gpu/drm/bridge/panel.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > index 67fe19e5a9c6..8ed8a70799c7 100644 > --- a/drivers/gpu/drm/bridge/panel.c > +++ b/drivers/gpu/drm/bridge/panel.c > @@ -82,11 +82,14 @@ static int panel_bridge_attach(struct drm_bridge *bridge) > > drm_mode_connector_attach_encoder(&panel_bridge->connector, > bridge->encoder); > + drm_atomic_helper_connector_reset(&panel_bridge->connector); I'm not sure we want to tie the ->reset stuff in with other helpers. One of the design goals I had with all things atomic was to make helpers much more modular, and imo the reset is one such piece - a driver might want to instead reconstruct state from hw using something like i915's hw state readout/compare/verify framework. If we can solve this init ordering issue without adding a depency on the reset stuff that would be a bit neater imo. But that's just my 2 cents. Cheers, Daniel > > ret = drm_panel_attach(panel_bridge->panel, &panel_bridge->connector); > if (ret < 0) > return ret; > > + drm_connector_register(&panel_bridge->connector); > + > return 0; > } > > -- > 2.11.0 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [RESEND-CI v4 09/15] drm: add helper functions for YCBCR output handling
On Thu, Jun 22, 2017 at 11:42 AM, Sharma, Shashank wrote: >> You should explain in 1-2 sentences what exactly this function does, and >> when a driver should use it. Just documenting the input/output stuff >> doesn't make the kerneldoc all that useful. > > Did you miss the first 3 lines above ? > "get the most suitable output. > Find the best suitable HDMI output considering source capability, sink > capability and user's choice (expressed in form of drm property)" > Or you mean that's not enough ? Indeed. Usually I sort the paramaters first, then the text. There should also be an empty line before the text starts. Would be great if you can do that since you'll respin anyway. -Daniel -- 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 https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v5 7/7] drm: create hdmi output property
On Thu, Jun 22, 2017 at 10:33 AM, Sharma, Shashank wrote: >> - The property values should be limited to what the driver can support, I >>guess that would mean limiting the available ycbcr modes? Or does all >>our hw support all the modes, including 420 (on the sink side)? > > This property is targeted at DRM layer, so naturally its for all the HWs > along with Intel HW, so it serves a big range. > All our HDMI 1.4 sources support RGB444, and after this series, can support > YCBCR444. > All HDMI 2.0 sources should support YCBCR420, but they can declare this > using a bool variable which I added in patch 3 (ycbcr420_allowed) > As we are targeting both HDMI 1.4 as well as HDMI 2.0 (Src and Sink), as a > whole we are covering all options. Yes, we need to define values for everything, since it's a generic property. But for a given driver imo we should only allow the values that are actually supported. An example would be the rotation property, which supporsts X/Y-mirror and rotation by 90° steps. But on a given i915 platform we only register support for the stuff the driver/hw can do, e.g. pre-gen9 do not register 90/270° rotation. I think we should do the same here. See drm_plane_create_rotation_property(), specifically the supported_rotations parameter. Cheers, Daniel -- 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 https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/2] drm: atmel-hlcdc: add missing .set_property helper to the crtc
On Thu, Jun 22, 2017 at 07:03:10AM +0200, Peter Rosin wrote: > The default implementation should be used. > > Signed-off-by: Peter Rosin Purely optional refactor idea, since you're not the first one to stumble over this, and in hindsight it makes sense to have these functions as the defaults: - Move all the set_property legacy2atomic helper functions into the core, renaming them to e.g. drm_atomic_default_crtc_set_property. But we'd do this for connectors and planes too ofc. - Use them as the default handler if drm_drv_uses_atomic_modeset() is true. - Nuke the default assignment from all drivers. This way there's less boilerplate, less confusion (some people thought you must have atomic userspace to use atomic properties, which isn't true as long as this is wired up) and happier driver developers :-) But since you've volunteered already to fix the fbdev clut stuff this really is just an idea. -Daniel > --- > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c > b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c > index 5348985..cc00ce3 100644 > --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c > +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c > @@ -429,6 +429,7 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs > = { > .atomic_destroy_state = atmel_hlcdc_crtc_destroy_state, > .enable_vblank = atmel_hlcdc_crtc_enable_vblank, > .disable_vblank = atmel_hlcdc_crtc_disable_vblank, > + .set_property = drm_atomic_helper_crtc_set_property, > }; > > int atmel_hlcdc_crtc_create(struct drm_device *dev) > -- > 2.1.4 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v2 13/14] drm: stm: remove dead code and pointless local lut storage
On Thu, Jun 22, 2017 at 11:49:34AM +, Philippe CORNU wrote: > > > On 06/22/2017 08:06 AM, Peter Rosin wrote: > > The redundant fb helper .load_lut is no longer used, and can not > > work right without also providing the fb helpers .gamma_set and > > .gamma_get thus rendering the code in this driver suspect. > > > > Hi Peter, > STM32 chipsets supports 8-bit CLUT mode but this driver version does not > support it "yet" (final patch has not been upstreamed because it was a > too big fbdev patch for simply adding CLUT...). > > Regarding your patch below, if it helps you to ease the drm framework > update then I am agree to "acknowledge it" asap, else if you are not in > a hurry, I would prefer a better and definitive patch handling 8-bit > CLUT properly and I am ok to help or/and to do it : ) I'll take your ack, since your 8bit lut support will be massively simpler with Peter's series here :-) > Extra questions: > - any plan to update modetest with the DRM_FORMAT_C8 support + gamma > get/set? We have gamma igts, well for the fancy new atomic color manager stuff. Testing drivers with modetest is kinda not that awesome :-) -Daniel > - do you have a simple way to test clut with fbdev, last year we where > using an old version of the SDL but I am still looking for a small piece > of code to do it (else I will do it myself but C8 on fbdev is not really > a priority ;-) > > best regards, > Philippe > > > Just remove the dead code. > > > > Signed-off-by: Peter Rosin > > --- > > drivers/gpu/drm/stm/ltdc.c | 12 > > drivers/gpu/drm/stm/ltdc.h | 1 - > > 2 files changed, 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c > > index 1b9483d..87829b9 100644 > > --- a/drivers/gpu/drm/stm/ltdc.c > > +++ b/drivers/gpu/drm/stm/ltdc.c > > @@ -375,17 +375,6 @@ static irqreturn_t ltdc_irq(int irq, void *arg) > >* DRM_CRTC > >*/ > > > > -static void ltdc_crtc_load_lut(struct drm_crtc *crtc) > > -{ > > - struct ltdc_device *ldev = crtc_to_ltdc(crtc); > > - unsigned int i, lay; > > - > > - for (lay = 0; lay < ldev->caps.nb_layers; lay++) > > - for (i = 0; i < 256; i++) > > - reg_write(ldev->regs, LTDC_L1CLUTWR + lay * LAY_OFS, > > - ldev->clut[i]); > > -} > > - > > static void ltdc_crtc_enable(struct drm_crtc *crtc) > > { > > struct ltdc_device *ldev = crtc_to_ltdc(crtc); > > @@ -523,7 +512,6 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc > > *crtc, > > } > > > > static struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = { > > - .load_lut = ltdc_crtc_load_lut, > > .enable = ltdc_crtc_enable, > > .disable = ltdc_crtc_disable, > > .mode_set_nofb = ltdc_crtc_mode_set_nofb, > > diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h > > index d7a9c73..620ca55 100644 > > --- a/drivers/gpu/drm/stm/ltdc.h > > +++ b/drivers/gpu/drm/stm/ltdc.h > > @@ -27,7 +27,6 @@ struct ltdc_device { > > struct drm_panel *panel; > > struct mutex err_lock; /* protecting error_status */ > > struct ltdc_caps caps; > > - u32 clut[256]; /* color look up table */ > > u32 error_status; > > u32 irq_status; > > }; > > > ___ > Intel-gfx mailing list > intel-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH drm-intel] drm: arcpgu: arc_pgu_crtc_mode_valid() can be static
Signed-off-by: Fengguang Wu --- arcpgu_crtc.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c index 99fbdae..611af74 100644 --- a/drivers/gpu/drm/arc/arcpgu_crtc.c +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c @@ -64,8 +64,8 @@ static const struct drm_crtc_funcs arc_pgu_crtc_funcs = { .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, }; -enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc, -const struct drm_display_mode *mode) +static enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc, + const struct drm_display_mode *mode) { struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); long rate, clk_rate = mode->clock * 1000; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[drm-intel:drm-intel-nightly 1499/1516] drivers/gpu/drm/arc/arcpgu_crtc.c:67:22: sparse: symbol 'arc_pgu_crtc_mode_valid' was not declared. Should it be static?
tree: git://anongit.freedesktop.org/drm-intel drm-intel-nightly head: cd6f0ef4478545aa014d92cabe4d794bfe54fe33 commit: 2b3d860efa3461af109469e6de2eea48f6ef5cdd [1499/1516] drm: arcpgu: Use crtc->mode_valid() callback reproduce: # apt-get install sparse git checkout 2b3d860efa3461af109469e6de2eea48f6ef5cdd make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) >> drivers/gpu/drm/arc/arcpgu_crtc.c:67:22: sparse: symbol >> 'arc_pgu_crtc_mode_valid' was not declared. Should it be static? Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [RESEND-CI v4 09/15] drm: add helper functions for YCBCR output handling
Regards Shashank On 6/23/2017 2:42 PM, Daniel Vetter wrote: On Thu, Jun 22, 2017 at 11:42 AM, Sharma, Shashank wrote: You should explain in 1-2 sentences what exactly this function does, and when a driver should use it. Just documenting the input/output stuff doesn't make the kerneldoc all that useful. Did you miss the first 3 lines above ? "get the most suitable output. Find the best suitable HDMI output considering source capability, sink capability and user's choice (expressed in form of drm property)" Or you mean that's not enough ? Indeed. Usually I sort the paramaters first, then the text. There should also be an empty line before the text starts. Would be great if you can do that since you'll respin anyway. Sure, not a problem. - Shashank -Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v5 7/7] drm: create hdmi output property
Regards Shashank On 6/23/2017 2:50 PM, Daniel Vetter wrote: On Thu, Jun 22, 2017 at 10:33 AM, Sharma, Shashank wrote: - The property values should be limited to what the driver can support, I guess that would mean limiting the available ycbcr modes? Or does all our hw support all the modes, including 420 (on the sink side)? This property is targeted at DRM layer, so naturally its for all the HWs along with Intel HW, so it serves a big range. All our HDMI 1.4 sources support RGB444, and after this series, can support YCBCR444. All HDMI 2.0 sources should support YCBCR420, but they can declare this using a bool variable which I added in patch 3 (ycbcr420_allowed) As we are targeting both HDMI 1.4 as well as HDMI 2.0 (Src and Sink), as a whole we are covering all options. Yes, we need to define values for everything, since it's a generic property. But for a given driver imo we should only allow the values that are actually supported. An example would be the rotation property, which supporsts X/Y-mirror and rotation by 90° steps. But on a given i915 platform we only register support for the stuff the driver/hw can do, e.g. pre-gen9 do not register 90/270° rotation. I think we should do the same here. See drm_plane_create_rotation_property(), specifically the supported_rotations parameter. Ah, I got your point now, and its valid. If you see the I915 level handlers of YCBCR functions (added in patch 10) they are taking care of blocking anything which is not supported by driver for this platform, based on: - The source capability - The sink capability - User preference of the property. So on a whole, set of Intel platforms cover all the values of property, but at driver level we make sure to allow only what is suitable for this source + sink combination. - Shashank Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v12 0/3] Enhancement to intel_dp_aux_backlight driver
On Thu, Jun 22, 2017 at 12:03:36PM -0700, Puthikorn Voravootivat wrote: > This patch set contain 3 patches which are already reviewed by DK. > Another 6 patches in previous version was already merged in v7 and v9. > - First patch sets the PWM freqency to match data in panel vbt. > - Next patch adds heuristic to determine whether we should use AUX > or PWM pin to adjust panel backlight brightness. > - Last patch adds support for dynamic brightness. > > Change log: > v12: > - Rebase Thanks for doing the rebase, applied. -Daniel > > v11: > - Reorder patches in v10 to make the last patch come first > - Fix nits > > v10: > - Add heuristic in patch #1 > - Add _unsafe mod option in patch #1, #2 > - handle frequency set error in patch #3 > > v9: > - Fix nits in v8 > > v8: > - Drop 4 patches that was already merged > - Move DP_EDP_BACKLIGHT_AUX_ENABLE_CAP check to patch #2 to avoid > behavior change if only apply patch #1 > - Add TODO to warn about enable backlight twice in patch #2 > - Use DIV_ROUND_CLOSEST instead of just "/" in patch #5 > - Fix bug calculate pn in patch #5 > - Clarify commit message / code comment in patch #5 > > v7: > - Add check in intel_dp_pwm_pin_display_control_capable in patch #4 > - Add option in patch #6 to enable DPCD or not > - Change definition in patch #8 and implementation in #9 to use Khz > - Fix compiler warning from build bot in patch #9 > > v6: > - Address review from Dhinakaran > - Make PWM frequency to have highest value of Pn that make the > frequency still within 25% of desired frequency. > > v5: > - Split first patch in v4 to 3 patches > - Bump priority for "Correctly enable backlight brightness adjustment via > DPCD" > - Make logic clearer for the case that both PWM pin and AUX are supported > - Add more log when write to register fail > - Add log when enable DBC > > v4: > - Rebase / minor typo fix. > > v3: > - Add new implementation of PWM frequency patch > > v2: > - Drop PWM frequency patch > - Address suggestion from Jani Nikula > > Puthikorn Voravootivat (3): > drm/i915: Set PWM divider to match desired frequency in vbt > drm/i915: Add heuristic to determine better way to adjust brightness > drm/i915: Add option to support dynamic backlight via DPCD > > drivers/gpu/drm/i915/i915_params.c| 12 +- > drivers/gpu/drm/i915/i915_params.h| 5 +- > drivers/gpu/drm/i915/intel_dp_aux_backlight.c | 185 > -- > 3 files changed, 186 insertions(+), 16 deletions(-) > > -- > 2.13.1.611.g7e3b11ae1-goog > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v5 7/7] drm: create hdmi output property
On Fri, Jun 23, 2017 at 03:35:30PM +0530, Sharma, Shashank wrote: > Regards > > Shashank > > > On 6/23/2017 2:50 PM, Daniel Vetter wrote: > > On Thu, Jun 22, 2017 at 10:33 AM, Sharma, Shashank > > wrote: > > > > - The property values should be limited to what the driver can support, > > > > I > > > > guess that would mean limiting the available ycbcr modes? Or does > > > > all > > > > our hw support all the modes, including 420 (on the sink side)? > > > This property is targeted at DRM layer, so naturally its for all the HWs > > > along with Intel HW, so it serves a big range. > > > All our HDMI 1.4 sources support RGB444, and after this series, can > > > support > > > YCBCR444. > > > All HDMI 2.0 sources should support YCBCR420, but they can declare this > > > using a bool variable which I added in patch 3 (ycbcr420_allowed) > > > As we are targeting both HDMI 1.4 as well as HDMI 2.0 (Src and Sink), as a > > > whole we are covering all options. > > Yes, we need to define values for everything, since it's a generic > > property. But for a given driver imo we should only allow the values > > that are actually supported. An example would be the rotation > > property, which supporsts X/Y-mirror and rotation by 90° steps. But on > > a given i915 platform we only register support for the stuff the > > driver/hw can do, e.g. pre-gen9 do not register 90/270° rotation. I > > think we should do the same here. See > > drm_plane_create_rotation_property(), specifically the > > supported_rotations parameter. > Ah, I got your point now, and its valid. > If you see the I915 level handlers of YCBCR functions (added in patch 10) > they are taking care of blocking > anything which is not supported by driver for this platform, based on: > - The source capability > - The sink capability > - User preference of the property. Yeah, runtime checks are needed on top (sometimes at least). But if we can't support a given mode for a given sink then we should take that into account when registering the property. Otherwise userspace tries stuff that can never succeed, which isn't a big problem with the TEST_ONLY atomic mode, just a bit silly. -Daniel > So on a whole, set of Intel platforms cover all the values of property, but > at driver level we make sure to allow only what is suitable for this source > + sink combination. > - Shashank > > > > Cheers, Daniel > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH drm-intel] drm: arcpgu: arc_pgu_crtc_mode_valid() can be static
On Fri, Jun 23, 2017 at 05:54:18PM +0800, kbuild test robot wrote: > > Signed-off-by: Fengguang Wu Oops, missed that, applied. Btw, for regression fixes like that, could you perhaps auto-generate the Fixes: line per the kernel process? Makes it easier for me to know where to apply something :-) Thanks, Daniel > --- > arcpgu_crtc.c |4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c > b/drivers/gpu/drm/arc/arcpgu_crtc.c > index 99fbdae..611af74 100644 > --- a/drivers/gpu/drm/arc/arcpgu_crtc.c > +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c > @@ -64,8 +64,8 @@ static const struct drm_crtc_funcs arc_pgu_crtc_funcs = { > .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, > }; > > -enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc, > - const struct drm_display_mode > *mode) > +static enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc, > + const struct > drm_display_mode *mode) > { > struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); > long rate, clk_rate = mode->clock * 1000; -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [kbuild-all] [RFC PATCH drm-intel] drm: arcpgu: arc_pgu_crtc_mode_valid() can be static
Hi Daniel, On Fri, Jun 23, 2017 at 12:30:17PM +0200, Daniel Vetter wrote: On Fri, Jun 23, 2017 at 05:54:18PM +0800, kbuild test robot wrote: Signed-off-by: Fengguang Wu Oops, missed that, applied. Btw, for regression fixes like that, could you perhaps auto-generate the Fixes: line per the kernel process? Makes it easier for me to know where to apply something :-) That's a good idea for commits in mainline, where the Fixes tag will look like Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()") However typically 0day auto-generated patches are pre-mainline ones, in which case only patch subject is available. How are we going to deal with this? Thanks, Fengguang --- arcpgu_crtc.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c index 99fbdae..611af74 100644 --- a/drivers/gpu/drm/arc/arcpgu_crtc.c +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c @@ -64,8 +64,8 @@ static const struct drm_crtc_funcs arc_pgu_crtc_funcs = { .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, }; -enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc, -const struct drm_display_mode *mode) +static enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc, + const struct drm_display_mode *mode) { struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); long rate, clk_rate = mode->clock * 1000; -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ kbuild-all mailing list kbuild-...@lists.01.org https://lists.01.org/mailman/listinfo/kbuild-all ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/vgem: Pin our pages for dmabuf exports
On Thu, Jun 22, 2017 at 02:46:17PM +0100, Chris Wilson wrote: > When the caller maps their dmabuf and we return an sg_table, the caller > doesn't expect the pages beneath that sg_table to vanish on a whim (i.e. > under mempressure). The contract is that the pages are pinned for the > duration of the mapping (from dma_buf_map_attachment() to > dma_buf_unmap_attachment). To comply, we need to introduce our own > vgem_object.pages_pin_count and elevate it across the mapping. However, > the drm_prime interface we use calls drv->prime_pin on dma_buf_attach > and drv->prime_unpin on dma_buf_detach, which while that does cover the > mapping is much broader than is desired -- but it will do for now. We could/should probably fix that ... Most drivers hold onto the mapping forever anyway. > v2: also hold the pin across prime_vmap/vunmap > > Reported-by: Tomi Sarvela > Testcase: igt/gem_concurrent_blit/*swap*vgem* > Fixes: 5ba6c9ff961a ("drm/vgem: Fix mmaping") > Signed-off-by: Chris Wilson > Cc: Tomi Sarvela > Cc: Laura Abbott > Cc: Sean Paul > Cc: Matthew Auld > Cc: Daniel Vetter > Cc: > --- > drivers/gpu/drm/vgem/vgem_drv.c | 81 > ++--- > drivers/gpu/drm/vgem/vgem_drv.h | 4 ++ > 2 files changed, 64 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c > index 18f401b442c2..c938af8c40cf 100644 > --- a/drivers/gpu/drm/vgem/vgem_drv.c > +++ b/drivers/gpu/drm/vgem/vgem_drv.c > @@ -52,6 +52,7 @@ static void vgem_gem_free_object(struct drm_gem_object *obj) > struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj); > > kvfree(vgem_obj->pages); > + mutex_destroy(&vgem_obj->pages_lock); > > if (obj->import_attach) > drm_prime_gem_destroy(obj, vgem_obj->table); > @@ -76,11 +77,15 @@ static int vgem_gem_fault(struct vm_fault *vmf) > if (page_offset > num_pages) > return VM_FAULT_SIGBUS; > > + ret = -ENOENT; > + mutex_lock(&obj->pages_lock); > if (obj->pages) { > get_page(obj->pages[page_offset]); > vmf->page = obj->pages[page_offset]; > ret = 0; > - } else { > + } > + mutex_unlock(&obj->pages_lock); > + if (ret) { > struct page *page; > > page = shmem_read_mapping_page( > @@ -161,6 +166,8 @@ static struct drm_vgem_gem_object > *__vgem_gem_create(struct drm_device *dev, > return ERR_PTR(ret); > } > > + mutex_init(&obj->pages_lock); > + > return obj; > } > > @@ -274,37 +281,66 @@ static const struct file_operations vgem_driver_fops = { > .release= drm_release, > }; > > +static struct page **vgem_pin_pages(struct drm_vgem_gem_object *bo) > +{ > + mutex_lock(&bo->pages_lock); > + if (bo->pages_pin_count++ == 0) { > + struct page **pages; > + > + pages = drm_gem_get_pages(&bo->base); > + if (IS_ERR(pages)) { > + bo->pages_pin_count--; > + mutex_unlock(&bo->pages_lock); > + return pages; > + } > + > + bo->pages = pages; > + } > + mutex_unlock(&bo->pages_lock); > + > + return bo->pages; > +} > + > +static void vgem_unpin_pages(struct drm_vgem_gem_object *bo) > +{ > + mutex_lock(&bo->pages_lock); > + if (--bo->pages_pin_count == 0) { > + drm_gem_put_pages(&bo->base, bo->pages, true, true); > + bo->pages = NULL; > + } > + mutex_unlock(&bo->pages_lock); > +} > + > static int vgem_prime_pin(struct drm_gem_object *obj) > { > + struct drm_vgem_gem_object *bo = to_vgem_bo(obj); > long n_pages = obj->size >> PAGE_SHIFT; > struct page **pages; > > - /* Flush the object from the CPU cache so that importers can rely > - * on coherent indirect access via the exported dma-address. > - */ > - pages = drm_gem_get_pages(obj); > + pages = vgem_pin_pages(bo); > if (IS_ERR(pages)) > return PTR_ERR(pages); > > + /* Flush the object from the CPU cache so that importers can rely > + * on coherent indirect access via the exported dma-address. > + */ > drm_clflush_pages(pages, n_pages); Just spotted this, but at least on x86 dma is supposed to be coherent. We're the exception. But then this is all ill-defined with dma_buf, so meh. > - drm_gem_put_pages(obj, pages, true, false); > > return 0; > } > > -static struct sg_table *vgem_prime_get_sg_table(struct drm_gem_object *obj) > +static void vgem_prime_unpin(struct drm_gem_object *obj) > { > - struct sg_table *st; > - struct page **pages; > + struct drm_vgem_gem_object *bo = to_vgem_bo(obj); > > - pages = drm_gem_get_pages(obj); > - if (IS_ERR(pages)) > - return ERR_CAST(pages); > + vgem_unpin_pages(bo); > +} > > - st = drm_prime_pages_to_sg(pages, obj->size >> PAG
Re: [PATCH v2] drm/vgem: Pin our pages for dmabuf exports
On Fri, Jun 23, 2017 at 01:02:53PM +0200, Daniel Vetter wrote: > Anyway looks all good, will push to drm-misc-fixes. Correction, pushed to -misc-next because it conflicts with the dma-buf import stuff from Laura and other bits. If you want it in -fixes I need a backport. I left the cc: stable in just to annoy Greg a bit :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [kbuild-all] [RFC PATCH drm-intel] drm: arcpgu: arc_pgu_crtc_mode_valid() can be static
On Fri, Jun 23, 2017 at 06:51:05PM +0800, Fengguang Wu wrote: > Hi Daniel, > > On Fri, Jun 23, 2017 at 12:30:17PM +0200, Daniel Vetter wrote: > > On Fri, Jun 23, 2017 at 05:54:18PM +0800, kbuild test robot wrote: > > > > > > Signed-off-by: Fengguang Wu > > > > Oops, missed that, applied. > > > > Btw, for regression fixes like that, could you perhaps auto-generate the > > Fixes: line per the kernel process? Makes it easier for me to know where > > to apply something :-) > > That's a good idea for commits in mainline, where the Fixes tag will > look like > >Fixes: e21d2170f366 ("video: remove unnecessary > platform_set_drvdata()") > > However typically 0day auto-generated patches are pre-mainline ones, > in which case only patch subject is available. How are we going to > deal with this? Well many trees you're testing are stable and will never rebase (at least all the drm-intel|misc.git ones are supposed to be stable), hence the sha1 is stable. I guess other maintainers could just remove the line if they prefer to squash it in? Also makes it easier to know where to squash it into :-) -Daniel > > Thanks, > Fengguang > > > > --- > > > arcpgu_crtc.c |4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c > > > b/drivers/gpu/drm/arc/arcpgu_crtc.c > > > index 99fbdae..611af74 100644 > > > --- a/drivers/gpu/drm/arc/arcpgu_crtc.c > > > +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c > > > @@ -64,8 +64,8 @@ static const struct drm_crtc_funcs arc_pgu_crtc_funcs = > > > { > > > .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, > > > }; > > > > > > -enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc, > > > - const struct drm_display_mode > > > *mode) > > > +static enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc > > > *crtc, > > > + const struct > > > drm_display_mode *mode) > > > { > > > struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); > > > long rate, clk_rate = mode->clock * 1000; > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > ___ > > kbuild-all mailing list > > kbuild-...@lists.01.org > > https://lists.01.org/mailman/listinfo/kbuild-all -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/vgem: Pin our pages for dmabuf exports
Quoting Daniel Vetter (2017-06-23 12:02:53) > On Thu, Jun 22, 2017 at 02:46:17PM +0100, Chris Wilson wrote: > > + /* Flush the object from the CPU cache so that importers can rely > > + * on coherent indirect access via the exported dma-address. > > + */ > > drm_clflush_pages(pages, n_pages); > > Just spotted this, but at least on x86 dma is supposed to be coherent. > We're the exception. But then this is all ill-defined with dma_buf, so > meh. It's been a running debate on whether these are meant to be WC mapped or not. dmabuf does have its sync ioctls that do allow us to mix WB for clients and UC for device, which for us is very important. But there are a few edge cases like device importing pages from a dmabuf as above. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 101561] [bisected] [llvm] Blue color shift on videos in MPV when using vo=opengl[-hq]
https://bugs.freedesktop.org/show_bug.cgi?id=101561 --- Comment #4 from Andy Furniss --- Yea, this commit also breaks (asserts) Unigine Valley on R9 285. I'll open a bug on llvm tracker and link back to this one. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 101561] [bisected] [llvm] Blue color shift on videos in MPV when using vo=opengl[-hq]
https://bugs.freedesktop.org/show_bug.cgi?id=101561 --- Comment #5 from Andy Furniss --- (In reply to Andy Furniss from comment #4) > Yea, this commit also breaks (asserts) Unigine Valley on R9 285. > > I'll open a bug on llvm tracker and link back to this one. https://bugs.llvm.org/show_bug.cgi?id=33566 Though I couldn't add the author to CC :-( -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [kbuild-all] [RFC PATCH drm-intel] drm: arcpgu: arc_pgu_crtc_mode_valid() can be static
On Fri, Jun 23, 2017 at 01:08:17PM +0200, Daniel Vetter wrote: On Fri, Jun 23, 2017 at 06:51:05PM +0800, Fengguang Wu wrote: Hi Daniel, On Fri, Jun 23, 2017 at 12:30:17PM +0200, Daniel Vetter wrote: > On Fri, Jun 23, 2017 at 05:54:18PM +0800, kbuild test robot wrote: > > > > Signed-off-by: Fengguang Wu > > Oops, missed that, applied. > > Btw, for regression fixes like that, could you perhaps auto-generate the > Fixes: line per the kernel process? Makes it easier for me to know where > to apply something :-) That's a good idea for commits in mainline, where the Fixes tag will look like Fixes: e21d2170f366 ("video: remove unnecessary platform_set_drvdata()") However typically 0day auto-generated patches are pre-mainline ones, in which case only patch subject is available. How are we going to deal with this? Well many trees you're testing are stable and will never rebase (at least all the drm-intel|misc.git ones are supposed to be stable), hence the sha1 is stable. I guess other maintainers could just remove the line if they prefer to squash it in? Also makes it easier to know where to squash it into :-) Fair enough. I'll add the Fixes line. :) Thanks, Fengguang > > --- > > arcpgu_crtc.c |4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c > > index 99fbdae..611af74 100644 > > --- a/drivers/gpu/drm/arc/arcpgu_crtc.c > > +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c > > @@ -64,8 +64,8 @@ static const struct drm_crtc_funcs arc_pgu_crtc_funcs = { > > .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, > > }; > > > > -enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc, > > - const struct drm_display_mode *mode) > > +static enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc, > > + const struct drm_display_mode *mode) > > { > > struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc); > > long rate, clk_rate = mode->clock * 1000; > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > ___ > kbuild-all mailing list > kbuild-...@lists.01.org > https://lists.01.org/mailman/listinfo/kbuild-all -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ kbuild-all mailing list kbuild-...@lists.01.org https://lists.01.org/mailman/listinfo/kbuild-all ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/12] fbdev helper locking rework and deferred setup
On Fri, Jun 23, 2017 at 09:38:49AM +0200, Daniel Vetter wrote: > On Thu, Jun 22, 2017 at 4:54 PM, Liviu Dudau wrote: > > On Wed, Jun 21, 2017 at 08:28:03PM +0200, Daniel Vetter wrote: > >> Hi all, > >> > >> This is Thierry's deferred fbdev setup series, with the locking rework > >> almost > >> entirely redone. The much wider scope is to get rid of drm_modeset_lock_all > >> calls for atomic drivers and remove users of the fairly nasty > >> mode_config->acquire_ctx hack, which breaks when doing multiple atomic > >> commits. > >> > >> Testing&review very much appreciated, especially from people who care > >> about the > >> various fbdev emulation things and the deferred setup stuff. > > > > Tested on my Juno dev board on a bit of a convoluted setup: the dev board > > has > > built into the SoC 2x HDLCD instances and I also have an FPGA daughter board > > with Mali DP 650 running again on a 2x configuration. On boot I'm getting > > this > > warning: > > > > juno-r0-ld login: [ 241.986785] INFO: task kworker/3:1:80 blocked for more > > than 120 seconds. > > [ 241.993652] Not tainted 4.12.0-rc5-01275-g1e2237a19156 #2 > > [ 241.999689] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables > > this message. > > [ 242.007660] kworker/3:1 D080 2 0x > > [ 242.013340] Workqueue: events output_poll_execute [drm_kms_helper] > > [ 242.019637] Call trace: > > [ 242.022132] [] __switch_to+0x98/0xb0 > > [ 242.027373] [] __schedule+0x19c/0x5e8 > > [ 242.032699] [] schedule+0x38/0xa0 > > [ 242.037672] [] schedule_preempt_disabled+0x20/0x38 > > [ 242.044144] [] __mutex_lock.isra.0+0x140/0x530 > > [ 242.050262] [] __mutex_lock_slowpath+0x10/0x18 > > [ 242.056379] [] mutex_lock+0x30/0x38 > > [ 242.061597] [] > > drm_fb_helper_hotplug_event.part.22+0x20/0x100 [drm_kms_helper] > > [ 242.070611] [] drm_fb_helper_hotplug_event+0x24/0x38 > > [drm_kms_helper] > > [ 242.078828] [] drm_fbdev_cma_hotplug_event+0x10/0x20 > > [drm_kms_helper] > > [ 242.086984] [] hdlcd_fb_output_poll_changed+0x14/0x20 > > [hdlcd] > > [ 242.094495] [] drm_kms_helper_hotplug_event+0x28/0x38 > > [drm_kms_helper] > > [ 242.102801] [] output_poll_execute+0x1a0/0x1f0 > > [drm_kms_helper] > > [ 242.110424] [] process_one_work+0x1d4/0x330 > > [ 242.116280] [] worker_thread+0x48/0x468 > > [ 242.121781] [] kthread+0x12c/0x130 > > [ 242.126839] [] ret_from_fork+0x10/0x50 > > [ 242.132252] INFO: task kworker/5:1:82 blocked for more than 120 seconds. > > [ 242.139074] Not tainted 4.12.0-rc5-01275-g1e2237a19156 #2 > > [ 242.145099] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables > > this message. > > [ 242.153064] kworker/5:1 D082 2 0x > > [ 242.158724] Workqueue: events output_poll_execute [drm_kms_helper] > > [ 242.165019] Call trace: > > [ 242.167519] [] __switch_to+0x98/0xb0 > > [ 242.172755] [] __schedule+0x19c/0x5e8 > > [ 242.178079] [] schedule+0x38/0xa0 > > [ 242.183051] [] schedule_preempt_disabled+0x20/0x38 > > [ 242.189520] [] __mutex_lock.isra.0+0x140/0x530 > > [ 242.195639] [] __mutex_lock_slowpath+0x10/0x18 > > [ 242.201756] [] mutex_lock+0x30/0x38 > > [ 242.206972] [] > > drm_fb_helper_hotplug_event.part.22+0x20/0x100 [drm_kms_helper] > > [ 242.215984] [] drm_fb_helper_hotplug_event+0x24/0x38 > > [drm_kms_helper] > > [ 242.224201] [] drm_fbdev_cma_hotplug_event+0x10/0x20 > > [drm_kms_helper] > > [ 242.232367] [] malidp_output_poll_changed+0x14/0x20 > > [mali_dp] > > [ 242.239880] [] drm_kms_helper_hotplug_event+0x28/0x38 > > [drm_kms_helper] > > [ 242.248188] [] output_poll_execute+0x1a0/0x1f0 > > [drm_kms_helper] > > [ 242.255808] [] process_one_work+0x1d4/0x330 > > [ 242.261664] [] worker_thread+0x48/0x468 > > [ 242.267196] [] kthread+0x12c/0x130 > > [ 242.272312] [] ret_from_fork+0x10/0x50 > > > > Each hardware type has only one instance of each driver being connected to > > an > > output, HDLCD is connected to an HDMI monitor that is not "active" (i.e. > > input is switched to DP connection), while the Mali DP instance is > > connected to > > a monitor. > > > > Suggestions on where to look next are welcome. > > I'm betting I've fumbled an unlock path somewhere and now your stuck > trying to get a lock you can't get. Can you pls recompile with lockdep > enabled and repro? That will directly tell us where and what went > wrong. This is what I've got with lockdep debugging enabled. Not sure it is the same thing as the previous WARN, the stack trace doesn't match the one above (but I still get that one after 120s). [ 16.536405] [drm] found ARM HDLCD version r0p0 [ 16.670859] tda998x 0-0071: found TDA19988
[PATCH] drm/fb-helper: Support deferred setup
From: Thierry Reding FB helper code falls back to a 1024x768 mode if no outputs are connected or don't report back any modes upon initialization. This can be annoying because outputs that are added to FB helper later on can't be used with FB helper if they don't support a matching mode. The fallback is in place because VGA connectors can happen to report an unknown connection status even when they are in fact connected. Some drivers have custom solutions in place to defer FB helper setup until at least one output is connected. But the logic behind these solutions is always the same and there is nothing driver-specific about it, so a better alterative is to fix the FB helper core and add support for all drivers automatically. This patch adds support for deferred FB helper setup. It checks all the connectors for their connection status, and if all of them report to be disconnected marks the FB helper as needing deferred setup. Whet setup is deferred, the FB helper core will automatically retry setup after a hotplug event, and it will keep trying until it succeeds. v2: Rebase onto my entirely reworked fbdev helper locking. One big difference is that this version again drops&reacquires the fbdev lock (which is now fb_helper->lock, but before this patch series it was mode_config->mutex), because register_framebuffer must be able to recurse back into fbdev helper code for the initial screen setup. v3: __drm_fb_helper_initial_config must hold fb_helper->lock upon return, I've fumbled that in the deferred setup case (Liviu). Cc: Liviu Dudau Cc: John Stultz Cc: Thierry Reding (v1) Tested-by: John Stultz Signed-off-by: Thierry Reding (v1) Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_fb_helper.c | 110 ++-- include/drm/drm_fb_helper.h | 23 + 2 files changed, 106 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index e3d033df068f..834c7e7d7efc 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -521,6 +521,9 @@ int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper) if (!drm_fbdev_emulation) return -ENODEV; + if (READ_ONCE(fb_helper->deferred_setup)) + return 0; + mutex_lock(&fb_helper->lock); ret = restore_fbdev_mode(fb_helper); @@ -1693,6 +1696,23 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var, } EXPORT_SYMBOL(drm_fb_helper_pan_display); +static bool drm_fb_helper_maybe_connected(struct drm_fb_helper *helper) +{ + bool connected = false; + unsigned int i; + + for (i = 0; i < helper->connector_count; i++) { + struct drm_fb_helper_connector *fb = helper->connector_info[i]; + + if (fb->connector->status != connector_status_disconnected) { + connected = true; + break; + } + } + + return connected; +} + /* * Allocates the backing storage and sets up the fbdev info structure through * the ->fb_probe callback and then registers the fbdev and sets up the panic @@ -2352,8 +2372,6 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper, int i; DRM_DEBUG_KMS("\n"); - if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0) - DRM_DEBUG_KMS("No connectors reported connected with modes\n"); /* prevent concurrent modification of connector_count by hotplug */ lockdep_assert_held(&fb_helper->lock); @@ -2431,6 +2449,60 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper, kfree(enabled); } +/* Note: Drops&reacquires fb_helper->lock */ +static int __drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, + int bpp_sel) +{ + struct drm_device *dev = fb_helper->dev; + struct fb_info *info; + unsigned int width, height; + int ret; + + width = dev->mode_config.max_width; + height = dev->mode_config.max_height; + + if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0) + DRM_DEBUG_KMS("No connectors reported connected with modes\n"); + + /* +* If everything's disconnected, there's no use in attempting to set +* up fbdev. +*/ + if (!drm_fb_helper_maybe_connected(fb_helper)) { + DRM_INFO("No outputs connected, deferring setup\n"); + fb_helper->preferred_bpp = bpp_sel; + fb_helper->deferred_setup = true; + return 0; + } + + drm_setup_crtcs(fb_helper, width, height); + ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel); + if (ret) { + mutex_unlock(&fb_helper->lock); + return ret; + } + + fb_helper->deferred_setup = false; + mutex_unlock(&fb_helper->lock); + + info =
[Bug 98238] Witcher 2: objects are black when changing lod on Radeon Pitcairn
https://bugs.freedesktop.org/show_bug.cgi?id=98238 --- Comment #20 from almos --- I confirm that Marek's patch fixes Witcher 2. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/7] drm/bridge: Support hotplugging panel-bridge.
On 6/22/2017 7:04 PM, Boris Brezillon wrote: On Thu, 22 Jun 2017 15:16:47 +0200 Andrzej Hajda wrote: On 22.06.2017 14:41, Boris Brezillon wrote: On Thu, 22 Jun 2017 14:29:07 +0200 Andrzej Hajda wrote: On 22.06.2017 11:23, Boris Brezillon wrote: On Thu, 22 Jun 2017 13:47:43 +0530 Archit Taneja wrote: On 06/22/2017 01:20 PM, Benjamin Gaignard wrote: 2017-06-20 19:31 GMT+02:00 Eric Anholt : Archit Taneja writes: On 06/16/2017 08:13 PM, Eric Anholt wrote: Archit Taneja writes: On 06/16/2017 02:11 AM, Eric Anholt wrote: If the panel-bridge is being set up after the drm_mode_config_reset(), then the connector's state would never get initialized, and we'd dereference the NULL in the hotplug path. We also need to register the connector, so that userspace can get at it. Shouldn't the KMS driver make sure the panel-bridge is set up before drm_mode_config_reset? Is it the case when we're inserting the panel-bridge driver as a module? All the connectors that have been added are registered automatically when drm_dev_register() is called by the KMS driver. Registering a connector in the middle of setting up our driver is prone to race conditions if the userspace decides to use them immediately. Yeah, this is fixing initializing panel_bridge at DSI host_attach time, which in the case of a panel module that creates the DSI device (adv7533-style, like you said I should use as a reference) will be after drm_mode_config_reset() and drm_dev_register(). Okay. In the case of the msm kms driver, we defer probe until the adv7533 module is inserted, only then we proceed to drm_mode_config_reset() and drm_dev_register(). I assumed this was the general practice followed by most kms drivers. I.,e the kms driver defers probe until all connector related modules are inserted, and only then proceed to create a drm device. The problem, though, is the panel driver needs the MIPI DSI host to exist to call mipi_dsi_device_register_full() during the probe process. The adv7533 driver gets around this by registering the DSI device in the bridge attach step, but drm_panel doesn't have an attach step. I'm not sure how we can get around this. We had discussion about this on irc recently, but couldn't come up with a good conclusion. We could come up with a panel_attach() callback to make it similar to bridges, but that's just us avoiding the real issue. How about making DSI dev registration fully asynchronous, that is, DSI devs declared in the DT under the DSI host node will be registered/attached at probe time, and devs using another control bus (like the adv7533 controller over i2c) will be registered afterwards. That implies moving the drm_brige registration logic outside of the DSI host ->probe() path. The idea would be to check if all devs connected to the DSI bus are ready at dsi_host->attach() time. If they are, we can finally register the XXX -> DSI bridge. If they're not (because some devs connected to the DSI bus have not been probed yet), then we do not register the drm_bridge and wait for the next dsi_host->attach() event. I guess you assumes that dsi-host knows all devs connected to it, thanks to: - subnodes of the host - ie. devices controlled via dsi bus, - graph links from host ports/endpoints - ie. devices controlled by other buses, for example adv7533. Yep, but I think that's already a requirement when populating devices with the OF graph method (if one of the DSI output endpoint does not have a drm_bridge/panel attached to it, the DSI host driver returns -EPROBE_DEFER). I would separate both abstractions to make it more clear: 1. MIPI bus should be registered early - to allow create/bind devices on it, Exactly. 2. drm_bridge should be registered only if all required sinks (bridges/panels) are registered. That's true, until we find a solution to support add DRM bridge hotplug. First point seems OK, I am not sure about the 2nd one - if used consistently, it would require building pipeline from sink to source. Yes. Since drm_bridge_attach requires encoder to be not null pipeline creation would be painful: 1. Every driver must call drm_of_find_panel_or_bridge on sink(s) before registering bridge and cache the result for later use. Shouldn't be hard to do since dsi_host->attach() is called each time a sink is added (and ready to use). All you need to do is retrieve the bridge pointer and put it in a list embedded in the DSI host priv struct. Once you have all sinks added to this list (can be checked by counting the number of endpoints and DSI devs at probe time), you can register the DSI bridge and wait for someone to call ->attach() on it. Some of the kms drivers (including vc4) currently do the bridge registration, and call drm_bridge_attach() in the DSI host driver's probe path itself. I guess some of them would require some restructuring to work with the above approach. Other kms drivers (msm for example) already have a separate modeset_init path
[Bug 101565] Intermittent graphical corruption since "radeonsi: don't emit partial flushes at the end of IBs (v2)" (c9040dc9)
https://bugs.freedesktop.org/show_bug.cgi?id=101565 Bug ID: 101565 Summary: Intermittent graphical corruption since "radeonsi: don't emit partial flushes at the end of IBs (v2)" (c9040dc9) Product: Mesa Version: git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel@lists.freedesktop.org Reporter: g...@chown.ath.cx QA Contact: dri-devel@lists.freedesktop.org Created attachment 132156 --> https://bugs.freedesktop.org/attachment.cgi?id=132156&action=edit Example of graphical corruption Commit c9040dc9 seems to cause graphical corruption on VI for me. It looks like some caching issue, since the corruption is intermittent and mostly appears only for a single frame. Maybe some assumptions made about the kernel's handling of IBs aren't true? I can trigger issues reliably by running some OpenCL load in the background (for instance, an Ethereum miner). That will make X (glamor) and gnome-shell rendering glitch randomly with high probability. Reverting the commit fixes the problem completely. Attached is a screenshot, showing what kind of artefacts I encounter. (Using the screenshot tool doesn't work as it'll rerender everything before capture) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/13] drm/rockchip: Drop drm_vblank_cleanup
On Wed, Jun 21, 2017 at 10:28:45AM +0200, Daniel Vetter wrote: > Either not relevant (in the load error paths) or done better already > (in the unload code, by calling drm_atomic_helper_shutdown). Drop it. > > Cc: Mark Yao > Signed-off-by: Daniel Vetter Reviewed-by: Sean Paul > --- > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > index c6b1b7f3a2a3..b9fbf4b1e8f0 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > @@ -177,7 +177,6 @@ static int rockchip_drm_bind(struct device *dev) > rockchip_drm_fbdev_fini(drm_dev); > err_kms_helper_poll_fini: > drm_kms_helper_poll_fini(drm_dev); > - drm_vblank_cleanup(drm_dev); > err_unbind_all: > component_unbind_all(dev, drm_dev); > err_mode_config_cleanup: > @@ -200,7 +199,6 @@ static void rockchip_drm_unbind(struct device *dev) > drm_kms_helper_poll_fini(drm_dev); > > drm_atomic_helper_shutdown(drm_dev); > - drm_vblank_cleanup(drm_dev); > component_unbind_all(dev, drm_dev); > drm_mode_config_cleanup(drm_dev); > rockchip_iommu_cleanup(drm_dev); > -- > 2.11.0 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/13] drm/vmwgfx: Drop drm_vblank_cleanup
On Wed, Jun 21, 2017 at 10:28:48AM +0200, Daniel Vetter wrote: > Again stopping the vblank before uninstalling the irq handler is kinda > the wrong way round, but the fb_off stuff should take care of > disabling the dsiplay at least in most cases. So drop the > drm_vblank_cleanup code since it's not really doing anything, it looks > all cargo-culted. > > v2: Appease gcc better. > > Cc: Sinclair Yeh > Cc: Thomas Hellstrom > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 9 +++-- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 -- > drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 4 > drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 9 - > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 27 +-- > 5 files changed, 4 insertions(+), 47 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > index 3d94ea67a825..a93c0bb73452 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > @@ -1658,7 +1658,7 @@ int vmw_kms_init(struct vmw_private *dev_priv) > > int vmw_kms_close(struct vmw_private *dev_priv) > { > - int ret; > + int ret = 0; > > /* >* Docs says we should take the lock before calling this function > @@ -1666,11 +1666,8 @@ int vmw_kms_close(struct vmw_private *dev_priv) >* drm_encoder_cleanup which takes the lock we deadlock. >*/ > drm_mode_config_cleanup(dev_priv->dev); > - if (dev_priv->active_display_unit == vmw_du_screen_object) > - ret = vmw_kms_sou_close_display(dev_priv); > - else if (dev_priv->active_display_unit == vmw_du_screen_target) > - ret = vmw_kms_stdu_close_display(dev_priv); > - else > + if (dev_priv->active_display_unit != vmw_du_screen_object && > + dev_priv->active_display_unit != vmw_du_screen_target) I think this is equivalent to: if (dev_priv->active_display_unit == vmw_du_legacy) > ret = vmw_kms_ldu_close_display(dev_priv); > > return ret; > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h > index 5f8d678ae675..ff9c8389ff21 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h > @@ -390,7 +390,6 @@ int vmw_kms_update_proxy(struct vmw_resource *res, > * Screen Objects display functions - vmwgfx_scrn.c > */ > int vmw_kms_sou_init_display(struct vmw_private *dev_priv); > -int vmw_kms_sou_close_display(struct vmw_private *dev_priv); > int vmw_kms_sou_do_surface_dirty(struct vmw_private *dev_priv, >struct vmw_framebuffer *framebuffer, >struct drm_clip_rect *clips, > @@ -418,7 +417,6 @@ int vmw_kms_sou_readback(struct vmw_private *dev_priv, > * Screen Target Display Unit functions - vmwgfx_stdu.c > */ > int vmw_kms_stdu_init_display(struct vmw_private *dev_priv); > -int vmw_kms_stdu_close_display(struct vmw_private *dev_priv); > int vmw_kms_stdu_surface_dirty(struct vmw_private *dev_priv, > struct vmw_framebuffer *framebuffer, > struct drm_clip_rect *clips, > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c > index d3987bcf53f8..449ed4fba0f2 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c > @@ -582,13 +582,9 @@ int vmw_kms_ldu_init_display(struct vmw_private > *dev_priv) > > int vmw_kms_ldu_close_display(struct vmw_private *dev_priv) > { > - struct drm_device *dev = dev_priv->dev; > - > if (!dev_priv->ldu_priv) > return -ENOSYS; > > - drm_vblank_cleanup(dev); > - > BUG_ON(!list_empty(&dev_priv->ldu_priv->active)); > > kfree(dev_priv->ldu_priv); > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > index 8d7dc9def7c2..3b917c9b0c21 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > @@ -746,15 +746,6 @@ int vmw_kms_sou_init_display(struct vmw_private > *dev_priv) > return 0; > } > > -int vmw_kms_sou_close_display(struct vmw_private *dev_priv) > -{ > - struct drm_device *dev = dev_priv->dev; > - > - drm_vblank_cleanup(dev); > - > - return 0; > -} > - > static int do_dmabuf_define_gmrfb(struct vmw_private *dev_priv, > struct vmw_framebuffer *framebuffer) > { > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > index 50be1f034f9e..6aecba6cd5e2 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > @@ -1651,36 +1651,11 @@ int vmw_kms_stdu_init_display(struct vmw_private > *dev_priv) > > if (unlikely(ret != 0)) { > DRM_ERROR("Failed to initialize STDU %d", i); > - goto err_vblank_cleanup; > +
Re: [PATCH v2 07/11] drm: sun4i: add support for the TV encoder in H3 SoC
On Tue, Jun 13, 2017 at 05:51:42PM +0800, Icenowy Zheng wrote: > > > 于 2017年6月13日 GMT+08:00 下午3:44:32, Maxime Ripard > 写到: > >On Sun, Jun 11, 2017 at 02:43:42PM +0800, icen...@aosc.io wrote: > >> 在 2017-06-07 17:38,Maxime Ripard 写道: > >> > On Mon, Jun 05, 2017 at 12:01:45AM +0800, Icenowy Zheng wrote: > >> > > Allwinner H3 features a TV encoder similar to the one in earlier > >SoCs, > >> > > but has a internal fixed clock divider that divides the TCON1 > >clock > >> > > (called TVE clock in datasheet) by 11. > >> > > > >> > > Add support for it. > >> > > > >> > > Signed-off-by: Icenowy Zheng > >> > > --- > >> > > Changes in v2: > >> > > - Quirk part rewritten. > >> > > > >> > > drivers/gpu/drm/sun4i/sun4i_tv.c | 35 > >> > > ++- > >> > > 1 file changed, 34 insertions(+), 1 deletion(-) > >> > > > >> > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tv.c > >> > > b/drivers/gpu/drm/sun4i/sun4i_tv.c > >> > > index 338b9e5bb2a3..b9ff6d5ea67a 100644 > >> > > --- a/drivers/gpu/drm/sun4i/sun4i_tv.c > >> > > +++ b/drivers/gpu/drm/sun4i/sun4i_tv.c > >> > > @@ -13,6 +13,7 @@ > >> > > #include > >> > > #include > >> > > #include > >> > > +#include > >> > > #include > >> > > #include > >> > > > >> > > @@ -169,14 +170,21 @@ struct tv_mode { > >> > >const struct resync_parameters *resync_params; > >> > > }; > >> > > > >> > > +struct sun4i_tv_quirks { > >> > > + int fixed_divider; > >> > > +}; > >> > > + > >> > > struct sun4i_tv { > >> > >struct drm_connectorconnector; > >> > >struct drm_encoder encoder; > >> > > > >> > >struct clk *clk; > >> > > + struct clk *mod_clk; > >> > >struct regmap *regs; > >> > >struct reset_control*reset; > >> > > > >> > > + const struct sun4i_tv_quirks *quirks; > >> > > + > >> > >struct sun4i_drv*drv; > >> > > }; > >> > > > >> > > @@ -391,6 +399,12 @@ static void sun4i_tv_mode_set(struct > >> > > drm_encoder *encoder, > >> > >struct sun4i_tcon *tcon = crtc->tcon; > >> > >const struct tv_mode *tv_mode = sun4i_tv_find_tv_by_mode(mode); > >> > > > >> > > + if (tv->quirks->fixed_divider) { > >> > > + DRM_DEBUG_DRIVER("Applying fixed divider %d on TVE > >> > > clock\n", > >> > > + tv->quirks->fixed_divider); > >> > > + mode->crtc_clock *= tv->quirks->fixed_divider; > >> > > + } > >> > > + > >> > > >> > You're not allowed to change the mode in mode_set, and you > >shouldn't > >> > even change it. The output pixel clock is still 27MHz. > >> > > >> > You should implement that using the states, as we discussed > >already. > >> > >> After reading the comments at > >include/drm/drm_modeset_helper_vtables.h, > >> I think the atomic_check function is allowed to directly change > >> the adjust_mode of crtc_state. > >> > >> And according to other comments at include/drm/drm_modes.h, the > >> crtc_clock in adjust_mode should be the clock to be really feed > >> to the hardware. > >> > >> So I think I can just change this in atomic_check. > >> > >> However, the mode_set function of sun4i_tv seems to be not > >> regarding adjusted_mode and still use original mode. > >> > >> So my current design is: > >> - Multiply adjusted_mode in atomic_check. > >> - Feed adjust_mode to TCON code in mode_set function. > >> > >> Is this proper? > >> > >> (From my own understanding to the comments I think so.) > > > >No, it's not. The pixel clock in the mode is the pixel clock output > >physically by the connector. You want to use it for an intermediate > >clock in your pipeline. > > > >This is not the same clock, and not the same frequency. > > So should a multiplier parameter be passed via > tcon1_mode_set function? No, and I've explained a couple of times already what you needed to do. Please read again the previous messages. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/stm: Fixup for "drm/stm: ltdc: Add panel-bridge support"
On 06/22/2017 07:56 PM, Eric Anholt wrote: > Signed-off-by: Eric Anholt > --- > > This fixup would be squashed into patch 1 of your series. Hi Eric, and many thanks for the two patches, I will follow your suggestion for the v5 serie. By the way, do you have more comments on the "drm/stm: ltdc: Add panel-bridge support" patch? Many thanks :-) Philippe > drivers/gpu/drm/stm/ltdc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c > index 7d7e889f09c3..d1d28348512b 100644 > --- a/drivers/gpu/drm/stm/ltdc.c > +++ b/drivers/gpu/drm/stm/ltdc.c > @@ -953,7 +953,8 @@ int ltdc_load(struct drm_device *ddev) > bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DPI); > if (IS_ERR(bridge)) { > DRM_ERROR("Failed to create panel-bridge\n"); > - return PTR_ERR(bridge); > + ret = PTR_ERR(bridge); > + goto err; > } > ldev->is_panel_bridge = true; > } > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/13] RESEND: remove drm_vblank_cleanup from drivers
On Wed, Jun 21, 2017 at 10:28:37AM +0200, Daniel Vetter wrote: > Hi all, > > This is the resend of the driver patches which haven't seen and ack/r-b yet > and > so aren't merged yet. I'd really like to get them all merged. Review/acks very > much appreciated. > > Thanks, Daniel > > Daniel Vetter (13): > drm/amd|radeon: Drop drm_vblank_cleanup > drm/hibmc: Drop drm_vblank_cleanup > drm/kirin: Drop drm_vblank_cleanup > drm/i915: Drop drm_vblank_cleanup > drm/mtk: Drop drm_vblank_cleanup > drm/mxsfb: Drop drm_vblank_cleanup > drm/nouveau: Drop drm_vblank_cleanup > drm/rockchip: Drop drm_vblank_cleanup > drm/shmob: Drop drm_vblank_cleanup > drm/udl: Drop drm_vblank_cleanup > drm/vmwgfx: Drop drm_vblank_cleanup > drm/zte: Drop drm_vblank_cleanup > drm/vblank: Unexport drm_vblank_cleanup For the whole set (with vmwgfx nit resolved): Reviewed-by: Sean Paul > > drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 1 - > drivers/gpu/drm/drm_internal.h | 1 + > drivers/gpu/drm/drm_vblank.c| 19 ++--- > drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 ++- > drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 1 - > drivers/gpu/drm/i915/i915_drv.c | 6 +- > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 - > drivers/gpu/drm/mxsfb/mxsfb_drv.c | 1 - > drivers/gpu/drm/nouveau/nouveau_display.c | 2 -- > drivers/gpu/drm/radeon/radeon_irq_kms.c | 1 - > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 -- > drivers/gpu/drm/shmobile/shmob_drm_drv.c| 4 +--- > drivers/gpu/drm/udl/udl_main.c | 2 -- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 9 +++-- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.h | 2 -- > drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 4 > drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c| 9 - > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c| 27 > + > drivers/gpu/drm/zte/zx_drm_drv.c| 2 -- > include/drm/drm_vblank.h| 1 - > 20 files changed, 11 insertions(+), 87 deletions(-) > > -- > 2.11.0 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 100785] [regression, bisected] arb_gpu_shader5 piglit fail
https://bugs.freedesktop.org/show_bug.cgi?id=100785 --- Comment #14 from Hi-Angel --- Created attachment 132157 --> https://bugs.freedesktop.org/attachment.cgi?id=132157&action=edit fix FTR, attaching the fix. However I don't know when I get it sent to the ML because I can't finish piglit testing — GPU hangs. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/8] ASoC: dwc: Added a quirk DW_I2S_QUIRK_16BIT_IDX_OVERRIDE to dwc driver
From: Vijendar Mukunda Added quirk DW_I2S_QUIRK_16BIT_IDX_OVERRIDE to Designware driver. This quirk will set idx value to 1. By setting this quirk, it will override supported format as 16 bit resolution and bus width as 2 Bytes. Reviewed-by: Alex Deucher Signed-off-by: Vijendar Mukunda Signed-off-by: Alex Deucher --- include/sound/designware_i2s.h | 1 + sound/soc/dwc/dwc-i2s.c| 6 ++ 2 files changed, 7 insertions(+) diff --git a/include/sound/designware_i2s.h b/include/sound/designware_i2s.h index 5681855..830f5ca 100644 --- a/include/sound/designware_i2s.h +++ b/include/sound/designware_i2s.h @@ -47,6 +47,7 @@ struct i2s_platform_data { #define DW_I2S_QUIRK_COMP_REG_OFFSET(1 << 0) #define DW_I2S_QUIRK_COMP_PARAM1(1 << 1) + #define DW_I2S_QUIRK_16BIT_IDX_OVERRIDE (1 << 2) unsigned int quirks; unsigned int i2s_reg_comp1; unsigned int i2s_reg_comp2; diff --git a/sound/soc/dwc/dwc-i2s.c b/sound/soc/dwc/dwc-i2s.c index 9c46e41..9160676 100644 --- a/sound/soc/dwc/dwc-i2s.c +++ b/sound/soc/dwc/dwc-i2s.c @@ -496,6 +496,8 @@ static int dw_configure_dai(struct dw_i2s_dev *dev, idx = COMP1_TX_WORDSIZE_0(comp1); if (WARN_ON(idx >= ARRAY_SIZE(formats))) return -EINVAL; + if (dev->quirks & DW_I2S_QUIRK_16BIT_IDX_OVERRIDE) + idx = 1; dw_i2s_dai->playback.channels_min = MIN_CHANNEL_NUM; dw_i2s_dai->playback.channels_max = 1 << (COMP1_TX_CHANNELS(comp1) + 1); @@ -508,6 +510,8 @@ static int dw_configure_dai(struct dw_i2s_dev *dev, idx = COMP2_RX_WORDSIZE_0(comp2); if (WARN_ON(idx >= ARRAY_SIZE(formats))) return -EINVAL; + if (dev->quirks & DW_I2S_QUIRK_16BIT_IDX_OVERRIDE) + idx = 1; dw_i2s_dai->capture.channels_min = MIN_CHANNEL_NUM; dw_i2s_dai->capture.channels_max = 1 << (COMP1_RX_CHANNELS(comp1) + 1); @@ -543,6 +547,8 @@ static int dw_configure_dai_by_pd(struct dw_i2s_dev *dev, if (ret < 0) return ret; + if (dev->quirks & DW_I2S_QUIRK_16BIT_IDX_OVERRIDE) + idx = 1; /* Set DMA slaves info */ dev->play_dma_data.pd.data = pdata->play_dma_data; dev->capture_dma_data.pd.data = pdata->capture_dma_data; -- 2.5.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/8] Add ASoC support for AMD Stoney APUs
This patch set updates the AMD GPU and Audio CoProcessor (ACP) audio drivers and the designware i2s driver for Stoney (ST). ST is an APU similar to Carrizo (CZ) which already has ACP audio support. The i2s controller and ACP audio DMA engine are part of the GPU and both need updating so I would like to upstream the whole patch set via one tree if possible. The current code is based on drm-next, but I'm happy to rebase on whatever tree this ends up going through if there are any problems applying. The entire patch set can be viewed here: https://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-next-4.13-acp Thanks! Alex Akshu Agrawal (1): ASoC: AMD: Add machine driver for cz rt5650 Vijendar Mukunda (7): drm/amd/amdgpu: Added asic_type as ACP DMA driver platform data ASoC: dwc: Added a quirk DW_I2S_QUIRK_16BIT_IDX_OVERRIDE to dwc driver drm/amd/amdgpu: Added a dwc quirk for Stoney platform ASoC: AMD: added condition checks for CZ specific code ASoC: AMD: DMA driver changes for Stoney Platform ASoC: AMD: Buffer related changes for Stoney drm/amd/amdgpu: Disable ACP Power Gating for Stoney platform drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 54 +--- include/sound/designware_i2s.h | 1 + sound/soc/amd/Kconfig | 7 ++ sound/soc/amd/Makefile | 2 + sound/soc/amd/acp-pcm-dma.c | 215 sound/soc/amd/acp-rt5645.c | 210 +++ sound/soc/amd/acp.h | 9 ++ sound/soc/dwc/dwc-i2s.c | 6 + 8 files changed, 432 insertions(+), 72 deletions(-) create mode 100644 sound/soc/amd/acp-rt5645.c -- 2.5.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/8] ASoC: AMD: added condition checks for CZ specific code
From: Vijendar Mukunda Added condition checks for CZ specific code based on asic_type. Stoney specific code will be added in a future commit. Reviewed-by: Alex Deucher Signed-off-by: Vijendar Mukunda Signed-off-by: Alex Deucher --- sound/soc/amd/acp-pcm-dma.c | 62 - 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index dcbf997..e48ae5d 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -34,6 +34,8 @@ #define MAX_BUFFER (PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS) #define MIN_BUFFER MAX_BUFFER +#define CHIP_STONEY 14 +#define CHIP_CARRIZO 13 static const struct snd_pcm_hardware acp_pcm_hardware_playback = { .info = SNDRV_PCM_INFO_INTERLEAVED | @@ -419,7 +421,7 @@ static void acp_set_sram_bank_state(void __iomem *acp_mmio, u16 bank, } /* Initialize and bring ACP hardware to default state. */ -static int acp_init(void __iomem *acp_mmio) +static int acp_init(void __iomem *acp_mmio, u32 asic_type) { u16 bank; u32 val, count, sram_pte_offset; @@ -494,8 +496,10 @@ static int acp_init(void __iomem *acp_mmio) * Now, turn off all of them. This can't be done in 'poweron' of * ACP pm domain, as this requires ACP to be initialized. */ - for (bank = 1; bank < 48; bank++) - acp_set_sram_bank_state(acp_mmio, bank, false); + if (asic_type == CHIP_CARRIZO) { + for (bank = 1; bank < 48; bank++) + acp_set_sram_bank_state(acp_mmio, bank, false); + } return 0; } @@ -646,14 +650,18 @@ static int acp_dma_open(struct snd_pcm_substream *substream) if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { intr_data->play_stream = substream; - for (bank = 1; bank <= 4; bank++) - acp_set_sram_bank_state(intr_data->acp_mmio, bank, - true); + if (intr_data->asic_type == CHIP_CARRIZO) { + for (bank = 1; bank <= 4; bank++) + acp_set_sram_bank_state(intr_data->acp_mmio, + bank, true); + } } else { intr_data->capture_stream = substream; - for (bank = 5; bank <= 8; bank++) - acp_set_sram_bank_state(intr_data->acp_mmio, bank, - true); + if (intr_data->asic_type == CHIP_CARRIZO) { + for (bank = 5; bank <= 8; bank++) + acp_set_sram_bank_state(intr_data->acp_mmio, + bank, true); + } } return 0; @@ -869,14 +877,18 @@ static int acp_dma_close(struct snd_pcm_substream *substream) if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { adata->play_stream = NULL; - for (bank = 1; bank <= 4; bank++) - acp_set_sram_bank_state(adata->acp_mmio, bank, - false); - } else { + if (adata->asic_type == CHIP_CARRIZO) { + for (bank = 1; bank <= 4; bank++) + acp_set_sram_bank_state(adata->acp_mmio, bank, + false); + } + } else { adata->capture_stream = NULL; - for (bank = 5; bank <= 8; bank++) - acp_set_sram_bank_state(adata->acp_mmio, bank, - false); + if (adata->asic_type == CHIP_CARRIZO) { + for (bank = 5; bank <= 8; bank++) + acp_set_sram_bank_state(adata->acp_mmio, bank, +false); + } } /* Disable ACP irq, when the current stream is being closed and @@ -945,7 +957,7 @@ static int acp_audio_probe(struct platform_device *pdev) dev_set_drvdata(&pdev->dev, audio_drv_data); /* Initialize the ACP */ - acp_init(audio_drv_data->acp_mmio); + acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type); status = snd_soc_register_platform(&pdev->dev, &acp_asoc_platform); if (status != 0) { @@ -976,19 +988,23 @@ static int acp_pcm_resume(struct device *dev) u16 bank; struct audio_drv_data *adata = dev_get_drvdata(dev); - acp_init(adata->acp_mmio); + acp_init(adata->acp_mmio, adata->asic_type); if (adata->play_stream && adata->play_stream->runtime) { - for (bank = 1; bank <= 4; bank++) - acp_set_sram_bank_state(adata->acp_mmio, bank, + if (adata->asic_type == CHIP_CARRIZO) { +
[PATCH 3/8] drm/amd/amdgpu: Added a dwc quirk for Stoney platform
From: Vijendar Mukunda Added DW_I2S_QUIRK_16BIT_IDX_OVERRIDE quirk for Stoney. Supported format and bus width for I2S controller read from I2S Component Parameter registers. These are ready only registers. For Stoney, I2S Component Parameter registers are programmed to support 32 bit format and 4 bytes bus width only. By setting this quirk, it will override 32 bit format with 16 bit format and 2 bytes as bus width for Stoney. Reviewed-by: Alex Deucher Signed-off-by: Vijendar Mukunda Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c index 7a2a765..86b68f7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c @@ -321,14 +321,25 @@ static int acp_hw_init(void *handle) return -ENOMEM; } - i2s_pdata[0].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET; + if (adev->asic_type == CHIP_STONEY) { + i2s_pdata[0].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET | + DW_I2S_QUIRK_16BIT_IDX_OVERRIDE; + } else { + i2s_pdata[0].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET; + } i2s_pdata[0].cap = DWC_I2S_PLAY; i2s_pdata[0].snd_rates = SNDRV_PCM_RATE_8000_96000; i2s_pdata[0].i2s_reg_comp1 = ACP_I2S_COMP1_PLAY_REG_OFFSET; i2s_pdata[0].i2s_reg_comp2 = ACP_I2S_COMP2_PLAY_REG_OFFSET; + if (adev->asic_type == CHIP_STONEY) { + i2s_pdata[1].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET | + DW_I2S_QUIRK_COMP_PARAM1 | + DW_I2S_QUIRK_16BIT_IDX_OVERRIDE; + } else { + i2s_pdata[1].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET | + DW_I2S_QUIRK_COMP_PARAM1; + } - i2s_pdata[1].quirks = DW_I2S_QUIRK_COMP_REG_OFFSET | - DW_I2S_QUIRK_COMP_PARAM1; i2s_pdata[1].cap = DWC_I2S_RECORD; i2s_pdata[1].snd_rates = SNDRV_PCM_RATE_8000_96000; i2s_pdata[1].i2s_reg_comp1 = ACP_I2S_COMP1_CAP_REG_OFFSET; -- 2.5.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/8] drm/amd/amdgpu: Added asic_type as ACP DMA driver platform data
From: Vijendar Mukunda asic_type information is passed to ACP DMA Driver as platform data. We need this to determine whether the asic is Carrizo (CZ) or Stoney (ST) in the acp sound driver. Reviewed-by: Alex Deucher Signed-off-by: Vijendar Mukunda Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 4 sound/soc/amd/acp-pcm-dma.c | 8 ++-- sound/soc/amd/acp.h | 7 +++ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c index 06879d1..7a2a765 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c @@ -262,6 +262,7 @@ static int acp_hw_init(void *handle) uint64_t acp_base; struct device *dev; struct i2s_platform_data *i2s_pdata; + u32 asic_type; struct amdgpu_device *adev = (struct amdgpu_device *)handle; @@ -271,6 +272,7 @@ static int acp_hw_init(void *handle) if (!ip_block) return -EINVAL; + asic_type = adev->asic_type; r = amd_acp_hw_init(adev->acp.cgs_device, ip_block->version->major, ip_block->version->minor); /* -ENODEV means board uses AZ rather than ACP */ @@ -355,6 +357,8 @@ static int acp_hw_init(void *handle) adev->acp.acp_cell[0].name = "acp_audio_dma"; adev->acp.acp_cell[0].num_resources = 4; adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0]; + adev->acp.acp_cell[0].platform_data = &asic_type; + adev->acp.acp_cell[0].pdata_size = sizeof(asic_type); adev->acp.acp_cell[1].name = "designware-i2s"; adev->acp.acp_cell[1].num_resources = 1; diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index 08b1399..dcbf997 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -73,12 +73,6 @@ static const struct snd_pcm_hardware acp_pcm_hardware_capture = { .periods_max = CAPTURE_MAX_NUM_PERIODS, }; -struct audio_drv_data { - struct snd_pcm_substream *play_stream; - struct snd_pcm_substream *capture_stream; - void __iomem *acp_mmio; -}; - static u32 acp_reg_read(void __iomem *acp_mmio, u32 reg) { return readl(acp_mmio + (reg * 4)); @@ -916,6 +910,7 @@ static int acp_audio_probe(struct platform_device *pdev) int status; struct audio_drv_data *audio_drv_data; struct resource *res; + const u32 *pdata = pdev->dev.platform_data; audio_drv_data = devm_kzalloc(&pdev->dev, sizeof(struct audio_drv_data), GFP_KERNEL); @@ -932,6 +927,7 @@ static int acp_audio_probe(struct platform_device *pdev) audio_drv_data->play_stream = NULL; audio_drv_data->capture_stream = NULL; + audio_drv_data->asic_type = *pdata; res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) { diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h index 330832e..28cf914 100644 --- a/sound/soc/amd/acp.h +++ b/sound/soc/amd/acp.h @@ -84,6 +84,13 @@ struct audio_substream_data { void __iomem *acp_mmio; }; +struct audio_drv_data { + struct snd_pcm_substream *play_stream; + struct snd_pcm_substream *capture_stream; + void __iomem *acp_mmio; + u32 asic_type; +}; + enum { ACP_TILE_P1 = 0, ACP_TILE_P2, -- 2.5.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/8] ASoC: AMD: DMA driver changes for Stoney Platform
From: Vijendar Mukunda Added DMA driver changes for Stoney platform. Below are the key differences between Stoney and CZ: - Memory Gating is disabled - SRAM Banks won't be turned off - No Of SRAM Banks reduced to 6 - DAGB Garlic Interface used - 16 bit resolution is supported. - SRAM bank 1 & SRAM bank 2 will be used for playback scenario. - SRAM Bank 3 & SRAM Bank 4 will be used for Capture scenario. Reviewed-by: Alex Deucher Signed-off-by: Vijendar Mukunda Signed-off-by: Alex Deucher --- sound/soc/amd/acp-pcm-dma.c | 81 + sound/soc/amd/acp.h | 2 ++ 2 files changed, 61 insertions(+), 22 deletions(-) diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index e48ae5d..5b5e95d 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -139,8 +139,8 @@ static void config_dma_descriptor_in_sram(void __iomem *acp_mmio, * system memory <-> ACP SRAM */ static void set_acp_sysmem_dma_descriptors(void __iomem *acp_mmio, - u32 size, int direction, - u32 pte_offset) + u32 size, int direction, + u32 pte_offset, u32 asic_type) { u16 i; u16 dma_dscr_idx = PLAYBACK_START_DMA_DESCR_CH12; @@ -154,20 +154,38 @@ static void set_acp_sysmem_dma_descriptors(void __iomem *acp_mmio, (size / 2) - (i * (size/2)); dmadscr[i].src = ACP_INTERNAL_APERTURE_WINDOW_0_ADDRESS + (pte_offset * SZ_4K) + (i * (size/2)); - dmadscr[i].xfer_val |= - (ACP_DMA_ATTRIBUTES_DAGB_ONION_TO_SHAREDMEM << 16) | - (size / 2); + if (asic_type == CHIP_STONEY) { + dmadscr[i].xfer_val |= + (ACP_DMA_ATTRIBUTES_DAGB_GARLIC_TO_SHAREDMEM << 16) | + (size / 2); + } else { + dmadscr[i].xfer_val |= + (ACP_DMA_ATTRIBUTES_DAGB_ONION_TO_SHAREDMEM << 16) | + (size / 2); + } } else { dma_dscr_idx = CAPTURE_START_DMA_DESCR_CH14 + i; - dmadscr[i].src = ACP_SHARED_RAM_BANK_5_ADDRESS + - (i * (size/2)); - dmadscr[i].dest = ACP_INTERNAL_APERTURE_WINDOW_0_ADDRESS - + (pte_offset * SZ_4K) + - (i * (size/2)); - dmadscr[i].xfer_val |= - BIT(22) | - (ACP_DMA_ATTRIBUTES_SHAREDMEM_TO_DAGB_ONION << 16) | - (size / 2); + if (asic_type == CHIP_STONEY) { + dmadscr[i].src = ACP_SHARED_RAM_BANK_3_ADDRESS + + (i * (size/2)); + dmadscr[i].dest = + ACP_INTERNAL_APERTURE_WINDOW_0_ADDRESS + + (pte_offset * SZ_4K) + (i * (size/2)); + dmadscr[i].xfer_val |= + BIT(22) | + (ACP_DMA_ATTRIBUTES_SHARED_MEM_TO_DAGB_GARLIC << 16) | + (size / 2); + } else { + dmadscr[i].src = ACP_SHARED_RAM_BANK_5_ADDRESS + + (i * (size/2)); + dmadscr[i].dest = +ACP_INTERNAL_APERTURE_WINDOW_0_ADDRESS + + (pte_offset * SZ_4K) + (i * (size/2)); + dmadscr[i].xfer_val |= + BIT(22) | + (ACP_DMA_ATTRIBUTES_SHAREDMEM_TO_DAGB_ONION << 16) | + (size / 2); + } } config_dma_descriptor_in_sram(acp_mmio, dma_dscr_idx, &dmadscr[i]); @@ -188,7 +206,8 @@ static void set_acp_sysmem_dma_descriptors(void __iomem *acp_mmio, * ACP SRAM <-> I2S */ static void set_acp_to_i2s_dma_descriptors(void __iomem *acp_mmio, - u32 size, int direction) + u32 size, int direction, + u32 asic_type) { u16 i; @@ -209,8 +228,15 @@ static void set_acp_to_i2s_dma_descriptors(void __iomem *acp_mmio, dma_dscr_idx = CAPTURE_START_DMA_DESCR_CH15 + i; /* dmadscr[i].src is unused by hardware. */ dmadscr[i].src = 0
[PATCH 8/8] ASoC: AMD: Add machine driver for cz rt5650
From: Akshu Agrawal The driver is used for AMD board using rt5650 codec. Reviewed-by: Alex Deucher Signed-off-by: Akshu Agrawal Signed-off-by: Alex Deucher --- sound/soc/amd/Kconfig | 7 ++ sound/soc/amd/Makefile | 2 + sound/soc/amd/acp-rt5645.c | 210 + 3 files changed, 219 insertions(+) create mode 100644 sound/soc/amd/acp-rt5645.c diff --git a/sound/soc/amd/Kconfig b/sound/soc/amd/Kconfig index 78187eb..eb0ae60 100644 --- a/sound/soc/amd/Kconfig +++ b/sound/soc/amd/Kconfig @@ -2,3 +2,10 @@ config SND_SOC_AMD_ACP tristate "AMD Audio Coprocessor support" help This option enables ACP DMA support on AMD platform. +config SND_SOC_AMD_CZ_RT5645_MACH + tristate "AMD CZ support for RT5645" + select SND_SOC_RT5645 + select SND_SOC_AMD_ACP + depends on I2C_DESIGNWARE_PLATFORM + help +This option enables machine driver for rt5645. diff --git a/sound/soc/amd/Makefile b/sound/soc/amd/Makefile index 1a66ec0..eed64ff 100644 --- a/sound/soc/amd/Makefile +++ b/sound/soc/amd/Makefile @@ -1,3 +1,5 @@ snd-soc-acp-pcm-objs := acp-pcm-dma.o +snd-soc-acp-rt5645-mach-objs := acp-rt5645.o obj-$(CONFIG_SND_SOC_AMD_ACP) += snd-soc-acp-pcm.o +obj-$(CONFIG_SND_SOC_AMD_CZ_RT5645_MACH) += snd-soc-acp-rt5645-mach.o diff --git a/sound/soc/amd/acp-rt5645.c b/sound/soc/amd/acp-rt5645.c new file mode 100644 index 000..ebaafad --- /dev/null +++ b/sound/soc/amd/acp-rt5645.c @@ -0,0 +1,210 @@ +/* + * Machine driver for AMD ACP Audio engine using Realtek RT5645 codec + * + * Copyright 2017 Advanced Micro Devices, Inc. + * + * This file is modified from rt288 machine driver + * + * 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 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 COPYRIGHT HOLDER(S) OR AUTHOR(S) 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. + * + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "../codecs/rt5645.h" + +#define CZ_PLAT_CLK 2400 + +static struct snd_soc_jack cz_jack; + +static int cz_aif1_hw_params(struct snd_pcm_substream *substream, +struct snd_pcm_hw_params *params) +{ + int ret = 0; + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *codec_dai = rtd->codec_dai; + + ret = snd_soc_dai_set_pll(codec_dai, 0, RT5645_PLL1_S_MCLK, + CZ_PLAT_CLK, params_rate(params) * 512); + if (ret < 0) { + dev_err(rtd->dev, "can't set codec pll: %d\n", ret); + return ret; + } + + ret = snd_soc_dai_set_sysclk(codec_dai, RT5645_SCLK_S_PLL1, + params_rate(params) * 512, SND_SOC_CLOCK_OUT); + if (ret < 0) { + dev_err(rtd->dev, "can't set codec sysclk: %d\n", ret); + return ret; + } + + return ret; +} + +static int cz_init(struct snd_soc_pcm_runtime *rtd) +{ + int ret; + struct snd_soc_card *card; + struct snd_soc_codec *codec; + + codec = rtd->codec; + card = rtd->card; + + ret = snd_soc_card_jack_new(card, "Headset Jack", + SND_JACK_HEADPHONE | SND_JACK_MICROPHONE | + SND_JACK_BTN_0 | SND_JACK_BTN_1 | + SND_JACK_BTN_2 | SND_JACK_BTN_3, + &cz_jack, NULL, 0); + if (ret) { + dev_err(card->dev, "HP jack creation failed %d\n", ret); + return ret; + } + + rt5645_set_jack_detect(codec, &cz_jack, &cz_jack, &cz_jack); + + return 0; +} + +static struct snd_soc_ops cz_aif1_ops = { + .hw_params = cz_aif1_hw_params, +}; + +static struct snd_soc_dai_link cz_dai_rt5650[] = { + { + .name = "amd-rt5645-play", + .stream_name = "RT5645_AIF1", + .platform_name = "acp_audio_dma.0.auto", + .cpu_dai_name = "designware-i2s.1.auto", +
[PATCH 6/8] ASoC: AMD: Buffer related changes for Stoney
From: Vijendar Mukunda Stoney uses 16kb SRAM memory for playback and 16Kb for capture. Modified Max buffer size to have the correct mapping between System Memory and SRAM. Added snd_pcm_hardware structures for playback and capture for Stoney. Reviewed-by: Alex Deucher Signed-off-by: Vijendar Mukunda Signed-off-by: Alex Deucher --- sound/soc/amd/acp-pcm-dma.c | 68 + 1 file changed, 63 insertions(+), 5 deletions(-) diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index 5b5e95d..c9bb618 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -34,6 +34,10 @@ #define MAX_BUFFER (PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS) #define MIN_BUFFER MAX_BUFFER +#define ST_PLAYBACK_MAX_PERIOD_SIZE 8192 +#define ST_CAPTURE_MAX_PERIOD_SIZE 8192 +#define ST_MAX_BUFFER (ST_PLAYBACK_MAX_PERIOD_SIZE * PLAYBACK_MAX_NUM_PERIODS) +#define ST_MIN_BUFFER ST_MAX_BUFFER #define CHIP_STONEY 14 #define CHIP_CARRIZO 13 @@ -75,6 +79,44 @@ static const struct snd_pcm_hardware acp_pcm_hardware_capture = { .periods_max = CAPTURE_MAX_NUM_PERIODS, }; +static const struct snd_pcm_hardware acp_st_pcm_hardware_playback = { + .info = SNDRV_PCM_INFO_INTERLEAVED | + SNDRV_PCM_INFO_BLOCK_TRANSFER | SNDRV_PCM_INFO_MMAP | + SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_BATCH | + SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME, + .formats = SNDRV_PCM_FMTBIT_S16_LE | + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE, + .channels_min = 1, + .channels_max = 8, + .rates = SNDRV_PCM_RATE_8000_96000, + .rate_min = 8000, + .rate_max = 96000, + .buffer_bytes_max = ST_MAX_BUFFER, + .period_bytes_min = PLAYBACK_MIN_PERIOD_SIZE, + .period_bytes_max = ST_PLAYBACK_MAX_PERIOD_SIZE, + .periods_min = PLAYBACK_MIN_NUM_PERIODS, + .periods_max = PLAYBACK_MAX_NUM_PERIODS, +}; + +static const struct snd_pcm_hardware acp_st_pcm_hardware_capture = { + .info = SNDRV_PCM_INFO_INTERLEAVED | + SNDRV_PCM_INFO_BLOCK_TRANSFER | SNDRV_PCM_INFO_MMAP | + SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_BATCH | + SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME, + .formats = SNDRV_PCM_FMTBIT_S16_LE | + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE, + .channels_min = 1, + .channels_max = 2, + .rates = SNDRV_PCM_RATE_8000_48000, + .rate_min = 8000, + .rate_max = 48000, + .buffer_bytes_max = ST_MAX_BUFFER, + .period_bytes_min = CAPTURE_MIN_PERIOD_SIZE, + .period_bytes_max = ST_CAPTURE_MAX_PERIOD_SIZE, + .periods_min = CAPTURE_MIN_NUM_PERIODS, + .periods_max = CAPTURE_MAX_NUM_PERIODS, +}; + static u32 acp_reg_read(void __iomem *acp_mmio, u32 reg) { return readl(acp_mmio + (reg * 4)); @@ -657,10 +699,17 @@ static int acp_dma_open(struct snd_pcm_substream *substream) if (adata == NULL) return -ENOMEM; - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - runtime->hw = acp_pcm_hardware_playback; - else - runtime->hw = acp_pcm_hardware_capture; + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { + if (intr_data->asic_type == CHIP_STONEY) + runtime->hw = acp_st_pcm_hardware_playback; + else + runtime->hw = acp_pcm_hardware_playback; + } else { + if (intr_data->asic_type == CHIP_STONEY) + runtime->hw = acp_st_pcm_hardware_capture; + else + runtime->hw = acp_pcm_hardware_capture; + } ret = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS); @@ -894,10 +943,19 @@ static int acp_dma_trigger(struct snd_pcm_substream *substream, int cmd) static int acp_dma_new(struct snd_soc_pcm_runtime *rtd) { - return snd_pcm_lib_preallocate_pages_for_all(rtd->pcm, + struct audio_drv_data *adata = dev_get_drvdata(rtd->platform->dev); + + if (adata->asic_type == CHIP_STONEY) { + return snd_pcm_lib_preallocate_pages_for_all(rtd->pcm, + SNDRV_DMA_TYPE_DEV, + NULL, MIN_BUFFER, + ST_MAX_BUFFER); + } else { + return snd_pcm_lib_preallocate_pages_for_all(rtd->pcm, SNDRV_DMA_TYPE_DEV, NULL, MIN_BUFFER, MAX_BUFFER); + } } static int acp_dma_close(struct snd_pcm_substream *substream) -- 2.5.5 ___ dri-devel mailing
[PATCH 7/8] drm/amd/amdgpu: Disable ACP Power Gating for Stoney platform
From: Vijendar Mukunda Power Gating is disabled in Stoney platform. Reviewed-by: Alex Deucher Signed-off-by: Vijendar Mukunda Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 33 ++--- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c index 86b68f7..0e512fa 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c @@ -287,19 +287,20 @@ static int acp_hw_init(void *handle) return 0; else if (r) return r; + if (asic_type != CHIP_STONEY) { + adev->acp.acp_genpd = kzalloc(sizeof(struct acp_pm_domain), GFP_KERNEL); + if (adev->acp.acp_genpd == NULL) + return -ENOMEM; - adev->acp.acp_genpd = kzalloc(sizeof(struct acp_pm_domain), GFP_KERNEL); - if (adev->acp.acp_genpd == NULL) - return -ENOMEM; - - adev->acp.acp_genpd->gpd.name = "ACP_AUDIO"; - adev->acp.acp_genpd->gpd.power_off = acp_poweroff; - adev->acp.acp_genpd->gpd.power_on = acp_poweron; + adev->acp.acp_genpd->gpd.name = "ACP_AUDIO"; + adev->acp.acp_genpd->gpd.power_off = acp_poweroff; + adev->acp.acp_genpd->gpd.power_on = acp_poweron; - adev->acp.acp_genpd->cgs_dev = adev->acp.cgs_device; + adev->acp.acp_genpd->cgs_dev = adev->acp.cgs_device; - pm_genpd_init(&adev->acp.acp_genpd->gpd, NULL, false); + pm_genpd_init(&adev->acp.acp_genpd->gpd, NULL, false); + } adev->acp.acp_cell = kzalloc(sizeof(struct mfd_cell) * ACP_DEVS, GFP_KERNEL); @@ -388,12 +389,14 @@ static int acp_hw_init(void *handle) if (r) return r; - for (i = 0; i < ACP_DEVS ; i++) { - dev = get_mfd_cell_dev(adev->acp.acp_cell[i].name, i); - r = pm_genpd_add_device(&adev->acp.acp_genpd->gpd, dev); - if (r) { - dev_err(dev, "Failed to add dev to genpd\n"); - return r; + if (asic_type != CHIP_STONEY) { + for (i = 0; i < ACP_DEVS ; i++) { + dev = get_mfd_cell_dev(adev->acp.acp_cell[i].name, i); + r = pm_genpd_add_device(&adev->acp.acp_genpd->gpd, dev); + if (r) { + dev_err(dev, "Failed to add dev to genpd\n"); + return r; + } } } -- 2.5.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/8] drm/amd/amdgpu: Added asic_type as ACP DMA driver platform data
Am 23.06.2017 um 18:34 schrieb Alex Deucher: From: Vijendar Mukunda asic_type information is passed to ACP DMA Driver as platform data. We need this to determine whether the asic is Carrizo (CZ) or Stoney (ST) in the acp sound driver. Reviewed-by: Alex Deucher Signed-off-by: Vijendar Mukunda Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 4 sound/soc/amd/acp-pcm-dma.c | 8 ++-- sound/soc/amd/acp.h | 7 +++ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c index 06879d1..7a2a765 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c @@ -262,6 +262,7 @@ static int acp_hw_init(void *handle) uint64_t acp_base; struct device *dev; struct i2s_platform_data *i2s_pdata; + u32 asic_type; struct amdgpu_device *adev = (struct amdgpu_device *)handle; @@ -271,6 +272,7 @@ static int acp_hw_init(void *handle) if (!ip_block) return -EINVAL; + asic_type = adev->asic_type; r = amd_acp_hw_init(adev->acp.cgs_device, ip_block->version->major, ip_block->version->minor); /* -ENODEV means board uses AZ rather than ACP */ @@ -355,6 +357,8 @@ static int acp_hw_init(void *handle) adev->acp.acp_cell[0].name = "acp_audio_dma"; adev->acp.acp_cell[0].num_resources = 4; adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0]; + adev->acp.acp_cell[0].platform_data = &asic_type; + adev->acp.acp_cell[0].pdata_size = sizeof(asic_type); Have the painkillers jellyfied my brain or are you setting a pointer in the amdgpu device structure to some variable on the stack? If it's just for initialization then that's probably ok, but I would reset the pointer at the end of the function. Otherwise somebody might have the idea to dereference it later on and that can only lead to trouble. Christian. adev->acp.acp_cell[1].name = "designware-i2s"; adev->acp.acp_cell[1].num_resources = 1; diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c index 08b1399..dcbf997 100644 --- a/sound/soc/amd/acp-pcm-dma.c +++ b/sound/soc/amd/acp-pcm-dma.c @@ -73,12 +73,6 @@ static const struct snd_pcm_hardware acp_pcm_hardware_capture = { .periods_max = CAPTURE_MAX_NUM_PERIODS, }; -struct audio_drv_data { - struct snd_pcm_substream *play_stream; - struct snd_pcm_substream *capture_stream; - void __iomem *acp_mmio; -}; - static u32 acp_reg_read(void __iomem *acp_mmio, u32 reg) { return readl(acp_mmio + (reg * 4)); @@ -916,6 +910,7 @@ static int acp_audio_probe(struct platform_device *pdev) int status; struct audio_drv_data *audio_drv_data; struct resource *res; + const u32 *pdata = pdev->dev.platform_data; audio_drv_data = devm_kzalloc(&pdev->dev, sizeof(struct audio_drv_data), GFP_KERNEL); @@ -932,6 +927,7 @@ static int acp_audio_probe(struct platform_device *pdev) audio_drv_data->play_stream = NULL; audio_drv_data->capture_stream = NULL; + audio_drv_data->asic_type = *pdata; res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); if (!res) { diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h index 330832e..28cf914 100644 --- a/sound/soc/amd/acp.h +++ b/sound/soc/amd/acp.h @@ -84,6 +84,13 @@ struct audio_substream_data { void __iomem *acp_mmio; }; +struct audio_drv_data { + struct snd_pcm_substream *play_stream; + struct snd_pcm_substream *capture_stream; + void __iomem *acp_mmio; + u32 asic_type; +}; + enum { ACP_TILE_P1 = 0, ACP_TILE_P2, ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/4] drm: Plumb modifiers through plane init
This is the plumbing for supporting fb modifiers on planes. Modifiers have already been introduced to some extent, but this series will extend this to allow querying modifiers per plane. Based on this, the client to enable optimal modifications for framebuffers. This patch simply allows the DRM drivers to initialize their list of supported modifiers upon initializing the plane. v2: A minor addition from Daniel v3: * Updated commit message * s/INVALID/DRM_FORMAT_MOD_INVALID (Liviu) * Remove some excess newlines (Liviu) * Update comment for > 64 modifiers (Liviu) v4: Minor comment adjustments (Liviu) v5: Some new platforms added due to rebase v6: Add some missed plane inits (or maybe they're new - who knows at this point) (Daniel) Signed-off-by: Ben Widawsky Reviewed-by: Daniel Stone (v2) Reviewed-by: Liviu Dudau --- drivers/gpu/drm/arc/arcpgu_crtc.c | 1 + drivers/gpu/drm/arm/hdlcd_crtc.c| 1 + drivers/gpu/drm/arm/malidp_planes.c | 2 +- drivers/gpu/drm/armada/armada_crtc.c| 1 + drivers/gpu/drm/armada/armada_overlay.c | 1 + drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 ++- drivers/gpu/drm/drm_modeset_helper.c| 1 + drivers/gpu/drm/drm_plane.c | 36 - drivers/gpu/drm/drm_simple_kms_helper.c | 3 +++ drivers/gpu/drm/exynos/exynos_drm_plane.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 2 +- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 1 + drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +- drivers/gpu/drm/i915/intel_display.c| 5 +++- drivers/gpu/drm/i915/intel_sprite.c | 4 +-- drivers/gpu/drm/imx/ipuv3-plane.c | 4 +-- drivers/gpu/drm/mediatek/mtk_drm_plane.c| 2 +- drivers/gpu/drm/meson/meson_plane.c | 1 + drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 2 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 +-- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 +- drivers/gpu/drm/nouveau/nv50_display.c | 5 ++-- drivers/gpu/drm/omapdrm/omap_plane.c| 2 +- drivers/gpu/drm/pl111/pl111_display.c | 2 +- drivers/gpu/drm/qxl/qxl_display.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 4 +-- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 4 +-- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 +-- drivers/gpu/drm/sti/sti_cursor.c| 2 +- drivers/gpu/drm/sti/sti_gdp.c | 2 +- drivers/gpu/drm/sti/sti_hqvdp.c | 2 +- drivers/gpu/drm/stm/ltdc.c | 2 +- drivers/gpu/drm/sun4i/sun4i_layer.c | 2 +- drivers/gpu/drm/tegra/dc.c | 12 - drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 2 +- drivers/gpu/drm/vc4/vc4_plane.c | 2 +- drivers/gpu/drm/virtio/virtgpu_plane.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 4 +-- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c| 4 +-- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c| 4 +-- drivers/gpu/drm/zte/zx_plane.c | 2 +- include/drm/drm_plane.h | 22 ++- include/drm/drm_simple_kms_helper.h | 1 + include/uapi/drm/drm_fourcc.h | 11 44 files changed, 130 insertions(+), 49 deletions(-) diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c index 611af74a31c0..d0e3671a71ed 100644 --- a/drivers/gpu/drm/arc/arcpgu_crtc.c +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c @@ -217,6 +217,7 @@ static struct drm_plane *arc_pgu_plane_init(struct drm_device *drm) ret = drm_universal_plane_init(drm, plane, 0xff, &arc_pgu_plane_funcs, formats, ARRAY_SIZE(formats), + NULL, DRM_PLANE_TYPE_PRIMARY, NULL); if (ret) return ERR_PTR(ret); diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c index 1a3359c0f6cd..9b9c0913f55f 100644 --- a/drivers/gpu/drm/arm/hdlcd_crtc.c +++ b/drivers/gpu/drm/arm/hdlcd_crtc.c @@ -320,6 +320,7 @@ static struct drm_plane *hdlcd_plane_init(struct drm_device *drm) ret = drm_universal_plane_init(drm, plane, 0xff, &hdlcd_plane_funcs, formats, ARRAY_SIZE(formats), + NULL, DRM_PLANE_TYPE_PRIMARY, NULL); if (ret) { return ERR_PTR(ret); diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c index 600fa7bd7f52..60402e27882f 100644 --- a/drivers/gpu/drm/arm/malidp_planes.c +++ b/drivers/gpu/drm/arm/malidp_planes.c @@ -398,7 +398,7 @@ int malidp_de_planes_init(struct drm_device *drm) DRM_
[PATCH 4/4] drm/i915: Add support for CCS modifiers
v2: Support sprite plane. Support pipe C/D limitation on GEN9. This requires rebase on the correct Ville patches Cc: Daniel Stone Cc: Kristian Høgsberg Signed-off-by: Ben Widawsky --- drivers/gpu/drm/i915/intel_display.c | 34 +-- drivers/gpu/drm/i915/intel_sprite.c | 39 +++- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 877a51514c61..2a0e5cd26059 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -93,7 +93,17 @@ static const uint32_t skl_primary_formats[] = { DRM_FORMAT_VYUY, }; +static const uint64_t skl_format_modifiers_noccs[] = { + I915_FORMAT_MOD_Yf_TILED, + I915_FORMAT_MOD_Y_TILED, + I915_FORMAT_MOD_X_TILED, + DRM_FORMAT_MOD_LINEAR, + DRM_FORMAT_MOD_INVALID +}; + static const uint64_t skl_format_modifiers[] = { + I915_FORMAT_MOD_Yf_TILED_CCS, + I915_FORMAT_MOD_Y_TILED_CCS, I915_FORMAT_MOD_Yf_TILED, I915_FORMAT_MOD_Y_TILED, I915_FORMAT_MOD_X_TILED, @@ -13872,17 +13882,13 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier) return false; } case DRM_FORMAT_RGB565: - case DRM_FORMAT_XRGB: - case DRM_FORMAT_XBGR: - case DRM_FORMAT_ARGB: - case DRM_FORMAT_ABGR: case DRM_FORMAT_XRGB2101010: case DRM_FORMAT_XBGR2101010: case DRM_FORMAT_YUYV: case DRM_FORMAT_YVYU: case DRM_FORMAT_UYVY: case DRM_FORMAT_VYUY: - /* All i915 modifiers are fine */ + /* All non-ccs i915 modifiers are fine */ switch (modifier) { case DRM_FORMAT_MOD_LINEAR: case I915_FORMAT_MOD_X_TILED: @@ -13892,6 +13898,12 @@ static bool skl_mod_supported(uint32_t format, uint64_t modifier) default: return false; } + case DRM_FORMAT_XRGB: + case DRM_FORMAT_XBGR: + case DRM_FORMAT_ARGB: + case DRM_FORMAT_ABGR: + /* All i915 modifiers are fine */ + return true; default: return false; } @@ -14123,13 +14135,23 @@ intel_primary_plane_create(struct drm_i915_private *dev_priv, enum pipe pipe) primary->frontbuffer_bit = INTEL_FRONTBUFFER_PRIMARY(pipe); primary->check_plane = intel_check_primary_plane; - if (INTEL_GEN(dev_priv) >= 9) { + if (INTEL_GEN(dev_priv) >= 10) { intel_primary_formats = skl_primary_formats; num_formats = ARRAY_SIZE(skl_primary_formats); intel_format_modifiers = skl_format_modifiers; primary->update_plane = skylake_update_primary_plane; primary->disable_plane = skylake_disable_primary_plane; + } else if (INTEL_GEN(dev_priv) >= 9) { + intel_primary_formats = skl_primary_formats; + num_formats = ARRAY_SIZE(skl_primary_formats); + if (pipe >= PIPE_C) + intel_format_modifiers = skl_format_modifiers; + else + intel_format_modifiers = skl_format_modifiers_noccs; + + primary->update_plane = skylake_update_primary_plane; + primary->disable_plane = skylake_disable_primary_plane; } else if (INTEL_GEN(dev_priv) >= 4) { intel_primary_formats = i965_primary_formats; num_formats = ARRAY_SIZE(i965_primary_formats); diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index e80834cb1f4c..de4454a8ef9e 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1085,7 +1085,17 @@ static uint32_t skl_plane_formats[] = { DRM_FORMAT_VYUY, }; +static const uint64_t skl_plane_format_modifiers_noccs[] = { + I915_FORMAT_MOD_Yf_TILED, + I915_FORMAT_MOD_Y_TILED, + I915_FORMAT_MOD_X_TILED, + DRM_FORMAT_MOD_LINEAR, + DRM_FORMAT_MOD_INVALID +}; + static const uint64_t skl_plane_format_modifiers[] = { + I915_FORMAT_MOD_Yf_TILED_CCS, + I915_FORMAT_MOD_Y_TILED_CCS, I915_FORMAT_MOD_Yf_TILED, I915_FORMAT_MOD_Y_TILED, I915_FORMAT_MOD_X_TILED, @@ -1108,6 +1118,20 @@ static bool intel_sprite_plane_format_mod_supported(struct drm_plane *plane, modifier != DRM_FORMAT_MOD_LINEAR) return false; + switch (modifier) { + case I915_FORMAT_MOD_Yf_TILED_CCS: + case I915_FORMAT_MOD_Y_TILED_CCS: + switch (format) { + case DRM_FORMAT_ABGR: + case DRM_FORMAT_ARGB: + case DRM_FORMAT_XBGR: + case DRM_FORMAT_XRGB: + return true; +
[PATCH v4 0/4] Blobifiers (FKA GET_PLANE2)
These patches create the blob property for modifiers, and the implementation for i915 modifiers. This [generally] has been tested in Weston by Daniel Stone as well as an earlier version of kmscube. This patch series was formerly known as "GET_PLANE2" because the interface for obtaining the modifiers was extended throug GET_PLANE instead of a blob property. The modifier blob (blobifier) presents a list of modifiers per plane, and a related bitmask [1] back to userspace, via KMS, so that a client such as a compositor may utilize those modifiers for buffer creation via GBM, or buffer import via dmabuf/EGL. All vendors may optimize via their supported modifiers. For Intel, this is one of the pieces required to support end to end lossless compression (a memory bandwidth saving technique semi-common across GPUs). I do apologize if I missed some previous review comments. A ton of things came up and it took a while to spin this rev. Nothing was missing intentionally. [1] The bitmask is used to show the connection between which modifiers are supported by which formats. Ben Widawsky (4): drm: Plumb modifiers through plane init drm: Create a format/modifier blob drm/i915: Add format modifiers for Intel drm/i915: Add support for CCS modifiers drivers/gpu/drm/arc/arcpgu_crtc.c | 1 + drivers/gpu/drm/arm/hdlcd_crtc.c| 1 + drivers/gpu/drm/arm/malidp_planes.c | 2 +- drivers/gpu/drm/armada/armada_crtc.c| 1 + drivers/gpu/drm/armada/armada_overlay.c | 1 + drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 3 +- drivers/gpu/drm/drm_mode_config.c | 7 ++ drivers/gpu/drm/drm_modeset_helper.c| 1 + drivers/gpu/drm/drm_plane.c | 119 +- drivers/gpu/drm/drm_simple_kms_helper.c | 3 + drivers/gpu/drm/exynos/exynos_drm_plane.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 2 +- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 1 + drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +- drivers/gpu/drm/i915/intel_display.c| 155 +++- drivers/gpu/drm/i915/intel_sprite.c | 117 +- drivers/gpu/drm/imx/ipuv3-plane.c | 4 +- drivers/gpu/drm/mediatek/mtk_drm_plane.c| 2 +- drivers/gpu/drm/meson/meson_plane.c | 1 + drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 2 +- drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c | 4 +- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 +- drivers/gpu/drm/nouveau/nv50_display.c | 5 +- drivers/gpu/drm/omapdrm/omap_plane.c| 2 +- drivers/gpu/drm/pl111/pl111_display.c | 2 +- drivers/gpu/drm/qxl/qxl_display.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 4 +- drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 4 +- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 +- drivers/gpu/drm/sti/sti_cursor.c| 2 +- drivers/gpu/drm/sti/sti_gdp.c | 2 +- drivers/gpu/drm/sti/sti_hqvdp.c | 2 +- drivers/gpu/drm/stm/ltdc.c | 2 +- drivers/gpu/drm/sun4i/sun4i_layer.c | 2 +- drivers/gpu/drm/tegra/dc.c | 12 +- drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 2 +- drivers/gpu/drm/vc4/vc4_plane.c | 2 +- drivers/gpu/drm/virtio/virtgpu_plane.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c | 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c| 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c| 4 +- drivers/gpu/drm/zte/zx_plane.c | 2 +- include/drm/drm_mode_config.h | 6 + include/drm/drm_plane.h | 22 +++- include/drm/drm_simple_kms_helper.h | 1 + include/uapi/drm/drm_fourcc.h | 11 ++ include/uapi/drm/drm_mode.h | 50 47 files changed, 539 insertions(+), 49 deletions(-) -- 2.13.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm/i915: Add format modifiers for Intel
This was based on a patch originally by Kristian. It has been modified pretty heavily to use the new callbacks from the previous patch. v2: - Add LINEAR and Yf modifiers to list (Ville) - Combine i8xx and i965 into one list of formats (Ville) - Allow 1010102 formats for Y/Yf tiled (Ville) v3: - Handle cursor formats (Ville) - Put handling for LINEAR in the mod_support functions (Ville) v4: - List each modifier explicitly in supported modifiers (Ville) - Handle the CURSOR plane (Ville) v5: - Split out cursor and sprite handling (Ville) v6: - Actually use the sprite funcs (Emil) - Use unreachable (Emil) v7: - Only allow Intel modifiers and LINEAR (Ben) v8 - Fix spite assert introduced in v6 (Daniel) Cc: Ville Syrjälä Cc: Kristian H. Kristensen Cc: Emil Velikov Signed-off-by: Ben Widawsky --- drivers/gpu/drm/i915/intel_display.c | 136 +-- drivers/gpu/drm/i915/intel_sprite.c | 82 - 2 files changed, 211 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7d55c5e3df28..877a51514c61 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -72,6 +72,12 @@ static const uint32_t i965_primary_formats[] = { DRM_FORMAT_XBGR2101010, }; +static const uint64_t i9xx_format_modifiers[] = { + I915_FORMAT_MOD_X_TILED, + DRM_FORMAT_MOD_LINEAR, + DRM_FORMAT_MOD_INVALID +}; + static const uint32_t skl_primary_formats[] = { DRM_FORMAT_C8, DRM_FORMAT_RGB565, @@ -87,11 +93,24 @@ static const uint32_t skl_primary_formats[] = { DRM_FORMAT_VYUY, }; +static const uint64_t skl_format_modifiers[] = { + I915_FORMAT_MOD_Yf_TILED, + I915_FORMAT_MOD_Y_TILED, + I915_FORMAT_MOD_X_TILED, + DRM_FORMAT_MOD_LINEAR, + DRM_FORMAT_MOD_INVALID +}; + /* Cursor formats */ static const uint32_t intel_cursor_formats[] = { DRM_FORMAT_ARGB, }; +static const uint64_t cursor_format_modifiers[] = { + DRM_FORMAT_MOD_LINEAR, + DRM_FORMAT_MOD_INVALID +}; + static void i9xx_crtc_clock_get(struct intel_crtc *crtc, struct intel_crtc_state *pipe_config); static void ironlake_pch_clock_get(struct intel_crtc *crtc, @@ -13810,6 +13829,108 @@ void intel_plane_destroy(struct drm_plane *plane) kfree(to_intel_plane(plane)); } +static bool i8xx_mod_supported(uint32_t format, uint64_t modifier) +{ + switch (format) { + case DRM_FORMAT_C8: + case DRM_FORMAT_RGB565: + case DRM_FORMAT_XRGB1555: + case DRM_FORMAT_XRGB: + return modifier == DRM_FORMAT_MOD_LINEAR || + modifier == I915_FORMAT_MOD_X_TILED; + default: + return false; + } +} + +static bool i965_mod_supported(uint32_t format, uint64_t modifier) +{ + switch (format) { + case DRM_FORMAT_C8: + case DRM_FORMAT_RGB565: + case DRM_FORMAT_XRGB: + case DRM_FORMAT_XBGR: + case DRM_FORMAT_XRGB2101010: + case DRM_FORMAT_XBGR2101010: + return modifier == DRM_FORMAT_MOD_LINEAR || + modifier == I915_FORMAT_MOD_X_TILED; + default: + return false; + } +} + +static bool skl_mod_supported(uint32_t format, uint64_t modifier) +{ + switch (format) { + case DRM_FORMAT_C8: + switch (modifier) { + case DRM_FORMAT_MOD_LINEAR: + case I915_FORMAT_MOD_X_TILED: + case I915_FORMAT_MOD_Y_TILED: + return true; + default: + return false; + } + case DRM_FORMAT_RGB565: + case DRM_FORMAT_XRGB: + case DRM_FORMAT_XBGR: + case DRM_FORMAT_ARGB: + case DRM_FORMAT_ABGR: + case DRM_FORMAT_XRGB2101010: + case DRM_FORMAT_XBGR2101010: + case DRM_FORMAT_YUYV: + case DRM_FORMAT_YVYU: + case DRM_FORMAT_UYVY: + case DRM_FORMAT_VYUY: + /* All i915 modifiers are fine */ + switch (modifier) { + case DRM_FORMAT_MOD_LINEAR: + case I915_FORMAT_MOD_X_TILED: + case I915_FORMAT_MOD_Y_TILED: + case I915_FORMAT_MOD_Yf_TILED: + return true; + default: + return false; + } + default: + return false; + } +} + +static bool intel_primary_plane_format_mod_supported(struct drm_plane *plane, +uint32_t format, +uint64_t modifier) +{ + struct drm_i915_private *dev_priv = to_i915(plane->dev); + + if (WARN_ON(modifier == DRM_FORMAT_MOD_INVALID)) + return false; + + if ((modifier >> 56) != DRM_FORMA
[PATCH 2/4] drm: Create a format/modifier blob
Updated blob layout (Rob, Daniel, Kristian, xerpi) v2: * Removed __packed, and alignment (.+) * Fix indent in drm_format_modifier fields (Liviu) * Remove duplicated modifier > 64 check (Liviu) * Change comment about modifier (Liviu) * Remove arguments to blob creation, use plane instead (Liviu) * Fix data types (Ben) * Make the blob part of uapi (Daniel) v3: Remove unused ret field. Change i, and j to unsigned int (Emil) v4: Use plane->modifier_count instead of recounting (Daniel) Cc: Rob Clark Cc: Kristian H. Kristensen Signed-off-by: Ben Widawsky Reviewed-by: Daniel Stone (v2) Reviewed-by: Liviu Dudau (v2) Reviewed-by: Emil Velikov (v3) --- drivers/gpu/drm/drm_mode_config.c | 7 drivers/gpu/drm/drm_plane.c | 83 +++ include/drm/drm_mode_config.h | 6 +++ include/uapi/drm/drm_mode.h | 50 +++ 4 files changed, 146 insertions(+) diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index d9862259a2a7..6bfbc3839df5 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -337,6 +337,13 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.gamma_lut_size_property = prop; + prop = drm_property_create(dev, + DRM_MODE_PROP_IMMUTABLE | DRM_MODE_PROP_BLOB, + "IN_FORMATS", 0); + if (!prop) + return -ENOMEM; + dev->mode_config.modifiers = prop; + return 0; } diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c index d3fc561d7b48..8a2d5765837a 100644 --- a/drivers/gpu/drm/drm_plane.c +++ b/drivers/gpu/drm/drm_plane.c @@ -62,6 +62,86 @@ static unsigned int drm_num_planes(struct drm_device *dev) return num; } +static inline u32 * +formats_ptr(struct drm_format_modifier_blob *blob) +{ + return (u32 *)(((char *)blob) + blob->formats_offset); +} + +static inline struct drm_format_modifier * +modifiers_ptr(struct drm_format_modifier_blob *blob) +{ + return (struct drm_format_modifier *)(((char *)blob) + blob->modifiers_offset); +} + +static int create_in_format_blob(struct drm_device *dev, struct drm_plane *plane) +{ + const struct drm_mode_config *config = &dev->mode_config; + struct drm_property_blob *blob; + struct drm_format_modifier *mod; + size_t blob_size, formats_size, modifiers_size; + struct drm_format_modifier_blob *blob_data; + unsigned int i, j; + + formats_size = sizeof(*plane->format_types) * plane->format_count; + if (WARN_ON(!formats_size)) { + /* 0 formats are never expected */ + return 0; + } + + modifiers_size = + sizeof(struct drm_format_modifier) * plane->modifier_count; + + blob_size = sizeof(struct drm_format_modifier_blob); + /* Modifiers offset is a pointer to a struct with a 64 bit field so it +* should be naturally aligned to 8B. +*/ + blob_size += ALIGN(formats_size, 8); + blob_size += modifiers_size; + + blob = drm_property_create_blob(dev, blob_size, NULL); + if (IS_ERR(blob)) + return -1; + + blob_data = (struct drm_format_modifier_blob *)blob->data; + blob_data->version = FORMAT_BLOB_CURRENT; + blob_data->count_formats = plane->format_count; + blob_data->formats_offset = sizeof(struct drm_format_modifier_blob); + blob_data->count_modifiers = plane->modifier_count; + + blob_data->modifiers_offset = + ALIGN(blob_data->formats_offset + formats_size, 8); + + memcpy(formats_ptr(blob_data), plane->format_types, formats_size); + + /* If we can't determine support, just bail */ + if (!plane->funcs->format_mod_supported) + goto done; + + mod = modifiers_ptr(blob_data); + for (i = 0; i < plane->modifier_count; i++) { + for (j = 0; j < plane->format_count; j++) { + if (plane->funcs->format_mod_supported(plane, + plane->format_types[j], + plane->modifiers[i])) { + + mod->formats |= 1 << j; + } + } + + mod->modifier = plane->modifiers[i]; + mod->offset = 0; + mod->pad = 0; + mod++; + } + +done: + drm_object_attach_property(&plane->base, config->modifiers, + blob->base.id); + + return 0; +} + /** * drm_universal_plane_init - Initialize a new universal plane object * @dev: DRM device @@ -181,6 +261,9 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, drm_object_attach_property(&plane->base, c
Re: [PATCH 1/8] drm/amd/amdgpu: Added asic_type as ACP DMA driver platform data
On Fri, Jun 23, 2017 at 12:43 PM, Christian König wrote: > Am 23.06.2017 um 18:34 schrieb Alex Deucher: >> >> From: Vijendar Mukunda >> >> asic_type information is passed to ACP DMA Driver as platform data. >> We need this to determine whether the asic is Carrizo (CZ) or >> Stoney (ST) in the acp sound driver. >> >> Reviewed-by: Alex Deucher >> Signed-off-by: Vijendar Mukunda >> Signed-off-by: Alex Deucher >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 4 >> sound/soc/amd/acp-pcm-dma.c | 8 ++-- >> sound/soc/amd/acp.h | 7 +++ >> 3 files changed, 13 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c >> index 06879d1..7a2a765 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c >> @@ -262,6 +262,7 @@ static int acp_hw_init(void *handle) >> uint64_t acp_base; >> struct device *dev; >> struct i2s_platform_data *i2s_pdata; >> + u32 asic_type; >> struct amdgpu_device *adev = (struct amdgpu_device *)handle; >> @@ -271,6 +272,7 @@ static int acp_hw_init(void *handle) >> if (!ip_block) >> return -EINVAL; >> + asic_type = adev->asic_type; >> r = amd_acp_hw_init(adev->acp.cgs_device, >> ip_block->version->major, >> ip_block->version->minor); >> /* -ENODEV means board uses AZ rather than ACP */ >> @@ -355,6 +357,8 @@ static int acp_hw_init(void *handle) >> adev->acp.acp_cell[0].name = "acp_audio_dma"; >> adev->acp.acp_cell[0].num_resources = 4; >> adev->acp.acp_cell[0].resources = &adev->acp.acp_res[0]; >> + adev->acp.acp_cell[0].platform_data = &asic_type; >> + adev->acp.acp_cell[0].pdata_size = sizeof(asic_type); > > > Have the painkillers jellyfied my brain or are you setting a pointer in the > amdgpu device structure to some variable on the stack? > > If it's just for initialization then that's probably ok, but I would reset > the pointer at the end of the function. > > Otherwise somebody might have the idea to dereference it later on and that > can only lead to trouble. I think it's ok because the hotplug of the audio device is triggered from this function, so the audio device should be probed by the time this variable goes out of scope. That said, it would probably be better to use the asic_type from the GPU driver instance directly rather than a stack variable. Alex > > Christian. > > >> adev->acp.acp_cell[1].name = "designware-i2s"; >> adev->acp.acp_cell[1].num_resources = 1; >> diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c >> index 08b1399..dcbf997 100644 >> --- a/sound/soc/amd/acp-pcm-dma.c >> +++ b/sound/soc/amd/acp-pcm-dma.c >> @@ -73,12 +73,6 @@ static const struct snd_pcm_hardware >> acp_pcm_hardware_capture = { >> .periods_max = CAPTURE_MAX_NUM_PERIODS, >> }; >> -struct audio_drv_data { >> - struct snd_pcm_substream *play_stream; >> - struct snd_pcm_substream *capture_stream; >> - void __iomem *acp_mmio; >> -}; >> - >> static u32 acp_reg_read(void __iomem *acp_mmio, u32 reg) >> { >> return readl(acp_mmio + (reg * 4)); >> @@ -916,6 +910,7 @@ static int acp_audio_probe(struct platform_device >> *pdev) >> int status; >> struct audio_drv_data *audio_drv_data; >> struct resource *res; >> + const u32 *pdata = pdev->dev.platform_data; >> audio_drv_data = devm_kzalloc(&pdev->dev, sizeof(struct >> audio_drv_data), >> GFP_KERNEL); >> @@ -932,6 +927,7 @@ static int acp_audio_probe(struct platform_device >> *pdev) >> audio_drv_data->play_stream = NULL; >> audio_drv_data->capture_stream = NULL; >> + audio_drv_data->asic_type = *pdata; >> res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); >> if (!res) { >> diff --git a/sound/soc/amd/acp.h b/sound/soc/amd/acp.h >> index 330832e..28cf914 100644 >> --- a/sound/soc/amd/acp.h >> +++ b/sound/soc/amd/acp.h >> @@ -84,6 +84,13 @@ struct audio_substream_data { >> void __iomem *acp_mmio; >> }; >> +struct audio_drv_data { >> + struct snd_pcm_substream *play_stream; >> + struct snd_pcm_substream *capture_stream; >> + void __iomem *acp_mmio; >> + u32 asic_type; >> +}; >> + >> enum { >> ACP_TILE_P1 = 0, >> ACP_TILE_P2, > > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/9] drm/amdgpu: Separate placements and busy placements
This allows a BO to have busy placements that are not part of its normal placements. Users that want the busy placements to be the same can change the placement.busy_placement pointer and corresponding count to be the same as the regular placements. Signed-off-by: John Brooks --- drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 50 +- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 12d61ed..c407d2d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -416,6 +416,7 @@ struct amdgpu_bo { u32 prefered_domains; u32 allowed_domains; struct ttm_placeplacements[AMDGPU_GEM_DOMAIN_MAX + 1]; + struct ttm_placebusy_placements[AMDGPU_GEM_DOMAIN_MAX + 1]; struct ttm_placementplacement; struct ttm_buffer_objecttbo; struct ttm_bo_kmap_obj kmap; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 8ee6965..244a7d3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -116,9 +116,11 @@ bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object *bo) static void amdgpu_ttm_placement_init(struct amdgpu_device *adev, struct ttm_placement *placement, struct ttm_place *places, + struct ttm_place *busy_places, u32 domain, u64 flags) { u32 c = 0; + u32 bc = 0; if (domain & AMDGPU_GEM_DOMAIN_VRAM) { unsigned visible_pfn = adev->mc.visible_vram_size >> PAGE_SHIFT; @@ -135,7 +137,8 @@ static void amdgpu_ttm_placement_init(struct amdgpu_device *adev, if (flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS) places[c].flags |= TTM_PL_FLAG_CONTIGUOUS; - c++; + + busy_places[bc++] = places[c++]; } if (domain & AMDGPU_GEM_DOMAIN_GTT) { @@ -147,7 +150,8 @@ static void amdgpu_ttm_placement_init(struct amdgpu_device *adev, TTM_PL_FLAG_UNCACHED; else places[c].flags |= TTM_PL_FLAG_CACHED; - c++; + + busy_places[bc++] = places[c++]; } if (domain & AMDGPU_GEM_DOMAIN_CPU) { @@ -159,42 +163,47 @@ static void amdgpu_ttm_placement_init(struct amdgpu_device *adev, TTM_PL_FLAG_UNCACHED; else places[c].flags |= TTM_PL_FLAG_CACHED; - c++; + + busy_places[bc++] = places[c++]; } if (domain & AMDGPU_GEM_DOMAIN_GDS) { places[c].fpfn = 0; places[c].lpfn = 0; places[c].flags = TTM_PL_FLAG_UNCACHED | AMDGPU_PL_FLAG_GDS; - c++; + + busy_places[bc++] = places[c++]; } if (domain & AMDGPU_GEM_DOMAIN_GWS) { places[c].fpfn = 0; places[c].lpfn = 0; places[c].flags = TTM_PL_FLAG_UNCACHED | AMDGPU_PL_FLAG_GWS; - c++; + + busy_places[bc++] = places[c++]; } if (domain & AMDGPU_GEM_DOMAIN_OA) { places[c].fpfn = 0; places[c].lpfn = 0; places[c].flags = TTM_PL_FLAG_UNCACHED | AMDGPU_PL_FLAG_OA; - c++; + + busy_places[bc++] = places[c++]; } if (!c) { places[c].fpfn = 0; places[c].lpfn = 0; places[c].flags = TTM_PL_MASK_CACHING | TTM_PL_FLAG_SYSTEM; - c++; + + busy_places[bc++] = places[c++]; } placement->num_placement = c; placement->placement = places; - placement->num_busy_placement = c; - placement->busy_placement = places; + placement->num_busy_placement = bc; + placement->busy_placement = busy_places; } void amdgpu_ttm_placement_from_domain(struct amdgpu_bo *abo, u32 domain) @@ -202,7 +211,7 @@ void amdgpu_ttm_placement_from_domain(struct amdgpu_bo *abo, u32 domain) struct amdgpu_device *adev = amdgpu_ttm_adev(abo->tbo.bdev); amdgpu_ttm_placement_init(adev, &abo->placement, abo->placements, - domain, abo->flags); + abo->busy_placements, domain, abo->flags); } static void amdgpu_fill_placement_to_bo(struct amdgpu_bo *bo, @@ -212,10 +221,12 @@ static void amdgpu_fill_placement_to_bo(struct amdgpu_bo *bo, memcpy(bo->placements, placement->placement, placement->num_plac
[PATCH 5/9] drm/amdgpu: Track time of last page fault and last CS move in struct amdgpu_bo
Signed-off-by: John Brooks --- drivers/gpu/drm/amd/amdgpu/amdgpu.h| 3 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 5 + drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 ++ 3 files changed, 10 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index fe73633..245234e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -429,6 +429,9 @@ struct amdgpu_bo { void*metadata; u32 metadata_size; unsignedprime_shared_count; + unsigned long last_page_fault_jiffies; + unsigned long last_cs_move_jiffies; + /* list of all virtual address to which this bo * is associated to */ diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index aeee684..2fad8bd 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -306,6 +306,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p, struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); u64 initial_bytes_moved; uint32_t domain; + uint32_t old_mem; int r; if (bo->pin_count) @@ -322,6 +323,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p, retry: amdgpu_ttm_placement_from_domain(bo, domain); initial_bytes_moved = atomic64_read(&adev->num_bytes_moved); + old_mem = bo->tbo.mem.mem_type; r = ttm_bo_validate(&bo->tbo, &bo->placement, true, false); p->bytes_moved += atomic64_read(&adev->num_bytes_moved) - initial_bytes_moved; @@ -331,6 +333,9 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p, goto retry; } + if (bo->tbo.mem.mem_type != old_mem) + bo->last_cs_move_jiffies = jiffies; + return r; } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 0ff555a..31d1f21 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -977,6 +977,8 @@ int amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo) if (bo->mem.mem_type != TTM_PL_VRAM) return 0; + abo->last_page_fault_jiffies = jiffies; + size = bo->mem.num_pages << PAGE_SHIFT; offset = bo->mem.start << PAGE_SHIFT; /* TODO: figure out how to map scattered VRAM to the CPU */ -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/9] drm/amdgpu: Set/clear CPU_ACCESS_REQUIRED flag on page fault and CS
When the AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED flag is given by userspace, it should only be treated as a hint to initially place a BO somewhere CPU accessible, rather than having a permanent effect on BO placement. Instead of the flag being set in stone at BO creation, set the flag when a page fault occurs so that it goes somewhere CPU-visible, and clear it when the BO is requested by the GPU. However, clearing the CPU_ACCESS_REQUIRED flag may move a BO to invisible VRAM, which is likely to cause a page fault that moves it right back to GTT. When this happens too much, it is highly detrimental to performance. Only clear the flag on CS if: - The BO wasn't page faulted for a certain amount of time (currently 10 seconds, measured with jiffies), and - its last page fault didn't occur too soon (currently 500ms) after its last CS request, or vice versa. Setting the flag in amdgpu_fault_reserve_notify() also means that we can remove the loop to restrict lpfn to the end of visible VRAM, because amdgpu_ttm_placement_init() will do it for us. Signed-off-by: John Brooks --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 46 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 1 + 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 2fad8bd..73d6882 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -320,6 +320,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p, else domain = bo->allowed_domains; + amdgpu_bo_clear_cpu_access_required(bo); retry: amdgpu_ttm_placement_from_domain(bo, domain); initial_bytes_moved = atomic64_read(&adev->num_bytes_moved); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 31d1f21..a7d48a7 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -967,8 +967,8 @@ int amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo) { struct amdgpu_device *adev = amdgpu_ttm_adev(bo->bdev); struct amdgpu_bo *abo; - unsigned long offset, size, lpfn; - int i, r; + unsigned long offset, size; + int r; if (!amdgpu_ttm_bo_is_amdgpu_bo(bo)) return 0; @@ -991,18 +991,9 @@ int amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo) /* hurrah the memory is not visible ! */ atomic64_inc(&adev->num_vram_cpu_page_faults); + abo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED; amdgpu_ttm_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT); - lpfn = adev->mc.visible_vram_size >> PAGE_SHIFT; - for (i = 0; i < abo->placement.num_placement; i++) { - /* Try to move the BO into visible VRAM */ - if ((abo->placements[i].flags & TTM_PL_FLAG_VRAM) && - (!abo->placements[i].lpfn || -abo->placements[i].lpfn > lpfn)) - abo->placements[i].lpfn = lpfn; - } - abo->placement.busy_placement = abo->placement.placement; - abo->placement.num_busy_placement = abo->placement.num_placement; r = ttm_bo_validate(bo, &abo->placement, false, false); if (unlikely(r != 0)) return r; @@ -1057,3 +1048,34 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo) return bo->tbo.offset; } + +/** + * amdgpu_bo_clear_cpu_access_required + * @bo: BO to update + * + * Clears CPU_ACCESS_REQUIRED flag if the BO hasn't had a page fault in a while + * and it didn't have a page fault too soon after the last time it was moved to + * VRAM. + * + * Caller should have bo reserved. + * + */ +void amdgpu_bo_clear_cpu_access_required(struct amdgpu_bo *bo) +{ + const unsigned int page_fault_timeout_ms = 1; + const unsigned int min_period_ms = 500; + unsigned int ms_since_pf, period_ms; + + ms_since_pf = jiffies_to_msecs(jiffies - bo->last_page_fault_jiffies); + period_ms = jiffies_to_msecs(abs(bo->last_page_fault_jiffies - +bo->last_cs_move_jiffies)); + + /* +* Try to avoid a revolving door between GTT and VRAM. Clearing the +* flag may move this BO back to VRAM, so don't clear it if it's likely +* to page fault and go right back to GTT. +*/ + if ((!bo->last_page_fault_jiffies || !bo->last_cs_move_jiffies) || + (ms_since_pf > page_fault_timeout_ms && period_ms > min_period_ms)) + bo->flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED; +} diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h index 3824851..b0cb137 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_obj
[PATCH 9/9] drm/amdgpu: Reduce lock contention when evicting from visible VRAM
Performing expensive BO moves asynchronously reduces the direct burden on the CS path, but it can still indirectly cause occasional stalls because the worker may reserve the BO for a long time during evictions, and if this coincides with it being needed by CS, the CS path will have to wait. Instead of reserving the BO and keeping it reserved while we wait for ttm_bo_validate() to move it and perform any evictions, we can afford to be more surgical and re-implement the ttm_bo_validate() path in the worker function with some changes to make it friendlier to other threads. Specifically, if evictions are needed when moving a BO to visible VRAM, unreserve the BO while performing them, so as to not block other tasks for too long. Also, sleep for an interval between each eviction so that the worker doesn't hog lru_lock. Signed-off-by: John Brooks --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 88 +- drivers/gpu/drm/ttm/ttm_bo.c | 34 +++- include/drm/ttm/ttm_bo_driver.h| 13 + include/drm/ttm/ttm_placement.h| 6 ++ 4 files changed, 125 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index a69441d..854e037 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -337,10 +337,16 @@ static void amdgpu_bo_move_vis_vram_work_func(struct work_struct *work) struct amdgpu_bo *bo = container_of(work, struct amdgpu_bo, move_vis_vram_work); struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); + struct ttm_placement placement; + struct ttm_mem_reg mem; + struct ttm_mem_type_manager *man = &bo->tbo.bdev->man[TTM_PL_VRAM]; u64 initial_bytes_moved, bytes_moved; uint32_t old_mem; + uint32_t new_flags; int r; + mem.mm_node = NULL; + spin_lock(&adev->mm_stats.lock); if (adev->mm_stats.accum_us_vis <= 0 || adev->mm_stats.accum_us <= 0) { @@ -359,17 +365,97 @@ static void amdgpu_bo_move_vis_vram_work_func(struct work_struct *work) goto out; amdgpu_ttm_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_VRAM); + placement = bo->placement; + + if (ttm_bo_mem_compat(&placement, &bo->tbo.mem, &new_flags)) + goto out; + + mem.num_pages = bo->tbo.num_pages; + mem.size = mem.num_pages << PAGE_SHIFT; + mem.page_alignment = bo->tbo.mem.page_alignment; + mem.bus.io_reserved_vm = false; + mem.bus.io_reserved_count = 0; + + placement.num_busy_placement = 0; + old_mem = bo->tbo.mem.mem_type; initial_bytes_moved = atomic64_read(&adev->num_bytes_moved); - ttm_bo_validate(&bo->tbo, &bo->placement, false, false); + + r = ttm_bo_mem_space(&bo->tbo, &placement, &mem, false, false); + if (r == -ENOMEM) { + /* Unreserve the BO while we make space for it */ + struct ttm_bo_device *bdev = bo->tbo.bdev; + + amdgpu_bo_unreserve(bo); + do { + r = (*man->func->get_node)(man, NULL, + &placement.placement[0], + &mem); + if (unlikely(r != 0)) + return; + + if (mem.mm_node) + break; + + r = ttm_mem_evict_first(bdev, TTM_PL_VRAM, + &placement.placement[0], false, + false); + if (unlikely(r != 0)) + return; + + /* Sleep to give other threads the opportunity to grab +* lru_lock +*/ + msleep(20); + } while (1); + + if (!kref_read(&bo->tbo.kref)) { + /* The BO was deleted since we last held it. Abort. */ + if (mem.mm_node) + (*man->func->put_node)(man, &mem); + return; + } + + r = amdgpu_bo_reserve(bo, true); + if (r != 0) { + if (mem.mm_node) + (*man->func->put_node)(man, &mem); + return; + } + + mem.mem_type = TTM_PL_VRAM; + + r = ttm_bo_add_move_fence(&bo->tbo, man, &mem); + if (unlikely(r != 0)) + goto out; + + mem.placement = TTM_PL_FLAG_VRAM; + mem.placement |= (placement.placement[0].flags & + man->available_caching); + mem.placement |= ttm_bo_select_caching(man, +
[PATCH 2/9] drm/amdgpu: Add vis_vramlimit module parameter
Allow specifying a limit on visible VRAM via a module parameter. This is helpful for testing performance under visible VRAM pressure. Signed-off-by: John Brooks --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 8 3 files changed, 13 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index c407d2d..fe73633 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -74,6 +74,7 @@ */ extern int amdgpu_modeset; extern int amdgpu_vram_limit; +extern int amdgpu_vis_vram_limit; extern int amdgpu_gart_size; extern int amdgpu_moverate; extern int amdgpu_benchmarking; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 4c7c262..a14f458 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -73,6 +73,7 @@ #define KMS_DRIVER_PATCHLEVEL 0 int amdgpu_vram_limit = 0; +int amdgpu_vis_vram_limit = 0; int amdgpu_gart_size = -1; /* auto */ int amdgpu_moverate = -1; /* auto */ int amdgpu_benchmarking = 0; @@ -119,6 +120,9 @@ int amdgpu_lbpw = -1; MODULE_PARM_DESC(vramlimit, "Restrict VRAM for testing, in megabytes"); module_param_named(vramlimit, amdgpu_vram_limit, int, 0600); +MODULE_PARM_DESC(vis_vramlimit, "Restrict visible VRAM for testing, in megabytes"); +module_param_named(vis_vramlimit, amdgpu_vis_vram_limit, int, 0444); + MODULE_PARM_DESC(gartsize, "Size of PCIE/IGP gart to setup in megabytes (32, 64, etc., -1 = auto)"); module_param_named(gartsize, amdgpu_gart_size, int, 0600); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index c9b131b..0676a78 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1095,6 +1095,7 @@ static struct ttm_bo_driver amdgpu_bo_driver = { int amdgpu_ttm_init(struct amdgpu_device *adev) { int r; + u64 vis_vram_limit; r = amdgpu_ttm_global_init(adev); if (r) { @@ -1118,6 +1119,13 @@ int amdgpu_ttm_init(struct amdgpu_device *adev) DRM_ERROR("Failed initializing VRAM heap.\n"); return r; } + + /* Reduce size of CPU-visible VRAM if requested */ + vis_vram_limit = amdgpu_vis_vram_limit * 1024 * 1024; + if (amdgpu_vis_vram_limit > 0 && + vis_vram_limit <= adev->mc.visible_vram_size) + adev->mc.visible_vram_size = vis_vram_limit; + /* Change the size here instead of the init above so only lpfn is affected */ amdgpu_ttm_set_active_vram_size(adev, adev->mc.visible_vram_size); -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/9] Visible VRAM Management Improvements
This patch series is intended to improve performance when limited CPU-visible VRAM is under pressure. Moving BOs into visible VRAM is essentially a housekeeping task. It's faster to access them in VRAM than GTT, but it isn't a hard requirement for them to be in VRAM. As such, it is unnecessary to spend valuable time blocking on this in the page fault handler or during command submission. Doing so translates directly into a longer frame time (ergo stalls and stuttering). The problem worsens when attempting to move BOs into visible VRAM when it is full. This takes much longer than a simple move because other BOs have to be evicted, which involves finding and then moving potentially hundreds of other BOs, which is very time consuming. In the case of limited visible VRAM, it's important to do this sometime to keep the contents of visible VRAM fresh, but it does not need to be a blocking operation. If visible VRAM is full, the BO can be read from GTT in the meantime and the BO can be moved to VRAM later. Thus, I have made it so that neither the command submission code nor page fault handler spends time evicting BOs from visible VRAM, and instead this is deferred to a workqueue function that's queued when CS requests BOs flagged AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED. Speaking of CPU_ACCESS_REQUIRED, I've changed the handling of that flag so that the kernel driver can clear it later even if it was set by userspace. This is because the userspace graphics library can't know whether the application really needs it to be CPU_ACCESS_REQUIRED forever. The kernel driver can't know either, but it does know when page faults occur, and if a BO doesn't appear to have any page faults when it's moved somewhere inaccessible, the flag can be removed and it doesn't have to take up space in CPU-visible memory anymore. This change was based on IRC discussions with Michel. Patch 7 fixes a problem with BO moverate throttling that causes visible VRAM moves to not be throttled if total VRAM isn't full enough. I've also added a vis_vramlimit module parameter for debugging purposes. It's similar to the vramlimit parameter except it limits only visible VRAM. I have tested this patch set with the two games I know to be affected by visible VRAM pressure: DiRT Rally and Dying Light. It practically eliminates eviction-related stuttering in DiRT Rally as well as very low performance if visible VRAM is limited to 64MB. It also fixes severely low framerates that occurred in some areas of Dying Light. All my testing was done with an R9 290 with 4GB of visible VRAM with an Intel i7 4790. -- John Brooks (Frogging101) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/9] drm/amdgpu: Don't force BOs into visible VRAM for page faults
There is no need for page faults to force BOs into visible VRAM if it's full, and the time it takes to do so is great enough to cause noticeable stuttering. Add GTT as a possible placement so that if visible VRAM is full, page faults move BOs to GTT instead of evicting other BOs from VRAM. Signed-off-by: John Brooks --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 244a7d3..751bc05 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -981,10 +981,11 @@ int amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo) /* hurrah the memory is not visible ! */ atomic64_inc(&adev->num_vram_cpu_page_faults); - amdgpu_ttm_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_VRAM); + amdgpu_ttm_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_VRAM | +AMDGPU_GEM_DOMAIN_GTT); lpfn = adev->mc.visible_vram_size >> PAGE_SHIFT; for (i = 0; i < abo->placement.num_placement; i++) { - /* Force into visible VRAM */ + /* Try to move the BO into visible VRAM */ if ((abo->placements[i].flags & TTM_PL_FLAG_VRAM) && (!abo->placements[i].lpfn || abo->placements[i].lpfn > lpfn)) @@ -993,16 +994,13 @@ int amdgpu_bo_fault_reserve_notify(struct ttm_buffer_object *bo) abo->placement.busy_placement = abo->placement.placement; abo->placement.num_busy_placement = abo->placement.num_placement; r = ttm_bo_validate(bo, &abo->placement, false, false); - if (unlikely(r == -ENOMEM)) { - amdgpu_ttm_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_GTT); - return ttm_bo_validate(bo, &abo->placement, false, false); - } else if (unlikely(r != 0)) { + if (unlikely(r != 0)) return r; - } offset = bo->mem.start << PAGE_SHIFT; /* this should never happen */ - if ((offset + size) > adev->mc.visible_vram_size) + if (bo->mem.mem_type == TTM_PL_VRAM && + (offset + size) > adev->mc.visible_vram_size) return -EINVAL; return 0; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/9] drm/amdgpu: Don't force BOs into visible VRAM if they can go to GTT instead
amdgpu_ttm_placement_init() callers that are using both VRAM and GTT as domains usually don't want visible VRAM as a busy placement. Signed-off-by: John Brooks --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 751bc05..0ff555a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -138,7 +138,15 @@ static void amdgpu_ttm_placement_init(struct amdgpu_device *adev, if (flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS) places[c].flags |= TTM_PL_FLAG_CONTIGUOUS; - busy_places[bc++] = places[c++]; + /* Don't set limited visible VRAM as a busy placement if we can +* use GTT instead +*/ + if (!((flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) && + adev->mc.visible_vram_size < adev->mc.real_vram_size && + (domain & AMDGPU_GEM_DOMAIN_GTT))) + busy_places[bc++] = places[c]; + + c++; } if (domain & AMDGPU_GEM_DOMAIN_GTT) { -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 8/9] drm/amdgpu: Asynchronously move BOs to visible VRAM
Moving CPU-accessible BOs from GTT to visible VRAM reduces latency on the GPU and improves average framerate. However, it's an expensive operation. When visible VRAM is full and evictions are necessary, it can easily take tens of milliseconds. On the CS path, that directly increases the frame time and causes noticeable momentary stutters. Unlike other BO move operations, moving BOs to visible VRAM is a housekeeping task and does not have to happen immediately. As a compromise to allow evictions to occur and keep the contents of visible VRAM fresh, but without stalling the rendering pipeline, we can defer these moves to a worker thread. Add a work function that moves a BO into visible VRAM and evicting other BOs if necessary. And during CS, queue this work function for all requested CPU_ACCESS_REQUIRED BOs (subject to the usual move throttling). This decreases the frequency and severity of visible-VRAM-related stuttering. Signed-off-by: John Brooks --- drivers/gpu/drm/amd/amdgpu/amdgpu.h| 5 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 14 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 45 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 7 + 4 files changed, 65 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 81dbb93..a809742 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -436,6 +436,10 @@ struct amdgpu_bo { * is associated to */ struct list_headva; + + /* Work item for moving this BO to visible VRAM asynchronously */ + struct work_struct move_vis_vram_work; + /* Constant after initialization */ struct drm_gem_object gem_base; struct amdgpu_bo*parent; @@ -1583,6 +1587,7 @@ struct amdgpu_device { struct amdgpu_mman mman; struct amdgpu_vram_scratch vram_scratch; struct amdgpu_wbwb; + struct workqueue_struct *vis_vram_wq; atomic64_t vram_usage; atomic64_t vram_vis_usage; atomic64_t gtt_usage; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 25d6df6..9215611 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -341,14 +341,16 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p, if (p->bytes_moved < p->bytes_moved_threshold) { if (adev->mc.visible_vram_size < adev->mc.real_vram_size && (bo->flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED)) { - /* And don't move a CPU_ACCESS_REQUIRED BO to limited -* visible VRAM if we've depleted our allowance to do -* that. + /* Move CPU_ACCESS_REQUIRED BOs to limited visible VRAM +* asynchronously, if we're allowed. */ - if (p->bytes_moved_vis < p->bytes_moved_vis_threshold) - domain = bo->prefered_domains; - else + if (p->bytes_moved_vis < p->bytes_moved_vis_threshold) { + queue_work(adev->vis_vram_wq, + &bo->move_vis_vram_work); + return 0; + } else { domain = bo->allowed_domains; + } } else { domain = bo->prefered_domains; } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 27d8c77..a69441d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -93,6 +93,8 @@ static void amdgpu_ttm_bo_destroy(struct ttm_buffer_object *tbo) bo = container_of(tbo, struct amdgpu_bo, tbo); + cancel_work_sync(&bo->move_vis_vram_work); + amdgpu_update_memory_usage(adev, &bo->tbo.mem, NULL); drm_gem_object_release(&bo->gem_base); @@ -330,6 +332,47 @@ void amdgpu_bo_free_kernel(struct amdgpu_bo **bo, u64 *gpu_addr, *cpu_addr = NULL; } +static void amdgpu_bo_move_vis_vram_work_func(struct work_struct *work) +{ + struct amdgpu_bo *bo = container_of(work, struct amdgpu_bo, + move_vis_vram_work); + struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); + u64 initial_bytes_moved, bytes_moved; + uint32_t old_mem; + int r; + + spin_lock(&adev->mm_stats.lock); + if (adev->mm_stats.accum_us_vis <= 0 || + adev->mm_stats.accum_us <= 0) { + spin_unlock(&adev->mm_stats.lock
[PATCH 7/9] drm/amdgpu: Throttle visible VRAM moves separately
The BO move throttling code is designed to allow VRAM to fill quickly if it is relatively empty. However, this does not take into account situations where the visible VRAM is smaller than total VRAM, and total VRAM may not be close to full but the visible VRAM segment is under pressure. In such situations, visible VRAM would experience unrestricted swapping and performance would drop. Add a separate counter specifically for moves involving visible VRAM, and check it before moving BOs there. Fixes: 95844d20ae02 (drm/amdgpu: throttle buffer migrations at CS using a fixed MBps limit (v2)) Signed-off-by: John Brooks --- drivers/gpu/drm/amd/amdgpu/amdgpu.h| 6 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 76 +++--- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 10 ++-- 3 files changed, 70 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 245234e..81dbb93 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1161,7 +1161,9 @@ struct amdgpu_cs_parser { struct list_headvalidated; struct dma_fence*fence; uint64_tbytes_moved_threshold; + uint64_tbytes_moved_vis_threshold; uint64_tbytes_moved; + uint64_tbytes_moved_vis; struct amdgpu_bo_list_entry *evictable; /* user fence */ @@ -1595,6 +1597,7 @@ struct amdgpu_device { spinlock_t lock; s64 last_update_us; s64 accum_us; /* accumulated microseconds */ + s64 accum_us_vis; /* for visible VRAM */ u32 log2_max_MBps; } mm_stats; @@ -1930,7 +1933,8 @@ bool amdgpu_need_post(struct amdgpu_device *adev); void amdgpu_update_display_priority(struct amdgpu_device *adev); int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data); -void amdgpu_cs_report_moved_bytes(struct amdgpu_device *adev, u64 num_bytes); +void amdgpu_cs_report_moved_bytes(struct amdgpu_device *adev, u64 num_bytes, + u64 num_vis_bytes); void amdgpu_ttm_placement_from_domain(struct amdgpu_bo *abo, u32 domain); bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object *bo); int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 73d6882..25d6df6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -223,11 +223,13 @@ static s64 bytes_to_us(struct amdgpu_device *adev, u64 bytes) * ticks. The accumulated microseconds (us) are converted to bytes and * returned. */ -static u64 amdgpu_cs_get_threshold_for_moves(struct amdgpu_device *adev) +static void amdgpu_cs_get_threshold_for_moves(struct amdgpu_device *adev, + u64 *max_bytes, + u64 *max_vis_bytes) { s64 time_us, increment_us; - u64 max_bytes; u64 free_vram, total_vram, used_vram; + u64 free_vis_vram, total_vis_vram, used_vis_vram; /* Allow a maximum of 200 accumulated ms. This is basically per-IB * throttling. @@ -238,13 +240,21 @@ static u64 amdgpu_cs_get_threshold_for_moves(struct amdgpu_device *adev) */ const s64 us_upper_bound = 20; - if (!adev->mm_stats.log2_max_MBps) - return 0; + if (!adev->mm_stats.log2_max_MBps) { + *max_bytes = 0; + *max_vis_bytes = 0; + return; + } total_vram = adev->mc.real_vram_size - adev->vram_pin_size; used_vram = atomic64_read(&adev->vram_usage); free_vram = used_vram >= total_vram ? 0 : total_vram - used_vram; + total_vis_vram = adev->mc.visible_vram_size; + used_vis_vram = atomic64_read(&adev->vram_vis_usage); + free_vis_vram = used_vis_vram >= total_vis_vram ? 0 : total_vis_vram - + used_vis_vram; + spin_lock(&adev->mm_stats.lock); /* Increase the amount of accumulated us. */ @@ -252,7 +262,9 @@ static u64 amdgpu_cs_get_threshold_for_moves(struct amdgpu_device *adev) increment_us = time_us - adev->mm_stats.last_update_us; adev->mm_stats.last_update_us = time_us; adev->mm_stats.accum_us = min(adev->mm_stats.accum_us + increment_us, - us_upper_bound); + us_upper_bound); + adev->mm_stats.accum_us_vis = min(adev->mm_stats.accum_us_vis + + increment_us, us_upper_bound); /* This prevents the shor
[Bug 101565] Intermittent graphical corruption since "radeonsi: don't emit partial flushes at the end of IBs (v2)" (c9040dc9)
https://bugs.freedesktop.org/show_bug.cgi?id=101565 Marek Olšák changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #1 from Marek Olšák --- I reverted the commit. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 101330] segfault in pcsxr
https://bugs.freedesktop.org/show_bug.cgi?id=101330 --- Comment #6 from Vincent B. --- Core was generated by `pcsxr'. Program terminated with signal SIGSEGV, Segmentation fault. #0 0xe64f8128 in radeon_bo_do_map (bo=0x931b0f8) at radeon_drm_bo.c:466 466 radeon_drm_bo.c: Aucun fichier ou dossier de ce type. [Current thread is 1 (Thread 0xf5ab2fc0 (LWP 4362))] (gdb) bt #0 0xe64f8128 in radeon_bo_do_map (bo=0x931b0f8) at radeon_drm_bo.c:466 #1 0xe6529342 in r600_buffer_transfer_map (ctx=0x965c168, resource=0x955cc30, level=0, usage=3758099970, box=0xffad0cc8, ptransfer=0x9313f54) at r600_buffer_common.c:480 #2 0xe620e6d8 in tc_transfer_map (_pipe=0x9771850, resource=0x955cc30, level=0, usage=3758099970, box=0xffad0cc8, transfer=0x9313f54) at util/u_threaded_context.c:1396 #3 0xe5fd1384 in pipe_buffer_map_range (transfer=0x9313f54, access=, length=65536, offset=0, buffer=, pipe=) at ../../src/gallium/auxiliary/util/u_inlines.h:307 #4 st_bufferobj_map_range (ctx=0x97796c8, offset=0, length=65536, access=16438, obj=0x9313ed8, index=MAP_INTERNAL) at state_tracker/st_cb_bufferobjects.c:433 #5 0xe5fad656 in vbo_exec_vtx_map (exec=0x979dadc) at vbo/vbo_exec_draw.c:342 #6 0xe5f98114 in vbo_Vertex3fv (v=0xe6afaec0 ) at vbo/vbo_attrib_tmp.h:280 #7 0xe6ab5842 in primTileS () from /home/vb/.pcsxr/plugins//libpeopsxgl.so #8 0xe6aacf01 in GPUwriteDataMem () from /home/vb/.pcsxr/plugins//libpeopsxgl.so #9 0xe6aad44c in GPUdmaChain () from /home/vb/.pcsxr/plugins//libpeopsxgl.so #10 0x080961f1 in ?? () #11 0x08097042 in ?? () #12 0xf4d77fbd in ?? () Backtrace stopped: previous frame inner to this frame (corrupt stack?) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 101561] [bisected] [llvm] Blue color shift on videos in MPV when using vo=opengl[-hq]
https://bugs.freedesktop.org/show_bug.cgi?id=101561 --- Comment #6 from Andy Furniss --- (In reply to Andy Furniss from comment #5) > Though I couldn't add the author to CC :-( Oops ignore that bit - the author is addable, I was trying to add the llvm username which is different. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/stm: Fixup for "drm/stm: ltdc: Add panel-bridge support"
Philippe CORNU writes: > On 06/22/2017 07:56 PM, Eric Anholt wrote: >> Signed-off-by: Eric Anholt >> --- >> >> This fixup would be squashed into patch 1 of your series. > > Hi Eric, > and many thanks for the two patches, I will follow your suggestion for > the v5 serie. > By the way, do you have more comments on the "drm/stm: ltdc: Add > panel-bridge support" patch? I think that's all -- I was going to add my r-b and apply that one, when I found this little nitpick. signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/7] dt-bindings: Document the Raspberry Pi Touchscreen nodes.
On Thu, Jun 15, 2017 at 01:41:28PM -0700, Eric Anholt wrote: > This doesn't yet cover input, but the driver does get the display > working when the firmware is disabled from talking to our I2C lines. > > Signed-off-by: Eric Anholt > --- > .../panel/raspberrypi,7inch-touchscreen.txt| 49 > ++ > 1 file changed, 49 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/raspberrypi,7inch-touchscreen.txt Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: R7-250X: Vulkan: Doom lockups.
On Fri, Jun 16, 2017 at 1:02 AM, Emil Velikov wrote: > Hi Mike, > > On 16 June 2017 at 05:48, Mike Mestnik wrote: >> I can tell by the 2 color(green and violet) rendering that this driver >> is experimental. Attached is kern.log from some testing I've done. >> > You might be interested in the following bug report. It's the first > hit on google under "radv doom green" ;-) > > https://bugs.freedesktop.org/show_bug.cgi?id=99467 > > -Emil That's great, but I was more reporting the lockups. Like the DRM should be catching something it's not. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98238] Witcher 2: objects are black when changing lod on Radeon Pitcairn
https://bugs.freedesktop.org/show_bug.cgi?id=98238 --- Comment #21 from Shmerl --- It's now in master. It should eventually land in /etc/drirc in distros, but until then, build Mesa from source and set this variable when running the game: glsl_correct_derivatives_after_discard=true -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] DRM: Fix an incorrectly formatted table
The "supported input formats" table in dw_hdmi.h was incorrectly formatted, using "+" signs where "|" needs to be. That, in turn, causes the PDF build to fail. Fixes: def23aa7e9821a3dfe3fb7b139dd0229a89fdeb0 Signed-off-by: Jonathan Corbet --- include/drm/bridge/dw_hdmi.h | 70 ++-- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h index ed599bea3f6c..c00671fef53c 100644 --- a/include/drm/bridge/dw_hdmi.h +++ b/include/drm/bridge/dw_hdmi.h @@ -22,56 +22,56 @@ struct dw_hdmi; * 48bit bus. * * +--+--+--+ - * + Format Name + Format Code + Encodings + + * | Format Name | Format Code | Encodings | * +--+--+--+ - * + RGB 4:4:4 8bit + ``MEDIA_BUS_FMT_RGB888_1X24``+ ``V4L2_YCBCR_ENC_DEFAULT`` + + * | RGB 4:4:4 8bit | ``MEDIA_BUS_FMT_RGB888_1X24``| ``V4L2_YCBCR_ENC_DEFAULT`` | * +--+--+--+ - * + RGB 4:4:4 10bits + ``MEDIA_BUS_FMT_RGB101010_1X30`` + ``V4L2_YCBCR_ENC_DEFAULT`` + + * | RGB 4:4:4 10bits | ``MEDIA_BUS_FMT_RGB101010_1X30`` | ``V4L2_YCBCR_ENC_DEFAULT`` | * +--+--+--+ - * + RGB 4:4:4 12bits + ``MEDIA_BUS_FMT_RGB121212_1X36`` + ``V4L2_YCBCR_ENC_DEFAULT`` + + * | RGB 4:4:4 12bits | ``MEDIA_BUS_FMT_RGB121212_1X36`` | ``V4L2_YCBCR_ENC_DEFAULT`` | * +--+--+--+ - * + RGB 4:4:4 16bits + ``MEDIA_BUS_FMT_RGB161616_1X48`` + ``V4L2_YCBCR_ENC_DEFAULT`` + + * | RGB 4:4:4 16bits | ``MEDIA_BUS_FMT_RGB161616_1X48`` | ``V4L2_YCBCR_ENC_DEFAULT`` | * +--+--+--+ - * + YCbCr 4:4:4 8bit + ``MEDIA_BUS_FMT_YUV8_1X24`` + ``V4L2_YCBCR_ENC_601`` + - * + + + or ``V4L2_YCBCR_ENC_709``+ - * + + + or ``V4L2_YCBCR_ENC_XV601`` + - * + + + or ``V4L2_YCBCR_ENC_XV709`` + + * | YCbCr 4:4:4 8bit | ``MEDIA_BUS_FMT_YUV8_1X24`` | ``V4L2_YCBCR_ENC_601`` | + * | | | or ``V4L2_YCBCR_ENC_709``| + * | | | or ``V4L2_YCBCR_ENC_XV601`` | + * | | | or ``V4L2_YCBCR_ENC_XV709`` | * +--+--+--+ - * + YCbCr 4:4:4 10bits + ``MEDIA_BUS_FMT_YUV10_1X30`` + ``V4L2_YCBCR_ENC_601`` + - * + + + or ``V4L2_YCBCR_ENC_709``+ - * + + + or ``V4L2_YCBCR_ENC_XV601`` + - * + + + or ``V4L2_YCBCR_ENC_XV709`` + + * | YCbCr 4:4:4 10bits | ``MEDIA_BUS_FMT_YUV10_1X30`` | ``V4L2_YCBCR_ENC_601`` | + * | | | or ``V4L2_YCBCR_ENC_709``| + * | | | or ``V4L2_YCBCR_ENC_XV601`` | + * | | | or ``V4L2_YCBCR_ENC_XV709`` | * +--+--+--+ - * + YCbCr 4:4:4 12bits + ``MEDIA_BUS_FMT_YUV12_1X36`` + ``V4L2_YCBCR_ENC_601`` + - * + + + or ``V4L2_YCBCR_ENC_709``+ - * + + + or ``V4L2_YCBCR_ENC_XV601`` + - * + + + or ``V4L2_YCBCR_ENC_XV709`` + + * | YCbCr 4:4:4 12bits | ``MEDIA_BUS_FMT_YUV12_1X36`` | ``V4L2_YCBCR_ENC_601`` | + * | | | or ``V4L2_YCBCR_ENC_709``| + * | | | or ``V4L2_YCBCR_ENC_XV601`` | + * | | | or ``V4L2_YCBCR_ENC_XV709`` | * +--+--+--+ - * + YCbCr 4:4:4 16bits + ``MEDIA_BUS_FMT_YUV16_1X48`` + ``V4L2_YCBCR_ENC_601`` + - * + + + or ``V4L2_YCBCR_ENC_709``+ - * + + +
[Bug 101222] [amd-staging] No DisplayPort surround sound with DC on RX 480 (HDMI works)
https://bugs.freedesktop.org/show_bug.cgi?id=101222 James Le Cuirot changed: What|Removed |Added Summary|[amd-staging] No HDMI/DP|[amd-staging] No |surround sound with DC on |DisplayPort surround sound |RX 480 |with DC on RX 480 (HDMI ||works) --- Comment #9 from James Le Cuirot --- (In reply to Mikko Autio from comment #7) > (In reply to James Le Cuirot from comment #6) > > (In reply to Mikko Autio from comment #4) > > > Attached patch fixes multi-channel HDMI audio on my RX 460. > > > > I was about to shower you with praise but sadly that patch made no > > difference here. :( > > Maybe there is something else broken with DisplayPort audio? Yes! I switched the outputs around (one was just using an adapter) and now it works! I really should have tried that sooner. This arrangement is fine for me as the monitor is just stereo anyway. LPCM works and so does ALSA pass-through but PulseAudio is refusing to do pass-through for some reason. I've had it working in the past but I can live with that for now. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/9] Visible VRAM Management Improvements
Hi John, I haven't read your patches. Just a question based on the cover letter. I understand that visible VRAM is the biggest pain point. But could the same reasoning make sense for invisible VRAM? That is, doing all the migrations to VRAM in a workqueue? Regards, Felix On 17-06-23 01:39 PM, John Brooks wrote: > This patch series is intended to improve performance when limited CPU-visible > VRAM is under pressure. > > Moving BOs into visible VRAM is essentially a housekeeping task. It's faster > to > access them in VRAM than GTT, but it isn't a hard requirement for them to be > in > VRAM. As such, it is unnecessary to spend valuable time blocking on this in > the > page fault handler or during command submission. Doing so translates directly > into a longer frame time (ergo stalls and stuttering). > > The problem worsens when attempting to move BOs into visible VRAM when it is > full. This takes much longer than a simple move because other BOs have to be > evicted, which involves finding and then moving potentially hundreds of other > BOs, which is very time consuming. In the case of limited visible VRAM, it's > important to do this sometime to keep the contents of visible VRAM fresh, but > it does not need to be a blocking operation. If visible VRAM is full, the BO > can be read from GTT in the meantime and the BO can be moved to VRAM later. > > Thus, I have made it so that neither the command submission code nor page > fault > handler spends time evicting BOs from visible VRAM, and instead this is > deferred to a workqueue function that's queued when CS requests BOs flagged > AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED. > > Speaking of CPU_ACCESS_REQUIRED, I've changed the handling of that flag so > that > the kernel driver can clear it later even if it was set by userspace. This is > because the userspace graphics library can't know whether the application > really needs it to be CPU_ACCESS_REQUIRED forever. The kernel driver can't > know > either, but it does know when page faults occur, and if a BO doesn't appear to > have any page faults when it's moved somewhere inaccessible, the flag can be > removed and it doesn't have to take up space in CPU-visible memory anymore. > This change was based on IRC discussions with Michel. > > Patch 7 fixes a problem with BO moverate throttling that causes visible VRAM > moves to not be throttled if total VRAM isn't full enough. > > I've also added a vis_vramlimit module parameter for debugging purposes. It's > similar to the vramlimit parameter except it limits only visible VRAM. > > I have tested this patch set with the two games I know to be affected by > visible VRAM pressure: DiRT Rally and Dying Light. It practically eliminates > eviction-related stuttering in DiRT Rally as well as very low performance if > visible VRAM is limited to 64MB. It also fixes severely low framerates that > occurred in some areas of Dying Light. All my testing was done with an R9 290 > with 4GB of visible VRAM with an Intel i7 4790. > > -- > John Brooks (Frogging101) > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/6] dt-bindings: display: Add Synopsys DW MIPI DSI DRM bridge driver
On Mon, Jun 19, 2017 at 11:51 AM, Philippe CORNU wrote: > On 06/08/2017 05:40 PM, Rob Herring wrote: >> On Fri, Jun 02, 2017 at 04:37:11PM +0200, Philippe CORNU wrote: >>> This patch adds documentation of device tree bindings for the >>> Synopsys DesignWare MIPI DSI host DRM bridge driver. >>> >>> Signed-off-by: Philippe CORNU >>> --- >>> .../bindings/display/bridge/dw_mipi_dsi.txt| 30 >>> ++ >>> 1 file changed, 30 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt >>> >>> diff --git >>> a/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt >>> b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt >>> new file mode 100644 >>> index 000..1d7c438 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/display/bridge/dw_mipi_dsi.txt >>> @@ -0,0 +1,30 @@ >>> +Synopsys DesignWare MIPI DSI host controller >>> + >>> + >>> +This document defines device tree properties for the Synopsys DesignWare >>> MIPI >>> +DSI host controller. It doesn't constitue a device tree binding >>> specification >>> +by itself but is meant to be referenced by platform-specific device tree >>> +bindings. >>> + >>> +When referenced from platform device tree bindings the properties defined >>> in >>> +this document are defined as follows. The platform device tree bindings are >>> +responsible for defining whether each property is required or optional. >>> + >>> +- reg: Memory mapped base address and length of the DWC MIPI DSI >>> + registers. (mandatory) >>> + >>> +- clocks: References to all the clocks specified in the clock-names >>> property >>> + as specified in [1]. (mandatory) >>> + >>> +- clock-names: "pclk" is peripheral clock for either AHB and APB. >>> (mandatory) >> >> Seems strange there's not also a pixel or bit clock? Or this gets driven >> from the phy? >> > Hi Rob, > And many thanks for your comments :) > > There is a "physical" pixel clock entering into the "DSI controller IP" > but the "DSI controller driver" does not need to control (or read) it > with the dt because this clock information (the frequency) is also > available in panel timings and the drm/kms framework will propagate the > panel timings in the drm/kms "crtc/encoder/bridge&panel/connector..." > chain. Then, the DSI controller driver will compute phy parameters > according to these panel timings. > Adding a pixel clock dependency in the dt here is then not necessary as > the frequency information comes through the panel timings. Even if the Linux driver doesn't currently need to control the pixel clock it should still be defined in the binding. Bindings are what you physically have, not just what the driver needs or doesn't need. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 2/6] dt-bindings: display: Add Synopsys DW MIPI DSI DRM bridge driver
On Mon, Jun 19, 2017 at 06:28:01PM +0200, Philippe CORNU wrote: > This patch adds documentation of device tree bindings for the > Synopsys DesignWare MIPI DSI host DRM bridge driver. You missed Archit's comment on v3. Bindings are for h/w, not drivers. Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 5/6] dt-bindings: display: stm32: Add DSI host driver
On Mon, Jun 19, 2017 at 06:28:04PM +0200, Philippe CORNU wrote: > This patch adds documentation of device tree bindings for the STM32 > DSI host driver based on the Synopsys DW MIPI DSI bridge driver. Same comment here. With that, Acked-by: Rob Herring > > Signed-off-by: Philippe CORNU > Reviewed-by: Neil Armstrong > --- > .../devicetree/bindings/display/st,stm32-ltdc.txt | 104 > - > 1 file changed, 103 insertions(+), 1 deletion(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/7] drm/bridge: Support hotplugging panel-bridge.
Archit Taneja writes: > On 06/22/2017 01:20 PM, Benjamin Gaignard wrote: >> 2017-06-20 19:31 GMT+02:00 Eric Anholt : >>> Archit Taneja writes: >>> On 06/16/2017 08:13 PM, Eric Anholt wrote: > Archit Taneja writes: > >> On 06/16/2017 02:11 AM, Eric Anholt wrote: >>> If the panel-bridge is being set up after the drm_mode_config_reset(), >>> then the connector's state would never get initialized, and we'd >>> dereference the NULL in the hotplug path. We also need to register >>> the connector, so that userspace can get at it. >>> >> >> Shouldn't the KMS driver make sure the panel-bridge is set up before >> drm_mode_config_reset? Is it the case when we're inserting the >> panel-bridge driver as a module? >> >> >> All the connectors that have been added are registered automatically >> when drm_dev_register() is called by the KMS driver. Registering a >> connector in the middle of setting up our driver is prone to race >> conditions if the userspace decides to use them immediately. > > Yeah, this is fixing initializing panel_bridge at DSI host_attach time, > which in the case of a panel module that creates the DSI device > (adv7533-style, like you said I should use as a reference) will be after > drm_mode_config_reset() and drm_dev_register(). Okay. In the case of the msm kms driver, we defer probe until the adv7533 module is inserted, only then we proceed to drm_mode_config_reset() and drm_dev_register(). I assumed this was the general practice followed by most kms drivers. I.,e the kms driver defers probe until all connector related modules are inserted, and only then proceed to create a drm device. >>> >>> The problem, though, is the panel driver needs the MIPI DSI host to >>> exist to call mipi_dsi_device_register_full() during the probe process. >>> The adv7533 driver gets around this by registering the DSI device in the >>> bridge attach step, but drm_panel doesn't have an attach step. > > I'm not sure how we can get around this. We had discussion about this on irc > recently, but couldn't come up with a good conclusion. We could come up with a > panel_attach() callback to make it similar to bridges, but that's just us > avoiding > the real issue. > >>> >>> Another alternative is my original version of the panel driver that was >>> a mipi_dsi_device driver that registered the panel during the DSI device >>> probe. That's why vc4's panel lookup is during the MIPI DSI attach >>> phase, currently. >> > > This would require you to have a DSI device node in DT, rather than an i2c > node, right? I don't know if we should do that because of a limitation in > our drm_mipi_dsi and drm_panel frameworks. All versions of this patch have had one of those, because either that's where you attach the driver (the first, no-core-changes version of the driver) or you need it for the of-graph connections anyway. These later versions just haven't had a compatible string on the DSI device node. signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: R7-250X: Vulkan: Doom lockups.
After applying the patches from the bug report the lockup was immediate, black screen. Jun 23 15:50:48 agartha kernel: [ 199.021303] amdgpu :01:00.0: GPU fault detected: 146 0x0b22080c Jun 23 15:50:48 agartha kernel: [ 199.021308] amdgpu :01:00.0: VM_CONTEXT1_PROTECTION_FAULT_ADDR 0x00100059 Jun 23 15:50:48 agartha kernel: [ 199.021310] amdgpu :01:00.0: VM_CONTEXT1_PROTECTION_FAULT_STATUS 0x0200800C Jun 23 15:50:48 agartha kernel: [ 199.021313] amdgpu :01:00.0: VM fault (0x0c, vmid 1) at page 1048665, read from '' (0x) (8) Jun 23 15:53:33 agartha kernel: [ 363.707517] INFO: task Xorg:952 blocked for more than 120 seconds. Jun 23 15:53:33 agartha kernel: [ 363.707522] Not tainted 4.11.0-trunk-amd64 #1 Debian 4.11.3-1~exp1~cheako0~artful0 Jun 23 15:53:33 agartha kernel: [ 363.707524] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. Jun 23 15:53:33 agartha kernel: [ 363.707525] XorgD0 952950 0x0004 Jun 23 15:53:33 agartha kernel: [ 363.707527] Call Trace: Jun 23 15:53:33 agartha kernel: [ 363.707535] ? __schedule+0x3c2/0x8b0 Jun 23 15:53:33 agartha kernel: [ 363.707537] ? __kfifo_in+0x2d/0x40 Jun 23 15:53:33 agartha kernel: [ 363.707539] ? schedule+0x32/0x80 Jun 23 15:53:33 agartha kernel: [ 363.707574] ? amd_sched_entity_push_job+0xb9/0x100 [amdgpu] Jun 23 15:53:33 agartha kernel: [ 363.707576] ? remove_wait_queue+0x60/0x60 Jun 23 15:53:33 agartha kernel: [ 363.707593] ? amdgpu_job_submit+0x6e/0x90 [amdgpu] Jun 23 15:53:33 agartha kernel: [ 363.707608] ? amdgpu_vm_bo_split_mapping+0x510/0x6f0 [amdgpu] Jun 23 15:53:33 agartha kernel: [ 363.707629] ? amdgpu_gem_prime_export+0x50/0x50 [amdgpu] Jun 23 15:53:33 agartha kernel: [ 363.707644] ? amdgpu_vm_bo_update+0x148/0x340 [amdgpu] Jun 23 15:53:33 agartha kernel: [ 363.707657] ? amdgpu_gem_va_ioctl+0x3c9/0x400 [amdgpu] Jun 23 15:53:33 agartha kernel: [ 363.707659] ? __check_object_size+0xfb/0x196 Jun 23 15:53:33 agartha kernel: [ 363.707677] ? drm_ioctl+0x1ef/0x440 [drm] Jun 23 15:53:33 agartha kernel: [ 363.707684] ? drm_ioctl+0x1ef/0x440 [drm] Jun 23 15:53:33 agartha kernel: [ 363.707697] ? amdgpu_gem_metadata_ioctl+0x1c0/0x1c0 [amdgpu] Jun 23 15:53:33 agartha kernel: [ 363.707699] ? ep_scan_ready_list.constprop.13+0x24f/0x260 Jun 23 15:53:33 agartha kernel: [ 363.707701] ? __hrtimer_init+0xc0/0xc0 Jun 23 15:53:33 agartha kernel: [ 363.707702] ? timerqueue_add+0x54/0xa0 Jun 23 15:53:33 agartha kernel: [ 363.707713] ? amdgpu_drm_ioctl+0x49/0x80 [amdgpu] Jun 23 15:53:33 agartha kernel: [ 363.707714] ? do_vfs_ioctl+0x9f/0x600 Jun 23 15:53:33 agartha kernel: [ 363.707716] ? __sys_recvmsg+0x4e/0x90 Jun 23 15:53:33 agartha kernel: [ 363.707717] ? __sys_recvmsg+0x7d/0x90 Jun 23 15:53:33 agartha kernel: [ 363.707718] ? SyS_ioctl+0x74/0x80 Jun 23 15:53:33 agartha kernel: [ 363.707720] ? system_call_fast_compare_end+0xc/0x9b Jun 23 15:55:33 agartha kernel: [ 484.547113] INFO: task Xorg:952 blocked for more than 120 seconds. Jun 23 15:55:33 agartha kernel: [ 484.547119] Not tainted 4.11.0-trunk-amd64 #1 Debian 4.11.3-1~exp1~cheako0~artful0 Jun 23 15:55:33 agartha kernel: [ 484.547120] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. Jun 23 15:55:33 agartha kernel: [ 484.547122] XorgD0 952950 0x0004 Jun 23 15:55:33 agartha kernel: [ 484.547124] Call Trace: Jun 23 15:55:33 agartha kernel: [ 484.547132] ? __schedule+0x3c2/0x8b0 Jun 23 15:55:33 agartha kernel: [ 484.547134] ? __kfifo_in+0x2d/0x40 Jun 23 15:55:33 agartha kernel: [ 484.547136] ? schedule+0x32/0x80 Jun 23 15:55:33 agartha kernel: [ 484.547177] ? amd_sched_entity_push_job+0xb9/0x100 [amdgpu] Jun 23 15:55:33 agartha kernel: [ 484.547179] ? remove_wait_queue+0x60/0x60 Jun 23 15:55:33 agartha kernel: [ 484.547197] ? amdgpu_job_submit+0x6e/0x90 [amdgpu] Jun 23 15:55:33 agartha kernel: [ 484.547212] ? amdgpu_vm_bo_split_mapping+0x510/0x6f0 [amdgpu] Jun 23 15:55:33 agartha kernel: [ 484.547227] ? amdgpu_gem_prime_export+0x50/0x50 [amdgpu] Jun 23 15:55:33 agartha kernel: [ 484.547241] ? amdgpu_vm_bo_update+0x148/0x340 [amdgpu] Jun 23 15:55:33 agartha kernel: [ 484.547255] ? amdgpu_gem_va_ioctl+0x3c9/0x400 [amdgpu] Jun 23 15:55:33 agartha kernel: [ 484.547257] ? __check_object_size+0xfb/0x196 Jun 23 15:55:33 agartha kernel: [ 484.547273] ? drm_ioctl+0x1ef/0x440 [drm] Jun 23 15:55:33 agartha kernel: [ 484.547279] ? drm_ioctl+0x1ef/0x440 [drm] Jun 23 15:55:33 agartha kernel: [ 484.547293] ? amdgpu_gem_metadata_ioctl+0x1c0/0x1c0 [amdgpu] Jun 23 15:55:33 agartha kernel: [ 484.547295] ? ep_scan_ready_list.constprop.13+0x24f/0x260 Jun 23 15:55:33 agartha kernel: [ 484.547297] ? __hrtimer_init+0xc0/0xc0 Jun 23 15:55:33 agartha kernel: [ 484.547298] ? timerqueue_add+0x54/0xa0 Jun 23 15:55:33 agartha kernel: [ 484.547309] ? amdgpu_drm_ioctl+0x49/0x80 [amdgpu] Jun 23 15:55:33 agartha kernel: [ 484.
Re: [PATCH 1/7] drm/bridge: Support hotplugging panel-bridge.
Boris Brezillon writes: > On Thu, 22 Jun 2017 13:47:43 +0530 > Archit Taneja wrote: > >> On 06/22/2017 01:20 PM, Benjamin Gaignard wrote: >> > 2017-06-20 19:31 GMT+02:00 Eric Anholt : >> >> Archit Taneja writes: >> >> >> >>> On 06/16/2017 08:13 PM, Eric Anholt wrote: >> Archit Taneja writes: >> >> > On 06/16/2017 02:11 AM, Eric Anholt wrote: >> >> If the panel-bridge is being set up after the drm_mode_config_reset(), >> >> then the connector's state would never get initialized, and we'd >> >> dereference the NULL in the hotplug path. We also need to register >> >> the connector, so that userspace can get at it. >> >> >> > >> > Shouldn't the KMS driver make sure the panel-bridge is set up before >> > drm_mode_config_reset? Is it the case when we're inserting the >> > panel-bridge driver as a module? >> > >> > >> > All the connectors that have been added are registered automatically >> > when drm_dev_register() is called by the KMS driver. Registering a >> > connector in the middle of setting up our driver is prone to race >> > conditions if the userspace decides to use them immediately. >> >> Yeah, this is fixing initializing panel_bridge at DSI host_attach time, >> which in the case of a panel module that creates the DSI device >> (adv7533-style, like you said I should use as a reference) will be after >> drm_mode_config_reset() and drm_dev_register(). >> >>> >> >>> Okay. In the case of the msm kms driver, we defer probe until the >> >>> adv7533 module is inserted, only then we proceed to >> >>> drm_mode_config_reset() >> >>> and drm_dev_register(). I assumed this was the general practice followed >> >>> by >> >>> most kms drivers. I.,e the kms driver defers probe until all connector >> >>> related modules are inserted, and only then proceed to create a drm >> >>> device. >> >> >> >> The problem, though, is the panel driver needs the MIPI DSI host to >> >> exist to call mipi_dsi_device_register_full() during the probe process. >> >> The adv7533 driver gets around this by registering the DSI device in the >> >> bridge attach step, but drm_panel doesn't have an attach step. >> >> I'm not sure how we can get around this. We had discussion about this on irc >> recently, but couldn't come up with a good conclusion. We could come up with >> a >> panel_attach() callback to make it similar to bridges, but that's just us >> avoiding >> the real issue. > > How about making DSI dev registration fully asynchronous, that is, DSI > devs declared in the DT under the DSI host node will be > registered/attached at probe time, and devs using another control bus > (like the adv7533 controller over i2c) will be registered afterwards. > > That implies moving the drm_brige registration logic outside of the DSI > host ->probe() path. The idea would be to check if all devs connected > to the DSI bus are ready at dsi_host->attach() time. If they are, we > can finally register the XXX -> DSI bridge. If they're not (because > some devs connected to the DSI bus have not been probed yet), then we > do not register the drm_bridge and wait for the next dsi_host->attach() > event. > > With this solution, I think we can avoid the chicken&egg problem where > the adv7533 dev is waiting for the DSI host to be probed to be able to > register a DSI dev with mipi_dsi_device_register_full() and the DSI > host needs all devs to be registered to not return -EPROBE_DEFER. I've now tried having mipi_dsi_device_register_full() succeed early and just do the device-add part when the host shows up. The problem is there's still mipi_dsi_attach(), which needs to be delayed until the panel driver fills in the rest of mipi_dsi_device's fields. Why aren't those part of the info? signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 98619] amdgpu 0000:01:00.0: GPU fault detected: 146 0x09d88404
https://bugs.freedesktop.org/show_bug.cgi?id=98619 --- Comment #8 from Mike Mestnik --- I've confirmed this VM Fault is preset with the patch applied for Doom. https://lists.freedesktop.org/archives/dri-devel/2017-June/145248.html -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/9] Visible VRAM Management Improvements
On Fri, Jun 23, 2017 at 05:02:58PM -0400, Felix Kuehling wrote: > Hi John, > > I haven't read your patches. Just a question based on the cover letter. > > I understand that visible VRAM is the biggest pain point. But could the > same reasoning make sense for invisible VRAM? That is, doing all the > migrations to VRAM in a workqueue? > > Regards, > Felix > I don't see why not. In theory, all non-essential buffer moves could be done this way, and it would be relatively trivial to extend it to that. But I wanted to limit the scope of my changes, at least for this series. Testing takes a long time and I wanted to focus those testing efforts as much as possible, produce something well-tested (I hope), and get feedback on this limited application of the concept before expanding its reach. John > > On 17-06-23 01:39 PM, John Brooks wrote: > > This patch series is intended to improve performance when limited > > CPU-visible > > VRAM is under pressure. > > > > Moving BOs into visible VRAM is essentially a housekeeping task. It's > > faster to > > access them in VRAM than GTT, but it isn't a hard requirement for them to > > be in > > VRAM. As such, it is unnecessary to spend valuable time blocking on this in > > the > > page fault handler or during command submission. Doing so translates > > directly > > into a longer frame time (ergo stalls and stuttering). > > > > The problem worsens when attempting to move BOs into visible VRAM when it is > > full. This takes much longer than a simple move because other BOs have to be > > evicted, which involves finding and then moving potentially hundreds of > > other > > BOs, which is very time consuming. In the case of limited visible VRAM, it's > > important to do this sometime to keep the contents of visible VRAM fresh, > > but > > it does not need to be a blocking operation. If visible VRAM is full, the BO > > can be read from GTT in the meantime and the BO can be moved to VRAM later. > > > > Thus, I have made it so that neither the command submission code nor page > > fault > > handler spends time evicting BOs from visible VRAM, and instead this is > > deferred to a workqueue function that's queued when CS requests BOs flagged > > AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED. > > > > Speaking of CPU_ACCESS_REQUIRED, I've changed the handling of that flag so > > that > > the kernel driver can clear it later even if it was set by userspace. This > > is > > because the userspace graphics library can't know whether the application > > really needs it to be CPU_ACCESS_REQUIRED forever. The kernel driver can't > > know > > either, but it does know when page faults occur, and if a BO doesn't appear > > to > > have any page faults when it's moved somewhere inaccessible, the flag can be > > removed and it doesn't have to take up space in CPU-visible memory anymore. > > This change was based on IRC discussions with Michel. > > > > Patch 7 fixes a problem with BO moverate throttling that causes visible VRAM > > moves to not be throttled if total VRAM isn't full enough. > > > > I've also added a vis_vramlimit module parameter for debugging purposes. > > It's > > similar to the vramlimit parameter except it limits only visible VRAM. > > > > I have tested this patch set with the two games I know to be affected by > > visible VRAM pressure: DiRT Rally and Dying Light. It practically eliminates > > eviction-related stuttering in DiRT Rally as well as very low performance if > > visible VRAM is limited to 64MB. It also fixes severely low framerates that > > occurred in some areas of Dying Light. All my testing was done with an R9 > > 290 > > with 4GB of visible VRAM with an Intel i7 4790. > > > > -- > > John Brooks (Frogging101) > > > > ___ > > amd-gfx mailing list > > amd-...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 196117] amdgpu - RX 480 (polaris) - freeze during boot
https://bugzilla.kernel.org/show_bug.cgi?id=196117 --- Comment #6 from Paul K. Gerke (paulkge...@craftware.nl) --- Success! Everything seems to be fixed... "everything": I tried the amdstaging-kernel for 4.11.0+ and it works now! Jippieh! This is the log for the working driver (I cut away a bit of the beginning which is the same as for the other kernel logs): [ 102.328213] amdgpu :02:00.0: fence driver on ring 0 use gpu addr 0x0008, cpu addr 0x8efe66e4e008 [ 102.328265] amdgpu :02:00.0: fence driver on ring 1 use gpu addr 0x0018, cpu addr 0x8efe66e4e018 [ 102.328331] amdgpu :02:00.0: fence driver on ring 2 use gpu addr 0x0028, cpu addr 0x8efe66e4e028 [ 102.328377] amdgpu :02:00.0: fence driver on ring 3 use gpu addr 0x0038, cpu addr 0x8efe66e4e038 [ 102.328517] amdgpu :02:00.0: fence driver on ring 4 use gpu addr 0x0048, cpu addr 0x8efe66e4e048 [ 102.328568] amdgpu :02:00.0: fence driver on ring 5 use gpu addr 0x0058, cpu addr 0x8efe66e4e058 [ 102.328610] amdgpu :02:00.0: fence driver on ring 6 use gpu addr 0x0068, cpu addr 0x8efe66e4e068 [ 102.328646] amdgpu :02:00.0: fence driver on ring 7 use gpu addr 0x0078, cpu addr 0x8efe66e4e078 [ 102.328681] amdgpu :02:00.0: fence driver on ring 8 use gpu addr 0x0088, cpu addr 0x8efe66e4e088 [ 102.328706] amdgpu :02:00.0: fence driver on ring 9 use gpu addr 0x009c, cpu addr 0x8efe66e4e09c [ 102.328795] amdgpu :02:00.0: fence driver on ring 10 use gpu addr 0x00ac, cpu addr 0x8efe66e4e0ac [ 102.328833] amdgpu :02:00.0: fence driver on ring 11 use gpu addr 0x00bc, cpu addr 0x8efe66e4e0bc [ 102.328847] [drm] Found UVD firmware Version: 1.79 Family ID: 16 [ 102.329116] amdgpu :02:00.0: fence driver on ring 12 use gpu addr 0x00f40122d420, cpu addr 0xa54243c5a420 [ 102.329128] [drm] Found VCE firmware Version: 52.4 Binary ID: 3 [ 102.329209] amdgpu :02:00.0: fence driver on ring 13 use gpu addr 0x00dc, cpu addr 0x8efe66e4e0dc [ 102.329254] amdgpu :02:00.0: fence driver on ring 14 use gpu addr 0x00ec, cpu addr 0x8efe66e4e0ec [ 102.368327] amdgpu: [powerplay] [AVFS] Something is broken. See log! [ 102.370210] amdgpu: [powerplay] Can't find requested voltage id in vdd_dep_on_sclk table! [ 102.382613] [drm] ring test on 0 succeeded in 14 usecs [ 102.383115] [drm] ring test on 9 succeeded in 9 usecs [ 102.383134] [drm] ring test on 1 succeeded in 8 usecs [ 102.383144] [drm] ring test on 2 succeeded in 3 usecs [ 102.383153] [drm] ring test on 3 succeeded in 3 usecs [ 102.383163] [drm] ring test on 4 succeeded in 3 usecs [ 102.383176] [drm] ring test on 5 succeeded in 5 usecs [ 102.383185] [drm] ring test on 6 succeeded in 3 usecs [ 102.383195] [drm] ring test on 7 succeeded in 3 usecs [ 102.383204] [drm] ring test on 8 succeeded in 3 usecs [ 102.383248] [drm] ring test on 10 succeeded in 5 usecs [ 102.383256] [drm] ring test on 11 succeeded in 6 usecs [ 102.429390] [drm] ring test on 12 succeeded in 1 usecs [ 102.429399] [drm] UVD initialized successfully. [ 102.539389] [drm] ring test on 13 succeeded in 7 usecs [ 102.539400] [drm] ring test on 14 succeeded in 3 usecs [ 102.539402] [drm] VCE initialized successfully. [ 102.539658] [drm] ib test on ring 0 succeeded [ 102.539821] [drm] ib test on ring 1 succeeded [ 102.539870] [drm] ib test on ring 2 succeeded [ 102.539913] [drm] ib test on ring 3 succeeded [ 102.539955] [drm] ib test on ring 4 succeeded [ 102.539994] [drm] ib test on ring 5 succeeded [ 102.540031] [drm] ib test on ring 6 succeeded [ 102.540072] [drm] ib test on ring 7 succeeded [ 102.540112] [drm] ib test on ring 8 succeeded [ 103.041685] [drm] ib test on ring 9 succeeded [ 103.041722] [drm] ib test on ring 10 succeeded [ 103.041751] [drm] ib test on ring 11 succeeded [ 103.043080] [drm] ib test on ring 12 succeeded [ 103.043301] [drm] ib test on ring 13 succeeded [ 103.141511] [drm] fb mappable at 0xC1437000 [ 103.141519] [drm] vram apper at 0xC000 [ 103.141521] [drm] size 768 [ 103.141522] [drm] fb depth is 24 [ 103.141523] [drm]pitch is 6400 [ 103.141588] fbcon: amdgpudrmfb (fb0) is primary device [ 103.263499] Console: switching to colour frame buffer device 200x75 [ 103.319197] systemd-journald[303]: Sent WATCHDOG=1 notification. [ 103.365845] amdgpu :02:00.0: fb0: amdgpudrmfb frame buffer device [ 103.388258] [drm] Initialized amdgpu 3.17.0 20150101 for :02:00.0 on minor 0 - For anybody who wants to reproduce what I did, here the instructions which work for ubuntu 16.04: - git-clone the kernel at git://people.freedesktop.org/~agd5f/linux - checkout the branch amd-staging-4.11 - I specifically used commit 3e3a7c55b8de38e0557fe954f236ca8e8e925d85 - Use the config
[Bug 196117] amdgpu - RX 480 (polaris) - freeze during boot
https://bugzilla.kernel.org/show_bug.cgi?id=196117 --- Comment #7 from Paul K. Gerke (paulkge...@craftware.nl) --- Created attachment 257145 --> https://bugzilla.kernel.org/attachment.cgi?id=257145&action=edit kernel config file for a working for the RX480 system -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 196117] amdgpu - RX 480 (polaris) - freeze during boot
https://bugzilla.kernel.org/show_bug.cgi?id=196117 Paul K. Gerke (paulkge...@craftware.nl) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |PATCH_ALREADY_AVAILABLE --- Comment #8 from Paul K. Gerke (paulkge...@craftware.nl) --- Found the branch where this was already fixed... -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 101572] glMemoryBarrier is backwards
https://bugs.freedesktop.org/show_bug.cgi?id=101572 Bug ID: 101572 Summary: glMemoryBarrier is backwards Product: Mesa Version: git Hardware: All OS: All Status: NEW Severity: major Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel@lists.freedesktop.org Reporter: dark_syl...@yahoo.com.ar QA Contact: dri-devel@lists.freedesktop.org This bug may not just be in radeonsi. I noticed the error after seeing my Compute Shaders produce output from an input FBO (used as a texture) that had missing draws. According to spec, glMemoryBarrier reflects how the buffer will be *used afterwards*. So for example if I have a Compute Shader that writes to an SSBO and later this buffer is used as an index buffer I should call: glMemoryBarrier( GL_ELEMENT_ARRAY_BARRIER_BIT ); Because I will be using from this buffer later on as an index buffer. However it appears Mesa expects me to call instead: glMemoryBarrier( GL_SHADER_STORAGE_BARRIER_BIT ); because I am writing to this buffer as an SSBO before the barrier. The problem I encountered specifically is that I was drawing to an FBO, and later on this FBO is used as a regular texture (sampler2D) in a compute shader. According to the spec, I should call: glMemoryBarrier( GL_TEXTURE_FETCH_BARRIER_BIT ); However Mesa does not produce correct output unless I do: glMemoryBarrier( GL_FRAMEBUFFER_BARRIER_BIT ); I had to re-read the spec several times and I was left wondered if I was the one who got it backwards. After all the language in which it is written is very confusing without a single example; however I then found: http://malideveloper.arm.com/sample-code/introduction-compute-shaders-2/ which says: "It’s important to remember the semantics of glMemoryBarrier(). As argument it takes a bitfield of various buffer types. We specify how we will read data after the memory barrier. In this case, we are writing the buffer via an SSBO, but we’re reading it when we’re using it as a vertex buffer, hence GL_VERTEX_ATTRIB_ARRAY_BARRIER_BIT." I then consulted OpenGL SuperBible and the Programming Guide, and they both agree: "glMemoryBarrier(GL_ATOMIC_COUNTER_BARRIER_BIT); will ensure that any access to an atomic counter in a buffer object will reflect updates to that buffer by a shader. You should call glMemoryBarrier() with the GL_ATOMIC_COUNTER_BARRIER_BIT set when something has written to a buffer that you want to see reflected in the values of your atomic counters. If you update the values in a buffer using an atomic counter and then use that buffer for something else, the bit you include in the barriers parameter to glMemoryBarrier() should correspond to what you want that buffer to be used for, which will not necessarily include GL_ATOMIC_COUNTER_BARRIER_BIT." (from OpenGL Super Bible) "GL_TEXTURE_FETCH_BARRIER_BIT specifies that any fetch from a texture issued after the barrier should reflect data written to the texture by commands issued before the barrier. (...) GL_FRAMEBUFFER_BARRIER_BIT specifies that reads or writes through framebuffer attachments issued after the barrier will reflect data written to those attachments by shaders executed before the barrier. Further, writes to framebuffers issued after the barrier will be ordered with respect to writes performed by shaders before the barrier." (from OpenGL Programming Manual). It appears state_tracker/st_cb_texturebarrier.c also contains this bug because it ignores GL_TEXTURE_UPDATE_BARRIER_BIT & GL_BUFFER_UPDATE_BARRIER_BIT; as it assumes writes through texture/buffer_update will be done through Mesa functions which are always synchronized; instead of synchronizing the read and writes after the barrier. This doesn't sound like it has a trivial fix, TBH I have no problem in supporting a glMemoryBarrierMESA( writesToFlushBeforeBarrier, readsToFlushAfterBarrier ) which would behave more sanely and the way you'd want (and I get the fastest path); and you then just workaround the standard glMemoryBarrier by issuing a barrier to all bits. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel