Hi Liviu,
On 2016-06-08 11:05, liviu.dudau@xxxxxxx wrote:
On Wed, Jun 08, 2016 at 08:58:33AM +0200, Marek Szyprowski wrote:
On 2016-06-07 16:34, liviu.dudau@xxxxxxx wrote:
On Tue, Jun 07, 2016 at 03:11:14PM +0100, Robin Murphy wrote:
Hi Liviu,
On 07/06/16 14:35, liviu.dudau@xxxxxxx wrote:
On Tue, Jun 07, 2016 at 01:06:00PM +0100, Robin Murphy wrote:
Having just inadvertently merged -next into my working branch, I find
dev6d910bfa809e ("drm/hlcd: Use lockless gem BO free callback") adversely
affecting my board's ability to boot ;)
Since I (intentionally) don't have sufficient CMA to create a framebuffer,
drm_gem_cma_create() fails, unconditionally calls the now-NULL
drm->driver->gem_free_object() in its cleanup path, and fiery death
ensues...
Thanks for reporting this. What other changes other than reducing the CMA
allocation size do you have that I might need in order to reproduce this?
I've just confirmed a plain checkout of next-20160602, using arm64 defconfig
+ DRM + HDLCD + TDA998X and CMA_SIZE_MBYTES=1, booted on a Juno, does the
job:
[ 3.032402] hdlcd 7ff60000.hdlcd: bound 0-0070 (ops tda998x_ops)
[ 3.038388] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
[ 3.044970] [drm] No driver support for vblank timestamp query.
[ 3.076973] hdlcd 7ff60000.hdlcd: failed to allocate buffer with size
7680000
[ 3.084081] Bad mode in Synchronous Abort handler detected, code
0x86000004 -- IABT (current EL)
[ 3.092815] CPU: 3 PID: 6 Comm: kworker/u12:0 Not tainted
4.7.0-rc1-next-20160602 #686
[ 3.100682] Hardware name: ARM Juno development board (r1) (DT)
[ 3.106567] Workqueue: deferwq deferred_probe_work_func
[ 3.111761] task: ffff8009768a3e80 ti: ffff8009768e8000 task.ti:
ffff8009768e8000
[ 3.119198] PC is at 0x0
[ 3.121720] LR is at drm_gem_cma_create+0x128/0x130
...and so on.
Today's -next, on the other hand, dodges the bullet entirely:
[ 2.903645] [drm] found ARM HDLCD version r0p0
[ 2.908122] hdlcd 7ff60000.hdlcd: master bind failed: -22
[ 2.913505] tda998x: probe of 0-0070 failed with error -22
[ 2.919141] [drm] found ARM HDLCD version r0p0
[ 2.923609] hdlcd 7ff50000.hdlcd: master bind failed: -22
[ 2.928991] tda998x: probe of 0-0071 failed with error -22
OK, the problem is that commit 59ce4039727ef40 has changed the behaviour from when
there is no "memory-region" phandle in the DT: before it used to return -ENODEV, now
it returns -EINVAL.
Marek, I quite liked the old behaviour to detect if the DT had the optional (from
my driver's point of view) "memory-region" phandle. Plus the check for dev is superfluous
when using of_reserved_mem_device_init() as that uses dev->of_node for np so it would
crash before the check anyway. Maybe move the check there?
Until then I suggest reverting the 59ce4039727ef40 commit.
I've just send a fix for this issue. I'm sorry for the regression. I hope
the fix fill
quickly get into next to solve your problem.
Thanks for the patch, however I have some comments to it.
The additional check for null dev make sense, because the new function
of_reserved_mem_device_init_by_idx can be called with any device and node
pointer not
embedded with it, so I would like to keep it safe.
And I have to admit I find that scary. Why do you accept any node that is *not* related to
the device? If you want just the ability to parse multiple "memory-region" phandles (where
are the bindings defined for that?) I would have modified of_reserved_mem_device_init() to
the the parsing and accept either the single entry style or a node with multiple
"memory-region" phandles in it. Otherwise I can steal the "memory-region" of another device
and that device would have no idea that I have done this.
The idea is not to steal memory region from another device, but to let
driver to use multiple
regions assigned to the supported device with dma-mapping api. To use
them with dma-mapping
subsystem, one needs separate struct device for each reserved region.
The idea is to create
child devices of the device, which has memory-region property. Then for
each such child
device, driver can call of_reserved_mem_device_init_by_idx() to enable
usage of dma-mapping
api based on the selected reserved region. Such approach was already
used for long time in
the media/platform/s5p-mfc driver and now it has been converted to use
generic reserved
memory regions.
If you feel scary about this approach, maybe a check if the device
provided to
of_reserved_mem_device_init_by_idx() function is one of the successors
of the device hidden
behind the provided node pointer (or points to the same device)?
Can you point me to the latest thread where this patch has been discussed?
This patch is a part of "Exynos: MFC driver: reserved memory cleanup and
IOMMU support" thread
and has been around for a while:
https://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg97645.html
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
