[Bug 60879] [radeonsi] X11 can't start with acceleration enabled
https://bugs.freedesktop.org/show_bug.cgi?id=60879 Tom Stellard changed: What|Removed |Added Attachment #106006|0 |1 is obsolete|| --- Comment #90 from Tom Stellard --- Created attachment 106097 --> https://bugs.freedesktop.org/attachment.cgi?id=106097&action=edit Fix v4 Here is an updated patch that addresses Michel's comments. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/1d48e837/attachment.html>
[Bug 75649] Glitchy output using only HDMI on laptop with AMD Mobility Radeon HD 3450/3470
https://bugs.freedesktop.org/show_bug.cgi?id=75649 --- Comment #8 from tderensis at gmail.com --- I have been looking in the xf86-video-ati repo at drmmode_display.c and the drmmode_sf86crtc_resize() function but can't find anything so far. It seems from Paul's comments that the issue may be related to the refresh rate as well. Are there any suggestion as to places to look in the code? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/1b7493c8/attachment.html>
[Bug 81644] Random crashes on RadeonSI with Chromium.
https://bugs.freedesktop.org/show_bug.cgi?id=81644 --- Comment #90 from Aaron B --- My testing has been delayed since they pushed LLVM 3.5 to stable on Arch, but they changed one of the function's parameters. Then, 3.6 compiles perfectly for 64-bit, yet when compiling 32-bit, it fails to link. So I've been stuck using repro's of 10.2.7 which...seems better, but it still does crash. Maybe it a memory/buffer overrun problem, because the versions are either god-awful, or somewhat usable with only a rare hiccup, which more recent versions just getting worse and worse. But like I said, I can't compile basically anything right now until everyone gets their stuff straight on function names and parameters. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/bb539874/attachment.html>
[PATCH] drm/exynos: update to use component match support
On 2014? 09? 10? 19:24, Andrzej Hajda wrote:
> Hi Inki,
>
> To test it properly I have to fix init/remove bugs [1].
> Of course these bugs were not introduced by this patch,
> but they prevented some basic tests.
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/37266
>
> I have tested successfully your patch with trats and universal_c210 boards.
>
> Few additional comments below.
>
> On 09/01/2014 02:19 PM, Inki Dae wrote:
>> Update Exynos's DRM driver to use component match support rater than
>> add_components.
>>
>> Signed-off-by: Inki Dae
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 40
>> ++-
>> 1 file changed, 18 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> index feee991..dae62c2 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> @@ -503,16 +503,15 @@ static int compare_of(struct device *dev, void *data)
>> return dev == (struct device *)data;
>> }
>
> Nitpick.
>
> This is not a part of this patch but compare_of suggests it compares OF
> nodes but this function compares devices, maybe compare_dev would be better.
Agree.
>
>>
>> -static int exynos_drm_add_components(struct device *dev, struct master *m)
>> +static struct component_match *exynos_drm_match_add(struct device *dev)
>> {
>> +struct component_match *match = NULL;
>> struct component_dev *cdev;
>> unsigned int attach_cnt = 0;
>>
>> mutex_lock(&drm_component_lock);
>>
>> list_for_each_entry(cdev, &drm_component_list, list) {
>> -int ret;
>> -
>> /*
>> * Add components to master only in case that crtc and
>> * encoder/connector device objects exist.
>> @@ -527,16 +526,10 @@ static int exynos_drm_add_components(struct device
>> *dev, struct master *m)
>> /*
>> * fimd and dpi modules have same device object so add
>> * only crtc device object in this case.
>> - *
>> - * TODO. if dpi module follows driver-model driver then
>> - * below codes can be removed.
>> */
>> if (cdev->crtc_dev == cdev->conn_dev) {
>> -ret = component_master_add_child(m, compare_of,
>> -cdev->crtc_dev);
>> -if (ret < 0)
>> -return ret;
>> -
>> +component_match_add(dev, &match, compare_of,
>> +cdev->crtc_dev);
>> goto out_lock;
>> }
>>
>> @@ -546,11 +539,8 @@ static int exynos_drm_add_components(struct device
>> *dev, struct master *m)
>> * connector/encoder need pipe number of crtc when they
>> * are created.
>> */
>> -ret = component_master_add_child(m, compare_of, cdev->crtc_dev);
>> -ret |= component_master_add_child(m, compare_of,
>> -cdev->conn_dev);
>> -if (ret < 0)
>> -return ret;
>> +component_match_add(dev, &match, compare_of, cdev->crtc_dev);
>> +component_match_add(dev, &match, compare_of, cdev->conn_dev);
>>
>> out_lock:
>> mutex_lock(&drm_component_lock);
>> @@ -558,7 +548,7 @@ out_lock:
>>
>> mutex_unlock(&drm_component_lock);
>>
>> -return attach_cnt ? 0 : -ENODEV;
>> +return attach_cnt ? match : ERR_PTR(-EPROBE_DEFER);
>> }
>>
>> static int exynos_drm_bind(struct device *dev)
>> @@ -572,13 +562,13 @@ static void exynos_drm_unbind(struct device *dev)
>> }
>>
>> static const struct component_master_ops exynos_drm_ops = {
>> -.add_components = exynos_drm_add_components,
>> .bind = exynos_drm_bind,
>> .unbind = exynos_drm_unbind,
>> };
>>
>> static int exynos_drm_platform_probe(struct platform_device *pdev)
>> {
>> +struct component_match *match;
>> int ret;
>>
>> pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>> @@ -645,13 +635,19 @@ static int exynos_drm_platform_probe(struct
>> platform_device *pdev)
>> goto err_unregister_ipp_drv;
>> #endif
>>
>> -ret = component_master_add(&pdev->dev, &exynos_drm_ops);
>> -if (ret < 0)
>> -DRM_DEBUG_KMS("re-tried by last sub driver probed later.\n");
>> +match = exynos_drm_match_add(&pdev->dev);
>> +if (IS_ERR(match)) {
>> +ret = PTR_ERR(match);
>> +goto err_unregister_ipp_dev;
>> +}
>>
>> -return 0;
>> +return component_master_add_with_match(&pdev->dev, &exynos_drm_ops,
>> +match);
>
> In case component_master_add_with_match fails there will be no cleanup -
> platform devices and drivers will not be removed.
>
Right.
>> +
>
[Bug 81644] Random crashes on RadeonSI with Chromium.
https://bugs.freedesktop.org/show_bug.cgi?id=81644 --- Comment #91 from Michel D?nzer --- (In reply to comment #89) > Could someone please fix this already? Mesa 10.2.x, 10.3-RC and git are > simply unusable on 7770. I'm sorry to hear that. We're working on it, but since we haven't been able to reproduce these issues, we need your help for testing: Does the environment variable R600_DEBUG=nodma help? If not, can you try if Mesa 10.1 is stable for you, and if so, bisect between 10.1 and 10.2? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/f1718bab/attachment.html>
[Bug 81644] Random crashes on RadeonSI with Chromium.
https://bugs.freedesktop.org/show_bug.cgi?id=81644 --- Comment #92 from Aaron B --- Like I said, I tried. But, it's too hard to reproduce. Unless you do it over a 2 week time span, it's so difficult to find. Maybe he could do it better since his builds seem to crash more if it's completely unusable. But, if you're on arch and don't want to help, stick to 32 and 64-bit LLVM 3.4.2. I can't help for now. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/5b6271bf/attachment.html>
[Bug 83708] [vdpau,uvd] kernel oops, Unable to handle kernel paging request at virtual address
https://bugs.freedesktop.org/show_bug.cgi?id=83708 Michel D?nzer changed: What|Removed |Added Summary|mplayer plays video using |[vdpau,uvd] kernel oops, |gpu hardware|Unable to handle kernel |acceleraing(vdpau,uvd) |paging request at virtual |kernel panic, Unable to |address |handle kernel paging| |request at virtual address | Product|Mesa|DRI Version|9.2 |unspecified Component|Drivers/Gallium/radeonsi|DRM/Radeon --- Comment #1 from Michel D?nzer --- (In reply to comment #1) > Hi, I try to using gpu hardware accelerate to play video from mplayer > (vdpau+uvd) . The machine architecture is sparc64,video card is radeon > HD7450,OS version is redhat7,kernel version is 3.10.0, mplayer version is > 1.1-21,mesa version is 9.2.5-6. Please attach the output of dmesg (showing at least all radeon driver related initialization), the /var/log/Xorg.0.log file and the output of vdpauinfo. Can you try newer versions of the kernel and Mesa? P.S. AFAICT the 7450 is Northern Islands generation (Caicos) based, not Southern Islands based, otherwise I'd be very surprised you even got this far, given bug 82455. :) -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/d92cc6b1/attachment.html>
[PATCH] drm/exynos: use a new anon file for exynos gem mmaper
On 2014? 09? 10? 18:01, Daniel Vetter wrote:
> Ok I've stumbled over the exynos mmap stuff again while cleaning up
> drm legacy cruft and I just don't get what you're doing and why
> exactly exynos needs to be special.
>
> _All_ other drm drivers happily get along with the vma offset manger
> stuff to handle mmaps, but somehow exynos does something really crazy.
We are also using the vma offset manager stuff. We just added direct
mapping interface specific to Exynos additionally.
>
> Can you please explain the design justification for this and why
> switching to the standard gem mmap support isn't possible?
As I mentioned above, we are using the standard gem mmap support.
However, the standard gem mmap is required for on-demand paging mostly
suitable for Desktop. In case of ARM SoC, whole memory region requested
by userspace would be allocated once the gem creation interface is
called. In this case, it wouldn't need to map userspace with physical
page in page fault handler, and the use of the vma offset manager stuff
would be unnecessary step.
For the same question, Al Viro did,
http://lists.freedesktop.org/archives/dri-devel/2013-September/046207.html
Is there any issue I am missing , that could be incurred by Exynos codes?
Thanks,
Inki Dae
>
> Thanks, Daniel
>
>
> On Fri, Dec 20, 2013 at 11:36 AM, Inki Dae wrote:
>> This patch resolves potential deadlock issue that can be incurred
>> by changing file->f_op and filp->private_data to exynos specific
>> mapper ops and gem object temporarily.
>>
>> To resolve this issue, this patch creates a new anon file dedicated
>> to exynos specific mmaper, and making it used instead of existing one.
>>
>> Signed-off-by: Inki Dae
>> Signed-off-by: Kyungmin Park
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 21 +
>> drivers/gpu/drm/exynos/exynos_drm_drv.h |1 +
>> drivers/gpu/drm/exynos/exynos_drm_gem.c | 74
>> ++-
>> drivers/gpu/drm/exynos/exynos_drm_gem.h |3 ++
>> 4 files changed, 38 insertions(+), 61 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> index 22b8f5e..b5e5957 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> @@ -14,6 +14,8 @@
>> #include
>> #include
>>
>> +#include
>> +
>> #include
>>
>> #include "exynos_drm_drv.h"
>> @@ -150,9 +152,14 @@ static int exynos_drm_unload(struct drm_device *dev)
>> return 0;
>> }
>>
>> +static const struct file_operations exynos_drm_gem_fops = {
>> + .mmap = exynos_drm_gem_mmap_buffer,
>> +};
>> +
>> static int exynos_drm_open(struct drm_device *dev, struct drm_file *file)
>> {
>> struct drm_exynos_file_private *file_priv;
>> + struct file *anon_filp;
>> int ret;
>>
>> file_priv = kzalloc(sizeof(*file_priv), GFP_KERNEL);
>> @@ -167,6 +174,16 @@ static int exynos_drm_open(struct drm_device *dev,
>> struct drm_file *file)
>> file->driver_priv = NULL;
>> }
>>
>> + anon_filp = anon_inode_getfile("exynos_gem", &exynos_drm_gem_fops,
>> + NULL, 0);
>> + if (IS_ERR(anon_filp)) {
>> + kfree(file_priv);
>> + return PTR_ERR(anon_filp);
>> + }
>> +
>> + anon_filp->f_mode = FMODE_READ | FMODE_WRITE;
>> + file_priv->anon_filp = anon_filp;
>> +
>> return ret;
>> }
>>
>> @@ -179,6 +196,7 @@ static void exynos_drm_preclose(struct drm_device *dev,
>> static void exynos_drm_postclose(struct drm_device *dev, struct drm_file
>> *file)
>> {
>> struct exynos_drm_private *private = dev->dev_private;
>> + struct drm_exynos_file_private *file_priv;
>> struct drm_pending_vblank_event *v, *vt;
>> struct drm_pending_event *e, *et;
>> unsigned long flags;
>> @@ -204,6 +222,9 @@ static void exynos_drm_postclose(struct drm_device *dev,
>> struct drm_file *file)
>> }
>> spin_unlock_irqrestore(&dev->event_lock, flags);
>>
>> + file_priv = file->driver_priv;
>> + if (file_priv->anon_filp)
>> + fput(file_priv->anon_filp);
>>
>> kfree(file->driver_priv);
>> file->driver_priv = NULL;
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> index eaa1966..0eaf5a2 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> @@ -226,6 +226,7 @@ struct exynos_drm_ipp_private {
>> struct drm_exynos_file_private {
>> struct exynos_drm_g2d_private *g2d_priv;
>> struct exynos_drm_ipp_private *ipp_priv;
>> + struct file *anon_filp;
>> };
>>
>> /*
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> b/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> index 1ade191..49b8c9b 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
>> +++ b/drivers/gpu/drm/exy
[Bug 82551] monitor resolution wrongly set when using kernels > 3.13
https://bugzilla.kernel.org/show_bug.cgi?id=82551 --- Comment #9 from Javier Fernandez --- well, actually i havent tested upstream versions of 3.16 kernels, only Ubuntu kernels (which are patched) thats why i ask about installing upstream ones... what do u think? Also, i can remember earlier releases of 3.16 kernels on which radeon open source driver didnt work ( 3.16.0-8) 3.13 -> works 3.14 -> doesn't work 3.15 -> doesn't work 3.16 -> works seems so, yes -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 82551] monitor resolution wrongly set when using kernels > 3.13
https://bugzilla.kernel.org/show_bug.cgi?id=82551 --- Comment #10 from Javier Fernandez --- the kernel i am using now and on which open source driver is working is 3.16.0-14-lowlatency -- You are receiving this mail because: You are watching the assignee of the bug.
[Bug 35457] [rs690m] Graphics corruption with ati x1200
https://bugs.freedesktop.org/show_bug.cgi?id=35457 Appin Technology Lab changed: What|Removed |Added Assignee|dri-devel at lists.freedesktop |atl.puducherry at gmail.com |.org| -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/13dc6ba2/attachment.html>
[PATCH] drm/exynos: use a new anon file for exynos gem mmaper
On Thu, Sep 11, 2014 at 11:16:53AM +0900, Inki Dae wrote: > On 2014? 09? 10? 18:01, Daniel Vetter wrote: > > Ok I've stumbled over the exynos mmap stuff again while cleaning up > > drm legacy cruft and I just don't get what you're doing and why > > exactly exynos needs to be special. > > > > _All_ other drm drivers happily get along with the vma offset manger > > stuff to handle mmaps, but somehow exynos does something really crazy. > > We are also using the vma offset manager stuff. We just added direct > mapping interface specific to Exynos additionally. > > > > > Can you please explain the design justification for this and why > > switching to the standard gem mmap support isn't possible? > > As I mentioned above, we are using the standard gem mmap support. > However, the standard gem mmap is required for on-demand paging mostly > suitable for Desktop. In case of ARM SoC, whole memory region requested > by userspace would be allocated once the gem creation interface is > called. In this case, it wouldn't need to map userspace with physical > page in page fault handler, and the use of the vma offset manager stuff > would be unnecessary step. You don't need to do demand paging at all, you can simply put in all the ptes in one go and then never unbind it. So strictly speaking you don't need to roll your own mmap, but otoh other drivers (including i915) do their own special mmap too. And since you now have it you must support it forever anyway. Aside: We have patches floating around for i915 to prefault aggressively, so you're not the only ones who noticed the faulting overhead. ARM SoC really aren't all that special compared to traditional desktop gpus, so if you stumble over such issues please raise them on dri-devel so that we could look into useful generic solutions next time around. > For the same question, Al Viro did, > http://lists.freedesktop.org/archives/dri-devel/2013-September/046207.html > > Is there any issue I am missing , that could be incurred by Exynos codes? I've stumbled over it again because you're reusing the drm_vm_open_locked function, which really should just be an implementation detail of the core drm/gem mmap support. If you want to roll your own mmap (and that's ok, i915 has it and ttm also does it) then imo you should not reuse any of the core mmap code, but implement your own set of vm_ops. You don't need a faul handler for this (since it will never fault), and open/close would just grabbing/dropping a reference of the underlying gem object. Instead of trying to reuse the same vm_ops you use for normal gem mmaps, which just doesn't make a lot of sense to me. If exynos stops using drm_vm_open_locked then I can move it into the new drm_internal.h header since this function really should be private to drm.ko. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH 01/15] drm: Move dma functions into drm_legacy.h
On Wed, Sep 10, 2014 at 04:42:04PM +0200, David Herrmann wrote:
> Hi
>
> On Wed, Sep 10, 2014 at 12:43 PM, Daniel Vetter
> wrote:
> > Also drop the unneeded EXPORT_SYMBOL and sprinkle drm_legacy_ prefixes
> > where missing.
> >
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/drm_bufs.c | 2 +-
> > drivers/gpu/drm/drm_dma.c| 10 --
> > drivers/gpu/drm/drm_fops.c | 2 +-
> > drivers/gpu/drm/drm_legacy.h | 8
> > include/drm/drmP.h | 7 ---
> > 5 files changed, 14 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
> > index 9e04d6a43fa4..70ba89f66199 100644
> > --- a/drivers/gpu/drm/drm_bufs.c
> > +++ b/drivers/gpu/drm/drm_bufs.c
> > @@ -1338,7 +1338,7 @@ int drm_legacy_freebufs(struct drm_device *dev, void
> > *data,
> > task_pid_nr(current));
> > return -EINVAL;
> > }
> > - drm_free_buffer(dev, buf);
> > + drm_legacy_free_buffer(dev, buf);
> > }
> >
> > return 0;
> > diff --git a/drivers/gpu/drm/drm_dma.c b/drivers/gpu/drm/drm_dma.c
> > index 8a140a953754..9360c3915fe9 100644
> > --- a/drivers/gpu/drm/drm_dma.c
> > +++ b/drivers/gpu/drm/drm_dma.c
> > @@ -124,7 +124,7 @@ void drm_legacy_dma_takedown(struct drm_device *dev)
> > *
> > * Resets the fields of \p buf.
> > */
> > -void drm_free_buffer(struct drm_device *dev, struct drm_buf * buf)
> > +void drm_legacy_free_buffer(struct drm_device *dev, struct drm_buf * buf)
> > {
> > if (!buf)
> > return;
> > @@ -142,8 +142,8 @@ void drm_free_buffer(struct drm_device *dev, struct
> > drm_buf * buf)
> > *
> > * Frees each buffer associated with \p file_priv not already on the
> > hardware.
> > */
> > -void drm_core_reclaim_buffers(struct drm_device *dev,
> > - struct drm_file *file_priv)
> > +void drm_legacy_core_reclaim_buffers(struct drm_device *dev,
> > +struct drm_file *file_priv)
> > {
> > struct drm_device_dma *dma = dev->dma;
> > int i;
> > @@ -154,7 +154,7 @@ void drm_core_reclaim_buffers(struct drm_device *dev,
> > if (dma->buflist[i]->file_priv == file_priv) {
> > switch (dma->buflist[i]->list) {
> > case DRM_LIST_NONE:
> > - drm_free_buffer(dev, dma->buflist[i]);
> > + drm_legacy_free_buffer(dev,
> > dma->buflist[i]);
> > break;
> > case DRM_LIST_WAIT:
> > dma->buflist[i]->list = DRM_LIST_RECLAIM;
> > @@ -166,5 +166,3 @@ void drm_core_reclaim_buffers(struct drm_device *dev,
> > }
> > }
> > }
> > -
> > -EXPORT_SYMBOL(drm_core_reclaim_buffers);
> > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> > index b419990042b0..6a8c9379bd42 100644
> > --- a/drivers/gpu/drm/drm_fops.c
> > +++ b/drivers/gpu/drm/drm_fops.c
> > @@ -404,7 +404,7 @@ int drm_release(struct inode *inode, struct file *filp)
> > drm_master_release(dev, filp);
> >
> > if (drm_core_check_feature(dev, DRIVER_HAVE_DMA))
> > - drm_core_reclaim_buffers(dev, file_priv);
> > + drm_legacy_core_reclaim_buffers(dev, file_priv);
> >
> > drm_events_release(file_priv);
> >
> > diff --git a/drivers/gpu/drm/drm_legacy.h b/drivers/gpu/drm/drm_legacy.h
> > index 3049af5a01b3..36755ac4f244 100644
> > --- a/drivers/gpu/drm/drm_legacy.h
> > +++ b/drivers/gpu/drm/drm_legacy.h
> > @@ -92,4 +92,12 @@ int drm_legacy_lock(struct drm_device *d, void *v,
> > struct drm_file *f);
> > int drm_legacy_unlock(struct drm_device *d, void *v, struct drm_file *f);
> > int drm_legacy_lock_free(struct drm_lock_data *lock, unsigned int ctx);
> >
> > +/* DMA support */
> > +extern int drm_legacy_dma_setup(struct drm_device *dev);
> > +extern void drm_legacy_dma_takedown(struct drm_device *dev);
> > +extern void drm_legacy_free_buffer(struct drm_device *dev,
> > + struct drm_buf * buf);
> > +extern void drm_legacy_core_reclaim_buffers(struct drm_device *dev,
> > + struct drm_file *filp);
> > +
>
> "_core_" in the name doesn't really make much sense, but as long as
> it has "legacy", too, I guess nobody cares.
Will do.
>
> You might wanna drop all the "extern " prefix for functions while
> doing the move. It has no affect at all (default for function
> declarations). Otherwise, looks good:
Yeah, missed it here and with the sg stuff in the next patch. Will fix.
>
> Reviewed-by: David Herrmann
>
> Thanks
> David
>
> > #endif /* __DRM_LEGACY_H__ */
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index c5c9f0e44a7b..574e325d6b69 100644
> > --- a/include/drm/drmP.h
> > +++ b/
[PATCH 02/15] drm: Move sg functions into drm_legacy.h
On Wed, Sep 10, 2014 at 04:43:31PM +0200, David Herrmann wrote:
> Hi
>
> On Wed, Sep 10, 2014 at 12:43 PM, Daniel Vetter
> wrote:
> > Also sprinkle the drm_legacy_ prefix where missing.
> >
> > Signed-off-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/drm_ioctl.c | 4 ++--
> > drivers/gpu/drm/drm_legacy.h | 7 +++
> > drivers/gpu/drm/drm_scatter.c | 8
> > include/drm/drmP.h| 7 ---
> > 4 files changed, 13 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > index bfb3b85dbe2d..7d6df78bb25d 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -108,8 +108,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
> > DRM_IOCTL_DEF(DRM_IOCTL_AGP_UNBIND, drm_agp_unbind_ioctl,
> > DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> > #endif
> >
> > - DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_sg_alloc,
> > DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> > - DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_sg_free,
> > DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> > + DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_legacy_sg_alloc,
> > DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> > + DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_legacy_sg_free,
> > DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> >
> > DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED),
> >
> > diff --git a/drivers/gpu/drm/drm_legacy.h b/drivers/gpu/drm/drm_legacy.h
> > index 36755ac4f244..d8bc895bc376 100644
> > --- a/drivers/gpu/drm/drm_legacy.h
> > +++ b/drivers/gpu/drm/drm_legacy.h
> > @@ -100,4 +100,11 @@ extern void drm_legacy_free_buffer(struct drm_device
> > *dev,
> > extern void drm_legacy_core_reclaim_buffers(struct drm_device *dev,
> > struct drm_file *filp);
> >
> > +/* Scatter Gather Support */
> > +extern void drm_legacy_sg_cleanup(struct drm_device *dev);
> > +extern int drm_legacy_sg_alloc(struct drm_device *dev, void *data,
> > + struct drm_file *file_priv);
> > +extern int drm_legacy_sg_free(struct drm_device *dev, void *data,
> > + struct drm_file *file_priv);
> > +
> > #endif /* __DRM_LEGACY_H__ */
> > diff --git a/drivers/gpu/drm/drm_scatter.c b/drivers/gpu/drm/drm_scatter.c
> > index 1c78406f6e71..a02605c96224 100644
> > --- a/drivers/gpu/drm/drm_scatter.c
> > +++ b/drivers/gpu/drm/drm_scatter.c
> > @@ -78,8 +78,8 @@ void drm_legacy_sg_cleanup(struct drm_device *dev)
>
> Can you include "drm_legacy.h" from drm_scatter.c, please? Otherwise,
> we will not catch wrong function declarations in the header file.
Some later patches adds that ;-) But I've noticed that my include rules
are a bit inconsistent, the new ones are:
- All drm.ko source files that need legacy stuff include "drm_legacy.h".
That file in turn then includes
- Drivers only ever include
Thanks, Daniel
>
> Thanks
> David
>
> > # define ScatterHandle(x) (unsigned int)(x)
> > #endif
> >
> > -int drm_sg_alloc(struct drm_device *dev, void *data,
> > -struct drm_file *file_priv)
> > +int drm_legacy_sg_alloc(struct drm_device *dev, void *data,
> > + struct drm_file *file_priv)
> > {
> > struct drm_scatter_gather *request = data;
> > struct drm_sg_mem *entry;
> > @@ -194,8 +194,8 @@ int drm_sg_alloc(struct drm_device *dev, void *data,
> > return -ENOMEM;
> > }
> >
> > -int drm_sg_free(struct drm_device *dev, void *data,
> > - struct drm_file *file_priv)
> > +int drm_legacy_sg_free(struct drm_device *dev, void *data,
> > + struct drm_file *file_priv)
> > {
> > struct drm_scatter_gather *request = data;
> > struct drm_sg_mem *entry;
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index 574e325d6b69..1d1468bcd69f 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -1316,13 +1316,6 @@ void drm_prime_remove_buf_handle_locked(struct
> > drm_prime_file_private *prime_fpr
> >
> > extern int drm_vma_info(struct seq_file *m, void *data);
> >
> > - /* Scatter Gather Support (drm_scatter.h) */
> > -extern void drm_legacy_sg_cleanup(struct drm_device *dev);
> > -extern int drm_sg_alloc(struct drm_device *dev, void *data,
> > - struct drm_file *file_priv);
> > -extern int drm_sg_free(struct drm_device *dev, void *data,
> > - struct drm_file *file_priv);
> > -
> >/* ATI PCIGART support (ati_pcigart.h) */
> > extern int drm_ati_pcigart_init(struct drm_device *dev,
> > struct drm_ati_pcigart_info * gart_info);
> > --
> > 1.9.3
> >
> > ___
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 -
[PATCH] drm/exynos: update to use component match support
On 2014? 09? 10? 19:24, Andrzej Hajda wrote:
> Hi Inki,
>
> To test it properly I have to fix init/remove bugs [1].
> Of course these bugs were not introduced by this patch,
> but they prevented some basic tests.
I had tested my patch with trats2 board, and works well without below
patch set. hm.. it seems that there is other corner cases I missed. Can
you give me more details about basic tests?
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/37266
>
> I have tested successfully your patch with trats and universal_c210 boards.
Thanks for testing and above fixup patch set. Will look into them soon. :)
Thanks,
Inki Dae
>
> Few additional comments below.
>
> On 09/01/2014 02:19 PM, Inki Dae wrote:
>> Update Exynos's DRM driver to use component match support rater than
>> add_components.
>>
>> Signed-off-by: Inki Dae
>> ---
>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 40
>> ++-
>> 1 file changed, 18 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> index feee991..dae62c2 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>> @@ -503,16 +503,15 @@ static int compare_of(struct device *dev, void *data)
>> return dev == (struct device *)data;
>> }
>
> Nitpick.
>
> This is not a part of this patch but compare_of suggests it compares OF
> nodes but this function compares devices, maybe compare_dev would be better.
>
>>
>> -static int exynos_drm_add_components(struct device *dev, struct master *m)
>> +static struct component_match *exynos_drm_match_add(struct device *dev)
>> {
>> +struct component_match *match = NULL;
>> struct component_dev *cdev;
>> unsigned int attach_cnt = 0;
>>
>> mutex_lock(&drm_component_lock);
>>
>> list_for_each_entry(cdev, &drm_component_list, list) {
>> -int ret;
>> -
>> /*
>> * Add components to master only in case that crtc and
>> * encoder/connector device objects exist.
>> @@ -527,16 +526,10 @@ static int exynos_drm_add_components(struct device
>> *dev, struct master *m)
>> /*
>> * fimd and dpi modules have same device object so add
>> * only crtc device object in this case.
>> - *
>> - * TODO. if dpi module follows driver-model driver then
>> - * below codes can be removed.
>> */
>> if (cdev->crtc_dev == cdev->conn_dev) {
>> -ret = component_master_add_child(m, compare_of,
>> -cdev->crtc_dev);
>> -if (ret < 0)
>> -return ret;
>> -
>> +component_match_add(dev, &match, compare_of,
>> +cdev->crtc_dev);
>> goto out_lock;
>> }
>>
>> @@ -546,11 +539,8 @@ static int exynos_drm_add_components(struct device
>> *dev, struct master *m)
>> * connector/encoder need pipe number of crtc when they
>> * are created.
>> */
>> -ret = component_master_add_child(m, compare_of, cdev->crtc_dev);
>> -ret |= component_master_add_child(m, compare_of,
>> -cdev->conn_dev);
>> -if (ret < 0)
>> -return ret;
>> +component_match_add(dev, &match, compare_of, cdev->crtc_dev);
>> +component_match_add(dev, &match, compare_of, cdev->conn_dev);
>>
>> out_lock:
>> mutex_lock(&drm_component_lock);
>> @@ -558,7 +548,7 @@ out_lock:
>>
>> mutex_unlock(&drm_component_lock);
>>
>> -return attach_cnt ? 0 : -ENODEV;
>> +return attach_cnt ? match : ERR_PTR(-EPROBE_DEFER);
>> }
>>
>> static int exynos_drm_bind(struct device *dev)
>> @@ -572,13 +562,13 @@ static void exynos_drm_unbind(struct device *dev)
>> }
>>
>> static const struct component_master_ops exynos_drm_ops = {
>> -.add_components = exynos_drm_add_components,
>> .bind = exynos_drm_bind,
>> .unbind = exynos_drm_unbind,
>> };
>>
>> static int exynos_drm_platform_probe(struct platform_device *pdev)
>> {
>> +struct component_match *match;
>> int ret;
>>
>> pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>> @@ -645,13 +635,19 @@ static int exynos_drm_platform_probe(struct
>> platform_device *pdev)
>> goto err_unregister_ipp_drv;
>> #endif
>>
>> -ret = component_master_add(&pdev->dev, &exynos_drm_ops);
>> -if (ret < 0)
>> -DRM_DEBUG_KMS("re-tried by last sub driver probed later.\n");
>> +match = exynos_drm_match_add(&pdev->dev);
>> +if (IS_ERR(match)) {
>> +ret = PTR_ERR(match);
>> +goto err_unregister_ipp_dev;
>> +}
>>
>> -return 0;
>> +
[Bug 83748] New: Only black content on screen, in the Tokyo flashback of the game "The Secret World"
https://bugs.freedesktop.org/show_bug.cgi?id=83748 Priority: medium Bug ID: 83748 Assignee: dri-devel at lists.freedesktop.org Summary: Only black content on screen, in the Tokyo flashback of the game "The Secret World" Severity: normal Classification: Unclassified OS: Linux (All) Reporter: john.ettedgui at gmail.com Hardware: x86-64 (AMD64) Status: NEW Version: git Component: Drivers/Gallium/radeonsi Product: Mesa Hey, I am trying to play the game "The Secret World" on my system. It is a Windows game so I am using Wine to run it. Initially the game runs, and I can see everything (cinematics and in-game stuff), but for some reason once I enter the Tokyo flashback, I cannot see the cinematic or anything in that dungeon. In the cinematic my screen seems to be completely black. Once back in-game, all content is dark/black, but I can kind of see where walls are or the overall shape of my character. Maybe that's what it would be with a null contrast and 0 brightness? Also the game is still running, I can interact with the npcs (if I manage to find them) etc... I tried using stable mesa 10.2.7 instead of daily git but it was the same. I then tried running the game on the same machine using the Catalyst drivers and all the visuals were there, both in the cinematic and in-game, so as far as I can see the issue shouldn't be in Wine. I don't understand why I'm getting that problem only in that part of the game, I've passed it using Catalyst, and now I'm able to continue the game normally with the gallium driver after this strange part... I am not sure what logs to add to this, since it is a visual issue and I don't see any kind of error message about it (although it could be hidden in the wine verbose logs I suppose). My system: Radeon 7970 running with Mesa, daily git or 10.2.7. llvm either daily git or 3.5.0 Arch Linux 64b with Linux kernel 3.16.2 I've tried both wine and wine-csmt, both have the same issue. Thanks! -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/992b2a12/attachment.html>
[PATCH RFC 4/4] drm: link connectors to backlight devices
On Wed, Sep 10, 2014 at 05:54:23PM +0200, David Herrmann wrote: > Backlight devices have always been managed independently of display > controllers. They're often controlled via different hardware interfaces > and their relationship to display-controllers varies vastly between > different boards. However, display brightness is obviously a property of > a display, and thus of a DRM connector. Therefore, it'd be really > appreciated if user-space APIs would highlight this relationship. > > The main runtime users of backlight interfaces are user-space compositors. > But currently they have to jump through hoops to find the correct > backlight device for a given connector. Furthermore, they need root > privileges to write to sysfs. sysfs has never been designed as run-time > non-root API. It does not provide file-contexts, run-time management or > any kind of API control. There is no way to control access to sysfs via > different links (in that case: mounts). Char-devs provide all this! > > So far, backlight APIs have been fairly trivial, so adding char-devs to > backlights is rather heavy-weight. Therefore, this patch introduces a new > API interface to modify backlight brightness via DRM: A "BRIGHTNESS" > property on DRM connectors. > > Instead of adding backlight hardware support to DRM, we rely on the > backlight-class and simply add a new API. Each DRM Connector can > optionally be linked to a backlight class device. Modifying the connector > property will have the same effect as writing into the "brightness" sysfs > file of the linked backlight class device. However, we now can manage > access to backlight devices via the same interface as access to > mode-setting on the underlying display. Furthermore, the connection > between displays and their backlight devices are visible in user-space. > > Obviously, matching backlights to displays cannot be solved magically > with this link. Therefore, we also add a user-space attribute to DRM > connectors called 'backlight'. If a DRM driver is incapable of matching > existing backlights to a connector, or if a given board has just crappy > backlight drivers, udev can write the name of a backlight-device into this > attribute and the connector-property will be re-linked to this backlight > device. The udev hwdb can be easily employed to track such quirks and > fixups for different board+GPU combinations. > Note that the name written into the 'backlight' attribute is saved on the > connector, so in case the real backlight device is probed after the DRM > card, the backlight will still get properly attached once probed. > > Signed-off-by: David Herrmann Nice you skid around all the pitfalls and trapdoors, I guess we've all been rather blind ;-) Two high-level comments: - We also want to forward "bl_power". cros was totally not happy when we stopped treating brightness == 0 as completely off (it upsets some panels terminally, so there's a vbt lower limit). Instead we expose this now through the bl_power knob. While at it I think we should expose all the other backlight properties too (read-only ofc for actual/max_brightness). - How does udev match on the drm connector name? They are not terribly stable atm, and if you reload your drm driver, or much more likely, have two gpus with two drm drivers they change. We probably should change the name allocation scheme to be per device instance instead of global first. Within a driver probe order is hopefully deterministic on a given platform, since even with super dynamic setups (based on dt/acpi) the firmware tables should change really. Cheers, Daniel > --- > drivers/gpu/drm/Kconfig | 1 + > drivers/gpu/drm/Makefile| 2 +- > drivers/gpu/drm/drm_backlight.c | 387 > > drivers/gpu/drm/drm_crtc.c | 45 +++-- > drivers/gpu/drm/drm_drv.c | 11 + > drivers/gpu/drm/drm_sysfs.c | 53 + > drivers/video/backlight/backlight.c | 3 + > include/drm/drm_backlight.h | 44 > include/drm/drm_crtc.h | 3 + > include/linux/backlight.h | 1 + > 10 files changed, 530 insertions(+), 20 deletions(-) > create mode 100644 drivers/gpu/drm/drm_backlight.c > create mode 100644 include/drm/drm_backlight.h > > diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig > index e3b4b0f..46bca34 100644 > --- a/drivers/gpu/drm/Kconfig > +++ b/drivers/gpu/drm/Kconfig > @@ -12,6 +12,7 @@ menuconfig DRM > select I2C > select I2C_ALGOBIT > select DMA_SHARED_BUFFER > + select BACKLIGHT_CLASS_DEVICE > help > Kernel-level support for the Direct Rendering Infrastructure (DRI) > introduced in XFree86 4.0. If you say Y here, you need to select > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile > index 9292a76..224544d 100644 > --- a/drivers/gpu/drm/Makefile > +++ b/drivers/gpu/drm/Makefile > @@ -14,7 +14,7 @@ drm-y :
[PATCH] drm/exynos: use a new anon file for exynos gem mmaper
Hi, On 09/11/2014 03:22 PM, Daniel Vetter wrote: > On Thu, Sep 11, 2014 at 11:16:53AM +0900, Inki Dae wrote: >> On 2014? 09? 10? 18:01, Daniel Vetter wrote: >>> Ok I've stumbled over the exynos mmap stuff again while cleaning up >>> drm legacy cruft and I just don't get what you're doing and why >>> exactly exynos needs to be special. >>> >>> _All_ other drm drivers happily get along with the vma offset manger >>> stuff to handle mmaps, but somehow exynos does something really crazy. >> >> We are also using the vma offset manager stuff. We just added direct >> mapping interface specific to Exynos additionally. >> >>> >>> Can you please explain the design justification for this and why >>> switching to the standard gem mmap support isn't possible? >> >> As I mentioned above, we are using the standard gem mmap support. >> However, the standard gem mmap is required for on-demand paging mostly >> suitable for Desktop. In case of ARM SoC, whole memory region requested >> by userspace would be allocated once the gem creation interface is >> called. In this case, it wouldn't need to map userspace with physical >> page in page fault handler, and the use of the vma offset manager stuff >> would be unnecessary step. > > You don't need to do demand paging at all, you can simply put in all the > ptes in one go and then never unbind it. So strictly speaking you don't > need to roll your own mmap, but otoh other drivers (including i915) do > their own special mmap too. And since you now have it you must support it > forever anyway. I agree with Daniel. The exynos drm specific mmap ioctl can be substituted to standard gem mmap if exynos mmap is implemented for direct mapping, actually gem cma does it from drm_gem_cma_mmap_obj. Thanks. > > Aside: We have patches floating around for i915 to prefault aggressively, > so you're not the only ones who noticed the faulting overhead. ARM SoC > really aren't all that special compared to traditional desktop gpus, so if > you stumble over such issues please raise them on dri-devel so that we > could look into useful generic solutions next time around. > >> For the same question, Al Viro did, >> http://lists.freedesktop.org/archives/dri-devel/2013-September/046207.html >> >> Is there any issue I am missing , that could be incurred by Exynos codes? > > I've stumbled over it again because you're reusing the drm_vm_open_locked > function, which really should just be an implementation detail of the core > drm/gem mmap support. > > If you want to roll your own mmap (and that's ok, i915 has it and ttm also > does it) then imo you should not reuse any of the core mmap code, but > implement your own set of vm_ops. You don't need a faul handler for this > (since it will never fault), and open/close would just grabbing/dropping a > reference of the underlying gem object. Instead of trying to reuse the > same vm_ops you use for normal gem mmaps, which just doesn't make a lot of > sense to me. > > If exynos stops using drm_vm_open_locked then I can move it into the new > drm_internal.h header since this function really should be private to > drm.ko. > > Thanks, Daniel >
[PATCH] drm/exynos: update to use component match support
On 09/11/2014 08:37 AM, Inki Dae wrote:
> On 2014? 09? 10? 19:24, Andrzej Hajda wrote:
>> Hi Inki,
>>
>> To test it properly I have to fix init/remove bugs [1].
>> Of course these bugs were not introduced by this patch,
>> but they prevented some basic tests.
> I had tested my patch with trats2 board, and works well without below
> patch set. hm.. it seems that there is other corner cases I missed. Can
> you give me more details about basic tests?
As the component framework is about bringing up/down the master when
all/some components becomes available/unavailable I have tested
what happens when I change availability of components using bind/unbind
sysfs properties, for example:
echo 11c8.dsi >/sys/bus/platform/drivers/exynos-dsi/unbind
echo 11c8.dsi >/sys/bus/platform/drivers/exynos-dsi/bind
Regards
Andrzej
>
>> [1]: http://permalink.gmane.org/gmane.linux.kernel.samsung-soc/37266
>>
>> I have tested successfully your patch with trats and universal_c210 boards.
> Thanks for testing and above fixup patch set. Will look into them soon. :)
>
> Thanks,
> Inki Dae
>
>> Few additional comments below.
>>
>> On 09/01/2014 02:19 PM, Inki Dae wrote:
>>> Update Exynos's DRM driver to use component match support rater than
>>> add_components.
>>>
>>> Signed-off-by: Inki Dae
>>> ---
>>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 40
>>> ++-
>>> 1 file changed, 18 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> index feee991..dae62c2 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
>>> @@ -503,16 +503,15 @@ static int compare_of(struct device *dev, void *data)
>>> return dev == (struct device *)data;
>>> }
>> Nitpick.
>>
>> This is not a part of this patch but compare_of suggests it compares OF
>> nodes but this function compares devices, maybe compare_dev would be better.
>>
>>>
>>> -static int exynos_drm_add_components(struct device *dev, struct master *m)
>>> +static struct component_match *exynos_drm_match_add(struct device *dev)
>>> {
>>> + struct component_match *match = NULL;
>>> struct component_dev *cdev;
>>> unsigned int attach_cnt = 0;
>>>
>>> mutex_lock(&drm_component_lock);
>>>
>>> list_for_each_entry(cdev, &drm_component_list, list) {
>>> - int ret;
>>> -
>>> /*
>>> * Add components to master only in case that crtc and
>>> * encoder/connector device objects exist.
>>> @@ -527,16 +526,10 @@ static int exynos_drm_add_components(struct device
>>> *dev, struct master *m)
>>> /*
>>> * fimd and dpi modules have same device object so add
>>> * only crtc device object in this case.
>>> -*
>>> -* TODO. if dpi module follows driver-model driver then
>>> -* below codes can be removed.
>>> */
>>> if (cdev->crtc_dev == cdev->conn_dev) {
>>> - ret = component_master_add_child(m, compare_of,
>>> - cdev->crtc_dev);
>>> - if (ret < 0)
>>> - return ret;
>>> -
>>> + component_match_add(dev, &match, compare_of,
>>> + cdev->crtc_dev);
>>> goto out_lock;
>>> }
>>>
>>> @@ -546,11 +539,8 @@ static int exynos_drm_add_components(struct device
>>> *dev, struct master *m)
>>> * connector/encoder need pipe number of crtc when they
>>> * are created.
>>> */
>>> - ret = component_master_add_child(m, compare_of, cdev->crtc_dev);
>>> - ret |= component_master_add_child(m, compare_of,
>>> - cdev->conn_dev);
>>> - if (ret < 0)
>>> - return ret;
>>> + component_match_add(dev, &match, compare_of, cdev->crtc_dev);
>>> + component_match_add(dev, &match, compare_of, cdev->conn_dev);
>>>
>>> out_lock:
>>> mutex_lock(&drm_component_lock);
>>> @@ -558,7 +548,7 @@ out_lock:
>>>
>>> mutex_unlock(&drm_component_lock);
>>>
>>> - return attach_cnt ? 0 : -ENODEV;
>>> + return attach_cnt ? match : ERR_PTR(-EPROBE_DEFER);
>>> }
>>>
>>> static int exynos_drm_bind(struct device *dev)
>>> @@ -572,13 +562,13 @@ static void exynos_drm_unbind(struct device *dev)
>>> }
>>>
>>> static const struct component_master_ops exynos_drm_ops = {
>>> - .add_components = exynos_drm_add_components,
>>> .bind = exynos_drm_bind,
>>> .unbind = exynos_drm_unbind,
>>> };
>>>
>>> static int exynos_drm_platform_probe(struct platform_device *pdev)
>>> {
>>> + struct component_match *match;
>>> int ret;
>>>
>>> pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
>>> @@ -645,13 +635,19 @@ static int exy
[PATCH] drm: Move dma functions into drm_legacy.h
Also drop the unneeded EXPORT_SYMBOL and sprinkle drm_legacy_ prefixes
where missing.
v2: Drop the confusing _core_ and drop extern, both suggested by
David.
Cc: David Herrmann
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_bufs.c | 2 +-
drivers/gpu/drm/drm_dma.c| 10 --
drivers/gpu/drm/drm_fops.c | 2 +-
drivers/gpu/drm/drm_legacy.h | 8
include/drm/drmP.h | 7 ---
5 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index 9e04d6a43fa4..70ba89f66199 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -1338,7 +1338,7 @@ int drm_legacy_freebufs(struct drm_device *dev, void
*data,
task_pid_nr(current));
return -EINVAL;
}
- drm_free_buffer(dev, buf);
+ drm_legacy_free_buffer(dev, buf);
}
return 0;
diff --git a/drivers/gpu/drm/drm_dma.c b/drivers/gpu/drm/drm_dma.c
index 8a140a953754..1b1dd356a1e4 100644
--- a/drivers/gpu/drm/drm_dma.c
+++ b/drivers/gpu/drm/drm_dma.c
@@ -124,7 +124,7 @@ void drm_legacy_dma_takedown(struct drm_device *dev)
*
* Resets the fields of \p buf.
*/
-void drm_free_buffer(struct drm_device *dev, struct drm_buf * buf)
+void drm_legacy_free_buffer(struct drm_device *dev, struct drm_buf * buf)
{
if (!buf)
return;
@@ -142,8 +142,8 @@ void drm_free_buffer(struct drm_device *dev, struct drm_buf
* buf)
*
* Frees each buffer associated with \p file_priv not already on the hardware.
*/
-void drm_core_reclaim_buffers(struct drm_device *dev,
- struct drm_file *file_priv)
+void drm_legacy_reclaim_buffers(struct drm_device *dev,
+ struct drm_file *file_priv)
{
struct drm_device_dma *dma = dev->dma;
int i;
@@ -154,7 +154,7 @@ void drm_core_reclaim_buffers(struct drm_device *dev,
if (dma->buflist[i]->file_priv == file_priv) {
switch (dma->buflist[i]->list) {
case DRM_LIST_NONE:
- drm_free_buffer(dev, dma->buflist[i]);
+ drm_legacy_free_buffer(dev, dma->buflist[i]);
break;
case DRM_LIST_WAIT:
dma->buflist[i]->list = DRM_LIST_RECLAIM;
@@ -166,5 +166,3 @@ void drm_core_reclaim_buffers(struct drm_device *dev,
}
}
}
-
-EXPORT_SYMBOL(drm_core_reclaim_buffers);
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index b419990042b0..3bb6234d072a 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -404,7 +404,7 @@ int drm_release(struct inode *inode, struct file *filp)
drm_master_release(dev, filp);
if (drm_core_check_feature(dev, DRIVER_HAVE_DMA))
- drm_core_reclaim_buffers(dev, file_priv);
+ drm_legacy_reclaim_buffers(dev, file_priv);
drm_events_release(file_priv);
diff --git a/drivers/gpu/drm/drm_legacy.h b/drivers/gpu/drm/drm_legacy.h
index 3049af5a01b3..0e0df225dec6 100644
--- a/drivers/gpu/drm/drm_legacy.h
+++ b/drivers/gpu/drm/drm_legacy.h
@@ -92,4 +92,12 @@ int drm_legacy_lock(struct drm_device *d, void *v, struct
drm_file *f);
int drm_legacy_unlock(struct drm_device *d, void *v, struct drm_file *f);
int drm_legacy_lock_free(struct drm_lock_data *lock, unsigned int ctx);
+/* DMA support */
+int drm_legacy_dma_setup(struct drm_device *dev);
+void drm_legacy_dma_takedown(struct drm_device *dev);
+void drm_legacy_free_buffer(struct drm_device *dev,
+ struct drm_buf * buf);
+void drm_legacy_reclaim_buffers(struct drm_device *dev,
+ struct drm_file *filp);
+
#endif /* __DRM_LEGACY_H__ */
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index c5c9f0e44a7b..574e325d6b69 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1158,13 +1158,6 @@ void drm_clflush_virt_range(void *addr, unsigned long
length);
* DMA quiscent + idle. DMA quiescent usually requires the hardware lock.
*/
- /* DMA support (drm_dma.h) */
-extern int drm_legacy_dma_setup(struct drm_device *dev);
-extern void drm_legacy_dma_takedown(struct drm_device *dev);
-extern void drm_free_buffer(struct drm_device *dev, struct drm_buf * buf);
-extern void drm_core_reclaim_buffers(struct drm_device *dev,
-struct drm_file *filp);
-
/* IRQ support (drm_irq.h) */
extern int drm_control(struct drm_device *dev, void *data,
struct drm_file *file_priv);
--
1.9.3
[PATCH] drm: Move sg functions into drm_legacy.h
Also sprinkle the drm_legacy_ prefix where missing.
v2: Drop extern from function declarations and include "drm_legacy.h"
in drm_scatter.c, spotted by David.
Cc: David Herrmann
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_ioctl.c | 4 ++--
drivers/gpu/drm/drm_legacy.h | 7 +++
drivers/gpu/drm/drm_scatter.c | 9 +
include/drm/drmP.h| 7 ---
4 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index bfb3b85dbe2d..7d6df78bb25d 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -108,8 +108,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_AGP_UNBIND, drm_agp_unbind_ioctl,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
#endif
- DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_sg_alloc,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
- DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_sg_free,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
+ DRM_IOCTL_DEF(DRM_IOCTL_SG_ALLOC, drm_legacy_sg_alloc,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
+ DRM_IOCTL_DEF(DRM_IOCTL_SG_FREE, drm_legacy_sg_free,
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF(DRM_IOCTL_WAIT_VBLANK, drm_wait_vblank, DRM_UNLOCKED),
diff --git a/drivers/gpu/drm/drm_legacy.h b/drivers/gpu/drm/drm_legacy.h
index 0e0df225dec6..3cc0efa7304a 100644
--- a/drivers/gpu/drm/drm_legacy.h
+++ b/drivers/gpu/drm/drm_legacy.h
@@ -100,4 +100,11 @@ void drm_legacy_free_buffer(struct drm_device *dev,
void drm_legacy_reclaim_buffers(struct drm_device *dev,
struct drm_file *filp);
+/* Scatter Gather Support */
+void drm_legacy_sg_cleanup(struct drm_device *dev);
+int drm_legacy_sg_alloc(struct drm_device *dev, void *data,
+ struct drm_file *file_priv);
+int drm_legacy_sg_free(struct drm_device *dev, void *data,
+ struct drm_file *file_priv);
+
#endif /* __DRM_LEGACY_H__ */
diff --git a/drivers/gpu/drm/drm_scatter.c b/drivers/gpu/drm/drm_scatter.c
index 1c78406f6e71..4f0f3b36d537 100644
--- a/drivers/gpu/drm/drm_scatter.c
+++ b/drivers/gpu/drm/drm_scatter.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include "drm_legacy.h"
#define DEBUG_SCATTER 0
@@ -78,8 +79,8 @@ void drm_legacy_sg_cleanup(struct drm_device *dev)
# define ScatterHandle(x) (unsigned int)(x)
#endif
-int drm_sg_alloc(struct drm_device *dev, void *data,
-struct drm_file *file_priv)
+int drm_legacy_sg_alloc(struct drm_device *dev, void *data,
+ struct drm_file *file_priv)
{
struct drm_scatter_gather *request = data;
struct drm_sg_mem *entry;
@@ -194,8 +195,8 @@ int drm_sg_alloc(struct drm_device *dev, void *data,
return -ENOMEM;
}
-int drm_sg_free(struct drm_device *dev, void *data,
- struct drm_file *file_priv)
+int drm_legacy_sg_free(struct drm_device *dev, void *data,
+ struct drm_file *file_priv)
{
struct drm_scatter_gather *request = data;
struct drm_sg_mem *entry;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 574e325d6b69..1d1468bcd69f 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1316,13 +1316,6 @@ void drm_prime_remove_buf_handle_locked(struct
drm_prime_file_private *prime_fpr
extern int drm_vma_info(struct seq_file *m, void *data);
- /* Scatter Gather Support (drm_scatter.h) */
-extern void drm_legacy_sg_cleanup(struct drm_device *dev);
-extern int drm_sg_alloc(struct drm_device *dev, void *data,
- struct drm_file *file_priv);
-extern int drm_sg_free(struct drm_device *dev, void *data,
- struct drm_file *file_priv);
-
/* ATI PCIGART support (ati_pcigart.h) */
extern int drm_ati_pcigart_init(struct drm_device *dev,
struct drm_ati_pcigart_info * gart_info);
--
1.9.3
[PATCH] drm: Move legacy buffer structures to
A few odd cases:
- mgag200 someho had a totally unused drm_dma_handle_t. Remove it.
- i915 still uses the legacy pci dma alloc api, so grows an include.
Everything else fairly standard.
v2: Include "drm_legacy.h" in drm.ko source files for consistency.
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_dma.c | 1 +
drivers/gpu/drm/drm_info.c| 1 +
drivers/gpu/drm/drm_pci.c | 1 +
drivers/gpu/drm/i915/i915_drv.h | 9 ++-
drivers/gpu/drm/mgag200/mgag200_drv.h | 2 -
drivers/gpu/drm/via/via_verifier.c| 1 +
include/drm/drmP.h| 118 ++
include/drm/drm_legacy.h | 109 +++
8 files changed, 125 insertions(+), 117 deletions(-)
diff --git a/drivers/gpu/drm/drm_dma.c b/drivers/gpu/drm/drm_dma.c
index 1b1dd356a1e4..ea481800ef56 100644
--- a/drivers/gpu/drm/drm_dma.c
+++ b/drivers/gpu/drm/drm_dma.c
@@ -35,6 +35,7 @@
#include
#include
+#include "drm_legacy.h"
/**
* Initialize the DMA data.
diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c
index d1c5904bc473..0780541f7935 100644
--- a/drivers/gpu/drm/drm_info.c
+++ b/drivers/gpu/drm/drm_info.c
@@ -35,6 +35,7 @@
#include
#include
+#include "drm_legacy.h"
/**
* Called when "/proc/dri/.../name" is read.
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 02ab8c52f311..fd29f03645b8 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include "drm_legacy.h"
/**
* drm_pci_alloc - Allocate a PCI consistent memory block, for DMA.
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e3ca8dfa60df..17dfce0f4e68 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -42,6 +42,7 @@
#include
#include
#include
+#include /* for struct drm_dma_handle */
#include
#include
#include
@@ -288,8 +289,10 @@ struct intel_opregion {
struct intel_overlay;
struct intel_overlay_error_state;
+struct drm_local_map;
+
struct drm_i915_master_private {
- drm_local_map_t *sarea;
+ struct drm_local_map *sarea;
struct _drm_i915_sarea *sarea_priv;
};
#define I915_FENCE_REG_NONE -1
@@ -1476,7 +1479,7 @@ struct drm_i915_private {
struct drm_i915_gem_object *semaphore_obj;
uint32_t last_seqno, next_seqno;
- drm_dma_handle_t *status_page_dmah;
+ struct drm_dma_handle *status_page_dmah;
struct resource mch_res;
/* protects the irq masks */
@@ -1881,7 +1884,7 @@ struct drm_i915_gem_object {
struct drm_file *pin_filp;
/** for phy allocated objects */
- drm_dma_handle_t *phys_handle;
+ struct drm_dma_handle *phys_handle;
union {
struct i915_gem_userptr {
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h
b/drivers/gpu/drm/mgag200/mgag200_drv.h
index 2e2b76aa4e17..c03e347f3ffd 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -190,8 +190,6 @@ struct mga_device {
resource_size_t rmmio_size;
void __iomem*rmmio;
- drm_local_map_t *framebuffer;
-
struct mga_mc mc;
struct mga_mode_infomode_info;
diff --git a/drivers/gpu/drm/via/via_verifier.c
b/drivers/gpu/drm/via/via_verifier.c
index 9dbc92bd1512..0677bbf4ec7e 100644
--- a/drivers/gpu/drm/via/via_verifier.c
+++ b/drivers/gpu/drm/via/via_verifier.c
@@ -31,6 +31,7 @@
#include "via_3d_reg.h"
#include
#include
+#include
#include "via_verifier.h"
#include "via_drv.h"
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index ff43b4a14656..d40edf096154 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -79,6 +79,9 @@ struct module;
struct drm_file;
struct drm_device;
struct drm_agp_head;
+struct drm_local_map;
+struct drm_device_dma;
+struct drm_dma_handle;
struct device_node;
struct videomode;
@@ -275,57 +278,6 @@ struct drm_ioctl_desc {
#define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)\
[DRM_IOCTL_NR(DRM_##ioctl)] = {.cmd = DRM_##ioctl, .func = _func,
.flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl, .name = #ioctl}
-/**
- * DMA buffer.
- */
-struct drm_buf {
- int idx; /**< Index into master buflist */
- int total; /**< Buffer size */
- int order; /**< log-base-2(total) */
- int used; /**< Amount of buffer in use (for DMA) */
- unsigned long offset; /**< Byte offset (used internally) */
- void *address; /**< Address of buffer */
- unsigned long bus_address; /**< Bus address of buffer */
- struct drm_buf *next; /**< Kernel-only: used for free list */
- __volatile__ int waiting; /**< On kerne
[PATCH] drm/exynos: use a new anon file for exynos gem mmaper
On 2014? 09? 11? 15:22, Daniel Vetter wrote: > On Thu, Sep 11, 2014 at 11:16:53AM +0900, Inki Dae wrote: >> On 2014? 09? 10? 18:01, Daniel Vetter wrote: >>> Ok I've stumbled over the exynos mmap stuff again while cleaning up >>> drm legacy cruft and I just don't get what you're doing and why >>> exactly exynos needs to be special. >>> >>> _All_ other drm drivers happily get along with the vma offset manger >>> stuff to handle mmaps, but somehow exynos does something really crazy. >> >> We are also using the vma offset manager stuff. We just added direct >> mapping interface specific to Exynos additionally. >> >>> >>> Can you please explain the design justification for this and why >>> switching to the standard gem mmap support isn't possible? >> >> As I mentioned above, we are using the standard gem mmap support. >> However, the standard gem mmap is required for on-demand paging mostly >> suitable for Desktop. In case of ARM SoC, whole memory region requested >> by userspace would be allocated once the gem creation interface is >> called. In this case, it wouldn't need to map userspace with physical >> page in page fault handler, and the use of the vma offset manager stuff >> would be unnecessary step. > > You don't need to do demand paging at all, you can simply put in all the > ptes in one go and then never unbind it. So strictly speaking you don't > need to roll your own mmap, but otoh other drivers (including i915) do > their own special mmap too. And since you now have it you must support it > forever anyway. > > Aside: We have patches floating around for i915 to prefault aggressively, > so you're not the only ones who noticed the faulting overhead. ARM SoC > really aren't all that special compared to traditional desktop gpus, so if > you stumble over such issues please raise them on dri-devel so that we > could look into useful generic solutions next time around. > >> For the same question, Al Viro did, >> http://lists.freedesktop.org/archives/dri-devel/2013-September/046207.html >> >> Is there any issue I am missing , that could be incurred by Exynos codes? > > I've stumbled over it again because you're reusing the drm_vm_open_locked > function, which really should just be an implementation detail of the core > drm/gem mmap support. Ah, right. that is critical issue. I shouldn't had used drm_vm_open_locked. Sorry for this. > > If you want to roll your own mmap (and that's ok, i915 has it and ttm also > does it) then imo you should not reuse any of the core mmap code, but > implement your own set of vm_ops. You don't need a faul handler for this > (since it will never fault), and open/close would just grabbing/dropping a > reference of the underlying gem object. Instead of trying to reuse the > same vm_ops you use for normal gem mmaps, which just doesn't make a lot of > sense to me. > > If exynos stops using drm_vm_open_locked then I can move it into the new > drm_internal.h header since this function really should be private to > drm.ko. Sorry for blocking you. I will fix it soon. Thanks, Inki Dae > > Thanks, Daniel >
[PATCH] drm/exynos: use a new anon file for exynos gem mmaper
On 2014? 09? 11? 16:01, Joonyoung Shim wrote: > Hi, > > On 09/11/2014 03:22 PM, Daniel Vetter wrote: >> On Thu, Sep 11, 2014 at 11:16:53AM +0900, Inki Dae wrote: >>> On 2014? 09? 10? 18:01, Daniel Vetter wrote: Ok I've stumbled over the exynos mmap stuff again while cleaning up drm legacy cruft and I just don't get what you're doing and why exactly exynos needs to be special. _All_ other drm drivers happily get along with the vma offset manger stuff to handle mmaps, but somehow exynos does something really crazy. >>> >>> We are also using the vma offset manager stuff. We just added direct >>> mapping interface specific to Exynos additionally. >>> Can you please explain the design justification for this and why switching to the standard gem mmap support isn't possible? >>> >>> As I mentioned above, we are using the standard gem mmap support. >>> However, the standard gem mmap is required for on-demand paging mostly >>> suitable for Desktop. In case of ARM SoC, whole memory region requested >>> by userspace would be allocated once the gem creation interface is >>> called. In this case, it wouldn't need to map userspace with physical >>> page in page fault handler, and the use of the vma offset manager stuff >>> would be unnecessary step. >> >> You don't need to do demand paging at all, you can simply put in all the >> ptes in one go and then never unbind it. So strictly speaking you don't >> need to roll your own mmap, but otoh other drivers (including i915) do >> their own special mmap too. And since you now have it you must support it >> forever anyway. > > I agree with Daniel. The exynos drm specific mmap ioctl can be > substituted to standard gem mmap if exynos mmap is implemented for > direct mapping, actually gem cma does it from drm_gem_cma_mmap_obj. Right, we don't need mmap ioctl specific to Exynos. What we have to is to call a Exynos function to do direct mapping at the end of exynos_drm_gem_mmap function. As Daniel mentioned above, this way we don't also need even page fault handler. We really visited here and there to stick to the use of mmap ioctl specfic to Exynos. :) Thanks, Inki Dae > > Thanks. > >> >> Aside: We have patches floating around for i915 to prefault aggressively, >> so you're not the only ones who noticed the faulting overhead. ARM SoC >> really aren't all that special compared to traditional desktop gpus, so if >> you stumble over such issues please raise them on dri-devel so that we >> could look into useful generic solutions next time around. >> >>> For the same question, Al Viro did, >>> http://lists.freedesktop.org/archives/dri-devel/2013-September/046207.html >>> >>> Is there any issue I am missing , that could be incurred by Exynos codes? >> >> I've stumbled over it again because you're reusing the drm_vm_open_locked >> function, which really should just be an implementation detail of the core >> drm/gem mmap support. >> >> If you want to roll your own mmap (and that's ok, i915 has it and ttm also >> does it) then imo you should not reuse any of the core mmap code, but >> implement your own set of vm_ops. You don't need a faul handler for this >> (since it will never fault), and open/close would just grabbing/dropping a >> reference of the underlying gem object. Instead of trying to reuse the >> same vm_ops you use for normal gem mmaps, which just doesn't make a lot of >> sense to me. >> >> If exynos stops using drm_vm_open_locked then I can move it into the new >> drm_internal.h header since this function really should be private to >> drm.ko. >> >> Thanks, Daniel >> > >
[Bug 60879] [radeonsi] X11 can't start with acceleration enabled
https://bugs.freedesktop.org/show_bug.cgi?id=60879 --- Comment #91 from Michel D?nzer --- Created attachment 106113 --> https://bugs.freedesktop.org/attachment.cgi?id=106113&action=edit Another approach If Tom's v4 patch doesn't work, you can try this patch on top of it. If that still doesn't work, please provide the stderr debugging output about raster_config. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/990afd0b/attachment.html>
[PATCH RFC 1/4] backlight: use static initializers
On Wed, 10 Sep 2014, David Herrmann wrote:
> Use static initializers instead of setting up global variables during
> runtime. This reduces code size and execution time.
>
> Signed-off-by: David Herrmann
Reviewed-by: Jani Nikula
> ---
> drivers/video/backlight/backlight.c | 9 +++--
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/backlight/backlight.c
> b/drivers/video/backlight/backlight.c
> index bddc8b1..726c6c6 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -21,9 +21,9 @@
> #include
> #endif
>
> -static struct list_head backlight_dev_list;
> -static struct mutex backlight_dev_list_mutex;
> -static struct blocking_notifier_head backlight_notifier;
> +static LIST_HEAD(backlight_dev_list);
> +static DEFINE_MUTEX(backlight_dev_list_mutex);
> +static BLOCKING_NOTIFIER_HEAD(backlight_notifier);
>
> static const char *const backlight_types[] = {
> [BACKLIGHT_RAW] = "raw",
> @@ -582,9 +582,6 @@ static int __init backlight_class_init(void)
>
> backlight_class->dev_groups = bl_device_groups;
> backlight_class->pm = &backlight_class_dev_pm_ops;
> - INIT_LIST_HEAD(&backlight_dev_list);
> - mutex_init(&backlight_dev_list_mutex);
> - BLOCKING_INIT_NOTIFIER_HEAD(&backlight_notifier);
>
> return 0;
> }
> --
> 2.1.0
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Jani Nikula, Intel Open Source Technology Center
[PATCH RFC 2/4] backlight: use spin-lock to protect device list
On Wed, 10 Sep 2014, David Herrmann wrote:
> There is really no reason to use a mutex to protect a simple list. Convert
> the list-lock to a simple spinlock instead.
>
> The spin-locks prepare for a backlight_find() helper, which should
> preferably be usable from atomic context. A mutex would prevent that, so
> use an irq-save spinlock instead.
>
> Signed-off-by: David Herrmann
Reviewed-by: Jani Nikula
> ---
> drivers/video/backlight/backlight.c | 16 +---
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/video/backlight/backlight.c
> b/drivers/video/backlight/backlight.c
> index 726c6c6..33b64be 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -16,13 +16,14 @@
> #include
> #include
> #include
> +#include
>
> #ifdef CONFIG_PMAC_BACKLIGHT
> #include
> #endif
>
> static LIST_HEAD(backlight_dev_list);
> -static DEFINE_MUTEX(backlight_dev_list_mutex);
> +static DEFINE_SPINLOCK(backlight_dev_list_lock);
> static BLOCKING_NOTIFIER_HEAD(backlight_notifier);
>
> static const char *const backlight_types[] = {
> @@ -369,9 +370,9 @@ struct backlight_device *backlight_device_register(const
> char *name,
> mutex_unlock(&pmac_backlight_mutex);
> #endif
>
> - mutex_lock(&backlight_dev_list_mutex);
> + spin_lock_irq(&backlight_dev_list_lock);
> list_add(&new_bd->entry, &backlight_dev_list);
> - mutex_unlock(&backlight_dev_list_mutex);
> + spin_unlock_irq(&backlight_dev_list_lock);
>
> blocking_notifier_call_chain(&backlight_notifier,
>BACKLIGHT_REGISTERED, new_bd);
> @@ -384,15 +385,16 @@ bool backlight_device_registered(enum backlight_type
> type)
> {
> bool found = false;
> struct backlight_device *bd;
> + unsigned long flags;
>
> - mutex_lock(&backlight_dev_list_mutex);
> + spin_lock_irqsave(&backlight_dev_list_lock, flags);
> list_for_each_entry(bd, &backlight_dev_list, entry) {
> if (bd->props.type == type) {
> found = true;
> break;
> }
> }
> - mutex_unlock(&backlight_dev_list_mutex);
> + spin_unlock_irqrestore(&backlight_dev_list_lock, flags);
>
> return found;
> }
> @@ -409,9 +411,9 @@ void backlight_device_unregister(struct backlight_device
> *bd)
> if (!bd)
> return;
>
> - mutex_lock(&backlight_dev_list_mutex);
> + spin_lock_irq(&backlight_dev_list_lock);
> list_del(&bd->entry);
> - mutex_unlock(&backlight_dev_list_mutex);
> + spin_unlock_irq(&backlight_dev_list_lock);
>
> #ifdef CONFIG_PMAC_BACKLIGHT
> mutex_lock(&pmac_backlight_mutex);
> --
> 2.1.0
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Jani Nikula, Intel Open Source Technology Center
[Bug 83748] Only black content on screen, in the Tokyo flashback of the game "The Secret World"
https://bugs.freedesktop.org/show_bug.cgi?id=83748 --- Comment #1 from Michel D?nzer --- If you could create an apitrace demonstrating the problem, that should be useful. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/77991362/attachment.html>
[PATCH] drm/exynos: use a new anon file for exynos gem mmaper
On Thu, Sep 11, 2014 at 04:22:00PM +0900, Inki Dae wrote: > On 2014? 09? 11? 15:22, Daniel Vetter wrote: > > On Thu, Sep 11, 2014 at 11:16:53AM +0900, Inki Dae wrote: > >> On 2014? 09? 10? 18:01, Daniel Vetter wrote: > >>> Ok I've stumbled over the exynos mmap stuff again while cleaning up > >>> drm legacy cruft and I just don't get what you're doing and why > >>> exactly exynos needs to be special. > >>> > >>> _All_ other drm drivers happily get along with the vma offset manger > >>> stuff to handle mmaps, but somehow exynos does something really crazy. > >> > >> We are also using the vma offset manager stuff. We just added direct > >> mapping interface specific to Exynos additionally. > >> > >>> > >>> Can you please explain the design justification for this and why > >>> switching to the standard gem mmap support isn't possible? > >> > >> As I mentioned above, we are using the standard gem mmap support. > >> However, the standard gem mmap is required for on-demand paging mostly > >> suitable for Desktop. In case of ARM SoC, whole memory region requested > >> by userspace would be allocated once the gem creation interface is > >> called. In this case, it wouldn't need to map userspace with physical > >> page in page fault handler, and the use of the vma offset manager stuff > >> would be unnecessary step. > > > > You don't need to do demand paging at all, you can simply put in all the > > ptes in one go and then never unbind it. So strictly speaking you don't > > need to roll your own mmap, but otoh other drivers (including i915) do > > their own special mmap too. And since you now have it you must support it > > forever anyway. > > > > Aside: We have patches floating around for i915 to prefault aggressively, > > so you're not the only ones who noticed the faulting overhead. ARM SoC > > really aren't all that special compared to traditional desktop gpus, so if > > you stumble over such issues please raise them on dri-devel so that we > > could look into useful generic solutions next time around. > > > >> For the same question, Al Viro did, > >> http://lists.freedesktop.org/archives/dri-devel/2013-September/046207.html > >> > >> Is there any issue I am missing , that could be incurred by Exynos codes? > > > > I've stumbled over it again because you're reusing the drm_vm_open_locked > > function, which really should just be an implementation detail of the core > > drm/gem mmap support. > > Ah, right. that is critical issue. I shouldn't had used > drm_vm_open_locked. Sorry for this. > > > > > If you want to roll your own mmap (and that's ok, i915 has it and ttm also > > does it) then imo you should not reuse any of the core mmap code, but > > implement your own set of vm_ops. You don't need a faul handler for this > > (since it will never fault), and open/close would just grabbing/dropping a > > reference of the underlying gem object. Instead of trying to reuse the > > same vm_ops you use for normal gem mmaps, which just doesn't make a lot of > > sense to me. > > > > If exynos stops using drm_vm_open_locked then I can move it into the new > > drm_internal.h header since this function really should be private to > > drm.ko. > > Sorry for blocking you. I will fix it soon. No need to rush really, but if you fix this please cc me so that I can throw the header cleanup patch on top. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 81644] Random crashes on RadeonSI with Chromium.
https://bugs.freedesktop.org/show_bug.cgi?id=81644 --- Comment #93 from Maciej --- (In reply to comment #91) > I'm sorry to hear that. We're working on it, but since we haven't been able > to reproduce these issues, we need your help for testing: Does the > environment variable R600_DEBUG=nodma help? If not, can you try if Mesa 10.1 > is stable for you, and if so, bisect between 10.1 and 10.2? R600_DEBUG=nodma doesn't seem to help, got a hang while typing this comment. It doesn't happen with Mesa 10.1 from default Ubuntu 14.04 installation, but I need at least Mesa 10.3 and llvm-3.5 for any enjoyable gaming. My choice is either use Mesa for gaming, but then I can't use browser with any sort of hardware acceleration (or flash) or use fglrx for awesome 12fps desktop performance (cause fglrx is broken at all fronts). As for bisecting, I have no idea how to do that, nor I have time to learn - I'm just a Ubuntu user with no technical skills. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/3bb7643e/attachment.html>
[Bug 81644] Random crashes on RadeonSI with Chromium.
https://bugs.freedesktop.org/show_bug.cgi?id=81644 --- Comment #94 from Grigori Goronzy --- Maybe the crash actually happens because of glamor rendering - setting R600_DEBUG won't do anything in that case. Does this patch to Mesa make any difference? https://bugs.freedesktop.org/attachment.cgi?id=105745 -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/38720f94/attachment-0001.html>
[Bug 81382] Text console blanking does not go away
https://bugs.freedesktop.org/show_bug.cgi?id=81382 --- Comment #9 from Denys Vlasenko --- (In reply to comment #7) > Ok, this fix work, but cause another problem (tested with 3.15.5+patch and > 3.16.1). > > When display goes off, backlight goes off. > When display goes on, backlight is set to MAX. > When display goes off again, backligh remains MAX. > After pressing key, LCD works, backlight stay at MAX level. > When display goes off, backlight is still MAX. I would say it means that merely treating backlight value of 0 as MAX is not the best idea. Maybe we need an additional bool variable "failed to read initial BL value, don't ever try to set it", set it if initial read of BL value is 0, and if it is set, never try to change BL level? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/dcb34da8/attachment.html>
[Bug 83748] Only black content on screen, in the Tokyo flashback of the game "The Secret World"
https://bugs.freedesktop.org/show_bug.cgi?id=83748 --- Comment #2 from John --- Thank you for the prompt reply Michel. This is my first time with apitrace so I may have made a mistake. Here's the command line I've used: apitrace32 trace --api=gl /usr/bin/wine ".exe". I've tried running apitrace in 64bit but it never worked, I suppose because the game is 32bit. The result files are huge, is this expected? 5.3G, 327k, 236M I can't see how I could share those with you. (well apart from the 327k one). -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/a00cb2fe/attachment.html>
[Intel-gfx] [PULL] topic/vblank-rework
Hi Mario,
Can you please take a look at the patches I've submitted and review
them (at least the first 2)? Dave will close the 3.18 drm-next merge
window at the end of this week and I'd like to really get this in.
Thanks, Daniel
On Wed, Sep 10, 2014 at 5:45 PM, Mario Kleiner
wrote:
> On Wed, Sep 10, 2014 at 5:29 PM, Daniel Vetter
> wrote:
>> On Wed, Sep 10, 2014 at 4:19 PM, Mario Kleiner
>> wrote:
>>> Hmm, not quite an ack from my side for the pull in its current form. I
>>> said if the two remaining issues i mentioned are addressed, then i'm
>>> happy with it and can have my reviewed/acked-by. Looking at the code
>>> they haven't been adressed.
>>
>> Sorry about the confusion, I've somehow thought that you've retracted
>> those comments in Message-ID:
>>
>>
>> But I've missed that that was about just one of the issues.
>>
>
> Thought so. That one patch turns out to be crucial. My own software
> immediately complained loudly about broken vblank irqs and switched to
> lower performance fallbacks when that patch was missing.
>
> I'll test the patches on a few more cards in the next days - but so
> far things look good at least as far as my special test cases go.
>
>>> However, this is easily fixable on top of the current patches:
>>>
>>> 1. A vblank_disable_timeout module parameter of zero should always
>>> leave vblank irq's enabled and also override the drivers choice,
>>> otherwise a user can't override the driver on a broken driver/gpu
>>> combo, which is the only use case for having that module parameter.
>>> Currenty the disable_immediately flag overrides the users override ->
>>> Ouch.
>>>
>>> So in drm_vblank_put():
>>>
>>> ...
>>>
>>> /* Last user schedules interrupt disable */
>>> if (atomic_dec_and_test(&vblank->refcount)) {
>> Insert zero -> opt-out check <<<
>>>if (drm_vblank_offdelay == 0)
>>>return;
>> Remaining code continues <<<
>>>if (dev->vblank_disable_immediate || drm_vblank_offdelay < 0)
>>>vblank_disable_fn((unsigned long)vblank);
>>>else if (drm_vblank_offdelay > 0)
>>>mod_timer(&vblank->disable_timer, jiffies +
>>> ((drm_vblank_offdelay * HZ)/1000));
>>
>> Yeah, I guess that makes sense. I'm not really a fan of giving users
>> too powerful module options to hack around driver bugs since often
>> that means they'll never report the bug :( But we have the support now
>> to mark certain module options as debug-only and they'll taint the
>> kernel if set, so this is fixable.
>>
>> I'll follow up with the patch you've suggested.
>>
>
> Thanks. I think the modules parameters i usually care about will get
> proper testing and reporting, because while my software and users are
> good at detecting such problems, they wouldn't know how to fix them
> themselves, and at the same time they crucially depend on this stuff
> working, so this gets reported to me quickly and i can give them the
> module param workaround in private e-mail and take it from there with
> proper bug reports or patches.
>
>>> ...
>>>
>>> 2. For the "drm: Have the vblank counter account for the time ... "
>>> patch, we must opt-out of that last timestamp/counter update/bump if
>>> the driver doesn't support high-precision vblank timestamping,
>>> otherwise the vblank count and timestamp will be inconsistent with
>>> each other - or outright wrong in case of the timestamp. Rather
>>> deliver a slightly outdated, but correct count+timestamp pair to
>>> userspace, which is still useable for practical purposes, than a pair
>>> that's outright wrong and will definitely confuse clients.
>>>
>>> A simple fix in static void vblank_disable_and_save() would be to
>>> replace the new...
>>>
>>> if (!vblank->enabled) {
>>>
>>> ... check by ...
>>>
>>> if (!vblank->enabled &&
>>> ) {
>>
>> Yeah, makes sense (well the follow-up one ofc). I'll do a patch which
>> adds this and adds a comment. Aside I think it would be useful to add
>> a #define for the 0 return value, since the magic checks all over are
>> imo fairly hard to understand.
>>
>> I'll also float a patch for rfc about that.
>>
>
> Good!
>
> thanks,
> -mario
>
>> Thanks for your comments and again my apologies for missing that
>> there's still outstanding work left to do on this.
>>
>> Cheers, Daniel
>>
>>>
>>>
>>> On Wed, Sep 10, 2014 at 2:05 PM, Daniel Vetter
>>> wrote:
Hi Dave,
So here's the final bits of Ville's vblank rework with a bit of cleanup
from Mario on top.
The neat thing this finally allows is to immediately disable the vblank
interrupt on the last drm_vblank_put if the hardware has perfectly
accurate vblank counter and timestamp readout support. On i915 that
required piles of small adjustements from Ville since depending upon the
platform and port the vblank happens at different scanout lines.
Of course this is fully opt-in and per-device (we need that since gen2
doesn't have a hw vblank counter).
Mario reviewed the entire pile too and
[pull] drm/msm: msm-next for 3.18
On Wed, Sep 10, 2014 at 11:23:49AM -0400, Rob Clark wrote: > Hi Dave, main pull for 3.18: > > 1) add LVDS support for mdp4 (tested with auo B101XTN01.0 panel) > 2) add B101XTN01.0 panel > 3) bit of gpu refactoring to prepare for addition of addition gpu > generations beyond just a3xx > > Thierry, I wasn't quite sure if I should include the panel patch or if > you were going to send that.. it is easy enough for me to drop that > one patch and resend the pull if that is preferred. It's the only patch I had applied to the drm/panel tree and I'm currently on paternal leave until around v3.17-rc6 so won't get around to apply much more before the deadline, so I'm fine with Dave picking it up via your pull request. I'll drop the patch from my tree to avoid a conflict in linux-next. Thierry -- next part -- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/722b7bb5/attachment.sig>
[PATCH RFC 4/4] drm: link connectors to backlight devices
One extreme case - apple_gmux needs to be mapped to both the internal and discrete gpu. The same may be true for some other platform drivers on multi-gpu systems. Matthew Garrett | matthew.garrett at nebula.com -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/66ba7771/attachment.html>
[PATCH RFC 3/4] backlight: add kernel-internal backlight API
On Wed, Sep 10, 2014 at 05:54:22PM +0200, David Herrmann wrote:
[...]
> +void backlight_set_brightness(struct backlight_device *bd, unsigned int
> value,
> + enum backlight_update_reason reason)
> +{
> + mutex_lock(&bd->ops_lock);
> + if (bd->ops) {
> + value = clamp(value, 0U, (unsigned)bd->props.max_brightness);
max_brightness should really be unsigned to begin with...
> + pr_debug("set brightness to %u\n", value);
dev_dbg(&bd->dev, ...)?
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index adb14a8..bcc0dec 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -141,6 +141,22 @@ extern bool backlight_device_registered(enum
> backlight_type type);
> extern int backlight_register_notifier(struct notifier_block *nb);
> extern int backlight_unregister_notifier(struct notifier_block *nb);
>
> +struct backlight_device *backlight_device_lookup(const char *name);
> +void backlight_set_brightness(struct backlight_device *bd, unsigned int
> value,
> + enum backlight_update_reason reason);
> +
> +static inline void backlight_device_ref(struct backlight_device *bd)
> +{
> + if (bd)
> + get_device(&bd->dev);
> +}
Perhaps for consistency with get_device() this should return bd? That
way you can chain things like so:
priv->backlight = backlight_device_ref(bd);
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/661a9ada/attachment-0001.sig>
[PATCH RFC 3/4] backlight: add kernel-internal backlight API
Hi
On Thu, Sep 11, 2014 at 1:10 PM, Thierry Reding
wrote:
> On Wed, Sep 10, 2014 at 05:54:22PM +0200, David Herrmann wrote:
> [...]
>> +void backlight_set_brightness(struct backlight_device *bd, unsigned int
>> value,
>> + enum backlight_update_reason reason)
>> +{
>> + mutex_lock(&bd->ops_lock);
>> + if (bd->ops) {
>> + value = clamp(value, 0U, (unsigned)bd->props.max_brightness);
>
> max_brightness should really be unsigned to begin with...
>
>> + pr_debug("set brightness to %u\n", value);
>
> dev_dbg(&bd->dev, ...)?
I agree with both comments, but I tried to be consistent with what
brightness_store() does.
>> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
>> index adb14a8..bcc0dec 100644
>> --- a/include/linux/backlight.h
>> +++ b/include/linux/backlight.h
>> @@ -141,6 +141,22 @@ extern bool backlight_device_registered(enum
>> backlight_type type);
>> extern int backlight_register_notifier(struct notifier_block *nb);
>> extern int backlight_unregister_notifier(struct notifier_block *nb);
>>
>> +struct backlight_device *backlight_device_lookup(const char *name);
>> +void backlight_set_brightness(struct backlight_device *bd, unsigned int
>> value,
>> + enum backlight_update_reason reason);
>> +
>> +static inline void backlight_device_ref(struct backlight_device *bd)
>> +{
>> + if (bd)
>> + get_device(&bd->dev);
>> +}
>
> Perhaps for consistency with get_device() this should return bd? That
> way you can chain things like so:
>
> priv->backlight = backlight_device_ref(bd);
Makes sense, will change it. Same is actually true for _unref(), which
should return NULL unconditionally. This way, you can use:
priv->backlight = backlight_device_unref(priv->backlight);
to release a reference and reset the pointer at the same time.
Thanks
David
[PATCH RFC 3/4] backlight: add kernel-internal backlight API
On Thu, Sep 11, 2014 at 01:14:31PM +0200, David Herrmann wrote:
> Hi
>
> On Thu, Sep 11, 2014 at 1:10 PM, Thierry Reding
> wrote:
> > On Wed, Sep 10, 2014 at 05:54:22PM +0200, David Herrmann wrote:
> > [...]
> >> +void backlight_set_brightness(struct backlight_device *bd, unsigned int
> >> value,
> >> + enum backlight_update_reason reason)
> >> +{
> >> + mutex_lock(&bd->ops_lock);
> >> + if (bd->ops) {
> >> + value = clamp(value, 0U, (unsigned)bd->props.max_brightness);
> >
> > max_brightness should really be unsigned to begin with...
> >
> >> + pr_debug("set brightness to %u\n", value);
> >
> > dev_dbg(&bd->dev, ...)?
>
> I agree with both comments, but I tried to be consistent with what
> brightness_store() does.
Fair enough, this can be cleaned up in separate patches.
> >> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> >> index adb14a8..bcc0dec 100644
> >> --- a/include/linux/backlight.h
> >> +++ b/include/linux/backlight.h
> >> @@ -141,6 +141,22 @@ extern bool backlight_device_registered(enum
> >> backlight_type type);
> >> extern int backlight_register_notifier(struct notifier_block *nb);
> >> extern int backlight_unregister_notifier(struct notifier_block *nb);
> >>
> >> +struct backlight_device *backlight_device_lookup(const char *name);
> >> +void backlight_set_brightness(struct backlight_device *bd, unsigned int
> >> value,
> >> + enum backlight_update_reason reason);
> >> +
> >> +static inline void backlight_device_ref(struct backlight_device *bd)
> >> +{
> >> + if (bd)
> >> + get_device(&bd->dev);
> >> +}
> >
> > Perhaps for consistency with get_device() this should return bd? That
> > way you can chain things like so:
> >
> > priv->backlight = backlight_device_ref(bd);
>
> Makes sense, will change it. Same is actually true for _unref(), which
> should return NULL unconditionally. This way, you can use:
> priv->backlight = backlight_device_unref(priv->backlight);
> to release a reference and reset the pointer at the same time.
That looks somewhat odd to me. Wouldn't priv->backlight typically go
away after the unref anyway (presumably because priv is going to get
freed soon after)?
But I have no strong objections to returning NULL from _unref(), if code
doesn't need it it can always choose not to use the return value.
Thierry
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/b882b62b/attachment.sig>
[PATCH 3/4] drm: Simplify return value of drm_get_last_vbltimestamp
On 09/10/2014 05:36 PM, Daniel Vetter wrote:
> Imo u32 hints at a register value, but in reality all callers only
> care whether the sampled timestamp is precise or not. So give them
> just a bool.
>
> Also move the declaration out of drmP.h, it's only used in drm_irq.c.
All good. Maybe then also remove
EXPORT_SYMBOL(drm_get_last_vbltimestamp);
in this patch if the method is now static to drm_irq.c ? Up to you.
For all 4 patches...
Reviewed-by: Mario Kleiner
-mario
> Cc: Mario Kleiner
> Cc: Ville Syrj?l?
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_irq.c | 24 +++-
> include/drm/drmP.h| 2 --
> 2 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 922721ead29a..b16f0bcef959 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -70,6 +70,10 @@ module_param_named(vblankoffdelay, drm_vblank_offdelay,
> int, 0600);
> module_param_named(timestamp_precision_usec, drm_timestamp_precision, int,
> 0600);
> module_param_named(timestamp_monotonic, drm_timestamp_monotonic, int, 0600);
>
> +static bool
> +drm_get_last_vbltimestamp(struct drm_device *dev, int crtc,
> + struct timeval *tvblank, unsigned flags);
> +
> /**
>* drm_update_vblank_count - update the master vblank counter
>* @dev: DRM device
> @@ -89,7 +93,8 @@ module_param_named(timestamp_monotonic,
> drm_timestamp_monotonic, int, 0600);
> static void drm_update_vblank_count(struct drm_device *dev, int crtc)
> {
> struct drm_vblank_crtc *vblank = &dev->vblank[crtc];
> - u32 cur_vblank, diff, tslot, rc;
> + u32 cur_vblank, diff, tslot;
> + bool rc;
> struct timeval t_vblank;
>
> /*
> @@ -147,7 +152,7 @@ static void vblank_disable_and_save(struct drm_device
> *dev, int crtc)
> unsigned long irqflags;
> u32 vblcount;
> s64 diff_ns;
> - int vblrc;
> + bool vblrc;
> struct timeval tvblank;
> int count = DRM_TIMESTAMP_MAXRETRIES;
>
> @@ -171,7 +176,7 @@ static void vblank_disable_and_save(struct drm_device
> *dev, int crtc)
>* vblank interrupt is disabled.
>*/
> if (!vblank->enabled &&
> - drm_get_last_vbltimestamp(dev, crtc, &tvblank, 0) > 0) {
> + drm_get_last_vbltimestamp(dev, crtc, &tvblank, 0)) {
> drm_update_vblank_count(dev, crtc);
> spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
> return;
> @@ -219,7 +224,7 @@ static void vblank_disable_and_save(struct drm_device
> *dev, int crtc)
>* available. In that case we can't account for this and just
>* hope for the best.
>*/
> - if ((vblrc > 0) && (abs64(diff_ns) > 100)) {
> + if (vblrc && (abs64(diff_ns) > 100)) {
> /* Store new timestamp in ringbuffer. */
> vblanktimestamp(dev, crtc, vblcount + 1) = tvblank;
>
> @@ -786,10 +791,11 @@ static struct timeval get_drm_timestamp(void)
>* call, i.e., it isn't very precisely locked to the true vblank.
>*
>* Returns:
> - * Non-zero if timestamp is considered to be very precise, zero otherwise.
> + * True if timestamp is considered to be very precise, false otherwise.
>*/
> -u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc,
> - struct timeval *tvblank, unsigned flags)
> +static bool
> +drm_get_last_vbltimestamp(struct drm_device *dev, int crtc,
> + struct timeval *tvblank, unsigned flags)
> {
> int ret;
>
> @@ -801,7 +807,7 @@ u32 drm_get_last_vbltimestamp(struct drm_device *dev, int
> crtc,
> ret = dev->driver->get_vblank_timestamp(dev, crtc, &max_error,
> tvblank, flags);
> if (ret > 0)
> - return (u32) ret;
> + return true;
> }
>
> /* GPU high precision timestamp query unsupported or failed.
> @@ -809,7 +815,7 @@ u32 drm_get_last_vbltimestamp(struct drm_device *dev, int
> crtc,
>*/
> *tvblank = get_drm_timestamp();
>
> - return 0;
> + return false;
> }
> EXPORT_SYMBOL(drm_get_last_vbltimestamp);
>
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index ad952b08711e..2ccb0e715569 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1004,8 +1004,6 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
> extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
> extern void drm_vblank_cleanup(struct drm_device *dev);
>
> -extern u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc,
> - struct timeval *tvblank, unsigned flags);
> extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>int crtc, int *max_error,
>
[PATCH 3/4] drm: Simplify return value of drm_get_last_vbltimestamp
On Thu, Sep 11, 2014 at 1:28 PM, Mario Kleiner wrote: > On 09/10/2014 05:36 PM, Daniel Vetter wrote: >> >> Imo u32 hints at a register value, but in reality all callers only >> care whether the sampled timestamp is precise or not. So give them >> just a bool. >> >> Also move the declaration out of drmP.h, it's only used in drm_irq.c. > > > All good. Maybe then also remove > > EXPORT_SYMBOL(drm_get_last_vbltimestamp); Oh, I've missed that one when grepping. Will fix when applying. > in this patch if the method is now static to drm_irq.c ? Up to you. > > For all 4 patches... > > Reviewed-by: Mario Kleiner Thanks a lot, I plan to send the pull request to Dave tomorrow. Presuming nothing else fails meanwhile ;-)
Shareable bufmgr objects
Hi there, Here is a small modification I had to make to get buffers shared between Mesa and LibVA on Chrome OS. This is required to have refcounting properly between the 2 API otherwise, Mesa might end up calling exit() when the kernel tells it that one of the buffer object used in a batch buffer is invalid : http://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/drivers/dri/i965/intel_batchbuffer.c#n282 Thanks, - Lionel
[PATCH] intel: make bufmgr_gem shareable from different API
When using Mesa and LibVA in the same process, one would like to be
able bind buffers from the output of the decoder to a GL texture
through an EGLImage.
LibVA can reuse buffers allocated by Gbm through a file descriptor. It
will then wrap it into a drm_intel_bo with
drm_intel_bo_gem_create_from_prime().
Given both libraries are using libdrm to allocate and use buffer
objects, there is a need to have the buffer objects properly
refcounted. That is possible if both API use the same drm_intel_bo
objects, but that also requires that both API use the same
drm_intel_bufmgr object.
This patch modifies drm_intel_bufmgr_gem_init() so given a file
descriptor, it will look for an already existing drm_intel_bufmgr
using the same file descriptor and return that object.
Signed-off-by: Lionel Landwerlin
---
intel/intel_bufmgr_gem.c | 100 +--
1 file changed, 88 insertions(+), 12 deletions(-)
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 0e1cb0d..125c81c 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -94,6 +94,8 @@ struct drm_intel_gem_bo_bucket {
typedef struct _drm_intel_bufmgr_gem {
drm_intel_bufmgr bufmgr;
+ atomic_t refcount;
+
int fd;
int max_relocs;
@@ -3186,6 +3188,85 @@ drm_intel_bufmgr_gem_set_aub_annotations(drm_intel_bo
*bo,
bo_gem->aub_annotation_count = count;
}
+static pthread_mutex_t bufmgr_list_mutex = PTHREAD_MUTEX_INITIALIZER;
+static drm_intel_bufmgr_gem **bufmgr_list = NULL;
+static unsigned bufmgr_list_size = 0, bufmgr_list_nb;
+
+static drm_intel_bufmgr_gem *
+drm_intel_bufmgr_gem_find_or_create_for_fd(int fd, int *found)
+{
+drm_intel_bufmgr_gem *bufmgr_gem;
+
+assert(pthread_mutex_lock(&bufmgr_list_mutex) == 0);
+
+if (bufmgr_list == NULL) {
+bufmgr_list_size = 2;
+bufmgr_list = calloc(bufmgr_list_size,
sizeof(drm_intel_bufmgr_gem *));
+} else {
+unsigned i;
+for (i = 0; i < bufmgr_list_nb; i++) {
+bufmgr_gem = bufmgr_list[i];
+if (bufmgr_gem->fd == fd) {
+atomic_inc(&bufmgr_gem->refcount);
+*found = 1;
+goto exit;
+}
+}
+}
+
+bufmgr_gem = calloc(1, sizeof(*bufmgr_gem));
+if (bufmgr_gem == NULL)
+goto exit;
+
+bufmgr_gem->fd = fd;
+atomic_set(&bufmgr_gem->refcount, 1);
+
+ assert(pthread_mutex_init(&bufmgr_gem->lock, NULL) == 0);
+
+if (bufmgr_list_nb >= bufmgr_list_size) {
+bufmgr_list_size *= 2;
+bufmgr_list = realloc(bufmgr_list, bufmgr_list_size);
+assert(bufmgr_list != NULL);
+}
+bufmgr_list[bufmgr_list_nb] = bufmgr_gem;
+bufmgr_list_nb++;
+
+pthread_mutex_lock(&bufmgr_gem->lock);
+
+*found = 0;
+
+exit:
+pthread_mutex_unlock(&bufmgr_list_mutex);
+
+return bufmgr_gem;
+}
+
+static void
+drm_intel_bufmgr_gem_unref (drm_intel_bufmgr *bufmgr)
+{
+drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr;
+
+if (atomic_dec_and_test(&bufmgr_gem->refcount)) {
+unsigned i, compact_start = bufmgr_list_nb;
+
+assert(pthread_mutex_lock(&bufmgr_list_mutex) == 0);
+
+for (i = 0; i < bufmgr_list_nb; i++) {
+if (bufmgr_list[i] == bufmgr_gem) {
+compact_start = i;
+bufmgr_list_nb--;
+break;
+}
+}
+for (i = compact_start; i < bufmgr_list_nb; i++)
+bufmgr_list[i] = bufmgr_list[i + 1];
+
+pthread_mutex_unlock(&bufmgr_list_mutex);
+
+drm_intel_bufmgr_gem_destroy(bufmgr);
+}
+}
+
/**
* Initializes the GEM buffer manager, which uses the kernel to allocate, map,
* and manage map buffer objections.
@@ -3201,16 +3282,9 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
int ret, tmp;
bool exec2 = false;
- bufmgr_gem = calloc(1, sizeof(*bufmgr_gem));
- if (bufmgr_gem == NULL)
- return NULL;
-
- bufmgr_gem->fd = fd;
-
- if (pthread_mutex_init(&bufmgr_gem->lock, NULL) != 0) {
- free(bufmgr_gem);
- return NULL;
- }
+bufmgr_gem = drm_intel_bufmgr_gem_find_or_create_for_fd(fd, &ret);
+ if (bufmgr_gem && ret)
+ return &bufmgr_gem->bufmgr;
ret = drmIoctl(bufmgr_gem->fd,
DRM_IOCTL_I915_GEM_GET_APERTURE,
@@ -3245,7 +3319,7 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
else if (IS_GEN8(bufmgr_gem->pci_device))
bufmgr_gem->gen = 8;
else {
-
[PATCH] intel: make bufmgr_gem shareable from different API
On Thu, Sep 11, 2014 at 12:33:41PM +0100, Lionel Landwerlin wrote:
> When using Mesa and LibVA in the same process, one would like to be
> able bind buffers from the output of the decoder to a GL texture
> through an EGLImage.
>
> LibVA can reuse buffers allocated by Gbm through a file descriptor. It
> will then wrap it into a drm_intel_bo with
> drm_intel_bo_gem_create_from_prime().
>
> Given both libraries are using libdrm to allocate and use buffer
> objects, there is a need to have the buffer objects properly
> refcounted. That is possible if both API use the same drm_intel_bo
> objects, but that also requires that both API use the same
> drm_intel_bufmgr object.
The description is wrong though. Reusing buffers export and import
through a dmabuf, should work and be correctly refcounted already.
This patch adds the ability to use the same /dev/dri/card0 device fd
between two libraries. This implies that they share the same context and
address space, which is probably not what you want, but nevertheless
seems sensible if they are sharing the device fd in the first place.
I suspect this may break unwary users such as igt, which would fork
after creating a bufmgr, close the fds, but then open their own device
fd with the same fd as before. Not a huge issue, just something to check
in case it causes some fun fallout.
> This patch modifies drm_intel_bufmgr_gem_init() so given a file
> descriptor, it will look for an already existing drm_intel_bufmgr
> using the same file descriptor and return that object.
>
> Signed-off-by: Lionel Landwerlin
> ---
> intel/intel_bufmgr_gem.c | 100
> +--
> 1 file changed, 88 insertions(+), 12 deletions(-)
>
> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> index 0e1cb0d..125c81c 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -94,6 +94,8 @@ struct drm_intel_gem_bo_bucket {
> typedef struct _drm_intel_bufmgr_gem {
> drm_intel_bufmgr bufmgr;
>
> + atomic_t refcount;
> +
> int fd;
>
> int max_relocs;
> @@ -3186,6 +3188,85 @@ drm_intel_bufmgr_gem_set_aub_annotations(drm_intel_bo
> *bo,
> bo_gem->aub_annotation_count = count;
> }
>
> +static pthread_mutex_t bufmgr_list_mutex = PTHREAD_MUTEX_INITIALIZER;
> +static drm_intel_bufmgr_gem **bufmgr_list = NULL;
> +static unsigned bufmgr_list_size = 0, bufmgr_list_nb;
> +
> +static drm_intel_bufmgr_gem *
> +drm_intel_bufmgr_gem_find_or_create_for_fd(int fd, int *found)
> +{
> +drm_intel_bufmgr_gem *bufmgr_gem;
> +
> +assert(pthread_mutex_lock(&bufmgr_list_mutex) == 0);
> +
> +if (bufmgr_list == NULL) {
Just use an embedded list rather than array, that would greatly simplify
the search, cration and deletion.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
[PATCH] intel: make bufmgr_gem shareable from different API
Hi, On 11 September 2014 12:52, Chris Wilson wrote: > On Thu, Sep 11, 2014 at 12:33:41PM +0100, Lionel Landwerlin wrote: > > When using Mesa and LibVA in the same process, one would like to be > > able bind buffers from the output of the decoder to a GL texture > > through an EGLImage. > > > > LibVA can reuse buffers allocated by Gbm through a file descriptor. It > > will then wrap it into a drm_intel_bo with > > drm_intel_bo_gem_create_from_prime(). > > > > Given both libraries are using libdrm to allocate and use buffer > > objects, there is a need to have the buffer objects properly > > refcounted. That is possible if both API use the same drm_intel_bo > > objects, but that also requires that both API use the same > > drm_intel_bufmgr object. > > The description is wrong though. Reusing buffers export and import > through a dmabuf, should work and be correctly refcounted already. > Indeed. I've been using the attached patch to deal with the case where we have two EGLDisplays/DRIscreens that can share DRIimage objects (long story, and a much more ugly patch), and it works perfectly. The cover letter's description is right though, in that you get a cryptic message thanks to relocation having been totally skipped when you submit objects from a foreign bufmgr. Cheers, Daniel -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/6280b1ce/attachment.html> -- next part -- A non-text attachment was scrubbed... Name: mesa-intel-foreign-bufmgr.patch Type: text/x-patch Size: 2783 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/6280b1ce/attachment.bin>
[PATCH] intel: make bufmgr_gem shareable from different API
On 11/09/14 12:52, Chris Wilson wrote:
> On Thu, Sep 11, 2014 at 12:33:41PM +0100, Lionel Landwerlin wrote:
>> When using Mesa and LibVA in the same process, one would like to be
>> able bind buffers from the output of the decoder to a GL texture
>> through an EGLImage.
>>
>> LibVA can reuse buffers allocated by Gbm through a file descriptor. It
>> will then wrap it into a drm_intel_bo with
>> drm_intel_bo_gem_create_from_prime().
>>
>> Given both libraries are using libdrm to allocate and use buffer
>> objects, there is a need to have the buffer objects properly
>> refcounted. That is possible if both API use the same drm_intel_bo
>> objects, but that also requires that both API use the same
>> drm_intel_bufmgr object.
> The description is wrong though. Reusing buffers export and import
> through a dmabuf, should work and be correctly refcounted already.
>
> This patch adds the ability to use the same /dev/dri/card0 device fd
> between two libraries. This implies that they share the same context and
> address space, which is probably not what you want, but nevertheless
> seems sensible if they are sharing the device fd in the first place.
That's what I meant, sorry if it was unclear.
>
> I suspect this may break unwary users such as igt, which would fork
> after creating a bufmgr, close the fds, but then open their own device
> fd with the same fd as before. Not a huge issue, just something to check
> in case it causes some fun fallout.
Will have a look, thanks.
>
>> This patch modifies drm_intel_bufmgr_gem_init() so given a file
>> descriptor, it will look for an already existing drm_intel_bufmgr
>> using the same file descriptor and return that object.
>>
>> Signed-off-by: Lionel Landwerlin
>> ---
>> intel/intel_bufmgr_gem.c | 100
>> +--
>> 1 file changed, 88 insertions(+), 12 deletions(-)
>>
>> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
>> index 0e1cb0d..125c81c 100644
>> --- a/intel/intel_bufmgr_gem.c
>> +++ b/intel/intel_bufmgr_gem.c
>> @@ -94,6 +94,8 @@ struct drm_intel_gem_bo_bucket {
>> typedef struct _drm_intel_bufmgr_gem {
>> drm_intel_bufmgr bufmgr;
>>
>> +atomic_t refcount;
>> +
>> int fd;
>>
>> int max_relocs;
>> @@ -3186,6 +3188,85 @@ drm_intel_bufmgr_gem_set_aub_annotations(drm_intel_bo
>> *bo,
>> bo_gem->aub_annotation_count = count;
>> }
>>
>> +static pthread_mutex_t bufmgr_list_mutex = PTHREAD_MUTEX_INITIALIZER;
>> +static drm_intel_bufmgr_gem **bufmgr_list = NULL;
>> +static unsigned bufmgr_list_size = 0, bufmgr_list_nb;
>> +
>> +static drm_intel_bufmgr_gem *
>> +drm_intel_bufmgr_gem_find_or_create_for_fd(int fd, int *found)
>> +{
>> +drm_intel_bufmgr_gem *bufmgr_gem;
>> +
>> +assert(pthread_mutex_lock(&bufmgr_list_mutex) == 0);
>> +
>> +if (bufmgr_list == NULL) {
> Just use an embedded list rather than array, that would greatly simplify
> the search, cration and deletion.
> -Chris
>
I tried to use the embedded list, but from my understanding I need the
embedded structure at the top of the bufmgr struct. Is that possible?
Sounds like an ABI break.
Thanks,
-
Lionel
[PATCH RFC 4/4] drm: link connectors to backlight devices
Hi On Thu, Sep 11, 2014 at 8:48 AM, Daniel Vetter wrote: > Nice you skid around all the pitfalls and trapdoors, I guess we've all > been rather blind ;-) > > Two high-level comments: > - We also want to forward "bl_power". cros was totally not happy when we > stopped treating brightness == 0 as completely off (it upsets some > panels terminally, so there's a vbt lower limit). Instead we expose this > now through the bl_power knob. > > While at it I think we should expose all the other backlight properties > too (read-only ofc for actual/max_brightness). bl_power is easy to add. I guess v2 will have: "BACKLIGHT-POWER" (range 0-4) actual-brightness is a bit more tricky. Currently, DRM caches property values, so there is no read_property() hook. We'd have to add this. But it'll be quite nasty as we have to call into the backlight driver. So I think we want to run an async-interruptible worker on the backlight, drop the locks in the ioctl and wait for the job to finish. Not sure whether it's worth it.. maybe we can add this later. > - How does udev match on the drm connector name? They are not terribly > stable atm, and if you reload your drm driver, or much more likely, have > two gpus with two drm drivers they change. We probably should change the > name allocation scheme to be per device instance instead of global > first. Within a driver probe order is hopefully deterministic on a given > platform, since even with super dynamic setups (based on dt/acpi) the > firmware tables should change really. You can match on EDID attributes. Ok, so far this is pretty ugly as the EDID property is binary. But we can add rather trivial udev extensions to make EDID binary against text matching possible. While we're at it, I don't really like the brightness-value re-scaling. I currently expose BRIGHTNESS as rang 0-65535 and scale it to the backlight range. This works perfectly well as the backlight is usually a really small range, but it would be much simpler if we could expose the real range. However, this would require DRM property hotplugging. This is currently not supported by DRM.. and it would require multiple different properties for each connector as each might have a different range. But then, we have to suffix the name as we cannot have multiple properties with the same name.. Eh.. re-scaling doesn't sound that bad, does it? Ok, we could expose a separate property called MAX-BRIGHTNESS and drivers simply ignore the range-bounds of the BRIGHTNESS value and use MAX-BRIGHTNESS instead? Sounds ok'ish. Thanks David
[PATCH] intel: make bufmgr_gem shareable from different API
On Thu, Sep 11, 2014 at 01:21:13PM +0100, Lionel Landwerlin wrote: > On 11/09/14 12:52, Chris Wilson wrote: > >Just use an embedded list rather than array, that would greatly simplify > >the search, cration and deletion. > > I tried to use the embedded list, but from my understanding I need > the embedded structure at the top of the bufmgr struct. Is that > possible? Sounds like an ABI break. The drmMMListHead allows embedding anywhere within the parent, and drm_intel_bufmgr_gem is opaque so can be freely extended. -Chris -- Chris Wilson, Intel Open Source Technology Centre
[PATCH RFC 4/4] drm: link connectors to backlight devices
On Thu, 11 Sep 2014, Daniel Vetter wrote: > On Wed, Sep 10, 2014 at 05:54:23PM +0200, David Herrmann wrote: >> Backlight devices have always been managed independently of display >> controllers. They're often controlled via different hardware interfaces >> and their relationship to display-controllers varies vastly between >> different boards. However, display brightness is obviously a property of >> a display, and thus of a DRM connector. Therefore, it'd be really >> appreciated if user-space APIs would highlight this relationship. >> >> The main runtime users of backlight interfaces are user-space compositors. >> But currently they have to jump through hoops to find the correct >> backlight device for a given connector. Furthermore, they need root >> privileges to write to sysfs. sysfs has never been designed as run-time >> non-root API. It does not provide file-contexts, run-time management or >> any kind of API control. There is no way to control access to sysfs via >> different links (in that case: mounts). Char-devs provide all this! >> >> So far, backlight APIs have been fairly trivial, so adding char-devs to >> backlights is rather heavy-weight. Therefore, this patch introduces a new >> API interface to modify backlight brightness via DRM: A "BRIGHTNESS" >> property on DRM connectors. >> >> Instead of adding backlight hardware support to DRM, we rely on the >> backlight-class and simply add a new API. Each DRM Connector can >> optionally be linked to a backlight class device. Modifying the connector >> property will have the same effect as writing into the "brightness" sysfs >> file of the linked backlight class device. However, we now can manage >> access to backlight devices via the same interface as access to >> mode-setting on the underlying display. Furthermore, the connection >> between displays and their backlight devices are visible in user-space. >> >> Obviously, matching backlights to displays cannot be solved magically >> with this link. Therefore, we also add a user-space attribute to DRM >> connectors called 'backlight'. If a DRM driver is incapable of matching >> existing backlights to a connector, or if a given board has just crappy >> backlight drivers, udev can write the name of a backlight-device into this >> attribute and the connector-property will be re-linked to this backlight >> device. The udev hwdb can be easily employed to track such quirks and >> fixups for different board+GPU combinations. >> Note that the name written into the 'backlight' attribute is saved on the >> connector, so in case the real backlight device is probed after the DRM >> card, the backlight will still get properly attached once probed. >> >> Signed-off-by: David Herrmann > > Nice you skid around all the pitfalls and trapdoors, I guess we've all > been rather blind ;-) > > Two high-level comments: > - We also want to forward "bl_power". cros was totally not happy when we > stopped treating brightness == 0 as completely off (it upsets some > panels terminally, so there's a vbt lower limit). Instead we expose this > now through the bl_power knob. Part of the reason was that their backlight handling userspace only uses the sysfs interface, not drm, and thus doing dpms to switch the display off would be more work. (And slow, but that's another matter.) OTOH if you are already frobbing the connector, it's easy to do dpms, right? (Side note, another issue with using brightness == 0 for a kind of easy dpms is that, at least in theory, there are displays that work with ambient light when the backlight is off. So it doesn't really switch the display off anyway. Don't know if such displays are common though.) > While at it I think we should expose all the other backlight properties > too (read-only ofc for actual/max_brightness). The trouble here, and I think also the reason David chose to use range 0..U16_MAX for the backlight property, is the change that occurs when the connector-backlight link gets changed. If we expose max, and deal with the problems, then that doesn't need to be a separate property, just the max value for the brightness property. > - How does udev match on the drm connector name? They are not terribly > stable atm, and if you reload your drm driver, or much more likely, have > two gpus with two drm drivers they change. We probably should change the > name allocation scheme to be per device instance instead of global > first. Within a driver probe order is hopefully deterministic on a given > platform, since even with super dynamic setups (based on dt/acpi) the > firmware tables should change really. Are the backlight sysfs names stable, are acpi_backlightN always enumerated in the same order? BR, Jani. > > Cheers, Daniel >> --- >> drivers/gpu/drm/Kconfig | 1 + >> drivers/gpu/drm/Makefile| 2 +- >> drivers/gpu/drm/drm_backlight.c | 387 >> >> drivers/gpu/drm/drm_crtc.c | 4
[PATCH RFC 0/4] Linking DRM Connectors to Backlight Devices
Hi On Wed, Sep 10, 2014 at 10:40 PM, Matthew Garrett wrote: > On Wed, 2014-09-10 at 17:54 +0200, David Herrmann wrote: > >> * User-space currently has a hard-time figuring out which backlight device >> to >>use, and which backlight device belongs to which display. So far, most >>systems only provide backlight-devices for internal displays, so figuring >> out >>the connection is easy, but that might change with more capable external >>connectors. > > The parent device of the backlight will be the correct display, if the > kernel has a meaningful way to determine that. We could do a better job > in the ACPI code than we currently do, but (unfortunately) that requires > us to know the ACPI IDs that each GPU vendor uses. We also probe ACPI devices independently of PCI devices (or other buses). So the actual DRM device might be created much later than the backlight, thus it cannot be a parent of the backlight. We can try to find a common ancestor, though. >> This series tries to solve this problem with a much simpler approach: >> Instead of moving backlights into DRM, we simply link DRM properties to a >> backlight device. That is, the kernel manages a link between a connector and >> a >> backlight device (or n backlight devices) which can be modified by udev in >> case >> the kernel got it wrong (we don't want huge board-fixup-tables in the >> kernel). >> User-space can now use the simpl DRM API to manage backlights, and the kernel >> does not need any special driver code to make it work. > > This doesn't really simplify userspace significantly - something's still > going to have to make the same policy decision as we do right now, and > the kernel isn't really the right place to do that. This patch allows to add really simple udev rules that implement any policy we want. This way, we can keep the policy in user-space, but at the same time it's no longer part of the compositors. Instead, we have an independent place (udev rules) where to write that policy and tell the kernel. I think this is an improvement. But of course, the unprivileged access is the much more compelling argument. Thanks David
[PATCH v2 00/17] drm/exynos/ipp: image post processing fixes and improvements, part four
On 2014? 08? 28? 18:07, Andrzej Hajda wrote: > This set of patches contains various improvement and fixes > for exynos_drm ipp framework. > The patchset is based on exynos-drm-next branch. > > IPP framework was tested for regressions on exynos4210-trats target. > > In the 2nd version of the series I have included changes proposed by > Joonyoung Shim. > I have decided to resend whole series because the changes caused merge > conflicts and > two separate patches have been added to the series. > Changes are described in comit messages. Applied. Thanks, Inki Dae > > Regards > Andrzej > > > Andrzej Hajda (17): > drm/exynos/ipp: remove fake pm callbacks > drm/exynos/ipp: cancel works before command node clean > drm/exynos/ipp: move file reference from memory to command node > drm/exynos/ipp: remove only related commands on file close > drm/exynos/ipp: remove unused field in command node > drm/exynos/ipp: free partially allocated resources on error > drm/exynos/ipp: move nodes cleaning to separate function > drm/exynos/ipp: clean memory nodes on command node cleaning > drm/exynos/ipp: replace work_struct casting with better constructs > drm/exynos/ipp: stop hardware before freeing memory > drm/exynos/ipp: remove events during command cleaning > drm/exynos/fimc: avoid clearing overflow bits > drm/exynos/fimc: do not enable fimc twice > drm/exynos/fimc: simplify buffer queuing > drm/exynos/fimc: fix source buffer registers > drm/exynos/ipp: remove file argument from node related functions > drm/exynos/ipp: add file checks for ioctls > > drivers/gpu/drm/exynos/exynos_drm_fimc.c| 90 ++- > drivers/gpu/drm/exynos/exynos_drm_gsc.c | 3 +- > drivers/gpu/drm/exynos/exynos_drm_ipp.c | 397 > > drivers/gpu/drm/exynos/exynos_drm_ipp.h | 4 +- > drivers/gpu/drm/exynos/exynos_drm_rotator.c | 3 +- > 5 files changed, 195 insertions(+), 302 deletions(-) >
[PATCH v2] drm/exynos: update to use component match support
Update Exynos's DRM driver to use component match support rater than
add_components.
Changelog v2:
- release devices and drivers if failed.
- change compare_of to compare_dev.
Signed-off-by: Inki Dae
---
drivers/gpu/drm/exynos/exynos_drm_drv.c | 44 +++
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 5aae95c..3f6ec96 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -486,21 +486,20 @@ void exynos_drm_component_del(struct device *dev,
mutex_unlock(&drm_component_lock);
}
-static int compare_of(struct device *dev, void *data)
+static int compare_dev(struct device *dev, void *data)
{
return dev == (struct device *)data;
}
-static int exynos_drm_add_components(struct device *dev, struct master *m)
+static struct component_match *exynos_drm_match_add(struct device *dev)
{
+ struct component_match *match = NULL;
struct component_dev *cdev;
unsigned int attach_cnt = 0;
mutex_lock(&drm_component_lock);
list_for_each_entry(cdev, &drm_component_list, list) {
- int ret;
-
/*
* Add components to master only in case that crtc and
* encoder/connector device objects exist.
@@ -515,16 +514,10 @@ static int exynos_drm_add_components(struct device *dev,
struct master *m)
/*
* fimd and dpi modules have same device object so add
* only crtc device object in this case.
-*
-* TODO. if dpi module follows driver-model driver then
-* below codes can be removed.
*/
if (cdev->crtc_dev == cdev->conn_dev) {
- ret = component_master_add_child(m, compare_of,
- cdev->crtc_dev);
- if (ret < 0)
- return ret;
-
+ component_match_add(dev, &match, compare_dev,
+ cdev->crtc_dev);
goto out_lock;
}
@@ -534,11 +527,8 @@ static int exynos_drm_add_components(struct device *dev,
struct master *m)
* connector/encoder need pipe number of crtc when they
* are created.
*/
- ret = component_master_add_child(m, compare_of, cdev->crtc_dev);
- ret |= component_master_add_child(m, compare_of,
- cdev->conn_dev);
- if (ret < 0)
- return ret;
+ component_match_add(dev, &match, compare_dev, cdev->crtc_dev);
+ component_match_add(dev, &match, compare_dev, cdev->conn_dev);
out_lock:
mutex_lock(&drm_component_lock);
@@ -546,7 +536,7 @@ out_lock:
mutex_unlock(&drm_component_lock);
- return attach_cnt ? 0 : -ENODEV;
+ return attach_cnt ? match : ERR_PTR(-EPROBE_DEFER);
}
static int exynos_drm_bind(struct device *dev)
@@ -560,13 +550,13 @@ static void exynos_drm_unbind(struct device *dev)
}
static const struct component_master_ops exynos_drm_ops = {
- .add_components = exynos_drm_add_components,
.bind = exynos_drm_bind,
.unbind = exynos_drm_unbind,
};
static int exynos_drm_platform_probe(struct platform_device *pdev)
{
+ struct component_match *match;
int ret;
pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
@@ -633,13 +623,23 @@ static int exynos_drm_platform_probe(struct
platform_device *pdev)
goto err_unregister_ipp_drv;
#endif
- ret = component_master_add(&pdev->dev, &exynos_drm_ops);
+ match = exynos_drm_match_add(&pdev->dev);
+ if (IS_ERR(match)) {
+ ret = PTR_ERR(match);
+ goto err_unregister_resources;
+ }
+
+ ret = component_master_add_with_match(&pdev->dev, &exynos_drm_ops,
+ match);
if (ret < 0)
- DRM_DEBUG_KMS("re-tried by last sub driver probed later.\n");
+ goto err_unregister_resources;
- return 0;
+ return ret;
+
+err_unregister_resources:
#ifdef CONFIG_DRM_EXYNOS_IPP
+ exynos_platform_device_ipp_unregister();
err_unregister_ipp_drv:
platform_driver_unregister(&ipp_driver);
err_unregister_gsc_drv:
--
1.7.9.5
[Bug 82551] monitor resolution wrongly set when using kernels > 3.13
https://bugzilla.kernel.org/show_bug.cgi?id=82551 --- Comment #11 from Alex Deucher --- You'll have to ask ubuntu if they have added any special changes. I'm not able to reproduce any display problems. -- You are receiving this mail because: You are watching the assignee of the bug.
[PATCH RFC 4/4] drm: link connectors to backlight devices
On Thu, Sep 11, 2014 at 02:22:55PM +0200, David Herrmann wrote: > Hi > > On Thu, Sep 11, 2014 at 8:48 AM, Daniel Vetter wrote: > > Nice you skid around all the pitfalls and trapdoors, I guess we've all > > been rather blind ;-) > > > > Two high-level comments: > > - We also want to forward "bl_power". cros was totally not happy when we > > stopped treating brightness == 0 as completely off (it upsets some > > panels terminally, so there's a vbt lower limit). Instead we expose this > > now through the bl_power knob. > > > > While at it I think we should expose all the other backlight properties > > too (read-only ofc for actual/max_brightness). > > bl_power is easy to add. I guess v2 will have: > "BACKLIGHT-POWER" (range 0-4) > > actual-brightness is a bit more tricky. Currently, DRM caches property > values, so there is no read_property() hook. We'd have to add this. > But it'll be quite nasty as we have to call into the backlight driver. > So I think we want to run an async-interruptible worker on the > backlight, drop the locks in the ioctl and wait for the job to finish. > Not sure whether it's worth it.. maybe we can add this later. See Jani's reply - we probably don't need it, at least not in version 1. > > > - How does udev match on the drm connector name? They are not terribly > > stable atm, and if you reload your drm driver, or much more likely, have > > two gpus with two drm drivers they change. We probably should change the > > name allocation scheme to be per device instance instead of global > > first. Within a driver probe order is hopefully deterministic on a given > > platform, since even with super dynamic setups (based on dt/acpi) the > > firmware tables should change really. > > You can match on EDID attributes. Ok, so far this is pretty ugly as > the EDID property is binary. But we can add rather trivial udev > extensions to make EDID binary against text matching possible. Why EDID? This is purely about the drm connector name, e.g. if I have 2 gpus, both with an eDP connector (optimus, so just one panel) then the first driver gets eDP-1 as the name of it and the 2nd one eDP-2. Which is ok if both should control backlight brightness through the same driver, but a total mess if not just gpus get switched, but also backlight controllers. And if you reload you get then eDP-2 and eDP-3. Well at least in the past, that hilarity at least was fixed in commit b21e3afe2357c0f49348a5fb61247012bf8262ec Author: Ilia Mirkin Date: Wed Aug 7 22:34:48 2013 -0400 drm: use ida to allocate connector id What I think we need to do is to make these ida allocators per-device, so that both drivers have an eDP-1 connector. Otherwise you need to either match both or do funny tricks like "the first eDP connector, no matter which one on this gpu". After all we can now support more than one eDP (and more than one LVDS since a long time actually). Or how exactly is the udev hw db supposed to match this stuff for special cases. In general we need to duplicate the existing logic from libbacklight, like Matthew suggested. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[Bug 66963] Rv6xx dpm problems
https://bugs.freedesktop.org/show_bug.cgi?id=66963 --- Comment #245 from Kajzer --- (In reply to comment #244) > Created attachment 106085 [details] [review] > workaround for basic enablement > > As per feedback from the last few comments the attached patch forces the > performance level to high rather than auto which should fix the stability > issues and lower power usage due to clockgating, etc. and enables dpm by > default for rv6xx. That's great, thanks. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/2e968b49/attachment-0001.html>
[Bug 83742] [radeonsi KMS] Monitors on DP outputs not enabled
https://bugs.freedesktop.org/show_bug.cgi?id=83742 --- Comment #1 from Alex Deucher --- Can you bisect? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/474790e6/attachment.html>
[Bug 75649] Glitchy output using only HDMI on laptop with AMD Mobility Radeon HD 3450/3470
https://bugs.freedesktop.org/show_bug.cgi?id=75649 --- Comment #9 from Alex Deucher --- (In reply to comment #8) > I have been looking in the xf86-video-ati repo at drmmode_display.c and the > drmmode_sf86crtc_resize() function but can't find anything so far. > > It seems from Paul's comments that the issue may be related to the refresh > rate as well. Are there any suggestion as to places to look in the code? Everything that touches the hw is in the kernel driver. Does disabling dpm fix the issue? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/e8dc0dc3/attachment.html>
[Bug 81382] Text console blanking does not go away
https://bugs.freedesktop.org/show_bug.cgi?id=81382 --- Comment #10 from Alex Deucher --- (In reply to comment #7) > Ok, this fix work, but cause another problem (tested with 3.15.5+patch and > 3.16.1). > > When display goes off, backlight goes off. > When display goes on, backlight is set to MAX. > When display goes off again, backligh remains MAX. > After pressing key, LCD works, backlight stay at MAX level. > When display goes off, backlight is still MAX. Does the backlight respond correctly when adjusted via the sysfs blacklight interface? -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/43c9dfe4/attachment.html>
[PATCH 2/4] drm: Only update final vblank count when precise ts is available
On Wed, Sep 10, 2014 at 05:36:09PM +0200, Daniel Vetter wrote:
> Drivers without a hardware vblank counter simply can't account for the
> vblanks that happened while the vblank interrupt was off. To check
> this grab a vblank timestamp and if the result is dubious follow the
> normal save-and-disable logic.
>
> Drivers should prevent this by setting vblank_disable_allowed = false,
> but since running vblank interrupts constantly is not good for power
> consumption most drivers lie. Testing for precise vblank timestamps is
> the next best thing we can check for.
>
> Suggested-by: Mario Kleiner
> Cc: Mario Kleiner
> Cc: Matt Roper
> Cc: Ville Syrj?l?
> Signed-off-by: Daniel Vetter
Reviewed-by: Matt Roper
> ---
> drivers/gpu/drm/drm_irq.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 6eb015020af2..922721ead29a 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -163,8 +163,15 @@ static void vblank_disable_and_save(struct drm_device
> *dev, int crtc)
>* has been ticking all along until this time. This makes the
>* count account for the entire time between drm_vblank_on() and
>* drm_vblank_off().
> + *
> + * But only do this if precise vblank timestamps are available.
> + * Otherwise we might read a totally bogus timestamp since drivers
> + * lacking precise timestamp support rely upon sampling the system clock
> + * at vblank interrupt time. Which obviously won't work out well if the
> + * vblank interrupt is disabled.
>*/
> - if (!vblank->enabled) {
> + if (!vblank->enabled &&
> + drm_get_last_vbltimestamp(dev, crtc, &tvblank, 0) > 0) {
> drm_update_vblank_count(dev, crtc);
> spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
> return;
> --
> 1.9.3
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
[PATCH v3] drm/i915: Merge of visible and !visible paths for primary planes
From: Gustavo Padovan
Fold intel_pipe_set_base() in the update primary plane path merging
pieces of code that are common to both paths.
Basically the the pin/unpin procedures are the same for both paths
and some checks can also be shared (some of the were moved to the
check() stage)
v2: take Ville's comments:
- remove unnecessary plane check
- move mutex lock to inside the conditional
- make the pin fail message a debug one
- add a fixme for the fastboot hack
- call intel_frontbuffer_flip() after FBC update
v3: take more Ville's comments:
- fold update code under if (intel_crtc->active), and do the
visible/!visible split inside.
- check ret inside the same conditional we assign it
Suggested-by: Ville Syrj?l?
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/i915/intel_display.c | 139 +--
1 file changed, 82 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index b78f00a..8f3144e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11824,12 +11824,23 @@ intel_check_primary_plane(struct drm_plane *plane,
struct drm_rect *dest = &state->dst;
struct drm_rect *src = &state->src;
const struct drm_rect *clip = &state->clip;
+ int ret;
- return drm_plane_helper_check_update(plane, crtc, fb,
+ ret = drm_plane_helper_check_update(plane, crtc, fb,
src, dest, clip,
DRM_PLANE_HELPER_NO_SCALING,
DRM_PLANE_HELPER_NO_SCALING,
false, true, &state->visible);
+ if (ret)
+ return ret;
+
+ /* no fb bound */
+ if (state->visible && !fb) {
+ DRM_ERROR("No FB bound\n");
+ return -EINVAL;
+ }
+
+ return 0;
}
static int
@@ -11841,6 +11852,8 @@ intel_commit_primary_plane(struct drm_plane *plane,
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ enum pipe pipe = intel_crtc->pipe;
+ struct drm_framebuffer *old_fb = plane->fb;
struct drm_i915_gem_object *obj = intel_fb_obj(fb);
struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->fb);
struct intel_plane *intel_plane = to_intel_plane(plane);
@@ -11849,67 +11862,28 @@ intel_commit_primary_plane(struct drm_plane *plane,
intel_crtc_wait_for_pending_flips(crtc);
- /*
-* If clipping results in a non-visible primary plane, we'll disable
-* the primary plane. Note that this is a bit different than what
-* happens if userspace explicitly disables the plane by passing fb=0
-* because plane->fb still gets set and pinned.
-*/
- if (!state->visible) {
- mutex_lock(&dev->struct_mutex);
-
- /*
-* Try to pin the new fb first so that we can bail out if we
-* fail.
-*/
- if (plane->fb != fb) {
- ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
- if (ret) {
- mutex_unlock(&dev->struct_mutex);
- return ret;
- }
- }
-
- i915_gem_track_fb(old_obj, obj,
- INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe));
-
- if (intel_crtc->primary_enabled)
- intel_disable_primary_hw_plane(plane, crtc);
-
-
- if (plane->fb != fb)
- if (plane->fb)
- intel_unpin_fb_obj(old_obj);
+ if (intel_crtc_has_pending_flip(crtc)) {
+ DRM_ERROR("pipe is still busy with an old pageflip\n");
+ return -EBUSY;
+ }
+ if (plane->fb != fb) {
+ mutex_lock(&dev->struct_mutex);
+ ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
+ if (ret == 0)
+ i915_gem_track_fb(old_obj, obj,
+ INTEL_FRONTBUFFER_PRIMARY(pipe));
mutex_unlock(&dev->struct_mutex);
-
- } else {
- if (intel_crtc && intel_crtc->active &&
- intel_crtc->primary_enabled) {
- /*
-* FBC does not work on some platforms for rotated
-* planes, so disable it when rotation is not 0 and
-* update it when rotation is set back to 0.
-*
-* FIXME: This is redundant with the fbc update done in
-* the primary plane enable function except that tha
[Bug 75649] Glitchy output using only HDMI on laptop with AMD Mobility Radeon HD 3450/3470
https://bugs.freedesktop.org/show_bug.cgi?id=75649 --- Comment #10 from Alex Deucher --- If you are using dpm, make sure your ddx has this patch: http://cgit.freedesktop.org/xorg/driver/xf86-video-ati/commit/?id=c4ae0e2cbcc0e2ebf9f13ee92d59b5120254a1dc -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/a6b54d49/attachment.html>
[Bug 83505] AMD A4-5300 APU : only radeon.dpm=1 prevents random reboots with 3.16.1 kernel.
https://bugs.freedesktop.org/show_bug.cgi?id=83505 --- Comment #2 from Rpnpif --- I can confirm now that radeon.dpm=1 or none option get random immediate reboots. Only radeon.dpm=0 prevents this reboots as tested during one week. So I change the summary. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/fa366c32/attachment-0001.html>
[Bug 83505] AMD A4-5300 APU : only radeon.dpm=1 get random reboots with 3.16.1 kernel.
https://bugs.freedesktop.org/show_bug.cgi?id=83505 Rpnpif changed: What|Removed |Added Summary|AMD A4-5300 APU : only |AMD A4-5300 APU : only |radeon.dpm=1 prevents |radeon.dpm=1 get random |random reboots with 3.16.1 |reboots with 3.16.1 kernel. |kernel. | -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/6ad1f9a9/attachment.html>
[Bug 83505] AMD A4-5300 APU : radeon.dpm=1 get random reboots with 3.16.1 kernel.
https://bugs.freedesktop.org/show_bug.cgi?id=83505 Rpnpif changed: What|Removed |Added Summary|AMD A4-5300 APU : only |AMD A4-5300 APU : |radeon.dpm=1 get random |radeon.dpm=1 get random |reboots with 3.16.1 kernel. |reboots with 3.16.1 kernel. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/8232ff02/attachment.html>
[Bug 83505] AMD A4-5300 APU : radeon.dpm=1 get random reboots with 3.16.1 kernel.
https://bugs.freedesktop.org/show_bug.cgi?id=83505 --- Comment #3 from Rpnpif --- Same issue on another machine with ASRock K7S41GX motherboard and Radeon RV280 with 3.14 Kernel from Debian Wheezy-Backport. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/bc5e0d45/attachment.html>
[PATCH v2] drm/exynos: update to use component match support
On 09/11/2014 02:57 PM, Inki Dae wrote:
> Update Exynos's DRM driver to use component match support rater than
> add_components.
>
> Changelog v2:
> - release devices and drivers if failed.
> - change compare_of to compare_dev.
>
> Signed-off-by: Inki Dae
Modulo fixes I have posted earlier.
Tested-by: Andrzej Hajda
--
Regards
Andrzej
> ---
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 44
> +++
> 1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 5aae95c..3f6ec96 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -486,21 +486,20 @@ void exynos_drm_component_del(struct device *dev,
> mutex_unlock(&drm_component_lock);
> }
>
> -static int compare_of(struct device *dev, void *data)
> +static int compare_dev(struct device *dev, void *data)
> {
> return dev == (struct device *)data;
> }
>
> -static int exynos_drm_add_components(struct device *dev, struct master *m)
> +static struct component_match *exynos_drm_match_add(struct device *dev)
> {
> + struct component_match *match = NULL;
> struct component_dev *cdev;
> unsigned int attach_cnt = 0;
>
> mutex_lock(&drm_component_lock);
>
> list_for_each_entry(cdev, &drm_component_list, list) {
> - int ret;
> -
> /*
>* Add components to master only in case that crtc and
>* encoder/connector device objects exist.
> @@ -515,16 +514,10 @@ static int exynos_drm_add_components(struct device
> *dev, struct master *m)
> /*
>* fimd and dpi modules have same device object so add
>* only crtc device object in this case.
> - *
> - * TODO. if dpi module follows driver-model driver then
> - * below codes can be removed.
>*/
> if (cdev->crtc_dev == cdev->conn_dev) {
> - ret = component_master_add_child(m, compare_of,
> - cdev->crtc_dev);
> - if (ret < 0)
> - return ret;
> -
> + component_match_add(dev, &match, compare_dev,
> + cdev->crtc_dev);
> goto out_lock;
> }
>
> @@ -534,11 +527,8 @@ static int exynos_drm_add_components(struct device *dev,
> struct master *m)
>* connector/encoder need pipe number of crtc when they
>* are created.
>*/
> - ret = component_master_add_child(m, compare_of, cdev->crtc_dev);
> - ret |= component_master_add_child(m, compare_of,
> - cdev->conn_dev);
> - if (ret < 0)
> - return ret;
> + component_match_add(dev, &match, compare_dev, cdev->crtc_dev);
> + component_match_add(dev, &match, compare_dev, cdev->conn_dev);
>
> out_lock:
> mutex_lock(&drm_component_lock);
> @@ -546,7 +536,7 @@ out_lock:
>
> mutex_unlock(&drm_component_lock);
>
> - return attach_cnt ? 0 : -ENODEV;
> + return attach_cnt ? match : ERR_PTR(-EPROBE_DEFER);
> }
>
> static int exynos_drm_bind(struct device *dev)
> @@ -560,13 +550,13 @@ static void exynos_drm_unbind(struct device *dev)
> }
>
> static const struct component_master_ops exynos_drm_ops = {
> - .add_components = exynos_drm_add_components,
> .bind = exynos_drm_bind,
> .unbind = exynos_drm_unbind,
> };
>
> static int exynos_drm_platform_probe(struct platform_device *pdev)
> {
> + struct component_match *match;
> int ret;
>
> pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> @@ -633,13 +623,23 @@ static int exynos_drm_platform_probe(struct
> platform_device *pdev)
> goto err_unregister_ipp_drv;
> #endif
>
> - ret = component_master_add(&pdev->dev, &exynos_drm_ops);
> + match = exynos_drm_match_add(&pdev->dev);
> + if (IS_ERR(match)) {
> + ret = PTR_ERR(match);
> + goto err_unregister_resources;
> + }
> +
> + ret = component_master_add_with_match(&pdev->dev, &exynos_drm_ops,
> + match);
> if (ret < 0)
> - DRM_DEBUG_KMS("re-tried by last sub driver probed later.\n");
> + goto err_unregister_resources;
>
> - return 0;
> + return ret;
> +
> +err_unregister_resources:
>
> #ifdef CONFIG_DRM_EXYNOS_IPP
> + exynos_platform_device_ipp_unregister();
> err_unregister_ipp_drv:
> platform_driver_unregister(&ipp_driver);
> err_unregister_gsc_drv:
>
[Bug 83505] AMD A4-5300 APU : radeon.dpm=1 get random reboots with 3.16.1 kernel.
https://bugs.freedesktop.org/show_bug.cgi?id=83505 --- Comment #4 from Rpnpif --- Created attachment 106141 --> https://bugs.freedesktop.org/attachment.cgi?id=106141&action=edit Dmesg from machine based on ASRock K7S41GX without radeon.dpm -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/bf4dec15/attachment.html>
[Bug 83505] AMD A4-5300 APU : radeon.dpm=1 get random reboots with 3.16.1 kernel.
https://bugs.freedesktop.org/show_bug.cgi?id=83505 --- Comment #5 from Alex Deucher --- (In reply to comment #3) > Same issue on another machine with ASRock K7S41GX motherboard and Radeon > RV280 with 3.14 Kernel from Debian Wheezy-Backport. RV280 does not support dpm so this parameter is useless for this asic. -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/0c8bcef3/attachment.html>
[PATCH] intel: make bufmgr_gem shareable from different API
When using Mesa and LibVA in the same process, one would like to be
able bind buffers from the output of the decoder to a GL texture
through an EGLImage.
LibVA can reuse buffers allocated by Gbm through a file descriptor. It
will then wrap it into a drm_intel_bo with
drm_intel_bo_gem_create_from_prime().
The problem at the moment is that both library get a different
drm_intel_bufmgr object when they call drm_intel_bufmgr_gem_init()
even though they're using the same drm file descriptor. As a result,
instead of manipulating the same buffer object for a given file
descriptor, they get 2 different drm_intel_bo objects and 2 different
refcounts, leading one of the library to get errors from the kernel on
invalid BO when one of the 2 library is done with a shared buffer.
This patch modifies drm_intel_bufmgr_gem_init() so, given a file
descriptor, it will look for an already existing drm_intel_bufmgr
using the same file descriptor and return that object.
Signed-off-by: Lionel Landwerlin
---
intel/intel_bufmgr_gem.c | 82 +---
1 file changed, 70 insertions(+), 12 deletions(-)
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 0e1cb0d..ce43bc6 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -94,6 +94,8 @@ struct drm_intel_gem_bo_bucket {
typedef struct _drm_intel_bufmgr_gem {
drm_intel_bufmgr bufmgr;
+ atomic_t refcount;
+
int fd;
int max_relocs;
@@ -111,6 +113,8 @@ typedef struct _drm_intel_bufmgr_gem {
int num_buckets;
time_t time;
+ drmMMListHead managers;
+
drmMMListHead named;
drmMMListHead vma_cache;
int vma_count, vma_open, vma_max;
@@ -3186,6 +3190,65 @@ drm_intel_bufmgr_gem_set_aub_annotations(drm_intel_bo
*bo,
bo_gem->aub_annotation_count = count;
}
+static pthread_mutex_t bufmgr_list_mutex = PTHREAD_MUTEX_INITIALIZER;
+static drmMMListHead bufmgr_list = { NULL, NULL };
+
+static drm_intel_bufmgr_gem *
+drm_intel_bufmgr_gem_find_or_create_for_fd(int fd, int *found)
+{
+ drm_intel_bufmgr_gem *bufmgr_gem;
+
+ assert(pthread_mutex_lock(&bufmgr_list_mutex) == 0);
+
+ if (bufmgr_list.next == NULL) {
+ DRMINITLISTHEAD(&bufmgr_list);
+ } else {
+ DRMLISTFOREACHENTRY(bufmgr_gem, &bufmgr_list, managers) {
+ if (bufmgr_gem->fd == fd) {
+ atomic_inc(&bufmgr_gem->refcount);
+ *found = 1;
+ goto exit;
+ }
+ }
+ }
+
+ bufmgr_gem = calloc(1, sizeof(*bufmgr_gem));
+ if (bufmgr_gem == NULL)
+ goto exit;
+
+ bufmgr_gem->fd = fd;
+ atomic_set(&bufmgr_gem->refcount, 1);
+
+ DRMLISTADD(&bufmgr_gem->managers, &bufmgr_list);
+
+ assert(pthread_mutex_init(&bufmgr_gem->lock, NULL) == 0);
+
+ pthread_mutex_lock(&bufmgr_gem->lock);
+
+ *found = 0;
+
+exit:
+ pthread_mutex_unlock(&bufmgr_list_mutex);
+
+ return bufmgr_gem;
+}
+
+static void
+drm_intel_bufmgr_gem_unref (drm_intel_bufmgr *bufmgr)
+{
+ drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr;
+
+ if (atomic_dec_and_test(&bufmgr_gem->refcount)) {
+ assert(pthread_mutex_lock(&bufmgr_list_mutex) == 0);
+
+ DRMLISTDEL(&bufmgr_gem->managers);
+
+ pthread_mutex_unlock(&bufmgr_list_mutex);
+
+ drm_intel_bufmgr_gem_destroy(bufmgr);
+ }
+}
+
/**
* Initializes the GEM buffer manager, which uses the kernel to allocate, map,
* and manage map buffer objections.
@@ -3201,16 +3264,9 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
int ret, tmp;
bool exec2 = false;
- bufmgr_gem = calloc(1, sizeof(*bufmgr_gem));
- if (bufmgr_gem == NULL)
- return NULL;
-
- bufmgr_gem->fd = fd;
-
- if (pthread_mutex_init(&bufmgr_gem->lock, NULL) != 0) {
- free(bufmgr_gem);
- return NULL;
- }
+ bufmgr_gem = drm_intel_bufmgr_gem_find_or_create_for_fd(fd, &ret);
+ if (bufmgr_gem && ret)
+ return &bufmgr_gem->bufmgr;
ret = drmIoctl(bufmgr_gem->fd,
DRM_IOCTL_I915_GEM_GET_APERTURE,
@@ -3245,7 +3301,7 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
else if (IS_GEN8(bufmgr_gem->pci_device))
bufmgr_gem->gen = 8;
else {
- free(bufmgr_gem);
+ drm_intel_bufmgr_gem_unref(&bufmgr_gem->bufmgr);
return NULL;
}
@@ -3357,7 +3413,7 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
bufmgr_gem->bufmgr.bo_exec = drm_intel_gem_bo_exec;
bufmgr_gem->bufmgr.bo_busy = drm_intel_gem_bo_busy;
bufmgr_gem->bufmgr.bo_madvise = drm_intel_gem_bo_madvise;
- bufmgr_gem->bufmgr.destroy = drm_intel_bufmgr_gem_destroy;
+
Shareable bufmgr objects v2
Following Chris' review, here is an updated patch using drmMMListHead. I did a quick read of the benchmarks/tests files in igt, as far as I can see, drm_intel_bufmgr_destroy() is always called before the drm file descriptor is closed. So it seems this change shouldn't break anything. Cheers, - Lionel
[PATCH] intel: make bufmgr_gem shareable from different API
On Thu, Sep 11, 2014 at 04:36:20PM +0100, Lionel Landwerlin wrote:
> When using Mesa and LibVA in the same process, one would like to be
> able bind buffers from the output of the decoder to a GL texture
> through an EGLImage.
>
> LibVA can reuse buffers allocated by Gbm through a file descriptor. It
> will then wrap it into a drm_intel_bo with
> drm_intel_bo_gem_create_from_prime().
>
> The problem at the moment is that both library get a different
> drm_intel_bufmgr object when they call drm_intel_bufmgr_gem_init()
> even though they're using the same drm file descriptor. As a result,
> instead of manipulating the same buffer object for a given file
> descriptor, they get 2 different drm_intel_bo objects and 2 different
> refcounts, leading one of the library to get errors from the kernel on
> invalid BO when one of the 2 library is done with a shared buffer.
>
> This patch modifies drm_intel_bufmgr_gem_init() so, given a file
> descriptor, it will look for an already existing drm_intel_bufmgr
> using the same file descriptor and return that object.
>
> Signed-off-by: Lionel Landwerlin
> ---
> intel/intel_bufmgr_gem.c | 82
> +---
> 1 file changed, 70 insertions(+), 12 deletions(-)
>
> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> index 0e1cb0d..ce43bc6 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -94,6 +94,8 @@ struct drm_intel_gem_bo_bucket {
> typedef struct _drm_intel_bufmgr_gem {
> drm_intel_bufmgr bufmgr;
>
> + atomic_t refcount;
> +
> int fd;
>
> int max_relocs;
> @@ -111,6 +113,8 @@ typedef struct _drm_intel_bufmgr_gem {
> int num_buckets;
> time_t time;
>
> + drmMMListHead managers;
> +
> drmMMListHead named;
> drmMMListHead vma_cache;
> int vma_count, vma_open, vma_max;
> @@ -3186,6 +3190,65 @@ drm_intel_bufmgr_gem_set_aub_annotations(drm_intel_bo
> *bo,
> bo_gem->aub_annotation_count = count;
> }
>
> +static pthread_mutex_t bufmgr_list_mutex = PTHREAD_MUTEX_INITIALIZER;
> +static drmMMListHead bufmgr_list = { NULL, NULL };
We don't have a static initialializer? Oh well,
static drmMMListHead bufmgr_list = { &bufmgr_list, &bufmgr_list };
> +static drm_intel_bufmgr_gem *
> +drm_intel_bufmgr_gem_find_or_create_for_fd(int fd, int *found)
> +{
> + drm_intel_bufmgr_gem *bufmgr_gem;
> +
> + assert(pthread_mutex_lock(&bufmgr_list_mutex) == 0);
> +
> + if (bufmgr_list.next == NULL) {
> + DRMINITLISTHEAD(&bufmgr_list);
Not needed with the static initializer above.
> + } else {
> + DRMLISTFOREACHENTRY(bufmgr_gem, &bufmgr_list, managers) {
> + if (bufmgr_gem->fd == fd) {
> + atomic_inc(&bufmgr_gem->refcount);
> + *found = 1;
> + goto exit;
> + }
> + }
> + }
> +
> + bufmgr_gem = calloc(1, sizeof(*bufmgr_gem));
> + if (bufmgr_gem == NULL)
> + goto exit;
> +
> + bufmgr_gem->fd = fd;
> + atomic_set(&bufmgr_gem->refcount, 1);
> +
> + DRMLISTADD(&bufmgr_gem->managers, &bufmgr_list);
> +
> + assert(pthread_mutex_init(&bufmgr_gem->lock, NULL) == 0);
> +
> + pthread_mutex_lock(&bufmgr_gem->lock);
There is an issue with dropping the lock here. A second thread may try
to use the uninitialised bufmgr and crash. We need to hold the lock
until we have finished initialising the bufmgr. So this function can
just be reduced to a list search called with the lock held.
> +
> + *found = 0;
> +
> +exit:
> + pthread_mutex_unlock(&bufmgr_list_mutex);
> +
> + return bufmgr_gem;
> +}
> +
> +static void
> +drm_intel_bufmgr_gem_unref (drm_intel_bufmgr *bufmgr)
> +{
> + drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr;
> +
> + if (atomic_dec_and_test(&bufmgr_gem->refcount)) {
> + assert(pthread_mutex_lock(&bufmgr_list_mutex) == 0);
You need to recheck the reference count after grabbing the lock.
> +
> + DRMLISTDEL(&bufmgr_gem->managers);
> +
> + pthread_mutex_unlock(&bufmgr_list_mutex);
> +
> + drm_intel_bufmgr_gem_destroy(bufmgr);
> + }
> +}
--
Chris Wilson, Intel Open Source Technology Centre
[PATCH RFC 4/4] drm: link connectors to backlight devices
Hi On Thu, Sep 11, 2014 at 3:06 PM, Daniel Vetter wrote: > On Thu, Sep 11, 2014 at 02:22:55PM +0200, David Herrmann wrote: >> actual-brightness is a bit more tricky. Currently, DRM caches property >> values, so there is no read_property() hook. We'd have to add this. >> But it'll be quite nasty as we have to call into the backlight driver. >> So I think we want to run an async-interruptible worker on the >> backlight, drop the locks in the ioctl and wait for the job to finish. >> Not sure whether it's worth it.. maybe we can add this later. > > See Jani's reply - we probably don't need it, at least not in version 1. I couldn't see any comment regarding "actual-brightness". But I'm totally fine with dropping this. >> >> > - How does udev match on the drm connector name? They are not terribly >> > stable atm, and if you reload your drm driver, or much more likely, have >> > two gpus with two drm drivers they change. We probably should change the >> > name allocation scheme to be per device instance instead of global >> > first. Within a driver probe order is hopefully deterministic on a given >> > platform, since even with super dynamic setups (based on dt/acpi) the >> > firmware tables should change really. >> >> You can match on EDID attributes. Ok, so far this is pretty ugly as >> the EDID property is binary. But we can add rather trivial udev >> extensions to make EDID binary against text matching possible. > > Why EDID? This is purely about the drm connector name, e.g. if I have 2 > gpus, both with an eDP connector (optimus, so just one panel) then the > first driver gets eDP-1 as the name of it and the 2nd one eDP-2. Which is > ok if both should control backlight brightness through the same driver, > but a total mess if not just gpus get switched, but also backlight > controllers. > > And if you reload you get then eDP-2 and eDP-3. Well at least in the past, > that hilarity at least was fixed in > > commit b21e3afe2357c0f49348a5fb61247012bf8262ec > Author: Ilia Mirkin > Date: Wed Aug 7 22:34:48 2013 -0400 > > drm: use ida to allocate connector id > > What I think we need to do is to make these ida allocators per-device, so > that both drivers have an eDP-1 connector. Otherwise you need to either > match both or do funny tricks like "the first eDP connector, no matter > which one on this gpu". After all we can now support more than one eDP > (and more than one LVDS since a long time actually). > > Or how exactly is the udev hw db supposed to match this stuff for special > cases. In general we need to duplicate the existing logic from > libbacklight, like Matthew suggested. Yeah, I get what you mean. Names are not stable so even if we can match on the internal card, we cannot match on "lowest available eDP display". per-device IDs should be totally fine and fix this issue. We prefix connectors with the device name anyway, so no conflicts can arise. But I think we want to make this a udev builtin anyway. So we can easily implement the same logic as libbacklight. Thanks David
[Intel-gfx] [PATCH -v4 2/4] drm/i915: split intel_update_plane into check() and commit()
On Fri, 05 Sep 2014, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Due to the upcoming atomic modesetting feature we need to separate
> some update functions into a check step that can fail and a commit
> step that should, ideally, never fail.
>
> This commit splits intel_update_plane() and its commit part can still
> fail due to the fb pinning procedure.
This patch regresses our tests:
https://bugs.freedesktop.org/show_bug.cgi?id=83747
BR,
Jani.
>
> Signed-off-by: Gustavo Padovan
> ---
> drivers/gpu/drm/i915/intel_sprite.c | 233
> ++--
> 1 file changed, 141 insertions(+), 92 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 07a74ef..a4306cf 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -845,57 +845,24 @@ static bool colorkey_enabled(struct intel_plane
> *intel_plane)
> }
>
> static int
> -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> -struct drm_framebuffer *fb, int crtc_x, int crtc_y,
> -unsigned int crtc_w, unsigned int crtc_h,
> -uint32_t src_x, uint32_t src_y,
> -uint32_t src_w, uint32_t src_h)
> +intel_check_sprite_plane(struct drm_plane *plane,
> + struct intel_plane_state *state)
> {
> - struct drm_device *dev = plane->dev;
> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_crtc *intel_crtc = to_intel_crtc(state->crtc);
> struct intel_plane *intel_plane = to_intel_plane(plane);
> - enum pipe pipe = intel_crtc->pipe;
> + struct drm_framebuffer *fb = state->fb;
> struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
> struct drm_i915_gem_object *obj = intel_fb->obj;
> - struct drm_i915_gem_object *old_obj = intel_plane->obj;
> - int ret;
> - bool primary_enabled;
> - bool visible;
> + int crtc_x, crtc_y;
> + unsigned int crtc_w, crtc_h;
> + uint32_t src_x, src_y, src_w, src_h;
> + struct drm_rect *src = &state->src;
> + struct drm_rect *dst = &state->dst;
> + struct drm_rect *orig_src = &state->orig_src;
> + const struct drm_rect *clip = &state->clip;
> int hscale, vscale;
> int max_scale, min_scale;
> int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> - struct drm_rect src = {
> - /* sample coordinates in 16.16 fixed point */
> - .x1 = src_x,
> - .x2 = src_x + src_w,
> - .y1 = src_y,
> - .y2 = src_y + src_h,
> - };
> - struct drm_rect dst = {
> - /* integer pixels */
> - .x1 = crtc_x,
> - .x2 = crtc_x + crtc_w,
> - .y1 = crtc_y,
> - .y2 = crtc_y + crtc_h,
> - };
> - const struct drm_rect clip = {
> - .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
> - .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
> - };
> - const struct {
> - int crtc_x, crtc_y;
> - unsigned int crtc_w, crtc_h;
> - uint32_t src_x, src_y, src_w, src_h;
> - } orig = {
> - .crtc_x = crtc_x,
> - .crtc_y = crtc_y,
> - .crtc_w = crtc_w,
> - .crtc_h = crtc_h,
> - .src_x = src_x,
> - .src_y = src_y,
> - .src_w = src_w,
> - .src_h = src_h,
> - };
>
> /* Don't modify another pipe's plane */
> if (intel_plane->pipe != intel_crtc->pipe) {
> @@ -927,55 +894,55 @@ intel_update_plane(struct drm_plane *plane, struct
> drm_crtc *crtc,
> max_scale = intel_plane->max_downscale << 16;
> min_scale = intel_plane->can_scale ? 1 : (1 << 16);
>
> - drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
> + drm_rect_rotate(src, fb->width << 16, fb->height << 16,
> intel_plane->rotation);
>
> - hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);
> + hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale);
> BUG_ON(hscale < 0);
>
> - vscale = drm_rect_calc_vscale_relaxed(&src, &dst, min_scale, max_scale);
> + vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale);
> BUG_ON(vscale < 0);
>
> - visible = drm_rect_clip_scaled(&src, &dst, &clip, hscale, vscale);
> + state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
>
> - crtc_x = dst.x1;
> - crtc_y = dst.y1;
> - crtc_w = drm_rect_width(&dst);
> - crtc_h = drm_rect_height(&dst);
> + crtc_x = dst->x1;
> + crtc_y = dst->y1;
> + crtc_w = drm_rect_width(dst);
> + crtc_h = drm_rect_height(dst);
>
> - if (visible) {
> + if (state->visible) {
> /* check again in case clipping clamped the results */
> - hscale = drm_rect_calc_hscale(
[PATCH] intel: make bufmgr_gem shareable from different API
When using Mesa and LibVA in the same process, one would like to be
able bind buffers from the output of the decoder to a GL texture
through an EGLImage.
LibVA can reuse buffers allocated by Gbm through a file descriptor. It
will then wrap it into a drm_intel_bo with
drm_intel_bo_gem_create_from_prime().
The problem at the moment is that both library get a different
drm_intel_bufmgr object when they call drm_intel_bufmgr_gem_init()
even though they're using the same drm file descriptor. As a result,
instead of manipulating the same buffer object for a given file
descriptor, they get 2 different drm_intel_bo objects and 2 different
refcounts, leading one of the library to get errors from the kernel on
invalid BO when one of the 2 library is done with a shared buffer.
This patch modifies drm_intel_bufmgr_gem_init() so, given a file
descriptor, it will look for an already existing drm_intel_bufmgr
using the same file descriptor and return that object.
Signed-off-by: Lionel Landwerlin
---
intel/intel_bufmgr_gem.c | 60
1 file changed, 55 insertions(+), 5 deletions(-)
diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
index 0e1cb0d..0a2a62b 100644
--- a/intel/intel_bufmgr_gem.c
+++ b/intel/intel_bufmgr_gem.c
@@ -94,6 +94,8 @@ struct drm_intel_gem_bo_bucket {
typedef struct _drm_intel_bufmgr_gem {
drm_intel_bufmgr bufmgr;
+ atomic_t refcount;
+
int fd;
int max_relocs;
@@ -111,6 +113,8 @@ typedef struct _drm_intel_bufmgr_gem {
int num_buckets;
time_t time;
+ drmMMListHead managers;
+
drmMMListHead named;
drmMMListHead vma_cache;
int vma_count, vma_open, vma_max;
@@ -3186,6 +3190,40 @@ drm_intel_bufmgr_gem_set_aub_annotations(drm_intel_bo
*bo,
bo_gem->aub_annotation_count = count;
}
+static pthread_mutex_t bufmgr_list_mutex = PTHREAD_MUTEX_INITIALIZER;
+static drmMMListHead bufmgr_list = { &bufmgr_list, &bufmgr_list };
+
+static drm_intel_bufmgr_gem *
+drm_intel_bufmgr_gem_find(int fd)
+{
+ drm_intel_bufmgr_gem *bufmgr_gem;
+
+ DRMLISTFOREACHENTRY(bufmgr_gem, &bufmgr_list, managers) {
+ if (bufmgr_gem->fd == fd) {
+ atomic_inc(&bufmgr_gem->refcount);
+ return bufmgr_gem;
+ }
+ }
+
+ return NULL;
+}
+
+static void
+drm_intel_bufmgr_gem_unref(drm_intel_bufmgr *bufmgr)
+{
+ drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)bufmgr;
+
+ if (atomic_dec_and_test(&bufmgr_gem->refcount)) {
+ assert(pthread_mutex_lock(&bufmgr_list_mutex) == 0);
+
+ DRMLISTDEL(&bufmgr_gem->managers);
+
+ pthread_mutex_unlock(&bufmgr_list_mutex);
+
+ drm_intel_bufmgr_gem_destroy(bufmgr);
+ }
+}
+
/**
* Initializes the GEM buffer manager, which uses the kernel to allocate, map,
* and manage map buffer objections.
@@ -3201,15 +3239,21 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
int ret, tmp;
bool exec2 = false;
+ bufmgr_gem = drm_intel_bufmgr_gem_find(fd);
+ if (bufmgr_gem)
+ goto exit;
+
bufmgr_gem = calloc(1, sizeof(*bufmgr_gem));
if (bufmgr_gem == NULL)
- return NULL;
+ goto exit;
bufmgr_gem->fd = fd;
+ atomic_set(&bufmgr_gem->refcount, 1);
if (pthread_mutex_init(&bufmgr_gem->lock, NULL) != 0) {
free(bufmgr_gem);
- return NULL;
+ bufmgr_gem = NULL;
+ goto exit;
}
ret = drmIoctl(bufmgr_gem->fd,
@@ -3246,7 +3290,8 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
bufmgr_gem->gen = 8;
else {
free(bufmgr_gem);
- return NULL;
+ bufmgr_gem = NULL;
+ goto exit;
}
if (IS_GEN3(bufmgr_gem->pci_device) &&
@@ -3357,7 +3402,7 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
bufmgr_gem->bufmgr.bo_exec = drm_intel_gem_bo_exec;
bufmgr_gem->bufmgr.bo_busy = drm_intel_gem_bo_busy;
bufmgr_gem->bufmgr.bo_madvise = drm_intel_gem_bo_madvise;
- bufmgr_gem->bufmgr.destroy = drm_intel_bufmgr_gem_destroy;
+ bufmgr_gem->bufmgr.destroy = drm_intel_bufmgr_gem_unref;
bufmgr_gem->bufmgr.debug = 0;
bufmgr_gem->bufmgr.check_aperture_space =
drm_intel_gem_check_aperture_space;
@@ -3373,5 +3418,10 @@ drm_intel_bufmgr_gem_init(int fd, int batch_size)
DRMINITLISTHEAD(&bufmgr_gem->vma_cache);
bufmgr_gem->vma_max = -1; /* unlimited by default */
- return &bufmgr_gem->bufmgr;
+ DRMLISTADD(&bufmgr_gem->managers, &bufmgr_list);
+
+exit:
+ pthread_mutex_unlock(&bufmgr_list_mutex);
+
+ return bufmgr_gem != NULL ? &bufmgr_gem->bufmgr : NULL;
}
--
2.0.1
[Bug 83748] Only black content on screen, in the Tokyo flashback of the game "The Secret World"
https://bugs.freedesktop.org/show_bug.cgi?id=83748 --- Comment #3 from John --- Here is a trim frame I got running this command: apitrace trim --exact --frames=1236 wine-preloader.1.trace and then compressed. The frame 1236 is the in-game blacked out case, seen in qapitrace. https://mega.co.nz/#!s4RAAAhL!kjBFx0Yq-QgvQTtflnPoRsbOQpdYTBAwWDBTrlHuSLY I've done the same with frame 1183, which I believe is in the cinematic but since that one is completely black it's hard to really say for sure. https://mega.co.nz/#!49RySQhJ!jh6t2rDLrKDzcSNUKViWBwrpn10vNcuPGMu8QuDbXCw I've used mega as the files are too big to attach to the bug report. Thanks! -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/9cd6c7a5/attachment.html>
[PATCH] drm/i915: Fix regression in the sprite plane update split
From: Gustavo Padovan
7e4bf45dbd99a965c7b5d5944c6dc4246f171eb5 introduced the regression.
We fix it by doing the right assignment of crtc_y
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=83747
Signed-off-by: Gustavo Padovan
---
drivers/gpu/drm/i915/intel_sprite.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 90bb45f..78044bb 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1080,7 +1080,7 @@ intel_commit_sprite_plane(struct drm_plane *plane,
if (state->visible) {
crtc_x = state->dst.x1;
- crtc_y = state->dst.x2;
+ crtc_y = state->dst.y1;
crtc_w = drm_rect_width(&state->dst);
crtc_h = drm_rect_height(&state->dst);
src_x = state->src.x1;
--
1.9.3
[Bug 66940] Mobility Radeon HD 5650 doesn't resume from suspend on kernel 3.11 (linus and drm_next)
https://bugs.freedesktop.org/show_bug.cgi?id=66940 --- Comment #14 from Bernhard Held --- Suspend works again with my HD 5450 using branch 'drm-next-3.18' of git://people.freedesktop.org/~airlied/linux! Previously resume consistently produced this output with a subsequent crash of the X-server: [ 7025.164024] radeon :01:00.0: ring 5 stalled for more than 1msec [ 7025.164030] radeon :01:00.0: GPU lockup (waiting for 0x000a last fence id 0x0002 on ring 5) [ 7025.164034] [drm:uvd_v1_0_ib_test] *ERROR* radeon: fence wait failed (-35). [ 7025.164040] [drm:radeon_ib_ring_tests] *ERROR* radeon: failed testing IB on ring 5 (-35). Thanks for finally resolving this problem! -- You are receiving this mail because: You are the assignee for the bug. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20140911/e2b58ef4/attachment.html>
[drm-next-3.18 (-wip)] only works with b6c2b4 below evergreen (UVD)
On Thu, Sep 11, 2014 at 6:12 PM, Dieter N?tzel wrote: > Hello Alex and Christian, > > RV730 (AGP) need badly > > From b6c2b4faf90230ef9cf1a81f36cbccda4a606c59 Mon Sep 17 00:00:00 2001 > From: Alex Deucher > Date: Mon, 8 Sep 2014 13:16:39 -0400 > Subject: [PATCH] drm/radeon: only use me/pfp sync on evergreen+ > > The packet seems to cause hangs on some 7xx asics. > > bug: > https://bugs.freedesktop.org/show_bug.cgi?id=83616 > > Signed-off-by: Alex Deucher > > to get UVD going, again. Dave already pulled it into drm-next: http://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-fixes&id=b6c2b4faf90230ef9cf1a81f36cbccda4a606c59 Alex > > Thanks, > Dieter. > > PS > Examination of > [ 11.279944] radeon :01:00.0: (-1) pin WB bo failed > [ 11.279956] radeon :01:00.0: f6124800 unpin not necessary > [ 11.279975] radeon :01:00.0: disabling GPU acceleration > [ 11.331227] radeon :01:00.0: f610e000 unpin not necessary > is under way - I'm learning GIT...;-)
[drm-next-3.18 (-wip)] only works with b6c2b4 below evergreen (UVD)
On Thu, Sep 11, 2014 at 6:14 PM, Alex Deucher wrote: > On Thu, Sep 11, 2014 at 6:12 PM, Dieter N?tzel > wrote: >> Hello Alex and Christian, >> >> RV730 (AGP) need badly >> >> From b6c2b4faf90230ef9cf1a81f36cbccda4a606c59 Mon Sep 17 00:00:00 2001 >> From: Alex Deucher >> Date: Mon, 8 Sep 2014 13:16:39 -0400 >> Subject: [PATCH] drm/radeon: only use me/pfp sync on evergreen+ >> >> The packet seems to cause hangs on some 7xx asics. >> >> bug: >> https://bugs.freedesktop.org/show_bug.cgi?id=83616 >> >> Signed-off-by: Alex Deucher >> >> to get UVD going, again. > > Dave already pulled it into drm-next: er drm-fixes. > http://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-fixes&id=b6c2b4faf90230ef9cf1a81f36cbccda4a606c59 > > Alex > >> >> Thanks, >> Dieter. >> >> PS >> Examination of >> [ 11.279944] radeon :01:00.0: (-1) pin WB bo failed >> [ 11.279956] radeon :01:00.0: f6124800 unpin not necessary >> [ 11.279975] radeon :01:00.0: disabling GPU acceleration >> [ 11.331227] radeon :01:00.0: f610e000 unpin not necessary >> is under way - I'm learning GIT...;-)
[pull] radeon drm-next-3.18
Hi Dave, This pull adds concurrent buffer read support to radeon to avoid serialization when read buffers are shared between engines. The following changes since commit c4d922b14544d115232b7448a2ea7640ba901eb6: Merge branch 'msm-next' of git://people.freedesktop.org/~robclark/linux into drm-next (2014-09-11 20:53:57 +1000) are available in the git repository at: git://people.freedesktop.org/~agd5f/linux drm-next-3.18 for you to fetch changes up to 298593b609ecbf9e8a99e8a41c8c46acb3528468: drm/radeon: allow concurrent buffer reads (2014-09-11 10:46:02 -0400) Christian K?nig (3): drm/ttm: allow fence to be added as shared drm/radeon: add the infrastructure for concurrent buffer access drm/radeon: allow concurrent buffer reads drivers/gpu/drm/qxl/qxl_release.c | 1 + drivers/gpu/drm/radeon/cik.c | 25 ++- drivers/gpu/drm/radeon/cik_sdma.c | 25 ++- drivers/gpu/drm/radeon/evergreen_dma.c| 24 +- drivers/gpu/drm/radeon/r100.c | 21 + drivers/gpu/drm/radeon/r200.c | 21 + drivers/gpu/drm/radeon/r600.c | 23 +- drivers/gpu/drm/radeon/r600_dma.c | 25 ++- drivers/gpu/drm/radeon/radeon.h | 43 +- drivers/gpu/drm/radeon/radeon_asic.h | 74 --- drivers/gpu/drm/radeon/radeon_benchmark.c | 30 ++--- drivers/gpu/drm/radeon/radeon_cs.c| 10 ++--- drivers/gpu/drm/radeon/radeon_ib.c| 2 +- drivers/gpu/drm/radeon/radeon_semaphore.c | 38 ++-- drivers/gpu/drm/radeon/radeon_test.c | 24 +++--- drivers/gpu/drm/radeon/radeon_ttm.c | 12 ++--- drivers/gpu/drm/radeon/radeon_vm.c| 19 +++- drivers/gpu/drm/radeon/rv770_dma.c| 25 ++- drivers/gpu/drm/radeon/si_dma.c | 25 ++- drivers/gpu/drm/ttm/ttm_execbuf_util.c| 18 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 3 ++ drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 5 +++ include/drm/ttm/ttm_execbuf_util.h| 2 + 23 files changed, 284 insertions(+), 211 deletions(-)
[PATCH] drm/ttm: make sure format string cannot leak in
While zone->name is currently hard coded, the call to kobject_init_and_add()
should follow the more defensive argument list usage (as already done in
other places in ttm_memory.c) where "%s" is used instead of directly passing
in a variable as a format string.
Signed-off-by: Kees Cook
---
drivers/gpu/drm/ttm/ttm_memory.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index fa53df487875..1e688b603e46 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -300,7 +300,8 @@ static int ttm_mem_init_highmem_zone(struct ttm_mem_global
*glob,
zone->glob = glob;
glob->zone_highmem = zone;
ret = kobject_init_and_add(
- &zone->kobj, &ttm_mem_zone_kobj_type, &glob->kobj, zone->name);
+ &zone->kobj, &ttm_mem_zone_kobj_type, &glob->kobj, "%s",
+ zone->name);
if (unlikely(ret != 0)) {
kobject_put(&zone->kobj);
return ret;
--
1.9.1
--
Kees Cook
Chrome OS Security
