Re: [PATCH libdrm 1/2] xf86drm: inline drmIoctl()

2017-06-23 Thread Eric Engestrom
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.

2017-06-23 Thread Andrzej Hajda
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

2017-06-23 Thread Maxime Ripard
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.

2017-06-23 Thread Boris Brezillon
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

2017-06-23 Thread Daniel Vetter
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

2017-06-23 Thread Hans de Goede
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

2017-06-23 Thread Hans de Goede
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()

2017-06-23 Thread Hans de Goede
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

2017-06-23 Thread Hans de Goede
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

2017-06-23 Thread Hans de Goede
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

2017-06-23 Thread Hans de Goede
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

2017-06-23 Thread Daniel Vetter
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

2017-06-23 Thread Daniel Vetter
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.

2017-06-23 Thread Daniel Vetter
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

2017-06-23 Thread Daniel Vetter
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

2017-06-23 Thread Daniel Vetter
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

2017-06-23 Thread Daniel Vetter
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

2017-06-23 Thread Daniel Vetter
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

2017-06-23 Thread kbuild test robot

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?

2017-06-23 Thread kbuild test robot
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

2017-06-23 Thread Sharma, Shashank

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

2017-06-23 Thread Sharma, Shashank

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

2017-06-23 Thread Daniel Vetter
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

2017-06-23 Thread Daniel Vetter
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

2017-06-23 Thread Daniel Vetter
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

2017-06-23 Thread Fengguang Wu

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

2017-06-23 Thread Daniel Vetter
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

2017-06-23 Thread Daniel Vetter
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

2017-06-23 Thread Daniel Vetter
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

2017-06-23 Thread Chris Wilson
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]

2017-06-23 Thread bugzilla-daemon
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]

2017-06-23 Thread bugzilla-daemon
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

2017-06-23 Thread Fengguang Wu

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

2017-06-23 Thread Liviu Dudau
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

2017-06-23 Thread Daniel Vetter
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

2017-06-23 Thread bugzilla-daemon
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.

2017-06-23 Thread Archit Taneja



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)

2017-06-23 Thread bugzilla-daemon
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

2017-06-23 Thread Sean Paul
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

2017-06-23 Thread Sean Paul
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

2017-06-23 Thread Maxime Ripard
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"

2017-06-23 Thread Philippe CORNU


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

2017-06-23 Thread Sean Paul
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

2017-06-23 Thread bugzilla-daemon
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

2017-06-23 Thread Alex Deucher
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

2017-06-23 Thread Alex Deucher
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

2017-06-23 Thread Alex Deucher
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

2017-06-23 Thread Alex Deucher
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

2017-06-23 Thread 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);
 
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

2017-06-23 Thread Alex Deucher
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

2017-06-23 Thread Alex Deucher
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

2017-06-23 Thread Alex Deucher
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

2017-06-23 Thread Alex Deucher
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

2017-06-23 Thread Christian König

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

2017-06-23 Thread Ben Widawsky
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

2017-06-23 Thread Ben Widawsky
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)

2017-06-23 Thread Ben Widawsky
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

2017-06-23 Thread Ben Widawsky
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

2017-06-23 Thread Ben Widawsky
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

2017-06-23 Thread Alex Deucher
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

2017-06-23 Thread John Brooks
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

2017-06-23 Thread John Brooks
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

2017-06-23 Thread John Brooks
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

2017-06-23 Thread John Brooks
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

2017-06-23 Thread John Brooks
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

2017-06-23 Thread John Brooks
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

2017-06-23 Thread John Brooks
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

2017-06-23 Thread John Brooks
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

2017-06-23 Thread John Brooks
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

2017-06-23 Thread John Brooks
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)

2017-06-23 Thread bugzilla-daemon
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

2017-06-23 Thread bugzilla-daemon
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]

2017-06-23 Thread bugzilla-daemon
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"

2017-06-23 Thread Eric Anholt
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.

2017-06-23 Thread Rob Herring
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.

2017-06-23 Thread Mike Mestnik
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

2017-06-23 Thread bugzilla-daemon
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

2017-06-23 Thread Jonathan Corbet
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)

2017-06-23 Thread bugzilla-daemon
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

2017-06-23 Thread Felix Kuehling
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

2017-06-23 Thread Rob Herring
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

2017-06-23 Thread Rob Herring
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

2017-06-23 Thread Rob Herring
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.

2017-06-23 Thread Eric Anholt
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.

2017-06-23 Thread Mike Mestnik
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.

2017-06-23 Thread Eric Anholt
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

2017-06-23 Thread bugzilla-daemon
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

2017-06-23 Thread John Brooks
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

2017-06-23 Thread bugzilla-daemon
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

2017-06-23 Thread bugzilla-daemon
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

2017-06-23 Thread bugzilla-daemon
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

2017-06-23 Thread bugzilla-daemon
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