[PATCH] fbcon: use default if cursor blink interval is not valid
On Thu, May 19, 2016 at 10:22 PM, Scot Doyle wrote:
> On Thu, 19 May 2016, Pavel Machek wrote:
>> Hi!
>>
>> > Two current [1] and three previous [2] systems locked during boot
>> > because the cursor flash timer was set using an ops->cur_blink_jiffies
>> > value of 0. Previous patches attempted to solve the problem by moving
>> > variable initialization earlier in the setup sequence [2].
>> >
>> > Use the normal cursor blink default interval of 200 ms if
>> > ops->cur_blink_jiffies is not in the range specified in commit
>> > bd63364caa8d. Since invalid values are not used, specific system
>> > initialization timings should not cause lockups.
>> >
>> > [1] https://bugs.launchpad.net/bugs/1574814
>> > [2] see commits: 2a17d7e80f1d, f235f664a8af, a1e533ec07d5
>>
>> Acked-by: Pavel Machek
>>
>> > static void cursor_timer_handler(unsigned long dev_addr)
>> > {
>> > struct fb_info *info = (struct fb_info *) dev_addr;
>> > struct fbcon_ops *ops = info->fbcon_par;
>> >
>> > queue_work(system_power_efficient_wq, &info->queue);
>> > - mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies);
>> > + mod_timer(&ops->cursor_timer, jiffies +
>> > + cursor_blink_jiffies(ops->cur_blink_jiffies));
>> > }
>> >
>> > static void fbcon_add_cursor_timer(struct fb_info *info)
>>
>> And actually... perhaps mod_timer should have some check for too low
>> timeouts..?
>>
>> WARN_ON?
>> Pavel
>
>
> Interesting idea. I applied this patch to a couple systems and
> receive the same warning on both:
If 'jiffies' is passed to mod_timer() for same timer unusually OR
mod_timer() isn't called from the timer handler, it shoudln't cause
soft lockup.
In the case of fbcon, 'jiffies' is always passed to mod_timer() and
mod_timer() is called from the timer handler meantime, that is a
real lockup.
>
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 73164c3..f6c0b69 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -788,6 +788,7 @@ __mod_timer(struct timer_list *timer, unsigned long
> expires,
>
> timer_stats_timer_set_start_info(timer);
> BUG_ON(!timer->function);
> + WARN_ONCE(expires == jiffies, "timer should expire in the future");
>
> base = lock_timer_base(timer, &flags);
>
> --
>
> [2.060474] [ cut here ]
> [2.061613] WARNING: CPU: 0 PID: 164 at kernel/time/timer.c:791
> mod_timer+0x233/0x240
> [2.062740] timer should expire in the future
> [2.062757] CPU: 0 PID: 164 Comm: kworker/0:2 Not tainted 4.6.0+ #7
> [2.065870] Hardware name: Toshiba Leon, BIOS 12/04/2013
> [2.067828] Workqueue: events_power_efficient hub_init_func3
> [2.069762] 88007443bbb8 8139932b
> 88007443bc08
> [2.071701] 88007443bbf8 8112e57c
> 0317
> [2.073655] 88007486a0b0 fffea2da 88007486a000
> 0202
> [2.075594] Call Trace:
> [2.077503] [] dump_stack+0x4d/0x72
> [2.079426] [] __warn+0xcc/0xf0
> [2.081325] [] warn_slowpath_fmt+0x4f/0x60
> [2.083212] [] ? find_next_bit+0x15/0x20
> [2.085022] [] ? cpumask_next_and+0x2f/0x40
> [2.086696] [] mod_timer+0x233/0x240
> [2.088362] [] usb_hcd_submit_urb+0x3f2/0x8c0
> [2.090026] [] ? urb_destroy+0x24/0x30
> [2.091698] [] ? insert_work+0x58/0xb0
> [2.093349] [] usb_submit_urb+0x287/0x530
> [2.094985] [] hub_activate+0x1fd/0x5d0
> [2.096625] [] ? finish_task_switch+0x78/0x1f0
> [2.098268] [] hub_init_func3+0x1a/0x20
> [2.099908] [] process_one_work+0x140/0x3e0
> [2.101539] [] worker_thread+0x4e/0x480
> [2.103173] [] ? process_one_work+0x3e0/0x3e0
> [2.104790] [] ? process_one_work+0x3e0/0x3e0
> [2.106259] [] kthread+0xc9/0xe0
> [2.107731] [] ret_from_fork+0x22/0x40
> [2.109215] [] ? __kthread_parkme+0x70/0x70
> [2.110704] ---[ end trace 3519886a1a990d99 ]---
>
> mod_timer is called from over a thousand places. Should timers always
> expire in the future?
>
[PATCH] drm/dp/mst: Always clear proposed vcpi table for port.
Not clearing mst manager's proposed vcpis table for destroyed
connectors when the manager is stopped leaves it pointing to
unrefernced memory, this causes pagefault when the manager is
restarted when plugging back a branch.
Signed-off-by: Andrey Grodzovsky
---
drivers/gpu/drm/drm_dp_mst_topology.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 9971c46..cd6014b 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2881,11 +2881,9 @@ static void drm_dp_destroy_connector_work(struct
work_struct *work)
drm_dp_port_teardown_pdt(port, port->pdt);
if (!port->input && port->vcpi.vcpi > 0) {
- if (mgr->mst_state) {
- drm_dp_mst_reset_vcpi_slots(mgr, port);
- drm_dp_update_payload_part1(mgr);
- drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
- }
+ drm_dp_mst_reset_vcpi_slots(mgr, port);
+ drm_dp_update_payload_part1(mgr);
+ drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
}
kref_put(&port->kref, drm_dp_free_mst_port);
--
1.9.1
[PATCH V5 3/3] soc/tegra: pmc: Add support for IO pads power state and voltage
On Thursday 19 May 2016 09:24 PM, Jon Hunter wrote:
> On 12/05/16 13:21, Laxman Dewangan wrote:
>> The IO pins of Tegra SoCs are grouped for common control of IO
>> interface like setting voltage signal levels and power state of
>> the interface. The group is generally referred as IO pads. The
>> power state and voltage control of IO pins can be done at IO pads
>> level.
>>
>> Tegra generation SoC supports the power down of IO pads when it
>> is not used even in the active state of system. This saves power
>> from that IO interface. Also it supports multiple voltage level
>> in IO pins for interfacing on some of pads. The IO pad voltage is
>> automatically detected till T124, hence SW need not to configure
>> this. But from T210, the automatically detection logic has been
>> removed, hence SW need to explicitly set the IO pad voltage into
>> IO pad configuration registers.
>>
>> Add support to set the power states and voltage level of the IO pads
>> from client driver. The implementation for the APIs are in generic
>> which is applicable for all generation os Tegra SoC.
>>
>> IO pads ID and information of bit field for power state and voltage
>> level controls are added for Tegra124, Tegra132 and Tegra210. The SOR
>> driver is modified to use the new APIs.
>>
>> Signed-off-by: Laxman Dewangan
>>
>> ---
>> Changes from V1:
>> This is reworked on earlier path to have separation between IO rails and
>> io pads and add power state and voltage control APIs in single call.
>>
>> Changes from V2:
>> - Remove the tegra_io_rail_power_off/on() apis and change client (sor) driver
>> to use the new APIs for IO pad power.
>> - Remove the TEGRA_IO_RAIL_ macros.
>>
>> Changes from V3:
>> - Make all pad_id/io_pad_id to id.
>> - tegra_io_pad_ -> tegra_io_pads
>> - dpd_bit -> bit, pwr_mask/bit to mask/bit.
>> - Rename function to tegra_io_pads_{set,get}_voltage_config
>> - Make the io pad tables common for all SoC.
>> - Make io_pads enums.
>> - Add enums for voltage.
>>
>> Changes from V4:
>> - IO_PAD->IO_PADS
>> - TEGRA_IO_PADS_POWER_SOURCE_ -> TEGRA_IO_PADS_VCONF_
>> ---
>> drivers/gpu/drm/tegra/sor.c | 8 +-
>> drivers/soc/tegra/pmc.c | 221
>> ++--
>> include/soc/tegra/pmc.h | 132 ++
>> 3 files changed, 294 insertions(+), 67 deletions(-)
> [snip]
>
>> +static int tegra_io_pads_to_voltage_bit(const struct tegra_pmc_soc *soc,
>> +enum tegra_io_pads id)
>> +{
>> +/* T210 only supports io-pad voltage config bit */
>> +if (soc->io_pads_soc_mask != TEGRA_IO_PADS_T210)
>> return -EINVAL;
> If this is only supported for Tegra210, should these voltage functions
> be dependent on CONFIG_ARCH_TEGRA_210_SOC? If so, then I am also
> wondering if we should bother having the massive look-up table and just
> have a smaller table to translate the ID to bit for voltage as you had
> in your initial patch? Seems there are few io-pads that support the
> voltage configuration.
>
Voltage config supported from T210 onwards. Earlier it was auto detected
and so SW need not to write it.
I think let's have this as of now and we will make the different table
for voltage config in future when we add another chip support.
We will not use the SOC config for this, we will use only compatible
property and its data for getting the support.
[Bug 76490] Hang during boot when DPM is on (R9 270X)
https://bugs.freedesktop.org/show_bug.cgi?id=76490 --- Comment #114 from Daniel Exner --- (In reply to Alex Deucher from comment #113) > That tree is using the same code power management as radeon, just ported to > amdgpu. Ok, thx for the clarification. Then I'll patiently wait for a proper fix. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160519/e3c2589c/attachment.html>
[PATCH] drm/vmwgfx: use kmemdup
Use kmemdup when some other buffer is immediately copied into allocated
region. It replaces call to allocation followed by memcpy, by a single
call to kmemdup.
Signed-off-by: Muhammad Falak R Wani
---
drivers/gpu/drm/vmwgfx/vmwgfx_mob.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
index b6126a5..941bcfd 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c
@@ -319,18 +319,17 @@ int vmw_otables_setup(struct vmw_private *dev_priv)
int ret;
if (dev_priv->has_dx) {
- *otables = kmalloc(sizeof(dx_tables), GFP_KERNEL);
+ *otables = kmemdup(dx_tables, sizeof(dx_tables), GFP_KERNEL);
if (*otables == NULL)
return -ENOMEM;
- memcpy(*otables, dx_tables, sizeof(dx_tables));
dev_priv->otable_batch.num_otables = ARRAY_SIZE(dx_tables);
} else {
- *otables = kmalloc(sizeof(pre_dx_tables), GFP_KERNEL);
+ *otables = kmemdup(pre_dx_tables, sizeof(pre_dx_tables),
+ GFP_KERNEL);
if (*otables == NULL)
return -ENOMEM;
- memcpy(*otables, pre_dx_tables, sizeof(pre_dx_tables));
dev_priv->otable_batch.num_otables = ARRAY_SIZE(pre_dx_tables);
}
--
1.9.1
[Bug 76490] Hang during boot when DPM is on (R9 270X)
https://bugs.freedesktop.org/show_bug.cgi?id=76490 --- Comment #113 from Alex Deucher --- (In reply to Daniel Exner from comment #112) > If I read that correct R9 270X is a GCN 1.0 card and thus should be > supported by experimental drm-next-4.8-wip-si branch. > > Is it worth trying? AMDGPU is using a yet another PM system (PowerPlay) , so > perhaps it works better, without having to blacklist? That tree is using the same code power management as radeon, just ported to amdgpu. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160519/c04a35e7/attachment.html>
[Bug 76490] Hang during boot when DPM is on (R9 270X)
https://bugs.freedesktop.org/show_bug.cgi?id=76490 --- Comment #112 from Daniel Exner --- If I read that correct R9 270X is a GCN 1.0 card and thus should be supported by experimental drm-next-4.8-wip-si branch. Is it worth trying? AMDGPU is using a yet another PM system (PowerPlay) , so perhaps it works better, without having to blacklist? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160519/70fc7947/attachment.html>
[Bug 95474] Bioshock Infinite and DiRT Showdown perform very poorly on any GPU with GCN >=1.1
https://bugs.freedesktop.org/show_bug.cgi?id=95474 --- Comment #7 from Jan Ziak <0xe2.0x9a.0x9b at gmail.com> --- (In reply to Alex Deucher from comment #4) > Can you try a different kernel or mesa version? LLVM 3.8 + Mesa 11.2.2 -> same result -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160519/88db42f3/attachment-0001.html>
[PATCH] Revert "drm/core: Preserve the framebuffer after removing it."
On Thu, May 19, 2016 at 5:15 PM, Hans de Goede wrote:
> This reverts commit 13803132818c ("drm/core: Preserve the framebuffer
> after removing it.").
>
> This commit assumes that going through drm_framebuffer_remove() is not
> necessary because "the fbdev code or any system compositor should restore
> the planes anyway so there's no need to do it twice". But this is not true
> for secondary GPUs / slave outputs.
>
> This revert fixes the dgpu no longer suspending on laptops with
> switchable graphics after an external output which is connected
> to the dgpu has been used.
>
> And it fixes the WARN_ON to detect drm_framebuffer leaks in
> drm_mode_config_cleanup() triggering when unplugging an USB displaylink
> device; or when rmmod-ing the secondary GPU kms driver on laptops with
> switchable-graphics.
>
> Also this part of the reverted commit's commit-msg: "The old fb_id is
> zero'd, so there's no danger of being able to restore the fb from fb_id."
> is no longer true, the zero-ing does not happen until drm_framebuffer_free
> gets called, which does not happen until the last ref is dropped, so
> if a crtc's primary->fb is still pointing to this fb, the id will not
> get zero'd and userspace could potentially gain access to the removed
> fb again.
>
> Cc: stable at vger.kenrel.org
> Signed-off-by: Hans de Goede
We have a proper fix in drm-next:
commit f2d580b9a8149735cbc4b59c4a8df60173658140
Author: Maarten Lankhorst
Date: Wed May 4 14:38:26 2016 +0200
drm/core: Do not preserve framebuffer on rmfb, v4.
That thing took forever to get merged since no one seemed to have
cared and bothered with a tested-by. But it's on its way to stable
kernels now.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 92827] Tonga: No Sound over HDMI with connected AV receiver
https://bugs.freedesktop.org/show_bug.cgi?id=92827 --- Comment #3 from Maximilian Böhm --- At least something to await for! Would be neat to put this info somewhere in a non-dev press release. I fiddled many hours with this issue without knowing anything about the state of HDMI audio on AMDGPU. There must be countless frustrated users, even more when you think of the Ubuntu 16.04 shift away from fglrx. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160519/f55d4a1b/attachment.html>
[PATCH] amdgpu/uvd: use kmemdup
On Thu, May 19, 2016 at 01:18:10PM +0200, Christian König wrote: > Am 19.05.2016 um 13:11 schrieb Muhammad Falak R Wani: > >Use kmemdup when some other buffer is immediately copied into allocated > >region. It replaces call to allocation followed by memcpy, by a single > >call to kmemdup. > > > >Signed-off-by: Muhammad Falak R Wani > > NAK, actually using memcpy() is wrong in the first place. > > The UVD BO is in VRAM so the pointer is pointing to IO memory here, > so this should be memcpy_fromio() instead of memcpy(). > > Christian. > Should I send V2 with the required changes, and I had a query, If memcpy was wrong, did it still work or it just got un-noticed ?
[PATCH] Revert "drm/core: Preserve the framebuffer after removing it."
This reverts commit 13803132818c ("drm/core: Preserve the framebuffer
after removing it.").
This commit assumes that going through drm_framebuffer_remove() is not
necessary because "the fbdev code or any system compositor should restore
the planes anyway so there's no need to do it twice". But this is not true
for secondary GPUs / slave outputs.
This revert fixes the dgpu no longer suspending on laptops with
switchable graphics after an external output which is connected
to the dgpu has been used.
And it fixes the WARN_ON to detect drm_framebuffer leaks in
drm_mode_config_cleanup() triggering when unplugging an USB displaylink
device; or when rmmod-ing the secondary GPU kms driver on laptops with
switchable-graphics.
Also this part of the reverted commit's commit-msg: "The old fb_id is
zero'd, so there's no danger of being able to restore the fb from fb_id."
is no longer true, the zero-ing does not happen until drm_framebuffer_free
gets called, which does not happen until the last ref is dropped, so
if a crtc's primary->fb is still pointing to this fb, the id will not
get zero'd and userspace could potentially gain access to the removed
fb again.
Cc: stable at vger.kenrel.org
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/drm_crtc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e08f962..15f5cd7 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -3474,7 +3474,7 @@ int drm_mode_rmfb(struct drm_device *dev,
mutex_unlock(&dev->mode_config.fb_lock);
mutex_unlock(&file_priv->fbs_lock);
- drm_framebuffer_unreference(fb);
+ drm_framebuffer_remove(fb);
return 0;
@@ -3656,8 +3656,8 @@ void drm_fb_release(struct drm_file *priv)
list_for_each_entry_safe(fb, tfb, &priv->fbs, filp_head) {
list_del_init(&fb->filp_head);
- /* This drops the fpriv->fbs reference. */
- drm_framebuffer_unreference(fb);
+ /* This will also drop the fpriv->fbs reference. */
+ drm_framebuffer_remove(fb);
}
}
--
2.7.4
[PATCH 3/5] drm: mediatek: fixup drm_gem_object_lookup API change
On 18/05/16 18:07, Arnd Bergmann wrote:
> The drm_gem_object_lookup() function prototype changed while this
> driver was added, so it fails to build now:
>
> drivers/gpu/drm/mediatek/mtk_drm_gem.c: In function
> 'mtk_drm_gem_dumb_map_offset':
> drivers/gpu/drm/mediatek/mtk_drm_gem.c:142:30: error: passing argument 1 of
> 'drm_gem_object_lookup' from incompatible pointer type
> [-Werror=incompatible-pointer-types]
>obj = drm_gem_object_lookup(dev, file_priv, handle);
>
> This fixes the new caller as well.
>
> Signed-off-by: Arnd Bergmann
> Fixes: a8ad0bd84f98 ("drm: Remove unused drm_device from
> drm_gem_object_lookup()")
> ---
> drivers/gpu/drm/mediatek/mtk_drm_fb.c | 2 +-
> drivers/gpu/drm/mediatek/mtk_drm_gem.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> index 33d30c19f35f..147df85399ab 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> @@ -138,7 +138,7 @@ struct drm_framebuffer *mtk_drm_mode_fb_create(struct
> drm_device *dev,
> if (drm_format_num_planes(cmd->pixel_format) != 1)
> return ERR_PTR(-EINVAL);
>
> - gem = drm_gem_object_lookup(dev, file, cmd->handles[0]);
> + gem = drm_gem_object_lookup(file, cmd->handles[0]);
> if (!gem)
> return ERR_PTR(-ENOENT);
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> index a773bfaea913..fa2ec0cd00e8 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> @@ -139,7 +139,7 @@ int mtk_drm_gem_dumb_map_offset(struct drm_file
> *file_priv,
> struct drm_gem_object *obj;
> int ret;
>
> - obj = drm_gem_object_lookup(dev, file_priv, handle);
> + obj = drm_gem_object_lookup(file_priv, handle);
> if (!obj) {
> DRM_ERROR("failed to lookup gem object.\n");
> return -EINVAL;
>
Reviewed-by: Matthias Brugger
[PATCH 2/5] drm: mediatek: add CONFIG_OF dependency
On 18/05/16 18:07, Arnd Bergmann wrote: > The mediatek DRM driver can be configured for compile testing with > CONFIG_OF disabled, but then fails to link: > > drivers/gpu/built-in.o: In function `mtk_drm_bind': > analogix_dp_reg.c:(.text+0x52888): undefined reference to > `of_find_device_by_node' > analogix_dp_reg.c:(.text+0x52930): undefined reference to > `of_find_device_by_node' > > This adds an explicit Kconfig dependency. > > Signed-off-by: Arnd Bergmann > --- > drivers/gpu/drm/mediatek/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/mediatek/Kconfig > b/drivers/gpu/drm/mediatek/Kconfig > index 545973f6b743..67183e26971d 100644 > --- a/drivers/gpu/drm/mediatek/Kconfig > +++ b/drivers/gpu/drm/mediatek/Kconfig > @@ -3,6 +3,7 @@ config DRM_MEDIATEK > depends on DRM > depends on ARCH_MEDIATEK || (ARM && COMPILE_TEST) > depends on COMMON_CLK > + depends on OF > select DRM_GEM_CMA_HELPER > select DRM_KMS_HELPER > select DRM_MIPI_DSI > Reviewed-by: Matthias Brugger
[PATCH V5 3/3] soc/tegra: pmc: Add support for IO pads power state and voltage
On 12/05/16 13:21, Laxman Dewangan wrote:
> The IO pins of Tegra SoCs are grouped for common control of IO
> interface like setting voltage signal levels and power state of
> the interface. The group is generally referred as IO pads. The
> power state and voltage control of IO pins can be done at IO pads
> level.
>
> Tegra generation SoC supports the power down of IO pads when it
> is not used even in the active state of system. This saves power
> from that IO interface. Also it supports multiple voltage level
> in IO pins for interfacing on some of pads. The IO pad voltage is
> automatically detected till T124, hence SW need not to configure
> this. But from T210, the automatically detection logic has been
> removed, hence SW need to explicitly set the IO pad voltage into
> IO pad configuration registers.
>
> Add support to set the power states and voltage level of the IO pads
> from client driver. The implementation for the APIs are in generic
> which is applicable for all generation os Tegra SoC.
>
> IO pads ID and information of bit field for power state and voltage
> level controls are added for Tegra124, Tegra132 and Tegra210. The SOR
> driver is modified to use the new APIs.
>
> Signed-off-by: Laxman Dewangan
>
> ---
> Changes from V1:
> This is reworked on earlier path to have separation between IO rails and
> io pads and add power state and voltage control APIs in single call.
>
> Changes from V2:
> - Remove the tegra_io_rail_power_off/on() apis and change client (sor) driver
> to use the new APIs for IO pad power.
> - Remove the TEGRA_IO_RAIL_ macros.
>
> Changes from V3:
> - Make all pad_id/io_pad_id to id.
> - tegra_io_pad_ -> tegra_io_pads
> - dpd_bit -> bit, pwr_mask/bit to mask/bit.
> - Rename function to tegra_io_pads_{set,get}_voltage_config
> - Make the io pad tables common for all SoC.
> - Make io_pads enums.
> - Add enums for voltage.
>
> Changes from V4:
> - IO_PAD->IO_PADS
> - TEGRA_IO_PADS_POWER_SOURCE_ -> TEGRA_IO_PADS_VCONF_
> ---
> drivers/gpu/drm/tegra/sor.c | 8 +-
> drivers/soc/tegra/pmc.c | 221
> ++--
> include/soc/tegra/pmc.h | 132 ++
> 3 files changed, 294 insertions(+), 67 deletions(-)
[snip]
> +static int tegra_io_pads_to_voltage_bit(const struct tegra_pmc_soc *soc,
> + enum tegra_io_pads id)
> +{
> + /* T210 only supports io-pad voltage config bit */
> + if (soc->io_pads_soc_mask != TEGRA_IO_PADS_T210)
> return -EINVAL;
If this is only supported for Tegra210, should these voltage functions
be dependent on CONFIG_ARCH_TEGRA_210_SOC? If so, then I am also
wondering if we should bother having the massive look-up table and just
have a smaller table to translate the ID to bit for voltage as you had
in your initial patch? Seems there are few io-pads that support the
voltage configuration.
+#define TEGRA_IO_RAIL_VOLTAGE(_io_rail, _pos) \
+{ \
+ .io_rail_id = TEGRA_IO_RAIL_##_io_rail, \
+ .bit_position = _pos, \
+}
+
+static struct tegra_io_rail_voltage_bit_info
tegra210_io_rail_voltage_info[] = {
+ TEGRA_IO_RAIL_VOLTAGE(SDMMC1, 12),
+ TEGRA_IO_RAIL_VOLTAGE(SDMMC3, 13),
+ TEGRA_IO_RAIL_VOLTAGE(AUDIO_HV, 18),
+ TEGRA_IO_RAIL_VOLTAGE(DMIC, 20),
+ TEGRA_IO_RAIL_VOLTAGE(GPIO, 21),
+ TEGRA_IO_RAIL_VOLTAGE(SPI_HV, 23),
+};
Otherwise, looks fine to me.
Cheers
Jon
--
nvpublic
[PATCH] drm/amd/powerplay/hwmgr: use kmemdup
Use kmemdup when some other buffer is immediately copied into allocated
region. It replaces call to allocation followed by memcpy, by a single
call to kmemdup.
Signed-off-by: Muhammad Falak R Wani
---
drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
index 1faad92..df399fe 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
@@ -6056,11 +6056,11 @@ static int tonga_get_pp_table(struct pp_hwmgr *hwmgr,
char **table)
struct tonga_hwmgr *data = (struct tonga_hwmgr *)(hwmgr->backend);
if (!data->soft_pp_table) {
- data->soft_pp_table = kzalloc(hwmgr->soft_pp_table_size,
GFP_KERNEL);
+ data->soft_pp_table = kmemdup(hwmgr->soft_pp_table,
+ hwmgr->soft_pp_table_size,
+ GFP_KERNEL);
if (!data->soft_pp_table)
return -ENOMEM;
- memcpy(data->soft_pp_table, hwmgr->soft_pp_table,
- hwmgr->soft_pp_table_size);
}
*table = (char *)&data->soft_pp_table;
--
1.9.1
[PATCH] drm/amd/powerplay/hwmgr: use kmemdup
Use kmemdup when some other buffer is immediately copied into allocated
region. It replaces call to allocation followed by memcpy, by a single
call to kmemdup.
Signed-off-by: Muhammad Falak R Wani
---
drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
index 93768fa..b7dad50 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
@@ -4760,11 +4760,11 @@ static int polaris10_get_pp_table(struct pp_hwmgr
*hwmgr, char **table)
struct polaris10_hwmgr *data = (struct polaris10_hwmgr
*)(hwmgr->backend);
if (!data->soft_pp_table) {
- data->soft_pp_table = kzalloc(hwmgr->soft_pp_table_size,
GFP_KERNEL);
+ data->soft_pp_table = kmemdup(hwmgr->soft_pp_table,
+ hwmgr->soft_pp_table_size,
+ GFP_KERNEL);
if (!data->soft_pp_table)
return -ENOMEM;
- memcpy(data->soft_pp_table, hwmgr->soft_pp_table,
- hwmgr->soft_pp_table_size);
}
*table = (char *)&data->soft_pp_table;
--
1.9.1
[PATCH] drm/amd/powerplay/hwmgr: use kmemdup
Use kmemdup when some other buffer is immediately copied into allocated
region. It replaces call to allocation followed by memcpy, by a single
call to kmemdup.
Signed-off-by: Muhammad Falak R Wani
---
drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
index c94f9fa..1471ac3 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
@@ -5109,11 +5109,11 @@ static int fiji_get_pp_table(struct pp_hwmgr *hwmgr,
char **table)
struct fiji_hwmgr *data = (struct fiji_hwmgr *)(hwmgr->backend);
if (!data->soft_pp_table) {
- data->soft_pp_table = kzalloc(hwmgr->soft_pp_table_size,
GFP_KERNEL);
+ data->soft_pp_table = kmemdup(hwmgr->soft_pp_table,
+ hwmgr->soft_pp_table_size,
+ GFP_KERNEL);
if (!data->soft_pp_table)
return -ENOMEM;
- memcpy(data->soft_pp_table, hwmgr->soft_pp_table,
- hwmgr->soft_pp_table_size);
}
*table = (char *)&data->soft_pp_table;
--
1.9.1
[PATCH] amdgpu/uvd: use kmemdup
Use kmemdup when some other buffer is immediately copied into allocated region. It replaces call to allocation followed by memcpy, by a single call to kmemdup. Signed-off-by: Muhammad Falak R Wani --- drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c index 01abfc2..c977ab6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c @@ -295,12 +295,10 @@ int amdgpu_uvd_suspend(struct amdgpu_device *adev) size = amdgpu_bo_size(adev->uvd.vcpu_bo); ptr = adev->uvd.cpu_addr; - adev->uvd.saved_bo = kmalloc(size, GFP_KERNEL); + adev->uvd.saved_bo = kmemdup(ptr, size, GFP_KERNEL); if (!adev->uvd.saved_bo) return -ENOMEM; - memcpy(adev->uvd.saved_bo, ptr, size); - return 0; } -- 1.9.1
[Bug 92827] Tonga: No Sound over HDMI with connected AV receiver
https://bugs.freedesktop.org/show_bug.cgi?id=92827 --- Comment #2 from Alex Deucher --- Audio support requires DAL which is not upstream yet. E.g., https://cgit.freedesktop.org/~agd5f/linux/log/?h=drm-next-4.7-wip-dal -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160519/3cb318c5/attachment.html>
[Bug 92827] Tonga: No Sound over HDMI with connected AV receiver
https://bugs.freedesktop.org/show_bug.cgi?id=92827 --- Comment #1 from Maximilian Böhm --- I can still confirm this bug with Linux 4.6.0. :( -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160519/43fbc63c/attachment.html>
[PATCH v2 2/3] drm/arm: Add support for Mali Display Processors
On Mon, Apr 25, 2016 at 03:19:23PM +0100, Liviu Dudau wrote:
> Add support for the new family of Display Processors from ARM Ltd.
> This commit adds basic support for Mali DP500, DP550 and DP650
> parts, with only the display engine being supported at the moment.
>
> Cc: David Brown
> Cc: Brian Starkey
>
> Signed-off-by: Liviu Dudau
Hello,
I'm fully aware that the merge window is open and people are busy, however if
any of the DRM sub-maintainers could provide some Acks or Reviewed-bys I would
really appreciate it.
I plan to make the code available under a new branch to go into linux-next next
week.
Best regards,
Liviu
> ---
> drivers/gpu/drm/arm/Kconfig | 16 +
> drivers/gpu/drm/arm/Makefile| 2 +
> drivers/gpu/drm/arm/malidp_crtc.c | 259
> drivers/gpu/drm/arm/malidp_drv.c| 538 +
> drivers/gpu/drm/arm/malidp_drv.h| 54 +++
> drivers/gpu/drm/arm/malidp_hw.c | 774
>
> drivers/gpu/drm/arm/malidp_hw.h | 189 +
> drivers/gpu/drm/arm/malidp_planes.c | 337
> drivers/gpu/drm/arm/malidp_regs.h | 172
> 9 files changed, 2341 insertions(+)
> create mode 100644 drivers/gpu/drm/arm/malidp_crtc.c
> create mode 100644 drivers/gpu/drm/arm/malidp_drv.c
> create mode 100644 drivers/gpu/drm/arm/malidp_drv.h
> create mode 100644 drivers/gpu/drm/arm/malidp_hw.c
> create mode 100644 drivers/gpu/drm/arm/malidp_hw.h
> create mode 100644 drivers/gpu/drm/arm/malidp_planes.c
> create mode 100644 drivers/gpu/drm/arm/malidp_regs.h
>
> diff --git a/drivers/gpu/drm/arm/Kconfig b/drivers/gpu/drm/arm/Kconfig
> index eaed454..1b29065 100644
> --- a/drivers/gpu/drm/arm/Kconfig
> +++ b/drivers/gpu/drm/arm/Kconfig
> @@ -25,3 +25,19 @@ config DRM_HDLCD_SHOW_UNDERRUN
> Enable this option to show in red colour the pixels that the
> HDLCD device did not fetch from framebuffer due to underrun
> conditions.
> +
> +config DRM_MALI_DISPLAY
> + tristate "ARM Mali Display Processor"
> + depends on DRM && OF && (ARM || ARM64)
> + depends on COMMON_CLK
> + select DRM_ARM
> + select DRM_KMS_HELPER
> + select DRM_KMS_CMA_HELPER
> + select DRM_GEM_CMA_HELPER
> + select VIDEOMODE_HELPERS
> + help
> + Choose this option if you want to compile the ARM Mali Display
> + Processor driver. It supports the DP500, DP550 and DP650 variants
> + of the hardware.
> +
> + If compiled as a module it will be called mali-dp.
> diff --git a/drivers/gpu/drm/arm/Makefile b/drivers/gpu/drm/arm/Makefile
> index 89dcb7b..bb8b158 100644
> --- a/drivers/gpu/drm/arm/Makefile
> +++ b/drivers/gpu/drm/arm/Makefile
> @@ -1,2 +1,4 @@
> hdlcd-y := hdlcd_drv.o hdlcd_crtc.o
> obj-$(CONFIG_DRM_HDLCD) += hdlcd.o
> +mali-dp-y := malidp_drv.o malidp_hw.o malidp_planes.o malidp_crtc.o
> +obj-$(CONFIG_DRM_MALI_DISPLAY) += mali-dp.o
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
> b/drivers/gpu/drm/arm/malidp_crtc.c
> new file mode 100644
> index 000..12893d0
> --- /dev/null
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -0,0 +1,259 @@
> +/*
> + * (C) COPYRIGHT 2016 ARM Limited. All rights reserved.
> + * Author: Liviu Dudau
> + *
> + * This program is free software and is provided to you under the terms of
> the
> + * GNU General Public License version 2 as published by the Free Software
> + * Foundation, and any use by you of this program is subject to the terms
> + * of such GNU licence.
> + *
> + * ARM Mali DP500/DP550/DP650 driver (crtc operations)
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "malidp_drv.h"
> +#include "malidp_hw.h"
> +
> +static bool malidp_crtc_mode_fixup(struct drm_crtc *crtc,
> +const struct drm_display_mode *mode,
> +struct drm_display_mode *adjusted_mode)
> +{
> + struct malidp_drm *malidp = crtc_to_malidp_device(crtc);
> + struct malidp_hw_device *hwdev = malidp->dev;
> +
> + /*
> + * check that the hardware can drive the required clock rate,
> + * but skip the check if the clock is meant to be disabled (req_rate =
> 0)
> + */
> + long rate, req_rate = mode->crtc_clock * 1000;
> +
> + if (req_rate) {
> + rate = clk_round_rate(hwdev->mclk, req_rate);
> + if (rate < req_rate) {
> + DRM_DEBUG_DRIVER("mclk clock unable to reach %d kHz\n",
> + mode->crtc_clock);
> + return false;
> + }
> +
> + rate = clk_round_rate(hwdev->pxlclk, req_rate);
> + if (rate != req_rate) {
> + DRM_DEBUG_DRIVER("pxlclk doesn't support %ld Hz\n",
> + req_rate);
> + return false;
> + }
> + }
> +
> + return true;
> +}
> +
> +s
[Bug 95413] Oversaturation + Artifacts on screen refresh with Redwood GPU
https://bugs.freedesktop.org/show_bug.cgi?id=95413 Sawyer Bergeron changed: What|Removed |Added Resolution|--- |NOTABUG Status|NEW |RESOLVED --- Comment #6 from Sawyer Bergeron --- It would appear that there was a small issue regarding the card--there was some cat fur blocking the fan from spinning at full speed, and something was causing the motor to not compensate for that resistance. The card ended up getting fairly hot during operation, and it appears after cleaning the card that it is operating normally now. Sorry for the inconvenience this oversight on my part caused. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20160519/52c05662/attachment.html>
[PATCH v4] vga_switcheroo: Add helper for deferred probing
So far we've got one condition when DRM drivers need to defer probing
on a dual GPU system and it's coded separately into each of the relevant
drivers. As suggested by Daniel Vetter, deduplicate that code in the
drivers and move it to a new vga_switcheroo helper. This yields better
encapsulation of concepts and lets us add further checks in a central
place. (The existing check pertains to pre-retina MacBook Pros and an
additional check is expected to be needed for retinas.)
v2: This helper could eventually be used by audio clients as well,
so rephrase kerneldoc to refer to "client" instead of "GPU"
and move the single existing check in an if block specific
to PCI_CLASS_DISPLAY_VGA devices. Move documentation on
that check from kerneldoc to a comment. (Daniel Vetter)
v3: Mandate in kerneldoc that registration of client shall only
happen after calling this helper. (Daniel Vetter)
v4: Rebase on 412c8f7de011 ("drm/radeon: Return -EPROBE_DEFER when
amdkfd not loaded")
Cc: Daniel Vetter
Cc: Ben Skeggs
Cc: Alex Deucher
Signed-off-by: Lukas Wunner
---
drivers/gpu/drm/i915/i915_drv.c | 10 +-
drivers/gpu/drm/nouveau/nouveau_drm.c | 10 +-
drivers/gpu/drm/radeon/radeon_drv.c | 10 +-
drivers/gpu/vga/vga_switcheroo.c | 34 --
include/linux/vga_switcheroo.h| 2 ++
5 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index dba03c0..61bf5a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -35,11 +35,9 @@
#include "i915_trace.h"
#include "intel_drv.h"
-#include
#include
#include
#include
-#include
#include
#include
@@ -1030,13 +1028,7 @@ static int i915_pci_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
if (PCI_FUNC(pdev->devfn))
return -ENODEV;
- /*
-* apple-gmux is needed on dual GPU MacBook Pro
-* to probe the panel if we're the inactive GPU.
-*/
- if (IS_ENABLED(CONFIG_VGA_ARB) && IS_ENABLED(CONFIG_VGA_SWITCHEROO) &&
- apple_gmux_present() && pdev != vga_default_device() &&
- !vga_switcheroo_handler_flags())
+ if (vga_switcheroo_client_probe_defer(pdev))
return -EPROBE_DEFER;
return drm_get_pci_dev(pdev, ent, &driver);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index db5c7d0..a0bb1d0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -22,13 +22,11 @@
* Authors: Ben Skeggs
*/
-#include
#include
#include
#include
#include
#include
-#include
#include
#include "drmP.h"
@@ -315,13 +313,7 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
bool boot = false;
int ret;
- /*
-* apple-gmux is needed on dual GPU MacBook Pro
-* to probe the panel if we're the inactive GPU.
-*/
- if (IS_ENABLED(CONFIG_VGA_ARB) && IS_ENABLED(CONFIG_VGA_SWITCHEROO) &&
- apple_gmux_present() && pdev != vga_default_device() &&
- !vga_switcheroo_handler_flags())
+ if (vga_switcheroo_client_probe_defer(pdev))
return -EPROBE_DEFER;
/* remove conflicting drivers (vesafb, efifb etc) */
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
b/drivers/gpu/drm/radeon/radeon_drv.c
index b55aa74..a455dc7 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -34,11 +34,9 @@
#include "radeon_drv.h"
#include
-#include
#include
#include
#include
-#include
#include
#include
@@ -340,13 +338,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
if (ret == -EPROBE_DEFER)
return ret;
- /*
-* apple-gmux is needed on dual GPU MacBook Pro
-* to probe the panel if we're the inactive GPU.
-*/
- if (IS_ENABLED(CONFIG_VGA_ARB) && IS_ENABLED(CONFIG_VGA_SWITCHEROO) &&
- apple_gmux_present() && pdev != vga_default_device() &&
- !vga_switcheroo_handler_flags())
+ if (vga_switcheroo_client_probe_defer(pdev))
return -EPROBE_DEFER;
/* Get rid of things like offb */
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index cbd7c98..101e14c 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -30,6 +30,7 @@
#define pr_fmt(fmt) "vga_switcheroo: " fmt
+#include
#include
#include
#include
@@ -308,7 +309,8 @@ static int register_client(struct pci_dev *pdev,
*
* Register vga client (GPU). Enable vga_switcheroo if another GPU and a
* handler have already registered. The power state of the client is assumed
- * to be ON.
+ * to be ON. Beforehand, vga_switcheroo_client_probe_defer() shall be called
+ * to ensure that all prerequisites are met.
*
* Return: 0 on success
[PATCH] dma-buf/fence: make fence context 64 bit
On Thu, May 19, 2016 at 02:21:43PM +0200, Christian König wrote: > Am 19.05.2016 um 14:07 schrieb Daniel Vetter: > >On Thu, May 19, 2016 at 11:30 AM, Christian König > > wrote: > >>Am 19.05.2016 um 11:14 schrieb Daniel Vetter: > >>>On Thu, May 19, 2016 at 11:00:36AM +0200, Christian König wrote: > From: Christian König > > Fence contexts are created on the fly (for example) by the GPU scheduler > used > in the amdgpu driver as a result of an userspace request. Because of this > userspace could in theory force a wrap around of the 32bit context number > if it doesn't behave well. > > Avoid this by increasing the context number to 64bits. This way even when > userspace manages to allocate a billion contexts per second it takes more > than 500 years for the context number to wrap around. > > Signed-off-by: Christian König > >>>Hm, I think it'd be nice to wrap this up into a real struct and then > >>>manage them with some idr or whatever. For debugging we might also want to > >>>keep track of all fences on a given timeline and similar things, so > >>>there will be a need for this in the future. > >>> > >>>So if you go through every driver I think it's better to replace the type > >>>with struct fence_context *context while we're at it. Makes it a notch > >>>bigger since we need to add a little bit of error handling to all callers > >>>of fence_context_alloc. > >>> > >>>Volunteered? ;-) > >> > >>Well, that's exactly what I wanted to avoid. 64bit numbers are fast to > >>allocate and easy to compare. > >> > >>If I make it a structure then we would need to kmalloc() it and make sure it > >>is reference counted so it stays alive as long as any fence structure is > >>alive which is referring to it. > >> > >>The overhead sounds to much to me, especially since we currently don't have > >>a real use for that right now. > >Hm, I guess if you're worried about the kmalloc we could make > >fence_context embeddable. At least I assume you have to allcate > >something somewhere already to store the u64, and that something also > >needs to be refcounted already (or cleaned up suitably) to make sure > >it doesn't disappear before the fences go away. > > Nope, while it's true that we allocate the fence context with the command > submission context in an IOCTL for amdgpu freeing the command submission > context happens while fences using the fence context are still around. E.g. > the application could have dies, but some hardware operations are still > under way referencing the context. > > Keeping it as a structure can cause all kind of problems in OOM situations. > For example we don't have a limit on the number of contexts created. E.g. an > application could allocate a lot of command submissions context, submits a > single waiting command and exits. > > >I'm just raising this because the longer we wait with redoing this > >interface the more painful it'll be. Android at least does have a > >full-blown struct, and the reason is exclusively for debugging. And > >from what I've heard from android devs debugging fence lockups is a > >complete pain. That's why I think sooner or later there's no way > >around a full blown struct. > I actually don't think so. Having the context as a simple number gives much > greater flexibility to us because we don't need to worry about any overhead. > > For debugging you can just put the context number into a hashtable or tree > if you need to store additional information to it. The trouble is that you then still have the lifetime issues. But I guess we can fix this later on, so Ack. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
[PATCH] fbcon: use default if cursor blink interval is not valid
Hi Scot, > Use the normal cursor blink default interval of 200 ms if > ops->cur_blink_jiffies is not in the range specified in commit > bd63364caa8d. Since invalid values are not used, specific system > initialization timings should not cause lockups. This fixes an issue we're seeing with the ast driver on OpenPOWER machines, thanks! Acked-by: Jeremy Kerr Cheers, Jeremy
[PATCH] dma-buf/fence: make fence context 64 bit
Am 19.05.2016 um 14:07 schrieb Daniel Vetter: > On Thu, May 19, 2016 at 11:30 AM, Christian König > wrote: >> Am 19.05.2016 um 11:14 schrieb Daniel Vetter: >>> On Thu, May 19, 2016 at 11:00:36AM +0200, Christian König wrote: From: Christian König Fence contexts are created on the fly (for example) by the GPU scheduler used in the amdgpu driver as a result of an userspace request. Because of this userspace could in theory force a wrap around of the 32bit context number if it doesn't behave well. Avoid this by increasing the context number to 64bits. This way even when userspace manages to allocate a billion contexts per second it takes more than 500 years for the context number to wrap around. Signed-off-by: Christian König >>> Hm, I think it'd be nice to wrap this up into a real struct and then >>> manage them with some idr or whatever. For debugging we might also want to >>> keep track of all fences on a given timeline and similar things, so >>> there will be a need for this in the future. >>> >>> So if you go through every driver I think it's better to replace the type >>> with struct fence_context *context while we're at it. Makes it a notch >>> bigger since we need to add a little bit of error handling to all callers >>> of fence_context_alloc. >>> >>> Volunteered? ;-) >> >> Well, that's exactly what I wanted to avoid. 64bit numbers are fast to >> allocate and easy to compare. >> >> If I make it a structure then we would need to kmalloc() it and make sure it >> is reference counted so it stays alive as long as any fence structure is >> alive which is referring to it. >> >> The overhead sounds to much to me, especially since we currently don't have >> a real use for that right now. > Hm, I guess if you're worried about the kmalloc we could make > fence_context embeddable. At least I assume you have to allcate > something somewhere already to store the u64, and that something also > needs to be refcounted already (or cleaned up suitably) to make sure > it doesn't disappear before the fences go away. Nope, while it's true that we allocate the fence context with the command submission context in an IOCTL for amdgpu freeing the command submission context happens while fences using the fence context are still around. E.g. the application could have dies, but some hardware operations are still under way referencing the context. Keeping it as a structure can cause all kind of problems in OOM situations. For example we don't have a limit on the number of contexts created. E.g. an application could allocate a lot of command submissions context, submits a single waiting command and exits. > I'm just raising this because the longer we wait with redoing this > interface the more painful it'll be. Android at least does have a > full-blown struct, and the reason is exclusively for debugging. And > from what I've heard from android devs debugging fence lockups is a > complete pain. That's why I think sooner or later there's no way > around a full blown struct. I actually don't think so. Having the context as a simple number gives much greater flexibility to us because we don't need to worry about any overhead. For debugging you can just put the context number into a hashtable or tree if you need to store additional information to it. Christian. > -Daniel
[PATCH] amdgpu/uvd: use kmemdup
Am 19.05.2016 um 13:49 schrieb Muhammad Falak R Wani: > On Thu, May 19, 2016 at 01:18:10PM +0200, Christian König wrote: >> Am 19.05.2016 um 13:11 schrieb Muhammad Falak R Wani: >>> Use kmemdup when some other buffer is immediately copied into allocated >>> region. It replaces call to allocation followed by memcpy, by a single >>> call to kmemdup. >>> >>> Signed-off-by: Muhammad Falak R Wani >> NAK, actually using memcpy() is wrong in the first place. >> >> The UVD BO is in VRAM so the pointer is pointing to IO memory here, >> so this should be memcpy_fromio() instead of memcpy(). >> >> Christian. >> > Should I send V2 with the required changes, and I had a query, I can take care of fixing this. > If memcpy was wrong, did it still work or it just got un-noticed ? Both, on X86 memcpy from IO memory works fine. Only on other architectures you run into problems with that. Regards, Christian.
[PATCH] dma-buf/fence: make fence context 64 bit
On Thu, May 19, 2016 at 11:30 AM, Christian König wrote: > Am 19.05.2016 um 11:14 schrieb Daniel Vetter: >> >> On Thu, May 19, 2016 at 11:00:36AM +0200, Christian König wrote: >>> >>> From: Christian König >>> >>> Fence contexts are created on the fly (for example) by the GPU scheduler >>> used >>> in the amdgpu driver as a result of an userspace request. Because of this >>> userspace could in theory force a wrap around of the 32bit context number >>> if it doesn't behave well. >>> >>> Avoid this by increasing the context number to 64bits. This way even when >>> userspace manages to allocate a billion contexts per second it takes more >>> than 500 years for the context number to wrap around. >>> >>> Signed-off-by: Christian König >> >> Hm, I think it'd be nice to wrap this up into a real struct and then >> manage them with some idr or whatever. For debugging we might also want to >> keep track of all fences on a given timeline and similar things, so >> there will be a need for this in the future. >> >> So if you go through every driver I think it's better to replace the type >> with struct fence_context *context while we're at it. Makes it a notch >> bigger since we need to add a little bit of error handling to all callers >> of fence_context_alloc. >> >> Volunteered? ;-) > > > Well, that's exactly what I wanted to avoid. 64bit numbers are fast to > allocate and easy to compare. > > If I make it a structure then we would need to kmalloc() it and make sure it > is reference counted so it stays alive as long as any fence structure is > alive which is referring to it. > > The overhead sounds to much to me, especially since we currently don't have > a real use for that right now. Hm, I guess if you're worried about the kmalloc we could make fence_context embeddable. At least I assume you have to allcate something somewhere already to store the u64, and that something also needs to be refcounted already (or cleaned up suitably) to make sure it doesn't disappear before the fences go away. I'm just raising this because the longer we wait with redoing this interface the more painful it'll be. Android at least does have a full-blown struct, and the reason is exclusively for debugging. And from what I've heard from android devs debugging fence lockups is a complete pain. That's why I think sooner or later there's no way around a full blown struct. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH V5 0/3] soc/tegra: Add support for IO pads power and voltage control
Hi Jon/Thierry, On Thursday 12 May 2016 05:51 PM, Laxman Dewangan wrote: > The IO pins of Tegra SoCs are grouped for common control of IO interface > like setting voltage signal levels and power state of the interface. The > group is generally referred as IO pads. The power state and voltage control > of IO pins can be done at IO pads level. > > Tegra124 onwards IO pads support the power down state when system is > active. The voltage levels on IO pads are auto detected on Tegra124/ > Tegra132 but it is not in Tegra210. For Tegra210, the SW need to > explicitly configure the voltage levels of IO pads > > This series add the interface for the IO pad power state and voltage > configurations. Modifies the client to use new APIs. > Register the child devices to provide the client interface to configure > IO pads power state and voltage from Device tree. > Any comment on this series?
[PATCH] amdgpu/uvd: use kmemdup
Am 19.05.2016 um 13:11 schrieb Muhammad Falak R Wani: > Use kmemdup when some other buffer is immediately copied into allocated > region. It replaces call to allocation followed by memcpy, by a single > call to kmemdup. > > Signed-off-by: Muhammad Falak R Wani NAK, actually using memcpy() is wrong in the first place. The UVD BO is in VRAM so the pointer is pointing to IO memory here, so this should be memcpy_fromio() instead of memcpy(). Christian. > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c > index 01abfc2..c977ab6 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c > @@ -295,12 +295,10 @@ int amdgpu_uvd_suspend(struct amdgpu_device *adev) > size = amdgpu_bo_size(adev->uvd.vcpu_bo); > ptr = adev->uvd.cpu_addr; > > - adev->uvd.saved_bo = kmalloc(size, GFP_KERNEL); > + adev->uvd.saved_bo = kmemdup(ptr, size, GFP_KERNEL); > if (!adev->uvd.saved_bo) > return -ENOMEM; > > - memcpy(adev->uvd.saved_bo, ptr, size); > - > return 0; > } >
AMDGPU SI userspace support - experimental
Hi, The following branches add SI support to the amdgpu side of our graphics driver stack. git://people.freedesktop.org/~mareko/mesa amdgpu-si git://people.freedesktop.org/~mareko/libdrm amdgpu-si git://people.freedesktop.org/~mareko/xf86-video-amdgpu amdgpu-si Thanks to Ronie Salgado for starting this. This should be considered a work in progress, because it's not in a working state yet. Below is a summary of what works and what doesn't. The current kernel support in the drm-next-4.8-wip-si branch doesn't work. It hangs or deadlocks on the first command submission that only exercises EVENT_WRITE(ZPASS_DONE) and doesn't use draw packets. I think it's a rebase issue, because a previous version of the kernel support (based on kernel 4.4) still works and piglit on GBM gives the same or similar results as radeon. The X server has never worked for me. The screen is mostly black and contains funny colors at the bottom. Marek
[PATCH] gpu/nouveau/nouveau_acpi.c: Fix Type Mismatch ACPI warning
union acpi_object argv4 = {
> - .buffer.type = ACPI_TYPE_BUFFER,
> - .buffer.length = 4,
> - .buffer.pointer = args_buff
> - };
> -
> - /* ACPI is little endian, AABBCCDD becomes {DD,CC,BB,AA} */
> - for (i = 0; i < 4; i++)
> - args_buff[i] = (arg >> i * 8) & 0xFF;
> -
> *result = 0;
> obj = acpi_evaluate_dsm_typed(handle, nouveau_op_dsm_muid, 0x0100,
> - func, &argv4, ACPI_TYPE_BUFFER);
> + func, NULL, ACPI_TYPE_PACKAGE);
The last parameter you give to `acpi_evaluate_dsm_typed()` is the return type
you expect (see [3]), which will be a buffer if func is 0, and is
implementation dependent otherwise (see section 9.14.1 _DSM of [4]). So you
donât want to change it to ACPI_TYPE_PACKAGE. If you look at the
implementation
of `acpi_evaluate_dsm()` (which is called by `acpi_evaluate_dsm_typed()`), it
will automatically create a package for the 4th argument, if you pass it a NULL
pointer (see [5]).
[3]:
https://github.com/torvalds/linux/blob/46c13450624e36302547a2ac3695f2350fe7ffc3/include/acpi/acpi_bus.h#L69
[4]: http://www.acpi.info/DOWNLOADS/ACPI_5_Errata%20A.pdf
[5]:
https://github.com/torvalds/linux/blob/46c13450624e36302547a2ac3695f2350fe7ffc3/drivers/acpi/utils.c#L628
> if (!obj) {
> acpi_handle_info(handle, "failed to evaluate _DSM\n");
> return AE_ERROR;
> --
> 2.5.5
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160519/8c17ee53/attachment-0001.sig>
[PATCH v2 1/5] drm/dp: Add drm_dp_psr_setup_time()
From: Ville Syrjälä
Add a small helper to parse the PSR setup time from the DPCD PSR
capabilities and return the value in microseconds.
v2: Don't waste so many bytes on the psr_setup_time_us[] table
Cc: Daniel Vetter
Reviewed-by: Daniel Vetter
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/drm_dp_helper.c | 32
include/drm/drm_dp_helper.h | 2 ++
2 files changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index eeaf5a7c3aa7..1f914629031e 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -822,3 +822,35 @@ void drm_dp_aux_unregister(struct drm_dp_aux *aux)
i2c_del_adapter(&aux->ddc);
}
EXPORT_SYMBOL(drm_dp_aux_unregister);
+
+#define PSR_SETUP_TIME(x) [DP_PSR_SETUP_TIME_ ## x >> DP_PSR_SETUP_TIME_SHIFT]
= (x)
+
+/**
+ * drm_dp_psr_setup_time() - PSR setup in time usec
+ * @psr_cap: PSR capabilities from DPCD
+ *
+ * Returns:
+ * PSR setup time for the panel in microseconds, negative
+ * error code on failure.
+ */
+int drm_dp_psr_setup_time(const u8 psr_cap[EDP_PSR_RECEIVER_CAP_SIZE])
+{
+ static const u16 psr_setup_time_us[] = {
+ PSR_SETUP_TIME(330),
+ PSR_SETUP_TIME(275),
+ PSR_SETUP_TIME(165),
+ PSR_SETUP_TIME(110),
+ PSR_SETUP_TIME(55),
+ PSR_SETUP_TIME(0),
+ };
+ int i;
+
+ i = (psr_cap[1] & DP_PSR_SETUP_TIME_MASK) >> DP_PSR_SETUP_TIME_SHIFT;
+ if (i >= ARRAY_SIZE(psr_setup_time_us))
+ return -EINVAL;
+
+ return psr_setup_time_us[i];
+}
+EXPORT_SYMBOL(drm_dp_psr_setup_time);
+
+#undef PSR_SETUP_TIME
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 5a848e734422..6aa74f7d45b4 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -657,6 +657,8 @@ struct edp_vsc_psr {
#define EDP_VSC_PSR_UPDATE_RFB (1<<1)
#define EDP_VSC_PSR_CRC_VALUES_VALID (1<<2)
+int drm_dp_psr_setup_time(const u8 psr_cap[EDP_PSR_RECEIVER_CAP_SIZE]);
+
static inline int
drm_dp_max_link_rate(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
{
--
2.7.4
[PATCH] dma-buf/fence: make fence context 64 bit
Am 19.05.2016 um 11:14 schrieb Daniel Vetter:
> On Thu, May 19, 2016 at 11:00:36AM +0200, Christian König wrote:
>> From: Christian König
>>
>> Fence contexts are created on the fly (for example) by the GPU scheduler used
>> in the amdgpu driver as a result of an userspace request. Because of this
>> userspace could in theory force a wrap around of the 32bit context number
>> if it doesn't behave well.
>>
>> Avoid this by increasing the context number to 64bits. This way even when
>> userspace manages to allocate a billion contexts per second it takes more
>> than 500 years for the context number to wrap around.
>>
>> Signed-off-by: Christian König
> Hm, I think it'd be nice to wrap this up into a real struct and then
> manage them with some idr or whatever. For debugging we might also want to
> keep track of all fences on a given timeline and similar things, so
> there will be a need for this in the future.
>
> So if you go through every driver I think it's better to replace the type
> with struct fence_context *context while we're at it. Makes it a notch
> bigger since we need to add a little bit of error handling to all callers
> of fence_context_alloc.
>
> Volunteered? ;-)
Well, that's exactly what I wanted to avoid. 64bit numbers are fast to
allocate and easy to compare.
If I make it a structure then we would need to kmalloc() it and make
sure it is reference counted so it stays alive as long as any fence
structure is alive which is referring to it.
The overhead sounds to much to me, especially since we currently don't
have a real use for that right now.
Christian.
>
> Cheers, Daniel
>> ---
>> drivers/dma-buf/fence.c | 8
>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +-
>> drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 2 +-
>> drivers/gpu/drm/nouveau/nouveau_fence.h | 3 ++-
>> drivers/gpu/drm/radeon/radeon.h | 2 +-
>> drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 2 +-
>> drivers/staging/android/sync.h | 3 ++-
>> include/linux/fence.h | 7 ---
>> 8 files changed, 16 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
>> index 7b05dbe..4d51f9e 100644
>> --- a/drivers/dma-buf/fence.c
>> +++ b/drivers/dma-buf/fence.c
>> @@ -35,7 +35,7 @@ EXPORT_TRACEPOINT_SYMBOL(fence_emit);
>>* context or not. One device can have multiple separate contexts,
>>* and they're used if some engine can run independently of another.
>>*/
>> -static atomic_t fence_context_counter = ATOMIC_INIT(0);
>> +static atomic64_t fence_context_counter = ATOMIC64_INIT(0);
>>
>> /**
>>* fence_context_alloc - allocate an array of fence contexts
>> @@ -44,10 +44,10 @@ static atomic_t fence_context_counter = ATOMIC_INIT(0);
>>* This function will return the first index of the number of fences
>> allocated.
>>* The fence context is used for setting fence->context to a unique number.
>>*/
>> -unsigned fence_context_alloc(unsigned num)
>> +u64 fence_context_alloc(unsigned num)
>> {
>> BUG_ON(!num);
>> -return atomic_add_return(num, &fence_context_counter) - num;
>> +return atomic64_add_return(num, &fence_context_counter) - num;
>> }
>> EXPORT_SYMBOL(fence_context_alloc);
>>
>> @@ -513,7 +513,7 @@ EXPORT_SYMBOL(fence_wait_any_timeout);
>>*/
>> void
>> fence_init(struct fence *fence, const struct fence_ops *ops,
>> - spinlock_t *lock, unsigned context, unsigned seqno)
>> + spinlock_t *lock, u64 context, unsigned seqno)
>> {
>> BUG_ON(!lock);
>> BUG_ON(!ops || !ops->wait || !ops->enable_signaling ||
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 992f00b..da3d021 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -2032,7 +2032,7 @@ struct amdgpu_device {
>> struct amdgpu_irq_src hpd_irq;
>>
>> /* rings */
>> -unsignedfence_context;
>> +u64 fence_context;
>> unsignednum_rings;
>> struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
>> boolib_pool_ready;
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
>> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
>> index f5321e2..a69cdd5 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
>> @@ -125,7 +125,7 @@ struct etnaviv_gpu {
>> u32 completed_fence;
>> u32 retired_fence;
>> wait_queue_head_t fence_event;
>> -unsigned int fence_context;
>> +u64 fence_context;
>> spinlock_t fence_spinlock;
>>
>> /* worker for handling active-list retiring: */
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.h
>> b/drivers/gpu/drm/nouveau/nouveau_fence.h
>> index 2e3a62d..64c4ce7 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_fence.h
>> +++ b/drivers/gpu/drm/
[PATCH] dma-buf/fence: make fence context 64 bit
On Thu, May 19, 2016 at 11:00:36AM +0200, Christian König wrote:
> From: Christian König
>
> Fence contexts are created on the fly (for example) by the GPU scheduler used
> in the amdgpu driver as a result of an userspace request. Because of this
> userspace could in theory force a wrap around of the 32bit context number
> if it doesn't behave well.
>
> Avoid this by increasing the context number to 64bits. This way even when
> userspace manages to allocate a billion contexts per second it takes more
> than 500 years for the context number to wrap around.
>
> Signed-off-by: Christian König
Hm, I think it'd be nice to wrap this up into a real struct and then
manage them with some idr or whatever. For debugging we might also want to
keep track of all fences on a given timeline and similar things, so
there will be a need for this in the future.
So if you go through every driver I think it's better to replace the type
with struct fence_context *context while we're at it. Makes it a notch
bigger since we need to add a little bit of error handling to all callers
of fence_context_alloc.
Volunteered? ;-)
Cheers, Daniel
> ---
> drivers/dma-buf/fence.c | 8
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +-
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 2 +-
> drivers/gpu/drm/nouveau/nouveau_fence.h | 3 ++-
> drivers/gpu/drm/radeon/radeon.h | 2 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 2 +-
> drivers/staging/android/sync.h | 3 ++-
> include/linux/fence.h | 7 ---
> 8 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
> index 7b05dbe..4d51f9e 100644
> --- a/drivers/dma-buf/fence.c
> +++ b/drivers/dma-buf/fence.c
> @@ -35,7 +35,7 @@ EXPORT_TRACEPOINT_SYMBOL(fence_emit);
> * context or not. One device can have multiple separate contexts,
> * and they're used if some engine can run independently of another.
> */
> -static atomic_t fence_context_counter = ATOMIC_INIT(0);
> +static atomic64_t fence_context_counter = ATOMIC64_INIT(0);
>
> /**
> * fence_context_alloc - allocate an array of fence contexts
> @@ -44,10 +44,10 @@ static atomic_t fence_context_counter = ATOMIC_INIT(0);
> * This function will return the first index of the number of fences
> allocated.
> * The fence context is used for setting fence->context to a unique number.
> */
> -unsigned fence_context_alloc(unsigned num)
> +u64 fence_context_alloc(unsigned num)
> {
> BUG_ON(!num);
> - return atomic_add_return(num, &fence_context_counter) - num;
> + return atomic64_add_return(num, &fence_context_counter) - num;
> }
> EXPORT_SYMBOL(fence_context_alloc);
>
> @@ -513,7 +513,7 @@ EXPORT_SYMBOL(fence_wait_any_timeout);
> */
> void
> fence_init(struct fence *fence, const struct fence_ops *ops,
> - spinlock_t *lock, unsigned context, unsigned seqno)
> + spinlock_t *lock, u64 context, unsigned seqno)
> {
> BUG_ON(!lock);
> BUG_ON(!ops || !ops->wait || !ops->enable_signaling ||
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 992f00b..da3d021 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -2032,7 +2032,7 @@ struct amdgpu_device {
> struct amdgpu_irq_src hpd_irq;
>
> /* rings */
> - unsignedfence_context;
> + u64 fence_context;
> unsignednum_rings;
> struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
> boolib_pool_ready;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index f5321e2..a69cdd5 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -125,7 +125,7 @@ struct etnaviv_gpu {
> u32 completed_fence;
> u32 retired_fence;
> wait_queue_head_t fence_event;
> - unsigned int fence_context;
> + u64 fence_context;
> spinlock_t fence_spinlock;
>
> /* worker for handling active-list retiring: */
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.h
> b/drivers/gpu/drm/nouveau/nouveau_fence.h
> index 2e3a62d..64c4ce7 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fence.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.h
> @@ -57,7 +57,8 @@ struct nouveau_fence_priv {
> int (*context_new)(struct nouveau_channel *);
> void (*context_del)(struct nouveau_channel *);
>
> - u32 contexts, context_base;
> + u32 contexts;
> + u64 context_base;
> bool uevent;
> };
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 80b24a4..5633ee3 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -2386,7 +2386,7 @@ struct radeon_device {
> struct radeon_mman
[PATCH] drm/amd/powerplay/hwmgr: use kmemdup
On Thu, May 19, 2016 at 7:15 AM, Muhammad Falak R Wani
wrote:
> Use kmemdup when some other buffer is immediately copied into allocated
> region. It replaces call to allocation followed by memcpy, by a single
> call to kmemdup.
>
> Signed-off-by: Muhammad Falak R Wani
Applied the hwmgr patches.
Thanks!
Alex
> ---
> drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
> index c94f9fa..1471ac3 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
> @@ -5109,11 +5109,11 @@ static int fiji_get_pp_table(struct pp_hwmgr *hwmgr,
> char **table)
> struct fiji_hwmgr *data = (struct fiji_hwmgr *)(hwmgr->backend);
>
> if (!data->soft_pp_table) {
> - data->soft_pp_table = kzalloc(hwmgr->soft_pp_table_size,
> GFP_KERNEL);
> + data->soft_pp_table = kmemdup(hwmgr->soft_pp_table,
> + hwmgr->soft_pp_table_size,
> + GFP_KERNEL);
> if (!data->soft_pp_table)
> return -ENOMEM;
> - memcpy(data->soft_pp_table, hwmgr->soft_pp_table,
> - hwmgr->soft_pp_table_size);
> }
>
> *table = (char *)&data->soft_pp_table;
> --
> 1.9.1
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] fbcon: use default if cursor blink interval is not valid
Hi!
> Two current [1] and three previous [2] systems locked during boot
> because the cursor flash timer was set using an ops->cur_blink_jiffies
> value of 0. Previous patches attempted to solve the problem by moving
> variable initialization earlier in the setup sequence [2].
>
> Use the normal cursor blink default interval of 200 ms if
> ops->cur_blink_jiffies is not in the range specified in commit
> bd63364caa8d. Since invalid values are not used, specific system
> initialization timings should not cause lockups.
>
> [1] https://bugs.launchpad.net/bugs/1574814
> [2] see commits: 2a17d7e80f1d, f235f664a8af, a1e533ec07d5
Acked-by: Pavel Machek
> static void cursor_timer_handler(unsigned long dev_addr)
> {
> struct fb_info *info = (struct fb_info *) dev_addr;
> struct fbcon_ops *ops = info->fbcon_par;
>
> queue_work(system_power_efficient_wq, &info->queue);
> - mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies);
> + mod_timer(&ops->cursor_timer, jiffies +
> + cursor_blink_jiffies(ops->cur_blink_jiffies));
> }
>
> static void fbcon_add_cursor_timer(struct fb_info *info)
And actually... perhaps mod_timer should have some check for too low
timeouts..?
WARN_ON?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[PATCH] dma-buf/fence: make fence context 64 bit
From: Christian König
Fence contexts are created on the fly (for example) by the GPU scheduler used
in the amdgpu driver as a result of an userspace request. Because of this
userspace could in theory force a wrap around of the 32bit context number
if it doesn't behave well.
Avoid this by increasing the context number to 64bits. This way even when
userspace manages to allocate a billion contexts per second it takes more
than 500 years for the context number to wrap around.
Signed-off-by: Christian König
---
drivers/dma-buf/fence.c | 8
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +-
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 2 +-
drivers/gpu/drm/nouveau/nouveau_fence.h | 3 ++-
drivers/gpu/drm/radeon/radeon.h | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_fence.c | 2 +-
drivers/staging/android/sync.h | 3 ++-
include/linux/fence.h | 7 ---
8 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
index 7b05dbe..4d51f9e 100644
--- a/drivers/dma-buf/fence.c
+++ b/drivers/dma-buf/fence.c
@@ -35,7 +35,7 @@ EXPORT_TRACEPOINT_SYMBOL(fence_emit);
* context or not. One device can have multiple separate contexts,
* and they're used if some engine can run independently of another.
*/
-static atomic_t fence_context_counter = ATOMIC_INIT(0);
+static atomic64_t fence_context_counter = ATOMIC64_INIT(0);
/**
* fence_context_alloc - allocate an array of fence contexts
@@ -44,10 +44,10 @@ static atomic_t fence_context_counter = ATOMIC_INIT(0);
* This function will return the first index of the number of fences allocated.
* The fence context is used for setting fence->context to a unique number.
*/
-unsigned fence_context_alloc(unsigned num)
+u64 fence_context_alloc(unsigned num)
{
BUG_ON(!num);
- return atomic_add_return(num, &fence_context_counter) - num;
+ return atomic64_add_return(num, &fence_context_counter) - num;
}
EXPORT_SYMBOL(fence_context_alloc);
@@ -513,7 +513,7 @@ EXPORT_SYMBOL(fence_wait_any_timeout);
*/
void
fence_init(struct fence *fence, const struct fence_ops *ops,
-spinlock_t *lock, unsigned context, unsigned seqno)
+spinlock_t *lock, u64 context, unsigned seqno)
{
BUG_ON(!lock);
BUG_ON(!ops || !ops->wait || !ops->enable_signaling ||
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 992f00b..da3d021 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -2032,7 +2032,7 @@ struct amdgpu_device {
struct amdgpu_irq_src hpd_irq;
/* rings */
- unsignedfence_context;
+ u64 fence_context;
unsignednum_rings;
struct amdgpu_ring *rings[AMDGPU_MAX_RINGS];
boolib_pool_ready;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index f5321e2..a69cdd5 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -125,7 +125,7 @@ struct etnaviv_gpu {
u32 completed_fence;
u32 retired_fence;
wait_queue_head_t fence_event;
- unsigned int fence_context;
+ u64 fence_context;
spinlock_t fence_spinlock;
/* worker for handling active-list retiring: */
diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.h
b/drivers/gpu/drm/nouveau/nouveau_fence.h
index 2e3a62d..64c4ce7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.h
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.h
@@ -57,7 +57,8 @@ struct nouveau_fence_priv {
int (*context_new)(struct nouveau_channel *);
void (*context_del)(struct nouveau_channel *);
- u32 contexts, context_base;
+ u32 contexts;
+ u64 context_base;
bool uevent;
};
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 80b24a4..5633ee3 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2386,7 +2386,7 @@ struct radeon_device {
struct radeon_mman mman;
struct radeon_fence_driver fence_drv[RADEON_NUM_RINGS];
wait_queue_head_t fence_queue;
- unsignedfence_context;
+ u64 fence_context;
struct mutexring_lock;
struct radeon_ring ring[RADEON_NUM_RINGS];
boolib_pool_ready;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
index e959df6..26ac8e8 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fence.c
@@ -46,7 +46,7 @@ struct vmw_fence_manager {
bool goal_irq_on; /* Protected by @goal_irq_mutex */
bool seqno_valid; /* Protected
[PATCH 1/2] dma-buf/fence: add fence_collection fences
Hi Christian,
2016-05-19 Christian König :
> Am 19.05.2016 um 00:57 schrieb Chris Wilson:
> > On Wed, May 18, 2016 at 05:59:52PM -0300, Gustavo Padovan wrote:
> > > +static void collection_check_cb_func(struct fence *fence, struct
> > > fence_cb *cb)
> > > +{
> > > + struct fence_collection_cb *f_cb;
> > > + struct fence_collection *collection;
> > > +
> > > + f_cb = container_of(cb, struct fence_collection_cb, cb);
> > > + collection = f_cb->collection;
> > > +
> > > + if (atomic_dec_and_test(&collection->num_pending_fences))
> > > + fence_signal(&collection->base);
> > > +}
> > > +
> > > +static bool fence_collection_enable_signaling(struct fence *fence)
> > > +{
> > > + struct fence_collection *collection = to_fence_collection(fence);
> > > + int i;
> > > +
> > > + for (i = 0 ; i < collection->num_fences ; i++) {
> > > + if (fence_add_callback(collection->fences[i].fence,
> > > +&collection->fences[i].cb,
> > > +collection_check_cb_func)) {
> > > + atomic_dec(&collection->num_pending_fences);
> > > + }
> > > + }
> > We don't always have a convenient means to preallocate an array of
> > fences to use. Keeping a list of fences in addition to the array would
> > be easier to user in many circumstances.
>
> I agree that there is use for such an implementation as well, but as
> mentioned in the last review cycle we intentionally chose an array instead
> of a more complex implementation here.
>
> This way the array can be passed to function like fence_wait_any_timeout()
> as well.
>
> I also suggested to rename it to fence_array to make that difference clear
> and allow for another implementation to live side by side with this.
>
> My crux at the moment is that I need both for the amdgpu driver, an array
> based implementation and a collection like one.
>
> Gustavo would you mind if I take your patches and work a bit on this?
Please go ahead with this and let me know if you need anything from me.
Gustavo
[PATCH 2/2] drm/i915/mst: Reset MST after resume when necessary
A follow-up to the previous commit, we skip checking the status of the
MST device and completely reprobe it if drm_dp_mst_topology_mgr_resume()
returns -EINVAL.
Cc: stable at vger.kernel.org
Signed-off-by: Lyude
---
drivers/gpu/drm/i915/intel_dp.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index db6a0fd..5b62f7e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6063,6 +6063,7 @@ void intel_dp_mst_suspend(struct drm_device *dev)
void intel_dp_mst_resume(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_dp_mst_topology_mgr *mgr;
int i;
for (i = 0; i < I915_MAX_PORTS; i++) {
@@ -6075,8 +6076,14 @@ void intel_dp_mst_resume(struct drm_device *dev)
if (!intel_dig_port->dp.can_mst)
continue;
- ret =
drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
- if (ret != 0) {
+ mgr = &intel_dig_port->dp.mst_mgr;
+
+ ret = drm_dp_mst_topology_mgr_resume(mgr);
+ /* A full reset is required */
+ if (ret == -EINVAL) {
+ drm_dp_mst_topology_mgr_set_mst(mgr, false);
+ intel_dp_probe_mst(&intel_dig_port->dp);
+ } else if (ret != 0) {
intel_dp_check_mst_status(&intel_dig_port->dp);
}
}
--
2.5.5
[PATCH 1/2] drm/dp/mst: Reprobe EDID for MST ports on resume
As observed with the latest ThinkPad docks, we unfortunately can't rely
on docks keeping us updated with hotplug events that happened while we
were suspended. On top of that, even if the number of connectors remains
the same between suspend and resume it's still not safe to assume that
there were no hotplugs, since a different monitor might have been
plugged into a port another monitor previously occupied. As such, we
need to go through all of the MST ports and check whether or not their
EDIDs have changed.
In addition to that, we also now return -EINVAL from
drm_dp_mst_topology_mgr_resume to indicate to callers that they need to
reset the MST connection, and that they can't rely on any other method
of reprobing.
Cc: stable at vger.kernel.org
Signed-off-by: Lyude
---
drivers/gpu/drm/drm_dp_mst_topology.c | 92 ++-
1 file changed, 91 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 71ea052..cc68c74 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
@@ -2118,6 +2119,62 @@ void drm_dp_mst_topology_mgr_suspend(struct
drm_dp_mst_topology_mgr *mgr)
}
EXPORT_SYMBOL(drm_dp_mst_topology_mgr_suspend);
+static bool drm_dp_mst_edids_changed(struct drm_dp_mst_topology_mgr *mgr,
+struct drm_dp_mst_port *port)
+{
+ struct drm_device *dev;
+ struct drm_connector *connector;
+ struct drm_dp_mst_port *dport;
+ struct drm_dp_mst_branch *mstb;
+ struct edid *current_edid, *cached_edid;
+ bool ret = false;
+
+ port = drm_dp_get_validated_port_ref(mgr, port);
+ if (!port)
+ return false;
+
+ mstb = drm_dp_get_validated_mstb_ref(mgr, port->mstb);
+ if (mstb) {
+ list_for_each_entry(dport, &port->mstb->ports, next) {
+ ret = drm_dp_mst_edids_changed(mgr, dport);
+ if (ret)
+ break;
+ }
+
+ drm_dp_put_mst_branch_device(mstb);
+ if (ret)
+ goto out;
+ }
+
+ connector = port->connector;
+ if (!connector || !port->aux.ddc.algo)
+ goto out;
+
+ dev = connector->dev;
+ mutex_lock(&dev->mode_config.mutex);
+
+ current_edid = drm_get_edid(connector, &port->aux.ddc);
+ cached_edid = (void*)connector->edid_blob_ptr->data;
+
+ if ((current_edid && cached_edid && memcmp(current_edid, cached_edid,
+ sizeof(struct edid)) != 0) ||
+ (!current_edid && cached_edid) || (current_edid && !cached_edid)) {
+ ret = true;
+ DRM_DEBUG_KMS("EDID on %s changed, reprobing connectors\n",
+ connector->name);
+ }
+
+ mutex_unlock(&dev->mode_config.mutex);
+
+ if (current_edid)
+ kfree(current_edid);
+
+out:
+ drm_dp_put_port(port);
+
+ return ret;
+}
+
/**
* drm_dp_mst_topology_mgr_resume() - resume the MST manager
* @mgr: manager to resume
@@ -2127,9 +2184,15 @@ EXPORT_SYMBOL(drm_dp_mst_topology_mgr_suspend);
*
* if the device fails this returns -1, and the driver should do
* a full MST reprobe, in case we were undocked.
+ *
+ * if the device can no longer be trusted, this returns -EINVAL
+ * and the driver should unconditionally disconnect and reconnect
+ * the dock.
*/
int drm_dp_mst_topology_mgr_resume(struct drm_dp_mst_topology_mgr *mgr)
{
+ struct drm_dp_mst_branch *mstb;
+ struct drm_dp_mst_port *port;
int ret = 0;
mutex_lock(&mgr->lock);
@@ -2163,8 +2226,35 @@ int drm_dp_mst_topology_mgr_resume(struct
drm_dp_mst_topology_mgr *mgr)
drm_dp_check_mstb_guid(mgr->mst_primary, guid);
ret = 0;
- } else
+
+ /*
+* Some hubs also forget to notify us of hotplugs that happened
+* while we were in suspend, so we need to verify that the edid
+* hasn't changed for any of the connectors. If it has been,
+* we unfortunately can't rely on the dock updating us with
+* hotplug events, so indicate we need a full reconnect.
+*/
+
+ /* MST's I2C helpers can't be used while holding this lock */
+ mutex_unlock(&mgr->lock);
+
+ mstb = drm_dp_get_validated_mstb_ref(mgr, mgr->mst_primary);
+ if (mstb) {
+ list_for_each_entry(port, &mstb->ports, next) {
+ if (drm_dp_mst_edids_changed(mgr, port)) {
+ ret = -EINVAL;
+ break;
+ }
+ }
+
+
next-20160518 build: 2 failures 20 warnings (next-20160518)
Hi Chris,
On Wed, 18 May 2016 11:23:57 +0100 Chris Wilson
wrote:
>
> I'm pretty sure they didn't exist when I wrote the patch!
Yeah, but they do in the drm-misc tree where your patch was applied ...
so someone should do a fix patch to be applied to that tree. In the
mean time, I will apply the below as a merge fix in linux-next.
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> index 33d30c19f35f..147df85399ab 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_fb.c
> @@ -138,7 +138,7 @@ struct drm_framebuffer *mtk_drm_mode_fb_create(struct
> drm_device *dev,
> if (drm_format_num_planes(cmd->pixel_format) != 1)
> return ERR_PTR(-EINVAL);
>
> - gem = drm_gem_object_lookup(dev, file, cmd->handles[0]);
> + gem = drm_gem_object_lookup(file, cmd->handles[0]);
> if (!gem)
> return ERR_PTR(-ENOENT);
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> index a773bfaea913..fa2ec0cd00e8 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c
> @@ -139,7 +139,7 @@ int mtk_drm_gem_dumb_map_offset(struct drm_file
> *file_priv,
> struct drm_gem_object *obj;
> int ret;
>
> - obj = drm_gem_object_lookup(dev, file_priv, handle);
> + obj = drm_gem_object_lookup(file_priv, handle);
> if (!obj) {
> DRM_ERROR("failed to lookup gem object.\n");
> return -EINVAL;
--
Cheers,
Stephen Rothwell
[PATCH 1/2] dma-buf/fence: add fence_collection fences
On Thu, May 19, 2016 at 09:46:47AM +0200, Christian König wrote:
> Am 19.05.2016 um 00:57 schrieb Chris Wilson:
> >On Wed, May 18, 2016 at 05:59:52PM -0300, Gustavo Padovan wrote:
> >>+static void collection_check_cb_func(struct fence *fence, struct fence_cb
> >>*cb)
> >>+{
> >>+ struct fence_collection_cb *f_cb;
> >>+ struct fence_collection *collection;
> >>+
> >>+ f_cb = container_of(cb, struct fence_collection_cb, cb);
> >>+ collection = f_cb->collection;
> >>+
> >>+ if (atomic_dec_and_test(&collection->num_pending_fences))
> >>+ fence_signal(&collection->base);
> >>+}
> >>+
> >>+static bool fence_collection_enable_signaling(struct fence *fence)
> >>+{
> >>+ struct fence_collection *collection = to_fence_collection(fence);
> >>+ int i;
> >>+
> >>+ for (i = 0 ; i < collection->num_fences ; i++) {
> >>+ if (fence_add_callback(collection->fences[i].fence,
> >>+ &collection->fences[i].cb,
> >>+ collection_check_cb_func)) {
> >>+ atomic_dec(&collection->num_pending_fences);
> >>+ }
> >>+ }
> >We don't always have a convenient means to preallocate an array of
> >fences to use. Keeping a list of fences in addition to the array would
> >be easier to user in many circumstances.
>
> I agree that there is use for such an implementation as well, but as
> mentioned in the last review cycle we intentionally chose an array instead
> of a more complex implementation here.
>
> This way the array can be passed to function like fence_wait_any_timeout()
> as well.
+1 on fence_array.
> I also suggested to rename it to fence_array to make that difference clear
> and allow for another implementation to live side by side with this.
>
> My crux at the moment is that I need both for the amdgpu driver, an array
> based implementation and a collection like one.
>
> Gustavo would you mind if I take your patches and work a bit on this?
I think the goal is to start landing the atomic fence stuff in 4.8.
Probably simplest if we converge on a first iteration that I can pull into
drm-misc right after 4.7-rc1. Then you can both base your respective work
on top of that branch (it's a stable one, so can even base official
branches on it).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH 1/2] dma-buf/fence: add fence_collection fences
Am 19.05.2016 um 00:57 schrieb Chris Wilson:
> On Wed, May 18, 2016 at 05:59:52PM -0300, Gustavo Padovan wrote:
>> +static void collection_check_cb_func(struct fence *fence, struct fence_cb
>> *cb)
>> +{
>> +struct fence_collection_cb *f_cb;
>> +struct fence_collection *collection;
>> +
>> +f_cb = container_of(cb, struct fence_collection_cb, cb);
>> +collection = f_cb->collection;
>> +
>> +if (atomic_dec_and_test(&collection->num_pending_fences))
>> +fence_signal(&collection->base);
>> +}
>> +
>> +static bool fence_collection_enable_signaling(struct fence *fence)
>> +{
>> +struct fence_collection *collection = to_fence_collection(fence);
>> +int i;
>> +
>> +for (i = 0 ; i < collection->num_fences ; i++) {
>> +if (fence_add_callback(collection->fences[i].fence,
>> + &collection->fences[i].cb,
>> + collection_check_cb_func)) {
>> +atomic_dec(&collection->num_pending_fences);
>> +}
>> +}
> We don't always have a convenient means to preallocate an array of
> fences to use. Keeping a list of fences in addition to the array would
> be easier to user in many circumstances.
I agree that there is use for such an implementation as well, but as
mentioned in the last review cycle we intentionally chose an array
instead of a more complex implementation here.
This way the array can be passed to function like
fence_wait_any_timeout() as well.
I also suggested to rename it to fence_array to make that difference
clear and allow for another implementation to live side by side with this.
My crux at the moment is that I need both for the amdgpu driver, an
array based implementation and a collection like one.
Gustavo would you mind if I take your patches and work a bit on this?
>
> Just means we need a
>
> struct fence_collection_entry {
> struct fence *fence;
> struct list_head link;
> };
>
> int fence_collection_add(struct fence *_collection,
>struct fence *fence)
> {
> struct fence_collection *collection =
> to_fence_collection(_collection);
> struct fence_collection_entry *entry;
>
> entry = kmalloc(sizeof(*entry), GFP_KERNEL);
> if (!entry)
> return -ENOMEM;
>
> entry->fence = fence_get(fence);
> list_add_tail(&entry->link, &collection->fence_list);
> atomic_inc(&collection->num_pending_fences);
>
> return 0;
> }
>
> and a couple of list iterations as well as walking the arrays.
>
> (This fence_collection_add() needs to be documented to only be valid
> from the constructing thread before the fence is sealed for export/use.)
As suggested by Daniel as well I would prefer that the the array
implementation only gets the fences as already filled array in the
constructor. This is much more fail save.
Christian.
> -Chris
>
[PATCH] fbcon: use default if cursor blink interval is not valid
On Thu, 19 May 2016, Pavel Machek wrote:
> Hi!
>
> > Two current [1] and three previous [2] systems locked during boot
> > because the cursor flash timer was set using an ops->cur_blink_jiffies
> > value of 0. Previous patches attempted to solve the problem by moving
> > variable initialization earlier in the setup sequence [2].
> >
> > Use the normal cursor blink default interval of 200 ms if
> > ops->cur_blink_jiffies is not in the range specified in commit
> > bd63364caa8d. Since invalid values are not used, specific system
> > initialization timings should not cause lockups.
> >
> > [1] https://bugs.launchpad.net/bugs/1574814
> > [2] see commits: 2a17d7e80f1d, f235f664a8af, a1e533ec07d5
>
> Acked-by: Pavel Machek
>
> > static void cursor_timer_handler(unsigned long dev_addr)
> > {
> > struct fb_info *info = (struct fb_info *) dev_addr;
> > struct fbcon_ops *ops = info->fbcon_par;
> >
> > queue_work(system_power_efficient_wq, &info->queue);
> > - mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies);
> > + mod_timer(&ops->cursor_timer, jiffies +
> > + cursor_blink_jiffies(ops->cur_blink_jiffies));
> > }
> >
> > static void fbcon_add_cursor_timer(struct fb_info *info)
>
> And actually... perhaps mod_timer should have some check for too low
> timeouts..?
>
> WARN_ON?
> Pavel
Interesting idea. I applied this patch to a couple systems and
receive the same warning on both:
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 73164c3..f6c0b69 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -788,6 +788,7 @@ __mod_timer(struct timer_list *timer, unsigned long expires,
timer_stats_timer_set_start_info(timer);
BUG_ON(!timer->function);
+ WARN_ONCE(expires == jiffies, "timer should expire in the future");
base = lock_timer_base(timer, &flags);
--
[2.060474] [ cut here ]
[2.061613] WARNING: CPU: 0 PID: 164 at kernel/time/timer.c:791
mod_timer+0x233/0x240
[2.062740] timer should expire in the future
[2.062757] CPU: 0 PID: 164 Comm: kworker/0:2 Not tainted 4.6.0+ #7
[2.065870] Hardware name: Toshiba Leon, BIOS 12/04/2013
[2.067828] Workqueue: events_power_efficient hub_init_func3
[2.069762] 88007443bbb8 8139932b
88007443bc08
[2.071701] 88007443bbf8 8112e57c
0317
[2.073655] 88007486a0b0 fffea2da 88007486a000
0202
[2.075594] Call Trace:
[2.077503] [] dump_stack+0x4d/0x72
[2.079426] [] __warn+0xcc/0xf0
[2.081325] [] warn_slowpath_fmt+0x4f/0x60
[2.083212] [] ? find_next_bit+0x15/0x20
[2.085022] [] ? cpumask_next_and+0x2f/0x40
[2.086696] [] mod_timer+0x233/0x240
[2.088362] [] usb_hcd_submit_urb+0x3f2/0x8c0
[2.090026] [] ? urb_destroy+0x24/0x30
[2.091698] [] ? insert_work+0x58/0xb0
[2.093349] [] usb_submit_urb+0x287/0x530
[2.094985] [] hub_activate+0x1fd/0x5d0
[2.096625] [] ? finish_task_switch+0x78/0x1f0
[2.098268] [] hub_init_func3+0x1a/0x20
[2.099908] [] process_one_work+0x140/0x3e0
[2.101539] [] worker_thread+0x4e/0x480
[2.103173] [] ? process_one_work+0x3e0/0x3e0
[2.104790] [] ? process_one_work+0x3e0/0x3e0
[2.106259] [] kthread+0xc9/0xe0
[2.107731] [] ret_from_fork+0x22/0x40
[2.109215] [] ? __kthread_parkme+0x70/0x70
[2.110704] ---[ end trace 3519886a1a990d99 ]---
mod_timer is called from over a thousand places. Should timers always
expire in the future?
[PATCH] tty: vt: Fix soft lockup in fbcon cursor blink timer.
On Thu 2016-05-19 08:27:37, Ming Lei wrote: > On Thu, May 19, 2016 at 4:24 AM, Scot Doyle wrote: > > On Wed, 18 May 2016, Ming Lei wrote: > >> On Wed, May 18, 2016 at 4:49 AM, Pavel Machek wrote: > >> > On Tue 2016-05-17 11:41:04, David Daney wrote: > >> >> From: David Daney > >> >> > >> >> We are getting somewhat random soft lockups with this signature: > >> >> > >> >> [ 86.992215] [] el1_irq+0xa0/0x10c > >> >> [ 86.997082] [] cursor_timer_handler+0x30/0x54 > >> >> [ 87.002991] [] call_timer_fn+0x54/0x1a8 > >> >> [ 87.008378] [] run_timer_softirq+0x1c4/0x2bc > >> >> [ 87.014200] [] __do_softirq+0x114/0x344 > >> >> [ 87.019590] [] irq_exit+0x74/0x98 > >> >> [ 87.024458] [] __handle_domain_irq+0x98/0xfc > >> >> [ 87.030278] [] gic_handle_irq+0x94/0x190 > >> >> > >> >> This is caused by the vt visual_init() function calling into > >> >> fbcon_init() with a vc_cur_blink_ms value of zero. This is a > >> >> transient condition, as it is later set to a non-zero value. But, if > >> >> the timer happens to expire while the blink rate is zero, it goes into > >> >> an endless loop, and we get soft lockup. > >> >> > >> >> The fix is to initialize vc_cur_blink_ms before calling the con_init() > >> >> function. > >> >> > >> >> Signed-off-by: David Daney > >> >> Cc: stable at vger.kernel.org > >> > > >> > Acked-by: Pavel Machek > >> > >> Tested-by: Ming Lei > >> > >> Thanks David and Pavel for making it work! > >> > >> > > >> > (And it is amazing how many problems configurable blink speed caused). > >> > > >> > Thanks! > >> > Pavel > >> > > > > > > > Dann, Ming and David, thank you so much for all of your effort. > > > > There were three other reports in the past year, each leading to their own > > patch, of boot lockups occuring when the cursor flash timer was set using > > an ops->cur_blink_jiffies value of 0. I plan to propose a patch within > > the next day that will prevent this for all code paths. > > Given this issue caues system unusable, I suggest to merge David's > oneline patch first, then you can think and try to figure out 'perfect' > solution > for addressing all this kind of reports from last year. Actually, I'd merge [PATCH] fbcon: use default if cursor blink interval is not valid first. That one is obviously safe. Nice big overkill, but safe. Then nicer solution can be attempted... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[PATCH] drm: Nuke ->vblank_disable_allowed
On Wed, May 18, 2016 at 10:29:57PM +0200, Daniel Vetter wrote:
> This was added in
>
> commit 0a3e67a4caac273a3bfc4ced3da364830b1ab241
> Author: Jesse Barnes
> Date: Tue Sep 30 12:14:26 2008 -0700
>
> drm: Rework vblank-wait handling to allow interrupt reduction.
>
> to stay backwards-compatible with old UMS code that didn't even tell
> the kernel when it did a modeset, so that the kernel could
> save/restore vblank counters. At worst this means vblanks will be
> somewhat funky on a setup that very likely no one still runs.
>
> So let's just nuke it.
>
> Plan B would be to set it unconditionally in drm_vblank_init for kms
> drivers, instead of in each driver separately. So if this patch breaks
> anything please only restore the hunks in drmP.h and drm_irq.c, plus
> add a check for DRIVER_MODESET in drm_vblank_init.
>
> Stumbled over this in a discussion on irc with Chris.
>
> v2: Remove leftover debug gunk from psr hacking (Alex).
>
> Cc: Chris Wilson
> Cc: Alex Deucher
> Cc: Liviu Dudau
> Cc: Russell King
> Cc: Thierry Reding
> Cc: Eric Anholt
> Cc: Laurent Pinchart
> Cc: Inki Dae
> Cc: Tomi Valkeinen
> Cc: Mark Yao
> Cc: Sascha Hauer
> Cc: Philipp Zabel
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 1 -
> drivers/gpu/drm/arm/hdlcd_drv.c | 1 -
> drivers/gpu/drm/armada/armada_drv.c | 1 -
> drivers/gpu/drm/drm_irq.c | 6 --
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 7 ---
> drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 1 -
> drivers/gpu/drm/gma500/psb_drv.c| 1 -
> drivers/gpu/drm/i915/i915_dma.c | 3 ---
> drivers/gpu/drm/imx/imx-drm-core.c | 7 ---
> drivers/gpu/drm/radeon/radeon_irq_kms.c | 1 -
> drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 7 ---
> drivers/gpu/drm/tegra/drm.c | 1 -
> drivers/gpu/drm/vc4/vc4_kms.c | 2 --
> include/drm/drmP.h | 8
> 14 files changed, 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> index 9266c7b69808..835a3fa8d8df 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> @@ -219,7 +219,6 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
> if (r) {
> return r;
> }
> - adev->ddev->vblank_disable_allowed = true;
>
> /* enable msi */
> adev->irq.msi_enabled = false;
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index 2112f0b105e3..4f909378d581 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -379,7 +379,6 @@ static int hdlcd_drm_bind(struct device *dev)
> DRM_ERROR("failed to initialise vblank\n");
> goto err_vblank;
> }
> - drm->vblank_disable_allowed = true;
>
> drm_mode_config_reset(drm);
> drm_kms_helper_poll_init(drm);
For the hdlcd_drv.c part:
Acked-by: Liviu Dudau
Thanks,
Liviu
> diff --git a/drivers/gpu/drm/armada/armada_drv.c
> b/drivers/gpu/drm/armada/armada_drv.c
> index 531fcb946346..cb21c0b6374a 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -113,7 +113,6 @@ static int armada_drm_load(struct drm_device *dev,
> unsigned long flags)
> goto err_comp;
>
> dev->irq_enabled = true;
> - dev->vblank_disable_allowed = 1;
>
> ret = armada_fbdev_init(dev);
> if (ret)
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index d1b5fc20b2f8..1a0ae89087e8 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -348,9 +348,6 @@ static void vblank_disable_fn(unsigned long arg)
> unsigned int pipe = vblank->pipe;
> unsigned long irqflags;
>
> - if (!dev->vblank_disable_allowed)
> - return;
> -
> spin_lock_irqsave(&dev->vbl_lock, irqflags);
> if (atomic_read(&vblank->refcount) == 0 && vblank->enabled) {
> DRM_DEBUG("disabling vblank on crtc %u\n", pipe);
> @@ -437,8 +434,6 @@ int drm_vblank_init(struct drm_device *dev, unsigned int
> num_crtcs)
>"get_vblank_timestamp == NULL\n");
> }
>
> - dev->vblank_disable_allowed = false;
> -
> return 0;
>
> err:
> @@ -1555,7 +1550,6 @@ static void drm_legacy_vblank_post_modeset(struct
> drm_device *dev,
>
> if (vblank->inmodeset) {
> spin_lock_irqsave(&dev->vbl_lock, irqflags);
> - dev->vblank_disable_allowed = true;
> drm_reset_vblank_timestamp(dev, pipe);
> spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 21c719e8e02b..2dd820e23b0c 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/dr
[PATCH] fbcon: use default if cursor blink interval is not valid
On 05/18/2016 09:21 PM, Scot Doyle wrote:
> Two current [1] and three previous [2] systems locked during boot
> because the cursor flash timer was set using an ops->cur_blink_jiffies
> value of 0. Previous patches attempted to solve the problem by moving
> variable initialization earlier in the setup sequence [2].
>
> Use the normal cursor blink default interval of 200 ms if
> ops->cur_blink_jiffies is not in the range specified in commit
> bd63364caa8d. Since invalid values are not used, specific system
> initialization timings should not cause lockups.
>
This patch just papers over the problem that you yourself introduced in
commit bd63364caa8d ("vt: add cursor blink interval escape sequence").
As you know, I have a patch that fixes the problem at the source:
https://lkml.org/lkml/2016/5/17/455
I don't like the idea of silently ignoring bad values passed in from
other code (drivers/tty/vt/vt.c), and much less doing the check for bad
values each time the timer expires rather than just once, where the bad
value is first introduced.
I think it would be preferable to WARN() at the site the bad value is
introduced, so that we can easily find the real source of the problem.
Initialize cur_blink_jiffies to a sane default value, then if something
attempts to set it to a value that would cause soft lockup, WARN and
refuse to change it.
Also there is a stylistic issue...
> [1] https://bugs.launchpad.net/bugs/1574814
> [2] see commits: 2a17d7e80f1d, f235f664a8af, a1e533ec07d5
>
> Signed-off-by: Scot Doyle
> Cc: [v4.2]
> ---
> drivers/video/console/fbcon.c | 17 +
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> index 6e92917..da61d87 100644
> --- a/drivers/video/console/fbcon.c
> +++ b/drivers/video/console/fbcon.c
> @@ -396,13 +396,23 @@ static void fb_flashcursor(struct work_struct *work)
> console_unlock();
> }
>
> +static int cursor_blink_jiffies(int candidate)
> +{
> + if (candidate >= msecs_to_jiffies(50) &&
> + candidate <= msecs_to_jiffies(USHRT_MAX))
> + return candidate;
> + else
> + return HZ / 5;
You use msecs_to_jiffies() is several places, then here open code the
division. Please use msecs_to_jiffies(), that is it's intended job.
> +}
> +
> static void cursor_timer_handler(unsigned long dev_addr)
> {
> struct fb_info *info = (struct fb_info *) dev_addr;
> struct fbcon_ops *ops = info->fbcon_par;
>
> queue_work(system_power_efficient_wq, &info->queue);
> - mod_timer(&ops->cursor_timer, jiffies + ops->cur_blink_jiffies);
> + mod_timer(&ops->cursor_timer, jiffies +
> + cursor_blink_jiffies(ops->cur_blink_jiffies));
> }
>
> static void fbcon_add_cursor_timer(struct fb_info *info)
> @@ -417,7 +427,8 @@ static void fbcon_add_cursor_timer(struct fb_info *info)
>
> init_timer(&ops->cursor_timer);
> ops->cursor_timer.function = cursor_timer_handler;
> - ops->cursor_timer.expires = jiffies + ops->cur_blink_jiffies;
> + ops->cursor_timer.expires = jiffies +
> + cursor_blink_jiffies(ops->cur_blink_jiffies);
> ops->cursor_timer.data = (unsigned long ) info;
> add_timer(&ops->cursor_timer);
> ops->flags |= FBCON_FLAGS_CURSOR_TIMER;
> @@ -709,7 +720,6 @@ static int con2fb_acquire_newinfo(struct vc_data *vc,
> struct fb_info *info,
> }
>
> if (!err) {
> - ops->cur_blink_jiffies = HZ / 5;
> info->fbcon_par = ops;
>
> if (vc)
> @@ -957,7 +967,6 @@ static const char *fbcon_startup(void)
> ops->currcon = -1;
> ops->graphics = 1;
> ops->cur_rotate = -1;
> - ops->cur_blink_jiffies = HZ / 5;
> info->fbcon_par = ops;
> p->con_rotate = initial_rotation;
> set_blitting_type(vc, info);
>
