[PATCH v4 00/11] drm: add support for Atmel HLCDC Display Controller

[Laurent Pinchart]

Hi Boris,

On Thursday 28 August 2014 16:21:00 Boris BREZILLON wrote:
> On Thu, 28 Aug 2014 14:19:22 +0200 Laurent Pinchart wrote:
> > Hi Boris,
> 
> [...]
> 
> >> I don't have any VGA connector (or I'm missing something :-)),
> > 
> > My bad.
> 
> No problem.
> 
> >> but I have an LCD panel and an RGB to HDMI encoder connected on the same
> >> RGB connector.
> > 
> > There's no such thing as an RGB connector in DRM. Your SoC has a parallel
> > RGB video output (I assume it's a DPI bus). From a DRM point of view,
> > that bus corresponds to the output of the CRTC.
> 
> Okay, this mean I'll have to dispatch some of the code I've put in
> atmel_hlcdc_output.c into atmel_hlcdc_crtc.c (BTW, any chance you could
> take a look at this files ?).

Not in the very near future I'm afraid, I'm moving to a new flat in a couple 
of days, that will keep me pretty busy. If nobody has reviewed your patches in 
a week from now feel free to ping me.

> >>> As DRM hardcodes the pipeline model to CRTC -> encoder -> connector,
> >>> you will also need a DRM encoder in the VGA path. I suppose your board
> >>> has a VGA DAC, that's the component you should expose as a DRM encoder
> >>> (even if it can't be controlled and doesn't limit the valid modes).
> >> 
> >> Actually, my problem is that both devices are connected on the same RGB
> >> connector, and thus share the same display mode (resolution, HSYNC,
> >> VSYNC, RGB output mode, ...).
> >> This means that all remote devices have to agree on a specific mode if
> >> we want to mirror the display on several output devices, otherwise we
> >> must disable one of the output devices.
> > 
> > That's not really a problem. From a DRM perspective you need to model your
> > device as
> > 
> > ,--.   ,---.   ,-.
> > | CRTC | -+--> | Dummy Encoder | > | Panel Connector |
> > `--?  |`---?   `-?
> >   |,---.   ,-.
> >   \--> | HDMI Encoder  | > | HDMI Connector  |
> >`---?   `-?
> > 
> > The HDMI pipeline is pretty straightforward.
> > 
> > You have told me that the panel has a parallel RGB input without any
> > encoder in the panel pipeline (by the way, which panel model are you
> > using ?). However, DRM requires an encoder in every pipeline. You will
> > thus need to instantiate a dummy encoder. One option would be to set the
> > encoder and connector types to DRM_MODE_ENCODER_LVDS and
> > DRM_MODE_CONNECTOR_LVDS respectively, as that's what userspace usually
> > expects for panels. That doesn't reflect the reality in your case though,
> > so creating a new DRM_MODE_CONNECTOR_DPI type might be needed, possibly
> > to be used with DRM_MODE_ENCODER_NONE.
> > 
> > As neither encoder can modify the mode, the same mode will be output on
> > the two connectors.
> 
> There are still several things to I'd like to understand:
>  1) who's gonna configure the RGB bus output format (RGB444, RGB666,
> RGB888) which directly depends on the device connected on this bus:
> the CRTC or the dummy and HDMI encoders.

Your mileage my vary, but in general I believe this should be the 
responsibility of the CRTC driver (the HLCDC driver in your case), from 
information it gets from DT and/or queries dynamically from the encoders at 
runtime.

>  2) Where should the HDMI encoder/connector support be implemented:
> in drivers/gpu/drm/atmel-hlcdc, drivers/gpu/drm/bridge or somewhere
> else. My point is that I don't want to add specific support for the
> Sil902x transmitter chip in the hlcdc driver.

The HDMI encoder should definitely be handled by a standalone driver. We have 
two infrastructures for this at the moment, drm_bridge and drm_encoder_slave. 
I'd like to see them being merged. I need to implement support for an HDMI 
encoder as well, I'll see if I can give this a try.

> Sorry if these are silly questions, but I'm still trying to understand
> how my case should be modeled :-).

As I don't have straightforward answers I won't consider the questions as 
silly :-)

-- 
Regards,

Laurent Pinchart

[Bug 83205] GPU lockup when entering settings in Verdun game with HyperZ enabled

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=83205

--- Comment #1 from Hohahiu  ---
Hi, Cl?ment,
Marek recently posted patches [1] which are supposed to fix a hang with hyperz.
Could you test them? Also there are some other patches which fix rendering with
hyperz enabled [2]. As far as I understand these two series were reviewed by
not yet mainlined. 
[1] http://lists.freedesktop.org/archives/mesa-dev/2014-August/066519.html
[2] http://lists.freedesktop.org/archives/mesa-dev/2014-August/066389.html

-- 
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/20140829/43b8a431/attachment.html>

[Bug 82050] R9270X pyrit benchmark perf regressions with latest kernel/llvm

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=82050

--- Comment #31 from smoki  ---
(In reply to comment #30)
> Yes.
> 
> Minecraft is unplayable with latest Kernel+latest Mesa.
> 
> In the beginning, it's smooth.. after 30 sec or so it start to stuter a
> little... By the five minutes mark. if you move in the game, it pause for
> like 5 to 7 seconde, move for 2 secondes, pause for 5 secondes...
> 
> By pause I mean the whole system pause, mouse, other terminals.. Everything.
> 
> I think it's related to this bug, it started with the kernel 3.17 (Like the
> OP, I did update mesa, llvm, libdrm, glamor...)
> 
> going back to 3.16 did not fix it, I got to downgrade Mesa and LLVM too (to
> the first relase of the first of this month to be sure) and after that, I
> played for like 4 hours without any issue.
> 
> I tried to revert 150ac07b855b5c5f879bf6ce9ca421ccd1a6c938 but I got massive
> gfx corruptions.

 I only meant about XYZ game which stutter with, and where reverting
150ac07b855b5c5f879bf6ce9ca421ccd1a6c938 helps, if game stutter even
with/without reverting than that is i think another 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/20140829/4969153e/attachment.html>

[Bug 82455] Failed to allocate virtual address for buffer

[bugzilla-dae...@freedesktop.org]

STATUS   = 0x20C0
[  543.791220] radeon :0a:00.0:   SRBM_STATUS2  = 0x
[  543.791238] radeon :0a:00.0:   R_008674_CP_STALLED_STAT1 = 0x
[  543.791256] radeon :0a:00.0:   R_008678_CP_STALLED_STAT2 = 0x4000
[  543.791275] radeon :0a:00.0:   R_00867C_CP_BUSY_STAT = 0x8000
[  543.791293] radeon :0a:00.0:   R_008680_CP_STAT  = 0x80228647
[  543.791312] radeon :0a:00.0:   R_00D034_DMA_STATUS_REG   = 0x44C83D57
[  543.791331] radeon :0a:00.0:   R_00D834_DMA_STATUS_REG   = 0x44C83D57
[  543.791350] radeon :0a:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR  
0x
[  543.791369] radeon :0a:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS
0x
[  544.484393] radeon :0a:00.0: GRBM_SOFT_RESET=0xDDFF
[  544.484468] radeon :0a:00.0: SRBM_SOFT_RESET=0x0100
[  544.485647] radeon :0a:00.0:   GRBM_STATUS   = 0x3028
[  544.485668] radeon :0a:00.0:   GRBM_STATUS_SE0   = 0x0006
[  544.485687] radeon :0a:00.0:   GRBM_STATUS_SE1   = 0x0006
[  544.485705] radeon :0a:00.0:   SRBM_STATUS   = 0x20C0
[  544.485832] radeon :0a:00.0:   SRBM_STATUS2  = 0x
[  544.485851] radeon :0a:00.0:   R_008674_CP_STALLED_STAT1 = 0x
[  544.485869] radeon :0a:00.0:   R_008678_CP_STALLED_STAT2 = 0x
[  544.485887] radeon :0a:00.0:   R_00867C_CP_BUSY_STAT = 0x
[  544.485906] radeon :0a:00.0:   R_008680_CP_STAT  = 0x
[  544.485924] radeon :0a:00.0:   R_00D034_DMA_STATUS_REG   = 0x44C83D57
[  544.485943] radeon :0a:00.0:   R_00D834_DMA_STATUS_REG   = 0x44C83D57
[  544.486089] radeon :0a:00.0: GPU reset succeeded, trying to resume
[  544.499083] [drm] probing gen 2 caps for device 10b5:8648 = 838cd02/0
[  544.499102] [drm] PCIE gen 2 link speeds already enabled
[  544.517700] [drm] PCIE GART of 1024M enabled (table at 0x00276000).
[  544.517913] radeon :0a:00.0: WB enabled
[  544.517939] radeon :0a:00.0: fence driver on ring 0 use gpu addr
0x8c00 and cpu addr 0xf80008880c00
[  544.517961] radeon :0a:00.0: fence driver on ring 1 use gpu addr
0x8c04 and cpu addr 0xf80008880c04
[  544.517984] radeon :0a:00.0: fence driver on ring 2 use gpu addr
0x8c08 and cpu addr 0xf80008880c08
[  544.518005] radeon :0a:00.0: fence driver on ring 3 use gpu addr
0x8c0c and cpu addr 0xf80008880c0c
[  544.518027] radeon :0a:00.0: fence driver on ring 4 use gpu addr
0x8c10 and cpu addr 0xf80008880c10
[  544.532531] radeon :0a:00.0: fence driver on ring 5 use gpu addr
0x00075a18 and cpu addr 0x00ca10075a18
[  544.767646] [drm] ring test on 0 succeeded in 1 usecs
[  544.767669] [drm] ring test on 1 succeeded in 1 usecs
[  544.767689] [drm] ring test on 2 succeeded in 1 usecs
[  544.767773] [drm] ring test on 3 succeeded in 2 usecs
[  544.767803] [drm] ring test on 4 succeeded in 2 usecs
[  544.945163] [drm] ring test on 5 succeeded in 2 usecs
[  544.945186] [drm] UVD initialized successfully.
[  544.945245] [drm] Enabling audio 0 support
[  544.945258] [drm] Enabling audio 1 support
[  544.945270] [drm] Enabling audio 2 support
[  544.945281] [drm] Enabling audio 3 support
[  544.945292] [drm] Enabling audio 4 support
[  544.945304] [drm] Enabling audio 5 support
[  544.945388] [drm] ib test on ring 0 succeeded in 0 usecs
[  544.945487] [drm] ib test on ring 1 succeeded in 0 usecs
[  544.945573] [drm] ib test on ring 2 succeeded in 0 usecs
[  544.945637] [drm] ib test on ring 3 succeeded in 0 usecs
[  544.945699] [drm] ib test on ring 4 succeeded in 0 usecs

-- 
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/20140829/3fb05243/attachment-0001.html>

[Bug 82455] Failed to allocate virtual address for buffer

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=82455

--- Comment #30 from charlie <407883775 at qq.com> ---
>>It is only a white screen for a few minutes.

It is only a white screen for a few second

-- 
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/20140829/a6818ac9/attachment.html>

[Mesa-dev] [PATCH] drm/radeon: Add RADEON_GEM_CPU_ACCESS BO creation flag

[Michel Dänzer]

On 29.08.2014 00:01, Alex Deucher wrote:
> On Thu, Aug 28, 2014 at 4:57 AM, Christian K?nig
>  wrote:
>> Am 28.08.2014 um 08:56 schrieb Michel D?nzer:
>>
>>> From: Michel D?nzer 
>>>
>>> This flag is a hint that userspace expects the BO to be accessed by the
>>> CPU. We can use that hint to prevent such BOs from ever being stored in
>>> the CPU inaccessible part of VRAM.
>>>
>>> Signed-off-by: Michel D?nzer 
>>
>>
>> This patch is Reviewed-by: Christian K?nig 
> 
> Applied to my -next tree.

Thanks!


>> I think we need a similar negative flags as well, e.g.
>> RADEON_GEM_NO_CPU_ACCESS.
>>
>> This way we can stop forcing buffers into the visible VRAM while pinning
>> them for scanout.
> 
> How about the attached patch?

[...]

> diff --git a/drivers/gpu/drm/radeon/radeon_object.c 
> b/drivers/gpu/drm/radeon/radeon_object.c
> index 09b039a..b71e8e0 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.c
> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> @@ -314,10 +314,14 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 
> domain, u64 max_offset,
>   unsigned lpfn = 0;
>  
>   /* force to pin into visible video ram */
> - if (bo->placements[i].flags & TTM_PL_FLAG_VRAM)
> - lpfn = bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;
> - else
> + if (bo->placements[i].flags & TTM_PL_FLAG_VRAM) {
> + if (bo->flags & RADEON_GEM_NO_CPU_ACCESS)
> + lpfn = bo->rdev->mc.real_vram_size >> 
> PAGE_SHIFT;
> + else
> + lpfn = bo->rdev->mc.visible_vram_size >> 
> PAGE_SHIFT;

lpfn can be left at 0 if RADEON_GEM_NO_CPU_ACCESS is set, so this can
be simplified to:

if (!(bo->flags & RADEON_GEM_NO_CPU_ACCESS))
lpfn = bo->rdev->mc.visible_vram_size >> 
PAGE_SHIFT;


> diff --git a/include/uapi/drm/radeon_drm.h b/include/uapi/drm/radeon_drm.h
> index f755f20..d2346fd 100644
> --- a/include/uapi/drm/radeon_drm.h
> +++ b/include/uapi/drm/radeon_drm.h
> @@ -803,6 +803,8 @@ struct drm_radeon_gem_info {
>  #define RADEON_GEM_GTT_WC(1 << 2)
>  /* BO is expected to be accessed by the CPU */
>  #define RADEON_GEM_CPU_ACCESS(1 << 3)
> +/* BO is expected to not be accessed by the CPU */
> +#define RADEON_GEM_NO_CPU_ACCESS (1 << 4)

I'd use stronger wording for this, e.g.

/* CPU access is not expected to work for this BO */


-- 
Earthling Michel D?nzer|  http://www.amd.com
Libre software enthusiast  |Mesa and X developer

[Bug 83184] Screen flickering at low resolution when monitor is attached via VGA dongle to DVI port

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=83184

--- Comment #2 from Rainer K?nig  ---
We have the following physical connectors on board:

- 1 Display Port connector
- 1 DVI-I connector (which is used with a DVI->VGA dongle when the problem
occurs)
- 1 DP to LVDS connector

A detailled technical spec is available here:
ftp://ftp.ts.fujitsu.com/pub/Mainboard-OEM-Sales/Products/Mainboards/Industrial/D3003-S/Documentation_D3003-S/TechNotes/TechNotes_V3.1_Mini-ITX_D3003-S.pdf

-- 
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/20140829/5c7a8e5e/attachment.html>

[PATCH v2 16/17] drm/exynos/ipp: remove file argument from node related functions

[Joonyoung Shim]

Hi Andrzej,

On 08/28/2014 06:07 PM, Andrzej Hajda wrote:
> Since file pointer is preserved in c_node passing it
> as argument in node functions is redundant.
> 
> Signed-off-by: Andrzej Hajda 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_ipp.c | 14 ++
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c 
> b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> index 05f0f4e..fc8bb67 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> @@ -529,7 +529,6 @@ static int ipp_put_mem_node(struct drm_device *drm_dev,
>  
>  static struct drm_exynos_ipp_mem_node
>   *ipp_get_mem_node(struct drm_device *drm_dev,
> - struct drm_file *file,
>   struct drm_exynos_ipp_cmd_node *c_node,
>   struct drm_exynos_ipp_queue_buf *qbuf)
>  {
> @@ -560,7 +559,7 @@ static struct drm_exynos_ipp_mem_node
>   dma_addr_t *addr;
>  
>   addr = exynos_drm_gem_get_dma_addr(drm_dev,
> - qbuf->handle[i], file);
> + qbuf->handle[i], c_node->filp);
>   if (IS_ERR(addr)) {
>   DRM_ERROR("failed to get addr.\n");
>   ipp_put_mem_node(drm_dev, c_node, m_node);
> @@ -606,7 +605,6 @@ static void ipp_free_event(struct drm_pending_event 
> *event)
>  }
>  
>  static int ipp_get_event(struct drm_device *drm_dev,
> - struct drm_file *file,
>   struct drm_exynos_ipp_cmd_node *c_node,
>   struct drm_exynos_ipp_queue_buf *qbuf)
>  {
> @@ -618,7 +616,7 @@ static int ipp_get_event(struct drm_device *drm_dev,
>   e = kzalloc(sizeof(*e), GFP_KERNEL);
>   if (!e) {
>   spin_lock_irqsave(_dev->event_lock, flags);
> - file->event_space += sizeof(e->event);
> + c_node->filp->event_space += sizeof(e->event);
>   spin_unlock_irqrestore(_dev->event_lock, flags);
>   return -ENOMEM;
>   }
> @@ -630,7 +628,7 @@ static int ipp_get_event(struct drm_device *drm_dev,
>   e->event.prop_id = qbuf->prop_id;
>   e->event.buf_id[EXYNOS_DRM_OPS_DST] = qbuf->buf_id;
>   e->base.event = >event.base;
> - e->base.file_priv = file;
> + e->base.file_priv = c_node->filp;
>   e->base.destroy = ipp_free_event;
>   mutex_lock(_node->event_lock);
>   list_add_tail(>base.link, _node->event_list);
> @@ -899,7 +897,7 @@ int exynos_drm_ipp_queue_buf(struct drm_device *drm_dev, 
> void *data,
>   /* find command node */
>   c_node = ipp_find_obj(>prop_idr, >prop_lock,
>   qbuf->prop_id);
> - if (!c_node) {
> + if (!c_node || c_node->filp != file) {

I think this should go to patch 17.

Thanks.

>   DRM_ERROR("failed to get command node.\n");
>   return -ENODEV;
>   }
> @@ -908,7 +906,7 @@ int exynos_drm_ipp_queue_buf(struct drm_device *drm_dev, 
> void *data,
>   switch (qbuf->buf_type) {
>   case IPP_BUF_ENQUEUE:
>   /* get memory node */
> - m_node = ipp_get_mem_node(drm_dev, file, c_node, qbuf);
> + m_node = ipp_get_mem_node(drm_dev, c_node, qbuf);
>   if (IS_ERR(m_node)) {
>   DRM_ERROR("failed to get m_node.\n");
>   return PTR_ERR(m_node);
> @@ -921,7 +919,7 @@ int exynos_drm_ipp_queue_buf(struct drm_device *drm_dev, 
> void *data,
>*/
>   if (qbuf->ops_id == EXYNOS_DRM_OPS_DST) {
>   /* get event for destination buffer */
> - ret = ipp_get_event(drm_dev, file, c_node, qbuf);
> + ret = ipp_get_event(drm_dev, c_node, qbuf);
>   if (ret) {
>   DRM_ERROR("failed to get event.\n");
>   goto err_clean_node;
> 

[PATCH v2 17/17] drm/exynos/ipp: add file checks for ioctls

[Joonyoung Shim]

Hi Andrzej,

On 08/28/2014 06:07 PM, Andrzej Hajda wrote:
> Process should not have access to ipp nodes created by another
> process. The patch adds necessary checks.
> 
> Signed-off-by: Andrzej Hajda 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_ipp.c | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c 
> b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> index fc8bb67..d233cfc 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
> @@ -318,7 +318,8 @@ static void ipp_print_property(struct 
> drm_exynos_ipp_property *property,
>   sz->hsize, sz->vsize, config->flip, config->degree);
>  }
>  
> -static int ipp_find_and_set_property(struct drm_exynos_ipp_property 
> *property)
> +static int ipp_find_and_set_property(struct drm_exynos_ipp_property 
> *property,
> + struct drm_file *file)

This is ok, but i think ipp_find_and_set_property function is some
duplicated. If we add function to get c_node from struct
exynos_drm_ippdrv, it's easy to remove ipp_find_and_set_property.

Thanks.

>  {
>   struct exynos_drm_ippdrv *ippdrv;
>   struct drm_exynos_ipp_cmd_node *c_node;
> @@ -339,8 +340,12 @@ static int ipp_find_and_set_property(struct 
> drm_exynos_ipp_property *property)
>*/
>   mutex_lock(>cmd_lock);
>   list_for_each_entry(c_node, >cmd_list, list) {
> - if ((c_node->property.prop_id == prop_id) &&
> - (c_node->state == IPP_STATE_STOP)) {
> + if (c_node->property.prop_id == prop_id) {
> + if (c_node->filp != file)
> + break;
> + if (c_node->state != IPP_STATE_STOP)
> + break;
> +
>   mutex_unlock(>cmd_lock);
>   DRM_DEBUG_KMS("found cmd[%d]ippdrv[0x%x]\n",
>   property->cmd, (int)ippdrv);
> @@ -418,7 +423,7 @@ int exynos_drm_ipp_set_property(struct drm_device 
> *drm_dev, void *data,
>*/
>   if (property->prop_id) {
>   DRM_DEBUG_KMS("prop_id[%d]\n", property->prop_id);
> - return ipp_find_and_set_property(property);
> + return ipp_find_and_set_property(property, file);
>   }
>  
>   /* find ipp driver using ipp id */
> @@ -1032,7 +1037,7 @@ int exynos_drm_ipp_cmd_ctrl(struct drm_device *drm_dev, 
> void *data,
>  
>   c_node = ipp_find_obj(>prop_idr, >prop_lock,
>   cmd_ctrl->prop_id);
> - if (!c_node) {
> + if (!c_node || c_node->filp != file) {
>   DRM_ERROR("invalid command node list.\n");
>   return -ENODEV;
>   }
> 

[Bug 75276] Implement VGPR Register Spilling

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=75276

--- Comment #38 from t3st at sogetthis.com ---
Looks like I stepped on similar issue at
http://llvm.org/bugs/show_bug.cgi?id=20738 when dealing with OpenCL and rather
filed bug against LLVM (not that like if they have right sections for AMD GPUs
in theie bugzilla, but it seems to be LLVM issue, yeah?...).

Basically, running simple CL memory benchmark causes huge flood by error
messages from LLVM and then GPU locks up, which is not fun at all, obviously.

-- 
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/20140829/291b42bb/attachment.html>

[PATCH] drm: add checking DRM_FORMAT_NV12MT

[Joonyoung Shim]

Hi Dave,

On 07/29/2014 01:08 AM, Ville Syrj?l? wrote:
> On Mon, Jul 28, 2014 at 11:56:59AM -0400, Rob Clark wrote:
>> On Mon, Jul 28, 2014 at 12:47 AM, Joonyoung Shim
>>  wrote:
>>> If user NV12MT uses as pixel format, the Addfb2 ioctl is failed because
>>> of missing to check DRM_FORMAT_NV12MT. The NV12MT pixel format is
>>> supported by exynos4 and some qualcomm chipset and it is used by exynos
>>> drm driver.

If there is no any objection, could you merge this?

Thanks.

>>
>> tbh, format_check() should probably just be made to respect the
>> formats advertised by all the planes..
> 
> That can't be done until all drivers are converted to primary/cursor
> planes. Also I'm not sure if we should allow it even then since that
> would make it quite easy to sneak in new driver specific formats
> without anyone necessarily reviewing them.
> 
>>
>> BR,
>> -R
>>
>>> Signed-off-by: Joonyoung Shim 
>>> ---
>>>  drivers/gpu/drm/drm_crtc.c | 5 +
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>>> index 1ccf5cb..5d7bd49 100644
>>> --- a/drivers/gpu/drm/drm_crtc.c
>>> +++ b/drivers/gpu/drm/drm_crtc.c
>>> @@ -2960,6 +2960,7 @@ static int format_check(const struct drm_mode_fb_cmd2 
>>> *r)
>>> case DRM_FORMAT_NV61:
>>> case DRM_FORMAT_NV24:
>>> case DRM_FORMAT_NV42:
>>> +   case DRM_FORMAT_NV12MT:
>>> case DRM_FORMAT_YUV410:
>>> case DRM_FORMAT_YVU410:
>>> case DRM_FORMAT_YUV411:
>>> @@ -4800,6 +4801,7 @@ int drm_format_num_planes(uint32_t format)
>>> case DRM_FORMAT_NV61:
>>> case DRM_FORMAT_NV24:
>>> case DRM_FORMAT_NV42:
>>> +   case DRM_FORMAT_NV12MT:
>>> return 2;
>>> default:
>>> return 1;
>>> @@ -4835,6 +4837,7 @@ int drm_format_plane_cpp(uint32_t format, int plane)
>>> case DRM_FORMAT_NV61:
>>> case DRM_FORMAT_NV24:
>>> case DRM_FORMAT_NV42:
>>> +   case DRM_FORMAT_NV12MT:
>>> return plane ? 2 : 1;
>>> case DRM_FORMAT_YUV410:
>>> case DRM_FORMAT_YVU410:
>>> @@ -4878,6 +4881,7 @@ int drm_format_horz_chroma_subsampling(uint32_t 
>>> format)
>>> case DRM_FORMAT_NV21:
>>> case DRM_FORMAT_NV16:
>>> case DRM_FORMAT_NV61:
>>> +   case DRM_FORMAT_NV12MT:
>>> case DRM_FORMAT_YUV422:
>>> case DRM_FORMAT_YVU422:
>>> case DRM_FORMAT_YUV420:
>>> @@ -4907,6 +4911,7 @@ int drm_format_vert_chroma_subsampling(uint32_t 
>>> format)
>>> case DRM_FORMAT_YVU420:
>>> case DRM_FORMAT_NV12:
>>> case DRM_FORMAT_NV21:
>>> +   case DRM_FORMAT_NV12MT:
>>> return 2;
>>> default:
>>> return 1;
>>> --
>>> 1.8.1.2
>>>
>>> ___
>>> dri-devel mailing list
>>> dri-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

[Bug 83226] New: Allow use of ColorRange and ColorSpace in xorg.conf.d files

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=83226

  Priority: medium
Bug ID: 83226
  Assignee: dri-devel at lists.freedesktop.org
   Summary: Allow use of ColorRange and ColorSpace in xorg.conf.d
files
  Severity: enhancement
Classification: Unclassified
OS: Linux (All)
  Reporter: john.ettedgui at gmail.com
  Hardware: All
Status: NEW
   Version: unspecified
 Component: Drivers/Gallium/radeonsi
   Product: Mesa

Hello,

This a feature request, I hope it is the right place.

As my main computer is connected to an HD-TV that only supports limited range
of colors, I would like to be able to use limited range with radeon(si) but it
is not yet available unlike for Intel or Nvidia.
For now I'm able to set media players to limited range, but I would like to be
able to do it at the system level.

Being able to switch the color space would also be nice.

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/20140829/76a8f987/attachment.html>

[PATCH 2/9] drm/i915: trivial: remove unneed set to NULL

[Jani Nikula]

On Thu, 28 Aug 2014, Gustavo Padovan  wrote:
> From: Gustavo Padovan 
>
> At this point of the code the obj var is already NULL, so we don't
> need to set it again to NULL.

Reviewed-by: Jani Nikula 

> Signed-off-by: Gustavo Padovan 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index b2e4eac..05937fe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8253,7 +8253,6 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc 
> *crtc,
>   if (!obj) {
>   DRM_DEBUG_KMS("cursor off\n");
>   addr = 0;
> - obj = NULL;
>   mutex_lock(>struct_mutex);
>   goto finish;
>   }
> -- 
> 1.9.3
>
> ___
> 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 3/9] drm/i915: fix memleak in intel_set_config_save_state()

[Jani Nikula]

On Thu, 28 Aug 2014, Gustavo Padovan  wrote:
> From: Gustavo Padovan 
>
> If the save_encoder_crtcs or save_connector_encoders allocation fail
> we need to free everything we have already allocated.

There is no memleak, because the caller of intel_set_config_save_state()
checks the return value, and subsequently handles the error with a call
to intel_set_config_free(), which does the right thing.

I know one could argue this should be done within
intel_set_config_save_state() but I'm not convinced. I'd let this be as
it is.

Just in case Daniel disagrees with me here and wants to merge, the patch
does look correct. So r-b for correctness, but nak on merging from
me. ;)

BR,
Jani.


>
> Signed-off-by: Gustavo Padovan 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 05937fe..a8a8abe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11018,13 +11018,13 @@ static int intel_set_config_save_state(struct 
> drm_device *dev,
>   kcalloc(dev->mode_config.num_encoder,
>   sizeof(struct drm_crtc *), GFP_KERNEL);
>   if (!config->save_encoder_crtcs)
> - return -ENOMEM;
> + goto free_crtc;
>  
>   config->save_connector_encoders =
>   kcalloc(dev->mode_config.num_connector,
>   sizeof(struct drm_encoder *), GFP_KERNEL);
>   if (!config->save_connector_encoders)
> - return -ENOMEM;
> + goto free_encoder;
>  
>   /* Copy data. Note that driver private data is not affected.
>* Should anything bad happen only the expected state is
> @@ -11046,6 +11046,12 @@ static int intel_set_config_save_state(struct 
> drm_device *dev,
>   }
>  
>   return 0;
> +
> +free_encoder:
> + kfree(config->save_encoder_crtcs);
> +free_crtc:
> + kfree(config->save_crtc_enabled);
> + return -ENOMEM;
>  }
>  
>  static void intel_set_config_restore_state(struct drm_device *dev,
> -- 
> 1.9.3
>
> ___
> 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

[Intel-gfx] [PATCH 3/9] drm/i915: fix memleak in intel_set_config_save_state()

[Chris Wilson]

On Fri, Aug 29, 2014 at 10:38:43AM +0300, Jani Nikula wrote:
> On Thu, 28 Aug 2014, Gustavo Padovan  wrote:
> > From: Gustavo Padovan 
> >
> > If the save_encoder_crtcs or save_connector_encoders allocation fail
> > we need to free everything we have already allocated.
> 
> There is no memleak, because the caller of intel_set_config_save_state()
> checks the return value, and subsequently handles the error with a call
> to intel_set_config_free(), which does the right thing.
> 
> I know one could argue this should be done within
> intel_set_config_save_state() but I'm not convinced. I'd let this be as
> it is.
> 
> Just in case Daniel disagrees with me here and wants to merge, the patch
> does look correct. So r-b for correctness, but nak on merging from
> me. ;)

You just said it triggers a double free... And you would be right.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 4/9] drm/i915: split intel_update_plane into check() and commit()

[Ville Syrjälä]

On Thu, Aug 28, 2014 at 02:40:08PM -0300, 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.
> 
> Signed-off-by: Gustavo Padovan 
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 128 
> ++--
>  1 file changed, 93 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> b/drivers/gpu/drm/i915/intel_sprite.c
> index 4cbe286..b1cb606 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -844,22 +844,32 @@ static bool colorkey_enabled(struct intel_plane 
> *intel_plane)
>   return key.flags != I915_SET_COLORKEY_NONE;
>  }
>  
> +static bool get_visible(struct drm_rect *src, struct drm_rect *dst,

get_visisble() is not a good name here IMO, also I think this split is at
a slightly too low level. What we really want is to start creating some
kind of plane config struct that can be passed to both check and commit,
and then check can already store all the clipped coordinates etc. to the
plane config and commit can just look them up w/o recomputing them.

Initially you could just have one such struct on the stack in
intel_update_plane() and pass it to both functions. Later we'll need to
figure out how to pass around the plane configs for all planes during
the nuclear flip, but there's no need to worry about such things quite yet.

> + const struct drm_rect *clip,
> + int min_scale, int max_scale)
> +{
> + int hscale, vscale;
> +
> + 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);
> + BUG_ON(vscale < 0);
> +
> + return drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
> +}
> +
>  static int
> -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> +intel_check_sprite_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)
>  {
> - struct drm_device *dev = plane->dev;
>   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   struct intel_plane *intel_plane = to_intel_plane(plane);
> - enum pipe pipe = intel_crtc->pipe;
>   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 hscale, vscale;
>   int max_scale, min_scale;
> @@ -882,20 +892,6 @@ intel_update_plane(struct drm_plane *plane, struct 
> drm_crtc *crtc,
>   .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) {
> @@ -930,13 +926,7 @@ intel_update_plane(struct drm_plane *plane, struct 
> drm_crtc *crtc,
>   drm_rect_rotate(, fb->width << 16, fb->height << 16,
>   intel_plane->rotation);
>  
> - hscale = drm_rect_calc_hscale_relaxed(, , min_scale, max_scale);
> - BUG_ON(hscale < 0);
> -
> - vscale = drm_rect_calc_vscale_relaxed(, , min_scale, max_scale);
> - BUG_ON(vscale < 0);
> -
> - visible = drm_rect_clip_scaled(, , , hscale, vscale);
> + visible = get_visible(, , , max_scale, min_scale);
>  
>   crtc_x = dst.x1;
>   crtc_y = dst.y1;
> @@ -1027,6 +1017,54 @@ intel_update_plane(struct drm_plane *plane, struct 
> drm_crtc *crtc,
>   }
>   }
>  
> + return 0;
> +}
> +
> +static int
> +intel_commit_sprite_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)
> +{
> + struct drm_device *dev = plane->dev;
> + struct intel_crtc 

[Intel-gfx] [PATCH 9/9] drm/i915: split intel_pipe_set_base() into check() and commit()

[Ville Syrjälä]

On Thu, Aug 28, 2014 at 02:40:13PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> Take out some parts of  code that can fail from it and move them to
> intel_pipe_check_base(), the only failure point in intel_pipe_set_base()
> now is the fb pinning procudure.

I'd like to see intel_pipe_set_base() replaced entirely with the
primary plane setplane. Right now we have duplicated code paths all over
the place due to intel_pipe_set_base().

After that's done the same plane config treatment should be applied to
primary planes as I suggested for sprites.

> 
> Signed-off-by: Gustavo Padovan 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 39 
> 
>  1 file changed, 31 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index eb6febf..ead2f24 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2661,17 +2661,11 @@ static bool intel_crtc_has_pending_flip(struct 
> drm_crtc *crtc)
>  }
>  
>  static int
> -intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
> +intel_pipe_check_base(struct drm_crtc *crtc, int x, int y,
>   struct drm_framebuffer *fb)
>  {
>   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 = crtc->primary->fb;
> - struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> - struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
> - int ret;
>  
>   if (intel_crtc_has_pending_flip(crtc)) {
>   DRM_ERROR("pipe is still busy with an old pageflip\n");
> @@ -2691,6 +2685,22 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int 
> y,
>   return -EINVAL;
>   }
>  
> + return 0;
> +}
> +
> +static int
> +intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
> + struct drm_framebuffer *fb)
> +{
> + 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 = crtc->primary->fb;
> + struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> + struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
> + int ret;
> +
>   mutex_lock(>struct_mutex);
>   ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
>   if (ret == 0)
> @@ -9868,6 +9878,10 @@ free_work:
>   if (ret == -EIO) {
>  out_hang:
>   intel_crtc_wait_for_pending_flips(crtc);
> + ret = intel_pipe_check_base(crtc, crtc->x, crtc->y, fb);
> + if (ret)
> + return ret;
> +
>   ret = intel_pipe_set_base(crtc, crtc->x, crtc->y, fb);
>   if (ret == 0 && event)
>   drm_send_vblank_event(dev, pipe, event);
> @@ -11396,6 +11410,10 @@ static int intel_crtc_set_config(struct drm_mode_set 
> *set)
>  
>   intel_crtc_wait_for_pending_flips(set->crtc);
>  
> + ret = intel_pipe_check_base(set->crtc, set->x, set->y, set->fb);
> + if (ret)
> + goto fail;
> +
>   ret = intel_pipe_set_base(set->crtc,
> set->x, set->y, set->fb);
>  
> @@ -11620,12 +11638,17 @@ intel_check_primary_plane(struct drm_plane *plane, 
> struct drm_crtc *crtc,
>   .x2 = intel_crtc->active ? intel_crtc->config.pipe_src_w : 0,
>   .y2 = intel_crtc->active ? intel_crtc->config.pipe_src_h : 0,
>   };
> + int ret;
>  
> - return drm_plane_helper_check_update(plane, crtc, fb,
> + ret = drm_plane_helper_check_update(plane, crtc, fb,
>   , , ,
>   DRM_PLANE_HELPER_NO_SCALING,
>   DRM_PLANE_HELPER_NO_SCALING,
>   false, true, visible);
> + if (ret)
> + return ret;
> +
> + return intel_pipe_check_base(crtc, src.x1, src.y1, fb);
>  }
>  
>  static int
> -- 
> 1.9.3
> 
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrj?l?
Intel OTC

drm: sti: add HDMI driver

[Dan Carpenter]

Hello Benjamin Gaignard,

The patch 5402626c83a2: "drm: sti: add HDMI driver" from Jul 30,
2014, leads to the following static checker warning:

drivers/gpu/drm/sti/sti_hdmi.c:301 hdmi_avi_infoframe_config()
error: buffer overflow 'frame' 13 <= 13

drivers/gpu/drm/sti/sti_hdmi.c
   298  hdmi_write(hdmi, val, 
HDMI_SW_DI_N_PKT_WORD2(HDMI_IFRAME_SLOT_AVI));
   299  
   300  val = frame[0xC];
   301  val |= frame[0xD] << 8;

frame[] only has 0xD elements so we are one element past the end here.

   302  hdmi_write(hdmi, val, 
HDMI_SW_DI_N_PKT_WORD3(HDMI_IFRAME_SLOT_AVI));
   303  

regards,
dan carpenter

[Bug 83234] New: Opening Steam crashes X session

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=83234

  Priority: medium
Bug ID: 83234
  Assignee: dri-devel at lists.freedesktop.org
   Summary: Opening Steam crashes X session
  Severity: normal
Classification: Unclassified
OS: Linux (All)
  Reporter: gutigen at outlook.com
  Hardware: x86-64 (AMD64)
Status: NEW
   Version: git
 Component: Drivers/Gallium/radeonsi
   Product: Mesa

Created attachment 105418
  --> https://bugs.freedesktop.org/attachment.cgi?id=105418=edit
dmesg

Instantly after opening Steam X crashes. OpenGL apps work fine, tested with
Unigine Heaven (64 bit) and Xonotic (32 and 64bit).

-- 
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/20140829/1a31fb83/attachment.html>

[Bug 83234] Opening Steam crashes X session

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=83234

--- Comment #1 from Maciej  ---
Created attachment 105419
  --> https://bugs.freedesktop.org/attachment.cgi?id=105419=edit
Xorg.0.log

-- 
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/20140829/b58d8c5c/attachment.html>

[Bug 83234] Opening Steam crashes X session

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=83234

--- Comment #2 from Maciej  ---
Forgot to add, system is Ubuntu 14.10.

-- 
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/20140829/e0fb93d8/attachment.html>

[PATCHv2 0/8] Upstreaming the Android build and misc fixes

[Gore, Tim]

Hi Emil, sorry for the delay.
I still see the same patch error for patch 3/8. The patch diff
for intel/makefile.am has as part of its context:

#Eric Anholt 

Whereas the source file has

#Eric Anholt 

So the patch fails. Its trivial of course. Perhaps there is some automatic
Substitution getting done by some tool. Maybe git-am will automatically
Fix this, not sure, I don't have a workflow for git-am.
My source comes from
   Git://anongit.freedesktop.org/git/mesa/drm
Remote =origin
Merge = refs/heads/master

Other than that, as I say the patches look fine, and I have successfully built
Libdrm in an Android tree using these patches and "mm".
I am out of the office from 12:00 until Thurs 4th Sept now.

  Tim

> -Original Message-
> From: Emil Velikov [mailto:emil.l.velikov at gmail.com]
> Sent: Sunday, August 24, 2014 7:40 PM
> To: Gore, Tim; dri-devel at lists.freedesktop.org
> Cc: emil.l.velikov at gmail.com; Daniel Vetter (daniel at ffwll.ch)
> Subject: Re: [PATCHv2 0/8] Upstreaming the Android build and misc fixes
> 
> Hi Tim,
> 
> Gave the series another try, and there was some minor issues caused by
> commit
> 5d8357976a8 (configure: Support symbol visibility when available). Did not
> see any of the originally mentioned issues. Are you sure that you've tried it
> on top of fd.o/drm/master ?
> 
> Do you have any comments, would like to do any testing (note it will not
> work due to the mmap/mmap64 bionic issue, to be addressed next) or
> should I go ahead and commit the series ?
> 
> Thanks
> Emil
> 
> The series:
> http://lists.freedesktop.org/archives/dri-devel/2014-July/064952.html
> 
> Patchv3 3/8:
> http://lists.freedesktop.org/archives/dri-devel/2014-August/066748.html
> 
> 
> On 31/07/14 19:32, Emil Velikov wrote:
> > Strange, I've explicitly made sure that it's rebased on top of 
> > libdrm/master.
> > I will give it another try in a moment.
> >
> > FWIW, for Intel at least a mmap/mmap64 would be needed due to the
> > bionic shortage of their original implementation.
> >
> > As mentioned earlier, I'm seeking for a few acked/reviewed-by and the
> > rest of the fixes will follow in due time. Can I consider this acked-by ?
> >
> > Thanks for having a look
> > -Emil
> >
> > On 31/07/14 16:33, Gore, Tim wrote:
> >> Ok, I still had to manually apply some of patch 3/8 as it was
> >> corrupted And would not apply, but after this I was able to build
> >> within one of My android trees. I have not tested the resulting
> >> libraries yet, but the Overall build process seems ok.
> >>
> >>   Tim
> >>
> >>> -Original Message-
> >>> From: Emil Velikov [mailto:emil.l.velikov at gmail.com]
> >>> Sent: Tuesday, July 29, 2014 6:27 PM
> >>> To: dri-devel at lists.freedesktop.org
> >>> Cc: Gore, Tim
> >>> Subject: [PATCHv2 0/8] Upstreaming the Android build and misc fixes
> >>>
> >>> Changes since the orignal posting:
> >>>  - Rebased on top of master.
> >>>  - Used _H_FILES for header lists (_HEADERS is a no-go with
> >>> autotools)
> >>>  - Install the freedreno headers to {include_dir}/freedreno, similar
> >>> to the automake builds.
> >>>  - Correctly include $(hw)/Android.mk
> >>>
> >>> The series is also available at the fixes+android-v2 branch in
> >>> https://github.com/evelikov/libdrm.
> >>>
> >>> -Emil
> >>
> >

[Intel-gfx] [PATCH 3/9] drm/i915: fix memleak in intel_set_config_save_state()

[Jani Nikula]

On Fri, 29 Aug 2014, Chris Wilson  wrote:
> On Fri, Aug 29, 2014 at 10:38:43AM +0300, Jani Nikula wrote:
>> On Thu, 28 Aug 2014, Gustavo Padovan  wrote:
>> > From: Gustavo Padovan 
>> >
>> > If the save_encoder_crtcs or save_connector_encoders allocation fail
>> > we need to free everything we have already allocated.
>> 
>> There is no memleak, because the caller of intel_set_config_save_state()
>> checks the return value, and subsequently handles the error with a call
>> to intel_set_config_free(), which does the right thing.
>> 
>> I know one could argue this should be done within
>> intel_set_config_save_state() but I'm not convinced. I'd let this be as
>> it is.
>> 
>> Just in case Daniel disagrees with me here and wants to merge, the patch
>> does look correct. So r-b for correctness, but nak on merging from
>> me. ;)
>
> You just said it triggers a double free... And you would be right.

Thanks Chris. I'll retract any hint of r-b I was giving. NAK.

BR,
Jani.


> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCHv2 0/8] Upstreaming the Android build and misc fixes

[Emil Velikov]

Hi Tim,

On 29/08/14 10:44, Gore, Tim wrote:
> Hi Emil, sorry for the delay.
> I still see the same patch error for patch 3/8. The patch diff
> for intel/makefile.am has as part of its context:
> 
> #Eric Anholt 
> 
> Whereas the source file has
> 
> #Eric Anholt 
> 
> So the patch fails. Its trivial of course. Perhaps there is some automatic
> Substitution getting done by some tool.
The issue you're seeing here is (afaik) an anti-spam feature. The ML
software/parser automatically substitutes/obscures email addresses.

For any future case, I would recommend adding a remote branch rather than
picking patches from mailling-lists - not all of them play nice :\

> Maybe git-am will automatically
> Fix this, not sure, I don't have a workflow for git-am.
> My source comes from
>Git://anongit.freedesktop.org/git/mesa/drm
> Remote =origin
> Merge = refs/heads/master
> 
> Other than that, as I say the patches look fine, and I have successfully built
> Libdrm in an Android tree using these patches and "mm".
> I am out of the office from 12:00 until Thurs 4th Sept now.
> 
Nice. They build fine on my end as well. I've even had a couple of people test
them (with a mmap patch on top) alongside mesa 10.2 :)

I was hoping for an acked-by/reviewed-by, but it seems like I'll push the
series as is.

Thanks
Emil

>   Tim
> 

[PATCH 00/20] DRM: Core Cleanups

[David Herrmann]

Hi

More cleanups of DRM core code. Diffstat says:
 72 files changed, 832 insertions(+), 1038 deletions(-)

..which is already nice, but doesn't reflect that a lot of code is now hidden
from main headers. Furthermore, with this series applied, drmP.h no longer looks
as ugly as it is now (widly unstructured). It still ain't a beauty, though:
 include/drm/drmP.h   | 236 ---

Anyhow, highlights:
 - drop "drm_memory.h"
 - drop "drm_usb.c"
 - drop "struct drm_waitlist"
 - drop "struct drm_sigdata"
 - drop "struct drm_bus"
 - drop "drm_master->unique_size"

Two patches are driver related:
 - radeon: Patch #1, moves drm_buffer.c to radeon/
 - nouveau: Patch #17, splits drm_driver for nouveau
They're rather trivial. Everything else is DRM-core.

I've moved some legacy stuff around so it's easier to drop it once unused. If
someone disagrees with move, let me know and I'll drop the patch.

I've pushed this to the 0-day testing bots now. I'll comment on the patches in
case something shows up.

Comments welcome!
David


David Herrmann (20):
  drm/radeon: move drm_buffer to drm/radeon/
  drm: mark drm_buf and drm_map as legacy
  drm: move "struct drm_vma_entry" to drm_vm.c
  drm: move "struct drm_magic_entry" to drm_auth.c
  drm: drop unused "struct drm_waitlist"
  drm: move AGP definitions harder
  drm: replace weird conditional includes
  drm: drop __KERNEL__ protection in drmP.h
  drm: merge drm_memory.h into drm_memory.c
  drm: move __OS_HAS_AGP into drm_agpsupport.h
  drm: order includes alphabetically in drmP.h
  drm: drop DRM_DEBUG_CODE
  drm: inline "struct drm_sigdata"
  drm: move remaining includes in drmP.h to the top
  drm: simplify drm_*_set_unique()
  drm: drop unused drm_master->unique_size
  drm: add driver->set_busid() callback
  drm: Goody bye, drm_bus!
  drm: merge drm_usb into udl
  drm: move drm-lock API to drm_legacy.h

 Documentation/DocBook/drm.tmpl   |   3 +-
 drivers/gpu/drm/Kconfig  |   6 -
 drivers/gpu/drm/Makefile |   5 +-
 drivers/gpu/drm/armada/armada_drv.c  |   1 +
 drivers/gpu/drm/ast/ast_drv.c|   1 +
 drivers/gpu/drm/bochs/bochs_drv.c|   1 +
 drivers/gpu/drm/cirrus/cirrus_drv.c  |   1 +
 drivers/gpu/drm/drm_agpsupport.c |   1 +
 drivers/gpu/drm/drm_auth.c   |   6 +
 drivers/gpu/drm/drm_buffer.c | 181 
 drivers/gpu/drm/drm_bufs.c   |  89 ++--
 drivers/gpu/drm/drm_debugfs.c|   2 -
 drivers/gpu/drm/drm_drv.c|  11 +-
 drivers/gpu/drm/drm_fops.c   |  16 +--
 drivers/gpu/drm/drm_info.c   |  59 
 drivers/gpu/drm/drm_ioctl.c  |  27 ++--
 drivers/gpu/drm/drm_legacy.h |  39 +
 drivers/gpu/drm/drm_lock.c   |  35 ++---
 drivers/gpu/drm/drm_memory.c |  12 ++
 drivers/gpu/drm/drm_pci.c|  41 ++
 drivers/gpu/drm/drm_platform.c   |  38 ++---
 drivers/gpu/drm/drm_usb.c|  88 
 drivers/gpu/drm/drm_vm.c |  74 ++
 drivers/gpu/drm/exynos/exynos_drm_drv.c  |   1 +
 drivers/gpu/drm/gma500/psb_drv.c |   1 +
 drivers/gpu/drm/i810/i810_dma.c  |   4 +-
 drivers/gpu/drm/i810/i810_drv.c  |   1 +
 drivers/gpu/drm/i915/i915_dma.c  |   2 +-
 drivers/gpu/drm/i915/i915_drv.c  |   1 +
 drivers/gpu/drm/mga/mga_dma.c|  49 +++
 drivers/gpu/drm/mga/mga_drv.c|   1 +
 drivers/gpu/drm/mgag200/mgag200_drv.c|   1 +
 drivers/gpu/drm/msm/msm_drv.c|   1 +
 drivers/gpu/drm/nouveau/nouveau_drm.c|  19 ++-
 drivers/gpu/drm/omapdrm/omap_drv.c   |   1 +
 drivers/gpu/drm/qxl/qxl_drv.c|   1 +
 drivers/gpu/drm/r128/r128_cce.c  |   2 +-
 drivers/gpu/drm/r128/r128_drv.c  |   1 +
 drivers/gpu/drm/radeon/Makefile  |   2 +-
 drivers/gpu/drm/radeon/drm_buffer.c  | 177 +++
 drivers/gpu/drm/radeon/drm_buffer.h  | 148 +++
 drivers/gpu/drm/radeon/r300_cmdbuf.c |   2 +-
 drivers/gpu/drm/radeon/r600_cp.c |   2 +-
 drivers/gpu/drm/radeon/radeon.h  |  17 ++-
 drivers/gpu/drm/radeon/radeon_cp.c   |  22 +--
 drivers/gpu/drm/radeon/radeon_drv.c  |   2 +
 drivers/gpu/drm/radeon/radeon_ring.c |  21 ---
 drivers/gpu/drm/radeon/radeon_state.c|   2 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.c|   1 +
 drivers/gpu/drm/savage/savage_bci.c  |  23 +--
 drivers/gpu/drm/savage/savage_drv.c  |   1 +
 drivers/gpu/drm/shmobile/shmob_drm_drv.c |   1 +
 drivers/gpu/drm/sis/sis_drv.c|   1 +
 drivers/gpu/drm/sis/sis_mm.c |   6 +-
 drivers/gpu/drm/tdfx/tdfx_drv.c  |   1 +
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  |   1 +
 drivers/gpu/drm/udl/Kconfig  |   3 +-
 drivers/gpu/drm/udl/udl_connector.c  |   4 +-
 drivers/gpu/drm/udl/udl_drv.c   

[PATCH 01/20] drm/radeon: move drm_buffer to drm/radeon/

[David Herrmann]

Radeon UMS is the last user of drm_buffer. Move it out of sight so radeon
can drop it together with UMS.

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/Makefile  |   2 +-
 drivers/gpu/drm/drm_buffer.c  | 181 --
 drivers/gpu/drm/radeon/Makefile   |   2 +-
 drivers/gpu/drm/radeon/drm_buffer.c   | 177 +
 drivers/gpu/drm/radeon/drm_buffer.h   | 148 +++
 drivers/gpu/drm/radeon/r300_cmdbuf.c  |   2 +-
 drivers/gpu/drm/radeon/radeon_state.c |   2 +-
 include/drm/drm_buffer.h  | 148 ---
 8 files changed, 329 insertions(+), 333 deletions(-)
 delete mode 100644 drivers/gpu/drm/drm_buffer.c
 create mode 100644 drivers/gpu/drm/radeon/drm_buffer.c
 create mode 100644 drivers/gpu/drm/radeon/drm_buffer.h
 delete mode 100644 include/drm/drm_buffer.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 4a55d59..9b7cb3f 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -4,7 +4,7 @@

 ccflags-y := -Iinclude/drm

-drm-y   := drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
+drm-y   := drm_auth.o drm_bufs.o drm_cache.o \
drm_context.o drm_dma.o \
drm_fops.o drm_gem.o drm_ioctl.o drm_irq.o \
drm_lock.o drm_memory.o drm_drv.o drm_vm.o \
diff --git a/drivers/gpu/drm/drm_buffer.c b/drivers/gpu/drm/drm_buffer.c
deleted file mode 100644
index 86a4a4a..000
--- a/drivers/gpu/drm/drm_buffer.c
+++ /dev/null
@@ -1,181 +0,0 @@
-/**
- *
- * Copyright 2010 Pauli Nieminen.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sub license, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
- * THE COPYRIGHT HOLDERS, AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM,
- * DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
- * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
- * USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- *
- **/
-/*
- * Multipart buffer for coping data which is larger than the page size.
- *
- * Authors:
- * Pauli Nieminen 
- */
-
-#include 
-#include 
-
-/**
- * Allocate the drm buffer object.
- *
- *   buf: Pointer to a pointer where the object is stored.
- *   size: The number of bytes to allocate.
- */
-int drm_buffer_alloc(struct drm_buffer **buf, int size)
-{
-   int nr_pages = size / PAGE_SIZE + 1;
-   int idx;
-
-   /* Allocating pointer table to end of structure makes drm_buffer
-* variable sized */
-   *buf = kzalloc(sizeof(struct drm_buffer) + nr_pages*sizeof(char *),
-   GFP_KERNEL);
-
-   if (*buf == NULL) {
-   DRM_ERROR("Failed to allocate drm buffer object to hold"
-   " %d bytes in %d pages.\n",
-   size, nr_pages);
-   return -ENOMEM;
-   }
-
-   (*buf)->size = size;
-
-   for (idx = 0; idx < nr_pages; ++idx) {
-
-   (*buf)->data[idx] =
-   kmalloc(min(PAGE_SIZE, size - idx * PAGE_SIZE),
-   GFP_KERNEL);
-
-
-   if ((*buf)->data[idx] == NULL) {
-   DRM_ERROR("Failed to allocate %dth page for drm"
-   " buffer with %d bytes and %d pages.\n",
-   idx + 1, size, nr_pages);
-   goto error_out;
-   }
-
-   }
-
-   return 0;
-
-error_out:
-
-   for (; idx >= 0; --idx)
-   kfree((*buf)->data[idx]);
-
-   kfree(*buf);
-   return -ENOMEM;
-}
-EXPORT_SYMBOL(drm_buffer_alloc);
-
-/**
- * Copy the user data to the begin of the buffer and reset the processing
- * iterator.
- *
- *   user_data: A pointer the data that is copied to the buffer.
- *   size: The Number of bytes to copy.
- */
-int drm_buffer_copy_from_user(struct drm_buffer *buf,
- void __user *user_data, int size)
-{
-   int nr_pages = size / PAGE_SIZE + 1;
-   

[PATCH 02/20] drm: mark drm_buf and drm_map as legacy

[David Herrmann]

Move internal declarations to drm_legacy.h and add drm_legacy_*() prefix
to all legacy functions.

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_bufs.c  | 89 ++---
 drivers/gpu/drm/drm_drv.c   |  4 +-
 drivers/gpu/drm/drm_ioctl.c | 16 +++
 drivers/gpu/drm/drm_legacy.h| 15 +++
 drivers/gpu/drm/i915/i915_dma.c |  2 +-
 drivers/gpu/drm/mga/mga_dma.c   | 49 ++--
 drivers/gpu/drm/r128/r128_cce.c |  2 +-
 drivers/gpu/drm/radeon/r600_cp.c|  2 +-
 drivers/gpu/drm/radeon/radeon_cp.c  | 22 -
 drivers/gpu/drm/savage/savage_bci.c | 19 
 drivers/gpu/drm/via/via_map.c   |  2 +-
 include/drm/drmP.h  | 41 +
 12 files changed, 131 insertions(+), 132 deletions(-)

diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index 61acb8f..6f05d74 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -1,18 +1,13 @@
-/**
- * \file drm_bufs.c
- * Generic buffer template
- *
- * \author Rickard E. (Rik) Faith 
- * \author Gareth Hughes 
- */
-
 /*
- * Created: Thu Nov 23 03:10:50 2000 by gareth at valinux.com
+ * Legacy: Generic DRM Buffer Management
  *
  * Copyright 1999, 2000 Precision Insight, Inc., Cedar Park, Texas.
  * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
  * All Rights Reserved.
  *
+ * Author: Rickard E. (Rik) Faith 
+ * Author: Gareth Hughes 
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
  * to deal in the Software without restriction, including without limitation
@@ -39,6 +34,7 @@
 #include 
 #include 
 #include 
+#include "drm_legacy.h"

 static struct drm_map_list *drm_find_matching_map(struct drm_device *dev,
  struct drm_local_map *map)
@@ -365,9 +361,9 @@ static int drm_addmap_core(struct drm_device * dev, 
resource_size_t offset,
return 0;
 }

-int drm_addmap(struct drm_device * dev, resource_size_t offset,
-  unsigned int size, enum drm_map_type type,
-  enum drm_map_flags flags, struct drm_local_map ** map_ptr)
+int drm_legacy_addmap(struct drm_device * dev, resource_size_t offset,
+ unsigned int size, enum drm_map_type type,
+ enum drm_map_flags flags, struct drm_local_map **map_ptr)
 {
struct drm_map_list *list;
int rc;
@@ -378,7 +374,7 @@ int drm_addmap(struct drm_device * dev, resource_size_t 
offset,
return rc;
 }

-EXPORT_SYMBOL(drm_addmap);
+EXPORT_SYMBOL(drm_legacy_addmap);

 /**
  * Ioctl to specify a range of memory that is available for mapping by a
@@ -391,8 +387,8 @@ EXPORT_SYMBOL(drm_addmap);
  * \return zero on success or a negative value on error.
  *
  */
-int drm_addmap_ioctl(struct drm_device *dev, void *data,
-struct drm_file *file_priv)
+int drm_legacy_addmap_ioctl(struct drm_device *dev, void *data,
+   struct drm_file *file_priv)
 {
struct drm_map *map = data;
struct drm_map_list *maplist;
@@ -429,9 +425,9 @@ int drm_addmap_ioctl(struct drm_device *dev, void *data,
  * its being used, and free any associate resource (such as MTRR's) if it's not
  * being on use.
  *
- * \sa drm_addmap
+ * \sa drm_legacy_addmap
  */
-int drm_rmmap_locked(struct drm_device *dev, struct drm_local_map *map)
+int drm_legacy_rmmap_locked(struct drm_device *dev, struct drm_local_map *map)
 {
struct drm_map_list *r_list = NULL, *list_t;
drm_dma_handle_t dmah;
@@ -485,19 +481,19 @@ int drm_rmmap_locked(struct drm_device *dev, struct 
drm_local_map *map)

return 0;
 }
-EXPORT_SYMBOL(drm_rmmap_locked);
+EXPORT_SYMBOL(drm_legacy_rmmap_locked);

-int drm_rmmap(struct drm_device *dev, struct drm_local_map *map)
+int drm_legacy_rmmap(struct drm_device *dev, struct drm_local_map *map)
 {
int ret;

mutex_lock(>struct_mutex);
-   ret = drm_rmmap_locked(dev, map);
+   ret = drm_legacy_rmmap_locked(dev, map);
mutex_unlock(>struct_mutex);

return ret;
 }
-EXPORT_SYMBOL(drm_rmmap);
+EXPORT_SYMBOL(drm_legacy_rmmap);

 /* The rmmap ioctl appears to be unnecessary.  All mappings are torn down on
  * the last close of the device, and this is necessary for cleanup when things
@@ -514,8 +510,8 @@ EXPORT_SYMBOL(drm_rmmap);
  * \param arg pointer to a struct drm_map structure.
  * \return zero on success or a negative value on error.
  */
-int drm_rmmap_ioctl(struct drm_device *dev, void *data,
-   struct drm_file *file_priv)
+int drm_legacy_rmmap_ioctl(struct drm_device *dev, void *data,
+  struct drm_file *file_priv)
 {
struct drm_map *request = data;
struct drm_local_map *map = NULL;
@@ -546,7 +542,7 @@ int drm_rmmap_ioctl(struct drm_device *dev, void *data,
return 0;

[PATCH 03/20] drm: move "struct drm_vma_entry" to drm_vm.c

[David Herrmann]

Make all the drm_vma_entry handling local to drm_vm.c and hide it from
global headers. This requires to extract the inlined legacy drm_vma_entry
cleanup into a small helper and also move a weirdly placed drm_vma_info
helper into drm_vm.c.

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_fops.c | 10 +-
 drivers/gpu/drm/drm_info.c | 59 ---
 drivers/gpu/drm/drm_vm.c   | 76 ++
 include/drm/drmP.h |  8 ++---
 4 files changed, 79 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 6dbbb0f..12e6a1c 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -330,8 +330,6 @@ static void drm_legacy_dev_reinit(struct drm_device *dev)
  */
 int drm_lastclose(struct drm_device * dev)
 {
-   struct drm_vma_entry *vma, *vma_temp;
-
DRM_DEBUG("\n");

if (dev->driver->lastclose)
@@ -346,13 +344,7 @@ int drm_lastclose(struct drm_device * dev)
drm_agp_clear(dev);

drm_legacy_sg_cleanup(dev);
-
-   /* Clear vma list (only built for debugging) */
-   list_for_each_entry_safe(vma, vma_temp, >vmalist, head) {
-   list_del(>head);
-   kfree(vma);
-   }
-
+   drm_legacy_vma_flush(dev);
drm_legacy_dma_takedown(dev);

mutex_unlock(>struct_mutex);
diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c
index ecaf0fa..3c99f6f 100644
--- a/drivers/gpu/drm/drm_info.c
+++ b/drivers/gpu/drm/drm_info.c
@@ -223,62 +223,3 @@ int drm_gem_name_info(struct seq_file *m, void *data)

return 0;
 }
-
-#if DRM_DEBUG_CODE
-
-int drm_vma_info(struct seq_file *m, void *data)
-{
-   struct drm_info_node *node = (struct drm_info_node *) m->private;
-   struct drm_device *dev = node->minor->dev;
-   struct drm_vma_entry *pt;
-   struct vm_area_struct *vma;
-   unsigned long vma_count = 0;
-#if defined(__i386__)
-   unsigned int pgprot;
-#endif
-
-   mutex_lock(>struct_mutex);
-   list_for_each_entry(pt, >vmalist, head)
-   vma_count++;
-
-   seq_printf(m, "vma use count: %lu, high_memory = %pK, 0x%pK\n",
-  vma_count, high_memory,
-  (void *)(unsigned long)virt_to_phys(high_memory));
-
-   list_for_each_entry(pt, >vmalist, head) {
-   vma = pt->vma;
-   if (!vma)
-   continue;
-   seq_printf(m,
-  "\n%5d 0x%pK-0x%pK %c%c%c%c%c%c 0x%08lx000",
-  pt->pid,
-  (void *)vma->vm_start, (void *)vma->vm_end,
-  vma->vm_flags & VM_READ ? 'r' : '-',
-  vma->vm_flags & VM_WRITE ? 'w' : '-',
-  vma->vm_flags & VM_EXEC ? 'x' : '-',
-  vma->vm_flags & VM_MAYSHARE ? 's' : 'p',
-  vma->vm_flags & VM_LOCKED ? 'l' : '-',
-  vma->vm_flags & VM_IO ? 'i' : '-',
-  vma->vm_pgoff);
-
-#if defined(__i386__)
-   pgprot = pgprot_val(vma->vm_page_prot);
-   seq_printf(m, " %c%c%c%c%c%c%c%c%c",
-  pgprot & _PAGE_PRESENT ? 'p' : '-',
-  pgprot & _PAGE_RW ? 'w' : 'r',
-  pgprot & _PAGE_USER ? 'u' : 's',
-  pgprot & _PAGE_PWT ? 't' : 'b',
-  pgprot & _PAGE_PCD ? 'u' : 'c',
-  pgprot & _PAGE_ACCESSED ? 'a' : '-',
-  pgprot & _PAGE_DIRTY ? 'd' : '-',
-  pgprot & _PAGE_PSE ? 'm' : 'k',
-  pgprot & _PAGE_GLOBAL ? 'g' : 'l');
-#endif
-   seq_printf(m, "\n");
-   }
-   mutex_unlock(>struct_mutex);
-   return 0;
-}
-
-#endif
-
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index 24e045c..352e339 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -35,11 +35,18 @@

 #include 
 #include 
+#include 
 #if defined(__ia64__)
 #include 
 #include 
 #endif

+struct drm_vma_entry {
+   struct list_head head;
+   struct vm_area_struct *vma;
+   pid_t pid;
+};
+
 static void drm_vm_open(struct vm_area_struct *vma);
 static void drm_vm_close(struct vm_area_struct *vma);

@@ -662,3 +669,72 @@ int drm_mmap(struct file *filp, struct vm_area_struct *vma)
return ret;
 }
 EXPORT_SYMBOL(drm_mmap);
+
+void drm_legacy_vma_flush(struct drm_device *dev)
+{
+   struct drm_vma_entry *vma, *vma_temp;
+
+   /* Clear vma list (only needed for legacy drivers) */
+   list_for_each_entry_safe(vma, vma_temp, >vmalist, head) {
+   list_del(>head);
+   kfree(vma);
+   }
+}
+
+#if DRM_DEBUG_CODE
+
+int drm_vma_info(struct seq_file *m, void *data)
+{
+   struct drm_info_node *node = (struct drm_info_node *) m->private;
+ 

[PATCH 04/20] drm: move "struct drm_magic_entry" to drm_auth.c

[David Herrmann]

In drm_release(), we currently call drm_remove_magic() if the drm_file
has a drm-magic attached. Therefore, once drm_master_release() is called,
the magic-list _must_ be empty.

By dropping the no-op cleanup, we can move "struct drm_magic_entry" to
drm_auth.c and avoid exposing it to all of DRM.

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_auth.c | 6 ++
 drivers/gpu/drm/drm_drv.c  | 7 ---
 include/drm/drmP.h | 6 --
 3 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c
index 3cedae1..708a204 100644
--- a/drivers/gpu/drm/drm_auth.c
+++ b/drivers/gpu/drm/drm_auth.c
@@ -35,6 +35,12 @@

 #include 

+struct drm_magic_entry {
+   struct list_head head;
+   struct drm_hash_item hash_item;
+   struct drm_file *priv;
+};
+
 /**
  * Find the file with the given magic number.
  *
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index b1587e2..6645669 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -133,7 +133,6 @@ EXPORT_SYMBOL(drm_master_get);
 static void drm_master_destroy(struct kref *kref)
 {
struct drm_master *master = container_of(kref, struct drm_master, 
refcount);
-   struct drm_magic_entry *pt, *next;
struct drm_device *dev = master->minor->dev;
struct drm_map_list *r_list, *list_temp;

@@ -154,12 +153,6 @@ static void drm_master_destroy(struct kref *kref)
master->unique_len = 0;
}

-   list_for_each_entry_safe(pt, next, >magicfree, head) {
-   list_del(>head);
-   drm_ht_remove_item(>magiclist, >hash_item);
-   kfree(pt);
-   }
-
drm_ht_remove(>magiclist);

mutex_unlock(>struct_mutex);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 11c3575..0fdd813 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -290,12 +290,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}

-struct drm_magic_entry {
-   struct list_head head;
-   struct drm_hash_item hash_item;
-   struct drm_file *priv;
-};
-
 /**
  * DMA buffer.
  */
-- 
2.1.0

[PATCH 05/20] drm: drop unused "struct drm_waitlist"

[David Herrmann]

This structure is unused, drop it.

Signed-off-by: David Herrmann 
---
 include/drm/drmP.h | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 0fdd813..0bf66f9 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -320,17 +320,6 @@ struct drm_buf {
void *dev_private;   /**< Per-buffer private storage */
 };

-/** bufs is one longer than it has to be */
-struct drm_waitlist {
-   int count;  /**< Number of possible buffers */
-   struct drm_buf **bufs;  /**< List of pointers to buffers */
-   struct drm_buf **rp;/**< Read pointer */
-   struct drm_buf **wp;/**< Write pointer */
-   struct drm_buf **end;   /**< End pointer */
-   spinlock_t read_lock;
-   spinlock_t write_lock;
-};
-
 typedef struct drm_dma_handle {
dma_addr_t busaddr;
void *vaddr;
-- 
2.1.0

[PATCH 06/20] drm: move AGP definitions harder

[David Herrmann]

Move drm_agp_head to drm_agpsupport.h and drm_agp_mem into drm_legacy.h.
Unfortunately, drivers still heavily access drm_agp_head so we cannot
move it to drm_legacy.h. However, at least it's no longer visible in
drmP.h now (it's directly included from it, though).

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_agpsupport.c |  1 +
 drivers/gpu/drm/drm_legacy.h | 15 +++
 drivers/gpu/drm/drm_memory.c |  1 +
 drivers/gpu/drm/drm_vm.c |  1 +
 include/drm/drmP.h   | 30 +-
 include/drm/drm_agpsupport.h | 13 +
 6 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/drm_agpsupport.c b/drivers/gpu/drm/drm_agpsupport.c
index dde205c..4b2b4aa 100644
--- a/drivers/gpu/drm/drm_agpsupport.c
+++ b/drivers/gpu/drm/drm_agpsupport.c
@@ -34,6 +34,7 @@
 #include 
 #include 
 #include 
+#include "drm_legacy.h"

 #if __OS_HAS_AGP

diff --git a/drivers/gpu/drm/drm_legacy.h b/drivers/gpu/drm/drm_legacy.h
index 912fe81..d94c564 100644
--- a/drivers/gpu/drm/drm_legacy.h
+++ b/drivers/gpu/drm/drm_legacy.h
@@ -23,6 +23,9 @@
  * OTHER DEALINGS IN THE SOFTWARE.
  */

+#include 
+
+struct agp_memory;
 struct drm_device;
 struct drm_file;

@@ -63,4 +66,16 @@ int drm_legacy_freebufs(struct drm_device *d, void *v, 
struct drm_file *f);
 int drm_legacy_mapbufs(struct drm_device *d, void *v, struct drm_file *f);
 int drm_legacy_dma_ioctl(struct drm_device *d, void *v, struct drm_file *f);

+/*
+ * AGP Support
+ */
+
+struct drm_agp_mem {
+   unsigned long handle;
+   struct agp_memory *memory;
+   unsigned long bound;
+   int pages;
+   struct list_head head;
+};
+
 #endif /* __DRM_LEGACY_H__ */
diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
index 00c67c0..7888dad 100644
--- a/drivers/gpu/drm/drm_memory.c
+++ b/drivers/gpu/drm/drm_memory.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include "drm_legacy.h"

 #if __OS_HAS_AGP
 static void *agp_remap(unsigned long offset, unsigned long size,
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index 352e339..be25174 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #endif
+#include "drm_legacy.h"

 struct drm_vma_entry {
struct list_head head;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 0bf66f9..7a3c73c 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -80,6 +80,7 @@ struct module;

 struct drm_file;
 struct drm_device;
+struct drm_agp_head;

 struct device_node;
 struct videomode;
@@ -440,35 +441,6 @@ struct drm_device_dma {
 };

 /**
- * AGP memory entry.  Stored as a doubly linked list.
- */
-struct drm_agp_mem {
-   unsigned long handle;   /**< handle */
-   struct agp_memory *memory;
-   unsigned long bound;/**< address */
-   int pages;
-   struct list_head head;
-};
-
-/**
- * AGP data.
- *
- * \sa drm_agp_init() and drm_device::agp.
- */
-struct drm_agp_head {
-   struct agp_kern_info agp_info;  /**< AGP device information */
-   struct list_head memory;
-   unsigned long mode; /**< AGP mode */
-   struct agp_bridge_data *bridge;
-   int enabled;/**< whether the AGP bus as been 
enabled */
-   int acquired;   /**< whether the AGP device has been 
acquired */
-   unsigned long base;
-   int agp_mtrr;
-   int cant_use_aperture;
-   unsigned long page_mask;
-};
-
-/**
  * Scatter-gather memory.
  */
 struct drm_sg_mem {
diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h
index 86a0218..3bebeb4 100644
--- a/include/drm/drm_agpsupport.h
+++ b/include/drm/drm_agpsupport.h
@@ -8,6 +8,19 @@
 #include 
 #include 

+struct drm_agp_head {
+   struct agp_kern_info agp_info;
+   struct list_head memory;
+   unsigned long mode;
+   struct agp_bridge_data *bridge;
+   int enabled;
+   int acquired;
+   unsigned long base;
+   int agp_mtrr;
+   int cant_use_aperture;
+   unsigned long page_mask;
+};
+
 #if __OS_HAS_AGP

 void drm_free_agp(struct agp_memory * handle, int pages);
-- 
2.1.0

[PATCH 07/20] drm: replace weird conditional includes

[David Herrmann]

pte_wrprotect() is only used by drm_vm.c, so move the include there. Also
include it unconditionally, all architectures provide this header!

Furthermore, replace asm/current.h with sched.h, which includes
asm/current.h unconditionally. This way we get the same effect and avoid
direct asm/ includes. Furthermore, drop the weird __alpha__ protection.
It's safe to include sched.h everywhere (and the wait.h comment doesn't
apply, anyway).

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_vm.c |  1 +
 include/drm/drmP.h   | 10 +-
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index be25174..967e570 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #endif
+#include 
 #include "drm_legacy.h"

 struct drm_vma_entry {
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 7a3c73c..31fb300 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -36,12 +36,7 @@
 #define _DRM_P_H_

 #ifdef __KERNEL__
-#ifdef __alpha__
-/* add include of current.h so that "current" is defined
- * before static inline funcs in wait.h. Doing this so we
- * can build the DRM (part of PI DRI). 4/21/2000 S + B */
-#include 
-#endif /* __alpha__ */
+#include 
 #include 
 #include 
 #include 
@@ -58,9 +53,6 @@
 #include 
 #include 
 #include 
-#if defined(__alpha__) || defined(__powerpc__)
-#include/* For pte_wrprotect */
-#endif
 #include 
 #include 
 #include 
-- 
2.1.0

[PATCH 08/20] drm: drop __KERNEL__ protection in drmP.h

[David Herrmann]

drmP.h is internal to the kernel. No need to keep the __KERNEL__
protection.

Signed-off-by: David Herrmann 
---
 include/drm/drmP.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 31fb300..d3504c6 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -35,7 +35,6 @@
 #ifndef _DRM_P_H_
 #define _DRM_P_H_

-#ifdef __KERNEL__
 #include 
 #include 
 #include 
@@ -1544,5 +1543,4 @@ static __inline__ bool drm_can_sleep(void)
return true;
 }

-#endif /* __KERNEL__ */
 #endif
-- 
2.1.0

[PATCH 09/20] drm: merge drm_memory.h into drm_memory.c

[David Herrmann]

The drm_memory.h header is only used to define PAGE_AGP, which is only
used in drm_memory.c. Fold the header into drm_memory.c and drop it.

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_memory.c | 11 +
 include/drm/drmP.h   |  6 ++---
 include/drm/drm_memory.h | 59 
 3 files changed, 13 insertions(+), 63 deletions(-)
 delete mode 100644 include/drm/drm_memory.h

diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
index 7888dad..62fda6a 100644
--- a/drivers/gpu/drm/drm_memory.c
+++ b/drivers/gpu/drm/drm_memory.c
@@ -39,6 +39,17 @@
 #include "drm_legacy.h"

 #if __OS_HAS_AGP
+
+#ifdef HAVE_PAGE_AGP
+# include 
+#else
+# ifdef __powerpc__
+#  define PAGE_AGP __pgprot(_PAGE_KERNEL | _PAGE_NO_CACHE)
+# else
+#  define PAGE_AGP PAGE_KERNEL
+# endif
+#endif
+
 static void *agp_remap(unsigned long offset, unsigned long size,
   struct drm_device * dev)
 {
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d3504c6..294f7da 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -58,6 +58,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -1127,10 +1129,6 @@ extern void drm_vm_open_locked(struct drm_device *dev, 
struct vm_area_struct *vm
 extern void drm_vm_close_locked(struct drm_device *dev, struct vm_area_struct 
*vma);
 extern unsigned int drm_poll(struct file *filp, struct poll_table_struct 
*wait);

-   /* Memory management support (drm_memory.h) */
-#include 
-
-
/* Misc. IOCTL support (drm_ioctl.h) */
 extern int drm_irq_by_busid(struct drm_device *dev, void *data,
struct drm_file *file_priv);
diff --git a/include/drm/drm_memory.h b/include/drm/drm_memory.h
deleted file mode 100644
index 4baf57a..000
--- a/include/drm/drm_memory.h
+++ /dev/null
@@ -1,59 +0,0 @@
-/**
- * \file drm_memory.h
- * Memory management wrappers for DRM
- *
- * \author Rickard E. (Rik) Faith 
- * \author Gareth Hughes 
- */
-
-/*
- * Created: Thu Feb  4 14:00:34 1999 by faith at valinux.com
- *
- * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
- * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the next
- * paragraph) shall be included in all copies or substantial portions of the
- * Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
- * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- * OTHER DEALINGS IN THE SOFTWARE.
- */
-
-#include 
-#include 
-#include 
-
-/**
- * Cut down version of drm_memory_debug.h, which used to be called
- * drm_memory.h.
- */
-
-#if __OS_HAS_AGP
-
-#ifdef HAVE_PAGE_AGP
-#include 
-#else
-# ifdef __powerpc__
-#  define PAGE_AGP __pgprot(_PAGE_KERNEL | _PAGE_NO_CACHE)
-# else
-#  define PAGE_AGP PAGE_KERNEL
-# endif
-#endif
-
-#else  /* __OS_HAS_AGP */
-
-#endif
-- 
2.1.0

[PATCH 10/20] drm: move __OS_HAS_AGP into drm_agpsupport.h

[David Herrmann]

With drm_memory.h gone, there is no header left that uses __OS_HAS_AGP.
Move it into drm_agpsupport.h (which is itself included from drmP.h) to
hide it harder from public eyes.

Signed-off-by: David Herrmann 
---
 include/drm/drmP.h   | 2 --
 include/drm/drm_agpsupport.h | 3 +++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 294f7da..c6f337c 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -67,8 +67,6 @@

 #include 

-#define __OS_HAS_AGP (defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) && 
defined(MODULE)))
-
 struct module;

 struct drm_file;
diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h
index 3bebeb4..4f1724c 100644
--- a/include/drm/drm_agpsupport.h
+++ b/include/drm/drm_agpsupport.h
@@ -8,6 +8,9 @@
 #include 
 #include 

+#define __OS_HAS_AGP (defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) && \
+ defined(MODULE)))
+
 struct drm_agp_head {
struct agp_kern_info agp_info;
struct list_head memory;
-- 
2.1.0

[PATCH 11/20] drm: order includes alphabetically in drmP.h

[David Herrmann]

It is hardly possible to review the drmP.h includes, anymore. Order them
alphabetically, linux/ first, then asm/ and then local drm/ includes.

Since a long time ago, kernel headers have been converted to include
required headers themselves. No-one cares whether that means the compiler
has to include a header multiple times. In fact, GCC already does some
optimization regarding multiple inclusions if a sorrounding #ifndef is
present.

Signed-off-by: David Herrmann 
---
 include/drm/drmP.h | 46 +++---
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index c6f337c..8b3f3b7 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -35,38 +35,42 @@
 #ifndef _DRM_P_H_
 #define _DRM_P_H_

-#include 
-#include 
-#include 
-#include 
+#include 
+#include 
+#include 
+#include 
 #include 
+#include 
+#include 
 #include 
-#include 
-#include 
-#include 
+#include 
 #include 
-#include 
+#include 
+#include 
+#include 
 #include 
-#include 
 #include 
-#include 
-#include 
+#include 
+#include 
+#include 
 #include 
-#include 
-#include 
+#include 
+#include 
 #include 
-#include 
-#include 
-#include 
-#include 
 #include 
+#include 
+
+#include 
 #include 
+#include 
+
 #include 
+#include 
+#include 
+#include 
 #include 
 #include 

-#include 
-
 struct module;

 struct drm_file;
@@ -77,10 +81,6 @@ struct device_node;
 struct videomode;
 struct reservation_object;

-#include 
-#include 
-#include 
-
 /*
  * 4 debug categories are defined:
  *
-- 
2.1.0

[PATCH 12/20] drm: drop DRM_DEBUG_CODE

[David Herrmann]

DRM_DEBUG_CODE is currently always set, so distributions enable it. The
only reason to keep support in code is if developers wanted to disable
debug support. Sounds unlikely.

All the DRM_DEBUG() printks are still guarded by a drm_debug read. So if
its cacheline is read once, they're discarded pretty fast.. There should
hardly be any performance penalty, it's even guarded by unlikely().

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_debugfs.c|  2 --
 drivers/gpu/drm/drm_vm.c |  4 
 drivers/gpu/drm/radeon/radeon.h  | 17 -
 drivers/gpu/drm/radeon/radeon_ring.c | 21 -
 include/drm/drmP.h   | 12 
 5 files changed, 12 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 13bd429..4491dbd 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -49,9 +49,7 @@ static const struct drm_info_list drm_debugfs_list[] = {
{"clients", drm_clients_info, 0},
{"bufs", drm_bufs_info, 0},
{"gem_names", drm_gem_name_info, DRIVER_GEM},
-#if DRM_DEBUG_CODE
{"vma", drm_vma_info, 0},
-#endif
 };
 #define DRM_DEBUGFS_ENTRIES ARRAY_SIZE(drm_debugfs_list)

diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index 967e570..4b3e9c4 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -683,8 +683,6 @@ void drm_legacy_vma_flush(struct drm_device *dev)
}
 }

-#if DRM_DEBUG_CODE
-
 int drm_vma_info(struct seq_file *m, void *data)
 {
struct drm_info_node *node = (struct drm_info_node *) m->private;
@@ -738,5 +736,3 @@ int drm_vma_info(struct seq_file *m, void *data)
mutex_unlock(>struct_mutex);
return 0;
 }
-
-#endif
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 83a2461..5cfa574 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2761,18 +2761,25 @@ void radeon_atombios_fini(struct radeon_device *rdev);
 /*
  * RING helpers.
  */
-#if DRM_DEBUG_CODE == 0
+
+/**
+ * radeon_ring_write - write a value to the ring
+ *
+ * @ring: radeon_ring structure holding ring information
+ * @v: dword (dw) value to write
+ *
+ * Write a value to the requested ring buffer (all asics).
+ */
 static inline void radeon_ring_write(struct radeon_ring *ring, uint32_t v)
 {
+   if (ring->count_dw <= 0)
+   DRM_ERROR("radeon: writing more dwords to the ring than 
expected!\n");
+
ring->ring[ring->wptr++] = v;
ring->wptr &= ring->ptr_mask;
ring->count_dw--;
ring->ring_free_dw--;
 }
-#else
-/* With debugging this is just too big to inline */
-void radeon_ring_write(struct radeon_ring *ring, uint32_t v);
-#endif

 /*
  * ASICs macro.
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c 
b/drivers/gpu/drm/radeon/radeon_ring.c
index d656079..6f2a9bd 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -45,27 +45,6 @@
 static int radeon_debugfs_ring_init(struct radeon_device *rdev, struct 
radeon_ring *ring);

 /**
- * radeon_ring_write - write a value to the ring
- *
- * @ring: radeon_ring structure holding ring information
- * @v: dword (dw) value to write
- *
- * Write a value to the requested ring buffer (all asics).
- */
-void radeon_ring_write(struct radeon_ring *ring, uint32_t v)
-{
-#if DRM_DEBUG_CODE
-   if (ring->count_dw <= 0) {
-   DRM_ERROR("radeon: writing more dwords to the ring than 
expected!\n");
-   }
-#endif
-   ring->ring[ring->wptr++] = v;
-   ring->wptr &= ring->ptr_mask;
-   ring->count_dw--;
-   ring->ring_free_dw--;
-}
-
-/**
  * radeon_ring_supports_scratch_reg - check if the ring supports
  * writing to scratch registers
  *
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 8b3f3b7..8f55875 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -140,9 +140,6 @@ int drm_err(const char *func, const char *format, ...);
 /** \name Begin the DRM... */
 /*@{*/

-#define DRM_DEBUG_CODE 2 /**< Include debugging code if > 1, then
-also include looping detection. */
-
 #define DRM_MAGIC_HASH_ORDER  4  /**< Size of key hash table. Must be power of 
2. */

 /*@}*/
@@ -188,7 +185,6 @@ int drm_err(const char *func, const char *format, ...);
  * \param fmt printf() like format string.
  * \param arg arguments
  */
-#if DRM_DEBUG_CODE
 #define DRM_DEBUG(fmt, args...)
\
do {\
if (unlikely(drm_debug & DRM_UT_CORE))  \
@@ -210,12 +206,6 @@ int drm_err(const char *func, const char *format, ...);
if (unlikely(drm_debug & DRM_UT_PRIME)) \
drm_ut_debug_printk(__func__, fmt, ##args); \
} while (0)
-#else
-#define 

[PATCH 13/20] drm: inline "struct drm_sigdata"

[David Herrmann]

The sigdata structure is only used to group two fields in drm_device.
Inline it and make it an unnamed object.

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_lock.c | 15 ---
 include/drm/drmP.h | 12 +---
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c
index e26b59e..60f1481 100644
--- a/drivers/gpu/drm/drm_lock.c
+++ b/drivers/gpu/drm/drm_lock.c
@@ -120,7 +120,7 @@ int drm_lock(struct drm_device *dev, void *data, struct 
drm_file *file_priv)
sigaddset(>sigmask, SIGTTOU);
dev->sigdata.context = lock->context;
dev->sigdata.lock = master->lock.hw_lock;
-   block_all_signals(drm_notifier, >sigdata, >sigmask);
+   block_all_signals(drm_notifier, dev, >sigmask);
}

if (dev->driver->dma_quiescent && (lock->flags & _DRM_LOCK_QUIESCENT))
@@ -286,26 +286,27 @@ int drm_lock_free(struct drm_lock_data *lock_data, 
unsigned int context)
  * If the lock is not held, then let the signal proceed as usual.  If the lock
  * is held, then set the contended flag and keep the signal blocked.
  *
- * \param priv pointer to a drm_sigdata structure.
+ * \param priv pointer to a drm_device structure.
  * \return one if the signal should be delivered normally, or zero if the
  * signal should be blocked.
  */
 static int drm_notifier(void *priv)
 {
-   struct drm_sigdata *s = (struct drm_sigdata *) priv;
+   struct drm_device *dev = priv;
+   struct drm_hw_lock *lock = dev->sigdata.lock;
unsigned int old, new, prev;

/* Allow signal delivery if lock isn't held */
-   if (!s->lock || !_DRM_LOCK_IS_HELD(s->lock->lock)
-   || _DRM_LOCKING_CONTEXT(s->lock->lock) != s->context)
+   if (!lock || !_DRM_LOCK_IS_HELD(lock->lock)
+   || _DRM_LOCKING_CONTEXT(lock->lock) != dev->sigdata.context)
return 1;

/* Otherwise, set flag to force call to
   drmUnlock */
do {
-   old = s->lock->lock;
+   old = lock->lock;
new = old | _DRM_LOCK_CONT;
-   prev = cmpxchg(>lock->lock, old, new);
+   prev = cmpxchg(>lock, old, new);
} while (prev != old);
return 0;
 }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 8f55875..840a373 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -432,12 +432,6 @@ struct drm_sg_mem {
dma_addr_t *busaddr;
 };

-struct drm_sigdata {
-   int context;
-   struct drm_hw_lock *lock;
-};
-
-
 /**
  * Kernel side of a mapping
  */
@@ -1035,9 +1029,13 @@ struct drm_device {

struct drm_sg_mem *sg;  /**< Scatter gather memory */
unsigned int num_crtcs;  /**< Number of CRTCs on this 
device */
-   struct drm_sigdata sigdata;/**< For block_all_signals */
sigset_t sigmask;

+   struct {
+   int context;
+   struct drm_hw_lock *lock;
+   } sigdata;
+
struct drm_local_map *agp_buffer_map;
unsigned int agp_buffer_token;

-- 
2.1.0

[PATCH 14/20] drm: move remaining includes in drmP.h to the top

[David Herrmann]

Including headers somewhere else but at the top is ugly, deprecated and
was used in early days only to speed up compile-times. Those days are
over. Make headers independent and then move the inclusions to the top.

Signed-off-by: David Herrmann 
---
 include/drm/drmP.h   | 31 ---
 include/drm/drm_agpsupport.h | 10 --
 include/drm/drm_crtc.h   |  4 ++--
 3 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 840a373..a8b24fc 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1,17 +1,14 @@
-/**
- * \file drmP.h
- * Private header for Direct Rendering Manager
- *
- * \author Rickard E. (Rik) Faith 
- * \author Gareth Hughes 
- */
-
 /*
+ * Internal Header for the Direct Rendering Manager
+ *
  * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
  * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
  * Copyright (c) 2009-2010, Code Aurora Forum.
  * All rights reserved.
  *
+ * Author: Rickard E. (Rik) Faith 
+ * Author: Gareth Hughes 
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
  * to deal in the Software without restriction, including without limitation
@@ -64,8 +61,14 @@
 #include 
 #include 

-#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -564,8 +567,6 @@ struct drm_gem_object {
struct dma_buf_attachment *import_attach;
 };

-#include 
-
 /**
  * struct drm_master - drm master structure
  *
@@ -1228,10 +1229,6 @@ extern void drm_vblank_post_modeset(struct drm_device 
*dev, int crtc);
 extern int drm_modeset_ctl(struct drm_device *dev, void *data,
   struct drm_file *file_priv);

-   /* AGP/GART support (drm_agpsupport.h) */
-
-#include 
-
/* Stub support (drm_stub.h) */
 extern int drm_setmaster_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file_priv);
@@ -1397,8 +1394,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned 
long obj_size,
 struct vm_area_struct *vma);
 int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);

-#include 
-
 static inline void
 drm_gem_object_reference(struct drm_gem_object *obj)
 {
@@ -1472,8 +1467,6 @@ static __inline__ void drm_core_dropmap(struct 
drm_local_map *map)
 {
 }

-#include 
-
 struct drm_device *drm_dev_alloc(struct drm_driver *driver,
 struct device *parent);
 void drm_dev_ref(struct drm_device *dev);
diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h
index 4f1724c..055dc05 100644
--- a/include/drm/drm_agpsupport.h
+++ b/include/drm/drm_agpsupport.h
@@ -1,12 +1,16 @@
 #ifndef _DRM_AGPSUPPORT_H_
 #define _DRM_AGPSUPPORT_H_

+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
-#include 
-#include 
+#include 
+
+struct drm_device;
+struct drm_file;

 #define __OS_HAS_AGP (defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) && \
  defined(MODULE)))
@@ -61,6 +65,7 @@ int drm_agp_unbind_ioctl(struct drm_device *dev, void *data,
 int drm_agp_bind(struct drm_device *dev, struct drm_agp_binding *request);
 int drm_agp_bind_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file_priv);
+
 #else /* __OS_HAS_AGP */

 static inline void drm_free_agp(struct agp_memory * handle, int pages)
@@ -188,6 +193,7 @@ static inline int drm_agp_bind_ioctl(struct drm_device 
*dev, void *data,
 {
return -ENODEV;
 }
+
 #endif /* __OS_HAS_AGP */

 #endif /* _DRM_AGPSUPPORT_H_ */
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 0375d75..77d9763 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -31,8 +31,8 @@
 #include 
 #include 
 #include 
-#include 
-#include 
+#include 
+#include 
 #include 

 struct drm_device;
-- 
2.1.0

[PATCH 15/20] drm: simplify drm_*_set_unique()

[David Herrmann]

Lets use kasprintf() to avoid pre-allocating the buffer. This is really
nothing to optimize for speed and the input is trusted, so kasprintf() is
just fine.

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_pci.c  | 30 --
 drivers/gpu/drm/drm_platform.c | 31 ---
 2 files changed, 16 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 020cfd9..8efea6b 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -129,31 +129,17 @@ static int drm_get_pci_domain(struct drm_device *dev)

 static int drm_pci_set_busid(struct drm_device *dev, struct drm_master *master)
 {
-   int len, ret;
-   master->unique_len = 40;
-   master->unique_size = master->unique_len;
-   master->unique = kmalloc(master->unique_size, GFP_KERNEL);
-   if (master->unique == NULL)
+   master->unique = kasprintf(GFP_KERNEL, "pci:%04x:%02x:%02x.%d",
+   drm_get_pci_domain(dev),
+   dev->pdev->bus->number,
+   PCI_SLOT(dev->pdev->devfn),
+   PCI_FUNC(dev->pdev->devfn));
+   if (!master->unique)
return -ENOMEM;

-
-   len = snprintf(master->unique, master->unique_len,
-  "pci:%04x:%02x:%02x.%d",
-  drm_get_pci_domain(dev),
-  dev->pdev->bus->number,
-  PCI_SLOT(dev->pdev->devfn),
-  PCI_FUNC(dev->pdev->devfn));
-
-   if (len >= master->unique_len) {
-   DRM_ERROR("buffer overflow");
-   ret = -EINVAL;
-   goto err;
-   } else
-   master->unique_len = len;
-
+   master->unique_len = strlen(master->unique);
+   master->unique_size = master->unique_len + 1;
return 0;
-err:
-   return ret;
 }

 int drm_pci_set_unique(struct drm_device *dev,
diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
index d5b76f1..0c09ddd 100644
--- a/drivers/gpu/drm/drm_platform.c
+++ b/drivers/gpu/drm/drm_platform.c
@@ -70,35 +70,20 @@ err_free:

 static int drm_platform_set_busid(struct drm_device *dev, struct drm_master 
*master)
 {
-   int len, ret, id;
-
-   master->unique_len = 13 + strlen(dev->platformdev->name);
-   master->unique_size = master->unique_len;
-   master->unique = kmalloc(master->unique_len + 1, GFP_KERNEL);
-
-   if (master->unique == NULL)
-   return -ENOMEM;
+   int id;

id = dev->platformdev->id;
-
-   /* if only a single instance of the platform device, id will be
-* set to -1.. use 0 instead to avoid a funny looking bus-id:
-*/
-   if (id == -1)
+   if (id < 0)
id = 0;

-   len = snprintf(master->unique, master->unique_len,
-   "platform:%s:%02d", dev->platformdev->name, id);
-
-   if (len > master->unique_len) {
-   DRM_ERROR("Unique buffer overflowed\n");
-   ret = -EINVAL;
-   goto err;
-   }
+   master->unique = kasprintf(GFP_KERNEL, "platform:%s:%02d",
+   dev->platformdev->name, id);
+   if (!master->unique)
+   return -ENOMEM;

+   master->unique_len = strlen(master->unique);
+   master->unique_size = master->unique_len;
return 0;
-err:
-   return ret;
 }

 static struct drm_bus drm_platform_bus = {
-- 
2.1.0

[PATCH 16/20] drm: drop unused drm_master->unique_size

[David Herrmann]

This field is unused and there is really no reason to optimize
unique-allocations. Drop it.

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_ioctl.c| 1 -
 drivers/gpu/drm/drm_pci.c  | 4 +---
 drivers/gpu/drm/drm_platform.c | 1 -
 include/drm/drmP.h | 2 --
 4 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index aa1ac79..cb6b54a 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -189,7 +189,6 @@ drm_unset_busid(struct drm_device *dev,
kfree(master->unique);
master->unique = NULL;
master->unique_len = 0;
-   master->unique_size = 0;
 }

 /**
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 8efea6b..e266927 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -138,7 +138,6 @@ static int drm_pci_set_busid(struct drm_device *dev, struct 
drm_master *master)
return -ENOMEM;

master->unique_len = strlen(master->unique);
-   master->unique_size = master->unique_len + 1;
return 0;
 }

@@ -149,8 +148,7 @@ int drm_pci_set_unique(struct drm_device *dev,
int domain, bus, slot, func, ret;

master->unique_len = u->unique_len;
-   master->unique_size = u->unique_len + 1;
-   master->unique = kmalloc(master->unique_size, GFP_KERNEL);
+   master->unique = kmalloc(master->unique_len + 1, GFP_KERNEL);
if (!master->unique) {
ret = -ENOMEM;
goto err;
diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
index 0c09ddd..f197a2b 100644
--- a/drivers/gpu/drm/drm_platform.c
+++ b/drivers/gpu/drm/drm_platform.c
@@ -82,7 +82,6 @@ static int drm_platform_set_busid(struct drm_device *dev, 
struct drm_master *mas
return -ENOMEM;

master->unique_len = strlen(master->unique);
-   master->unique_size = master->unique_len;
return 0;
 }

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index a8b24fc..98b1eaf 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -574,7 +574,6 @@ struct drm_gem_object {
  * @minor: Link back to minor char device we are master for. Immutable.
  * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex.
  * @unique_len: Length of unique field. Protected by drm_global_mutex.
- * @unique_size: Amount allocated. Protected by drm_global_mutex.
  * @magiclist: Hash of used authentication tokens. Protected by struct_mutex.
  * @magicfree: List of used authentication tokens. Protected by struct_mutex.
  * @lock: DRI lock information.
@@ -585,7 +584,6 @@ struct drm_master {
struct drm_minor *minor;
char *unique;
int unique_len;
-   int unique_size;
struct drm_open_hash magiclist;
struct list_head magicfree;
struct drm_lock_data lock;
-- 
2.1.0

[PATCH 18/20] drm: Goody bye, drm_bus!

[David Herrmann]

..we will not miss you..

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_ioctl.c|  8 +---
 drivers/gpu/drm/drm_pci.c  |  6 --
 drivers/gpu/drm/drm_platform.c |  5 -
 drivers/gpu/drm/drm_usb.c  | 12 
 include/drm/drmP.h |  5 -
 5 files changed, 1 insertion(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 4770bd7..3a1349f 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -250,15 +250,9 @@ static int drm_set_busid(struct drm_device *dev, struct 
drm_file *file_priv)
drm_unset_busid(dev, master);
return ret;
}
-   } else if (dev->driver->bus && dev->driver->bus->set_busid) {
-   ret = dev->driver->bus->set_busid(dev, master);
-   if (ret) {
-   drm_unset_busid(dev, master);
-   return ret;
-   }
} else {
if (WARN(dev->unique == NULL,
-"No drm_bus.set_busid() implementation provided by "
+"No drm_driver.set_busid() implementation provided by "
 "%ps. Use drm_dev_set_unique() to set the unique "
 "name explicitly.", dev->driver))
return -EINVAL;
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 0400c37..7563130 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -254,10 +254,6 @@ void drm_pci_agp_destroy(struct drm_device *dev)
}
 }

-static struct drm_bus drm_pci_bus = {
-   .set_busid = drm_pci_set_busid,
-};
-
 /**
  * drm_get_pci_dev - Register a PCI device with the DRM subsystem
  * @pdev: PCI device
@@ -338,8 +334,6 @@ int drm_pci_init(struct drm_driver *driver, struct 
pci_driver *pdriver)

DRM_DEBUG("\n");

-   driver->bus = _pci_bus;
-
if (driver->driver_features & DRIVER_MODESET)
return pci_register_driver(pdriver);

diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
index 939cd22..5314c9d 100644
--- a/drivers/gpu/drm/drm_platform.c
+++ b/drivers/gpu/drm/drm_platform.c
@@ -86,10 +86,6 @@ int drm_platform_set_busid(struct drm_device *dev, struct 
drm_master *master)
 }
 EXPORT_SYMBOL(drm_platform_set_busid);

-static struct drm_bus drm_platform_bus = {
-   .set_busid = drm_platform_set_busid,
-};
-
 /**
  * drm_platform_init - Register a platform device with the DRM subsystem
  * @driver: DRM device driver
@@ -105,7 +101,6 @@ int drm_platform_init(struct drm_driver *driver, struct 
platform_device *platfor
 {
DRM_DEBUG("\n");

-   driver->bus = _platform_bus;
return drm_get_platform_dev(platform_device, driver);
 }
 EXPORT_SYMBOL(drm_platform_init);
diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c
index f2fe94a..9c43490 100644
--- a/drivers/gpu/drm/drm_usb.c
+++ b/drivers/gpu/drm/drm_usb.c
@@ -36,16 +36,6 @@ err_free:
 }
 EXPORT_SYMBOL(drm_get_usb_dev);

-static int drm_usb_set_busid(struct drm_device *dev,
-  struct drm_master *master)
-{
-   return 0;
-}
-
-static struct drm_bus drm_usb_bus = {
-   .set_busid = drm_usb_set_busid,
-};
-
 /**
  * drm_usb_init - Register matching USB devices with the DRM subsystem
  * @driver: DRM device driver
@@ -61,8 +51,6 @@ int drm_usb_init(struct drm_driver *driver, struct usb_driver 
*udriver)
int res;
DRM_DEBUG("\n");

-   driver->bus = _usb_bus;
-
res = usb_register(udriver);
return res;
 }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index c82f292..5ae388a 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -605,10 +605,6 @@ struct drm_master {
 #define DRM_SCANOUTPOS_INVBL(1 << 1)
 #define DRM_SCANOUTPOS_ACCURATE (1 << 2)

-struct drm_bus {
-   int (*set_busid)(struct drm_device *dev, struct drm_master *master);
-};
-
 /**
  * DRM driver structure. This structure represent the common code for
  * a family of cards. There will one drm_device for each card present
@@ -846,7 +842,6 @@ struct drm_driver {
const struct drm_ioctl_desc *ioctls;
int num_ioctls;
const struct file_operations *fops;
-   struct drm_bus *bus;

/* List of devices hanging off this driver with stealth attach. */
struct list_head legacy_dev_list;
-- 
2.1.0

[PATCH 19/20] drm: merge drm_usb into udl

[David Herrmann]

This merges all the remains of drm_usb into its only user, udl. We can
then drop all the drm_usb stuff, including dev->usbdev.

Signed-off-by: David Herrmann 
---
 Documentation/DocBook/drm.tmpl  |   3 +-
 drivers/gpu/drm/Kconfig |   6 ---
 drivers/gpu/drm/Makefile|   3 --
 drivers/gpu/drm/drm_usb.c   |  76 ---
 drivers/gpu/drm/udl/Kconfig |   3 +-
 drivers/gpu/drm/udl/udl_connector.c |   4 +-
 drivers/gpu/drm/udl/udl_drv.c   | 102 +---
 drivers/gpu/drm/udl/udl_drv.h   |   1 +
 drivers/gpu/drm/udl/udl_main.c  |   8 +--
 include/drm/drmP.h  |   1 -
 include/drm/drm_usb.h   |  15 --
 11 files changed, 70 insertions(+), 152 deletions(-)
 delete mode 100644 drivers/gpu/drm/drm_usb.c
 delete mode 100644 include/drm/drm_usb.h

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 5c299fa..99f7ee6 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -291,10 +291,9 @@ char *date;
   Device Registration
   
 A number of functions are provided to help with device registration.
-The functions deal with PCI, USB and platform devices, respectively.
+The functions deal with PCI and platform devices, respectively.
   
 !Edrivers/gpu/drm/drm_pci.c
-!Edrivers/gpu/drm/drm_usb.c
 !Edrivers/gpu/drm/drm_platform.c
   
 New drivers that no longer rely on the services provided by the
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index e3500f9..e3b4b0f 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -25,12 +25,6 @@ config DRM_MIPI_DSI
bool
depends on DRM

-config DRM_USB
-   tristate
-   depends on DRM
-   depends on USB_SUPPORT && USB_ARCH_HAS_HCD
-   select USB
-
 config DRM_KMS_HELPER
tristate
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 9b7cb3f..9292a76 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -22,8 +22,6 @@ drm-$(CONFIG_PCI) += ati_pcigart.o
 drm-$(CONFIG_DRM_PANEL) += drm_panel.o
 drm-$(CONFIG_OF) += drm_of.o

-drm-usb-y   := drm_usb.o
-
 drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
drm_plane_helper.o drm_dp_mst_topology.o
 drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
@@ -36,7 +34,6 @@ CFLAGS_drm_trace_points.o := -I$(src)

 obj-$(CONFIG_DRM)  += drm.o
 obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
-obj-$(CONFIG_DRM_USB)   += drm_usb.o
 obj-$(CONFIG_DRM_TTM)  += ttm/
 obj-$(CONFIG_DRM_TDFX) += tdfx/
 obj-$(CONFIG_DRM_R128) += r128/
diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c
deleted file mode 100644
index 9c43490..000
--- a/drivers/gpu/drm/drm_usb.c
+++ /dev/null
@@ -1,76 +0,0 @@
-#include 
-#include 
-#include 
-#include 
-
-int drm_get_usb_dev(struct usb_interface *interface,
-   const struct usb_device_id *id,
-   struct drm_driver *driver)
-{
-   struct drm_device *dev;
-   int ret;
-
-   DRM_DEBUG("\n");
-
-   dev = drm_dev_alloc(driver, >dev);
-   if (!dev)
-   return -ENOMEM;
-
-   dev->usbdev = interface_to_usbdev(interface);
-   usb_set_intfdata(interface, dev);
-
-   ret = drm_dev_register(dev, 0);
-   if (ret)
-   goto err_free;
-
-   DRM_INFO("Initialized %s %d.%d.%d %s on minor %d\n",
-driver->name, driver->major, driver->minor, driver->patchlevel,
-driver->date, dev->primary->index);
-
-   return 0;
-
-err_free:
-   drm_dev_unref(dev);
-   return ret;
-
-}
-EXPORT_SYMBOL(drm_get_usb_dev);
-
-/**
- * drm_usb_init - Register matching USB devices with the DRM subsystem
- * @driver: DRM device driver
- * @udriver: USB device driver
- *
- * Registers one or more devices matched by a USB driver with the DRM
- * subsystem.
- *
- * Return: 0 on success or a negative error code on failure.
- */
-int drm_usb_init(struct drm_driver *driver, struct usb_driver *udriver)
-{
-   int res;
-   DRM_DEBUG("\n");
-
-   res = usb_register(udriver);
-   return res;
-}
-EXPORT_SYMBOL(drm_usb_init);
-
-/**
- * drm_usb_exit - Unregister matching USB devices from the DRM subsystem
- * @driver: DRM device driver
- * @udriver: USB device driver
- *
- * Unregisters one or more devices matched by a USB driver from the DRM
- * subsystem.
- */
-void drm_usb_exit(struct drm_driver *driver,
- struct usb_driver *udriver)
-{
-   usb_deregister(udriver);
-}
-EXPORT_SYMBOL(drm_usb_exit);
-
-MODULE_AUTHOR("David Airlie");
-MODULE_DESCRIPTION("USB DRM support");
-MODULE_LICENSE("GPL and additional rights");
diff --git a/drivers/gpu/drm/udl/Kconfig b/drivers/gpu/drm/udl/Kconfig
index f025286..613ab06 100644
--- a/drivers/gpu/drm/udl/Kconfig
+++ b/drivers/gpu/drm/udl/Kconfig
@@ 

[PATCH 17/20] drm: add driver->set_busid() callback

[David Herrmann]

One step closer to dropping all the drm_bus_* code:
Add a driver->set_busid() callback and make all drivers use the generic
helpers. Nouveau is the only driver that uses two different bus-types with
the same drm_driver. This is totally broken if both buses are available on
the same machine (unlikely, but lets be safe). Therefore, we create two
different drivers for each platform during module_init() and set the
set_busid() callback respectively.

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/armada/armada_drv.c  |  1 +
 drivers/gpu/drm/ast/ast_drv.c|  1 +
 drivers/gpu/drm/bochs/bochs_drv.c|  1 +
 drivers/gpu/drm/cirrus/cirrus_drv.c  |  1 +
 drivers/gpu/drm/drm_ioctl.c  |  8 +++-
 drivers/gpu/drm/drm_pci.c|  3 ++-
 drivers/gpu/drm/drm_platform.c   |  3 ++-
 drivers/gpu/drm/exynos/exynos_drm_drv.c  |  1 +
 drivers/gpu/drm/gma500/psb_drv.c |  1 +
 drivers/gpu/drm/i810/i810_drv.c  |  1 +
 drivers/gpu/drm/i915/i915_drv.c  |  1 +
 drivers/gpu/drm/mga/mga_drv.c|  1 +
 drivers/gpu/drm/mgag200/mgag200_drv.c|  1 +
 drivers/gpu/drm/msm/msm_drv.c|  1 +
 drivers/gpu/drm/nouveau/nouveau_drm.c| 19 +--
 drivers/gpu/drm/omapdrm/omap_drv.c   |  1 +
 drivers/gpu/drm/qxl/qxl_drv.c|  1 +
 drivers/gpu/drm/r128/r128_drv.c  |  1 +
 drivers/gpu/drm/radeon/radeon_drv.c  |  2 ++
 drivers/gpu/drm/rcar-du/rcar_du_drv.c|  1 +
 drivers/gpu/drm/savage/savage_drv.c  |  1 +
 drivers/gpu/drm/shmobile/shmob_drm_drv.c |  1 +
 drivers/gpu/drm/sis/sis_drv.c|  1 +
 drivers/gpu/drm/tdfx/tdfx_drv.c  |  1 +
 drivers/gpu/drm/tilcdc/tilcdc_drv.c  |  1 +
 drivers/gpu/drm/udl/udl_drv.c|  6 ++
 drivers/gpu/drm/via/via_drv.c|  1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c  |  1 +
 drivers/staging/imx-drm/imx-drm-core.c   |  1 +
 include/drm/drmP.h   |  3 +++
 30 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_drv.c 
b/drivers/gpu/drm/armada/armada_drv.c
index e2d5792..f672e6a 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -308,6 +308,7 @@ static struct drm_driver armada_drm_driver = {
.postclose  = NULL,
.lastclose  = armada_drm_lastclose,
.unload = armada_drm_unload,
+   .set_busid  = drm_platform_set_busid,
.get_vblank_counter = drm_vblank_count,
.enable_vblank  = armada_drm_enable_vblank,
.disable_vblank = armada_drm_disable_vblank,
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index f19682a..9a32d9d 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -199,6 +199,7 @@ static struct drm_driver driver = {

.load = ast_driver_load,
.unload = ast_driver_unload,
+   .set_busid = drm_pci_set_busid,

.fops = _fops,
.name = DRIVER_NAME,
diff --git a/drivers/gpu/drm/bochs/bochs_drv.c 
b/drivers/gpu/drm/bochs/bochs_drv.c
index 9738e9b..98837bd 100644
--- a/drivers/gpu/drm/bochs/bochs_drv.c
+++ b/drivers/gpu/drm/bochs/bochs_drv.c
@@ -82,6 +82,7 @@ static struct drm_driver bochs_driver = {
.driver_features= DRIVER_GEM | DRIVER_MODESET,
.load   = bochs_load,
.unload = bochs_unload,
+   .set_busid  = drm_pci_set_busid,
.fops   = _fops,
.name   = "bochs-drm",
.desc   = "bochs dispi vga interface (qemu stdvga)",
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.c 
b/drivers/gpu/drm/cirrus/cirrus_drv.c
index 919c73b..e705335 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.c
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.c
@@ -128,6 +128,7 @@ static struct drm_driver driver = {
.driver_features = DRIVER_MODESET | DRIVER_GEM,
.load = cirrus_driver_load,
.unload = cirrus_driver_unload,
+   .set_busid = drm_pci_set_busid,
.fops = _driver_fops,
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index cb6b54a..4770bd7 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -244,7 +244,13 @@ static int drm_set_busid(struct drm_device *dev, struct 
drm_file *file_priv)
if (master->unique != NULL)
drm_unset_busid(dev, master);

-   if (dev->driver->bus && dev->driver->bus->set_busid) {
+   if (dev->driver->set_busid) {
+   ret = dev->driver->set_busid(dev, master);
+   if (ret) {
+   drm_unset_busid(dev, master);
+   return ret;
+   }
+   } else if (dev->driver->bus && dev->driver->bus->set_busid) {
ret = 

[PATCH 20/20] drm: move drm-lock API to drm_legacy.h

[David Herrmann]

Same as the other legacy APIs, most of this is internal, so prefix it with
drm_legacy_* and move into drm_legacy.h.

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_fops.c  |  6 +++---
 drivers/gpu/drm/drm_ioctl.c |  4 ++--
 drivers/gpu/drm/drm_legacy.h|  9 +
 drivers/gpu/drm/drm_lock.c  | 20 +++-
 drivers/gpu/drm/i810/i810_dma.c |  4 ++--
 drivers/gpu/drm/savage/savage_bci.c |  4 ++--
 drivers/gpu/drm/sis/sis_mm.c|  6 +++---
 drivers/gpu/drm/via/via_mm.c|  6 +++---
 include/drm/drmP.h  | 14 +++---
 9 files changed, 38 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 12e6a1c..b419990 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -268,11 +268,11 @@ static void drm_master_release(struct drm_device *dev, 
struct file *filp)
 {
struct drm_file *file_priv = filp->private_data;

-   if (drm_i_have_hw_lock(dev, file_priv)) {
+   if (drm_legacy_i_have_hw_lock(dev, file_priv)) {
DRM_DEBUG("File %p released, freeing lock for context %d\n",
  filp, 
_DRM_LOCKING_CONTEXT(file_priv->master->lock.hw_lock->lock));
-   drm_lock_free(_priv->master->lock,
- 
_DRM_LOCKING_CONTEXT(file_priv->master->lock.hw_lock->lock));
+   drm_legacy_lock_free(_priv->master->lock,
+
_DRM_LOCKING_CONTEXT(file_priv->master->lock.hw_lock->lock));
}
 }

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 3a1349f..187dfaa 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -82,8 +82,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_ADD_DRAW, drm_noop, 
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
DRM_IOCTL_DEF(DRM_IOCTL_RM_DRAW, drm_noop, 
DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),

-   DRM_IOCTL_DEF(DRM_IOCTL_LOCK, drm_lock, DRM_AUTH),
-   DRM_IOCTL_DEF(DRM_IOCTL_UNLOCK, drm_unlock, DRM_AUTH),
+   DRM_IOCTL_DEF(DRM_IOCTL_LOCK, drm_legacy_lock, DRM_AUTH),
+   DRM_IOCTL_DEF(DRM_IOCTL_UNLOCK, drm_legacy_unlock, DRM_AUTH),

DRM_IOCTL_DEF(DRM_IOCTL_FINISH, drm_noop, DRM_AUTH),

diff --git a/drivers/gpu/drm/drm_legacy.h b/drivers/gpu/drm/drm_legacy.h
index d94c564..14e 100644
--- a/drivers/gpu/drm/drm_legacy.h
+++ b/drivers/gpu/drm/drm_legacy.h
@@ -78,4 +78,13 @@ struct drm_agp_mem {
struct list_head head;
 };

+/*
+ * Generic Userspace Locking-API
+ */
+
+int drm_legacy_i_have_hw_lock(struct drm_device *d, struct drm_file *f);
+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);
+
 #endif /* __DRM_LEGACY_H__ */
diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c
index 60f1481..727b032 100644
--- a/drivers/gpu/drm/drm_lock.c
+++ b/drivers/gpu/drm/drm_lock.c
@@ -52,7 +52,8 @@ static int drm_lock_take(struct drm_lock_data *lock_data, 
unsigned int context);
  *
  * Add the current task to the lock wait queue, and attempt to take to lock.
  */
-int drm_lock(struct drm_device *dev, void *data, struct drm_file *file_priv)
+int drm_legacy_lock(struct drm_device *dev, void *data,
+   struct drm_file *file_priv)
 {
DECLARE_WAITQUEUE(entry, current);
struct drm_lock *lock = data;
@@ -146,7 +147,7 @@ int drm_lock(struct drm_device *dev, void *data, struct 
drm_file *file_priv)
  *
  * Transfer and free the lock.
  */
-int drm_unlock(struct drm_device *dev, void *data, struct drm_file *file_priv)
+int drm_legacy_unlock(struct drm_device *dev, void *data, struct drm_file 
*file_priv)
 {
struct drm_lock *lock = data;
struct drm_master *master = file_priv->master;
@@ -157,7 +158,7 @@ int drm_unlock(struct drm_device *dev, void *data, struct 
drm_file *file_priv)
return -EINVAL;
}

-   if (drm_lock_free(>lock, lock->context)) {
+   if (drm_legacy_lock_free(>lock, lock->context)) {
/* FIXME: Should really bail out here. */
}

@@ -250,7 +251,7 @@ static int drm_lock_transfer(struct drm_lock_data 
*lock_data,
  * Marks the lock as not held, via the \p cmpxchg instruction. Wakes any task
  * waiting on the lock queue.
  */
-int drm_lock_free(struct drm_lock_data *lock_data, unsigned int context)
+int drm_legacy_lock_free(struct drm_lock_data *lock_data, unsigned int context)
 {
unsigned int old, new, prev;
volatile unsigned int *lock = _data->hw_lock->lock;
@@ -324,7 +325,7 @@ static int drm_notifier(void *priv)
  * having to worry about starvation.
  */

-void drm_idlelock_take(struct drm_lock_data *lock_data)
+void drm_legacy_idlelock_take(struct drm_lock_data *lock_data)
 {
int ret;

@@ -341,9 +342,9 

[Bug 83234] Opening Steam crashes X session

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=83234

--- Comment #3 from smoki  ---
 Just a guess but this might be duplicate of bug 78604 as xserver 1.15 can
crash with gcc libraries steam provide.

-- 
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/20140829/95e21296/attachment.html>

[Bug 83205] GPU lockup when entering settings in Verdun game with HyperZ enabled

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=83205

Marek Ol??k  changed:

   What|Removed |Added

 Blocks||75112

-- 
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/20140829/8bcd1e06/attachment.html>

[Bug 75112] Meta Bug for HyperZ issues on r600g and radeonsi

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=75112

Marek Ol??k  changed:

   What|Removed |Added

 Depends on||83205

-- 
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/20140829/5497c91b/attachment.html>

[Bug 83205] GPU lockup when entering settings in Verdun game with HyperZ enabled

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=83205

--- Comment #2 from Marek Ol??k  ---
Please apply both patches mentioned above and test again. If it still hangs,
please record a trace file with apitrace (the recording can be done with hyperz
disabled, but please make it as small as possible).

https://github.com/apitrace/apitrace

-- 
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/20140829/95be436b/attachment.html>

[PATCH 01/20] drm/radeon: move drm_buffer to drm/radeon/

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:27PM +0200, David Herrmann wrote:
> Radeon UMS is the last user of drm_buffer. Move it out of sight so radeon
> can drop it together with UMS.
> 
> Signed-off-by: David Herrmann 
> ---
>  drivers/gpu/drm/Makefile  |   2 +-
>  drivers/gpu/drm/drm_buffer.c  | 181 
> --
>  drivers/gpu/drm/radeon/Makefile   |   2 +-
>  drivers/gpu/drm/radeon/drm_buffer.c   | 177 +
>  drivers/gpu/drm/radeon/drm_buffer.h   | 148 +++
>  drivers/gpu/drm/radeon/r300_cmdbuf.c  |   2 +-
>  drivers/gpu/drm/radeon/radeon_state.c |   2 +-
>  include/drm/drm_buffer.h  | 148 ---
>  8 files changed, 329 insertions(+), 333 deletions(-)
>  delete mode 100644 drivers/gpu/drm/drm_buffer.c
>  create mode 100644 drivers/gpu/drm/radeon/drm_buffer.c
>  create mode 100644 drivers/gpu/drm/radeon/drm_buffer.h
>  delete mode 100644 include/drm/drm_buffer.h

Reviewed-by: Thierry Reding 
-- 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/20140829/c1861ee1/attachment.sig>

[Bug 83205] GPU lockup when entering settings in Verdun game with HyperZ enabled

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=83205

--- Comment #3 from smoki  ---

 It is obvious but just as reminder, second patch Hohahiu linked is for
evergreen you need this one for radeonsi instead:

 http://lists.freedesktop.org/archives/mesa-dev/2014-August/066388.html

-- 
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/20140829/98eb1415/attachment.html>

[PATCH 02/20] drm: mark drm_buf and drm_map as legacy

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:28PM +0200, David Herrmann wrote:
> Move internal declarations to drm_legacy.h and add drm_legacy_*() prefix
> to all legacy functions.

Perhaps this could give a short explanation of why they are marked as
legacy and what the plan is to get rid of them.

[...]
> diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
[...]
> @@ -378,7 +374,7 @@ int drm_addmap(struct drm_device * dev, resource_size_t 
> offset,
>   return rc;
>  }
>  
> -EXPORT_SYMBOL(drm_addmap);
> +EXPORT_SYMBOL(drm_legacy_addmap);

Nit: might as well remove the unusual blank line between the closing
brace and the EXPORT_SYMBOL while at it.

> diff --git a/drivers/gpu/drm/mga/mga_dma.c b/drivers/gpu/drm/mga/mga_dma.c
[...]
>   offset = 0;
> - err = drm_addmap(dev, offset, warp_size,
> -  _DRM_AGP, _DRM_READ_ONLY, _priv->warp);
> + err = drm_legacy_addmap(dev, offset, warp_size,
> + _DRM_AGP, _DRM_READ_ONLY, _priv->warp);

The parameters are somewhat oddly wrapped here. Nothing that the patch
introduces, but perhaps low-hanging fruit while at it. There are a few
other occurrences in this file.

I found only minor whitespace nits, feel free to ignore those.

Reviewed-by: Thierry Reding 
-- 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/20140829/ec4bff29/attachment.sig>

[PATCH 03/20] drm: move "struct drm_vma_entry" to drm_vm.c

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:29PM +0200, David Herrmann wrote:
> Make all the drm_vma_entry handling local to drm_vm.c and hide it from
> global headers. This requires to extract the inlined legacy drm_vma_entry
> cleanup into a small helper and also move a weirdly placed drm_vma_info
> helper into drm_vm.c.
> 
> Signed-off-by: David Herrmann 
> ---
>  drivers/gpu/drm/drm_fops.c | 10 +-
>  drivers/gpu/drm/drm_info.c | 59 ---
>  drivers/gpu/drm/drm_vm.c   | 76 
> ++
>  include/drm/drmP.h |  8 ++---
>  4 files changed, 79 insertions(+), 74 deletions(-)

Reviewed-by: Thierry Reding 
-- 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/20140829/0d982f27/attachment-0001.sig>

[Bug 83205] GPU lockup when entering settings in Verdun game with HyperZ enabled

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=83205

--- Comment #4 from Cl?ment Gu?rin  ---
Thanks for the pointers. I will try as soon as I can.

-- 
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/20140829/f289dffc/attachment.html>

[PATCH 04/20] drm: move "struct drm_magic_entry" to drm_auth.c

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:30PM +0200, David Herrmann wrote:
> In drm_release(), we currently call drm_remove_magic() if the drm_file
> has a drm-magic attached. Therefore, once drm_master_release() is called,
> the magic-list _must_ be empty.
> 
> By dropping the no-op cleanup, we can move "struct drm_magic_entry" to
> drm_auth.c and avoid exposing it to all of DRM.
> 
> Signed-off-by: David Herrmann 
> ---
>  drivers/gpu/drm/drm_auth.c | 6 ++
>  drivers/gpu/drm/drm_drv.c  | 7 ---
>  include/drm/drmP.h | 6 --
>  3 files changed, 6 insertions(+), 13 deletions(-)

Reviewed-by: Thierry Reding 
-- 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/20140829/0b579f98/attachment.sig>

[PATCH 05/20] drm: drop unused "struct drm_waitlist"

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:31PM +0200, David Herrmann wrote:
> This structure is unused, drop it.
> 
> Signed-off-by: David Herrmann 
> ---
>  include/drm/drmP.h | 11 ---
>  1 file changed, 11 deletions(-)

Reviewed-by: Thierry Reding 
-- 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/20140829/98304f92/attachment.sig>

[PATCH 06/20] drm: move AGP definitions harder

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:32PM +0200, David Herrmann wrote:
> Move drm_agp_head to drm_agpsupport.h and drm_agp_mem into drm_legacy.h.
> Unfortunately, drivers still heavily access drm_agp_head so we cannot
> move it to drm_legacy.h. However, at least it's no longer visible in
> drmP.h now (it's directly included from it, though).
> 
> Signed-off-by: David Herrmann 
> ---
>  drivers/gpu/drm/drm_agpsupport.c |  1 +
>  drivers/gpu/drm/drm_legacy.h | 15 +++
>  drivers/gpu/drm/drm_memory.c |  1 +
>  drivers/gpu/drm/drm_vm.c |  1 +
>  include/drm/drmP.h   | 30 +-
>  include/drm/drm_agpsupport.h | 13 +
>  6 files changed, 32 insertions(+), 29 deletions(-)

I don't know exactly what you mean by "move harder" in the subject, but
other than that this looks good, so with a better subject this is:

Reviewed-by: Thierry Reding 
-- 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/20140829/eff4001f/attachment.sig>

[PATCH 07/20] drm: replace weird conditional includes

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:33PM +0200, David Herrmann wrote:
> pte_wrprotect() is only used by drm_vm.c, so move the include there. Also
> include it unconditionally, all architectures provide this header!
> 
> Furthermore, replace asm/current.h with sched.h, which includes
> asm/current.h unconditionally. This way we get the same effect and avoid
> direct asm/ includes. Furthermore, drop the weird __alpha__ protection.
> It's safe to include sched.h everywhere (and the wait.h comment doesn't
> apply, anyway).
> 
> Signed-off-by: David Herrmann 
> ---
>  drivers/gpu/drm/drm_vm.c |  1 +
>  include/drm/drmP.h   | 10 +-
>  2 files changed, 2 insertions(+), 9 deletions(-)

Reviewed-by: Thierry Reding 
-- 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/20140829/0e760bd6/attachment.sig>

[PATCH 08/20] drm: drop __KERNEL__ protection in drmP.h

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:34PM +0200, David Herrmann wrote:
> drmP.h is internal to the kernel. No need to keep the __KERNEL__
> protection.
> 
> Signed-off-by: David Herrmann 
> ---
>  include/drm/drmP.h | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Thierry Reding 
-- 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/20140829/88255ec7/attachment.sig>

[PATCH 09/20] drm: merge drm_memory.h into drm_memory.c

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:35PM +0200, David Herrmann wrote:
> The drm_memory.h header is only used to define PAGE_AGP, which is only
> used in drm_memory.c. Fold the header into drm_memory.c and drop it.
> 
> Signed-off-by: David Herrmann 
> ---
>  drivers/gpu/drm/drm_memory.c | 11 +
>  include/drm/drmP.h   |  6 ++---
>  include/drm/drm_memory.h | 59 
> 
>  3 files changed, 13 insertions(+), 63 deletions(-)
>  delete mode 100644 include/drm/drm_memory.h
> 
> diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
> index 7888dad..62fda6a 100644
> --- a/drivers/gpu/drm/drm_memory.c
> +++ b/drivers/gpu/drm/drm_memory.c
> @@ -39,6 +39,17 @@
>  #include "drm_legacy.h"
>  
>  #if __OS_HAS_AGP
> +
> +#ifdef HAVE_PAGE_AGP
> +# include 
> +#else

This check seems to be redundant. All architectures that support AGP
provide this header.

> +# ifdef __powerpc__
> +#  define PAGE_AGP   __pgprot(_PAGE_KERNEL | _PAGE_NO_CACHE)

Is this even necessary? It seems like PowerPC always defines
HAVE_PAGE_AGP.

> +# else
> +#  define PAGE_AGP   PAGE_KERNEL
> +# endif
> +#endif

Shouldn't this simply be moved into the asm/agp.h header for each of the
architectures that has AGP?

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/20140829/e9eb97c8/attachment-0001.sig>

[PATCH 1/2] drm: make idr_mutex a spinlock

[David Herrmann]

There is no reason to use a heavy mutex for idr protection. Use a spinlock
and make idr-allocation use idr_preload().

This patch also makes mode-object lookup irq-save, in case you ever wanna
lookup modeset objects from interrupts. This is just a side-effect of
avoiding a mutex.

Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_crtc.c | 34 --
 include/drm/drm_crtc.h |  4 ++--
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 61b6978..97eba56 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -283,8 +283,10 @@ static int drm_mode_object_get_reg(struct drm_device *dev,
 {
int ret;

-   mutex_lock(>mode_config.idr_mutex);
-   ret = idr_alloc(>mode_config.crtc_idr, register_obj ? obj : NULL, 
1, 0, GFP_KERNEL);
+   idr_preload(GFP_KERNEL);
+   spin_lock_irq(>mode_config.idr_lock);
+
+   ret = idr_alloc(>mode_config.crtc_idr, register_obj ? obj : NULL, 
1, 0, GFP_NOWAIT);
if (ret >= 0) {
/*
 * Set up the object linking under the protection of the idr
@@ -293,7 +295,9 @@ static int drm_mode_object_get_reg(struct drm_device *dev,
obj->id = ret;
obj->type = obj_type;
}
-   mutex_unlock(>mode_config.idr_mutex);
+
+   spin_unlock_irq(>mode_config.idr_lock);
+   idr_preload_end();

return ret < 0 ? ret : 0;
 }
@@ -322,9 +326,9 @@ int drm_mode_object_get(struct drm_device *dev,
 static void drm_mode_object_register(struct drm_device *dev,
 struct drm_mode_object *obj)
 {
-   mutex_lock(>mode_config.idr_mutex);
+   spin_lock_irq(>mode_config.idr_lock);
idr_replace(>mode_config.crtc_idr, obj, obj->id);
-   mutex_unlock(>mode_config.idr_mutex);
+   spin_unlock_irq(>mode_config.idr_lock);
 }

 /**
@@ -339,17 +343,18 @@ static void drm_mode_object_register(struct drm_device 
*dev,
 void drm_mode_object_put(struct drm_device *dev,
 struct drm_mode_object *object)
 {
-   mutex_lock(>mode_config.idr_mutex);
+   spin_lock_irq(>mode_config.idr_lock);
idr_remove(>mode_config.crtc_idr, object->id);
-   mutex_unlock(>mode_config.idr_mutex);
+   spin_unlock_irq(>mode_config.idr_lock);
 }

 static struct drm_mode_object *_object_find(struct drm_device *dev,
uint32_t id, uint32_t type)
 {
struct drm_mode_object *obj = NULL;
+   unsigned long flags;

-   mutex_lock(>mode_config.idr_mutex);
+   spin_lock_irqsave(>mode_config.idr_lock, flags);
obj = idr_find(>mode_config.crtc_idr, id);
if (obj && type != DRM_MODE_OBJECT_ANY && obj->type != type)
obj = NULL;
@@ -358,7 +363,7 @@ static struct drm_mode_object *_object_find(struct 
drm_device *dev,
/* don't leak out unref'd fb's */
if (obj && (obj->type == DRM_MODE_OBJECT_FB))
obj = NULL;
-   mutex_unlock(>mode_config.idr_mutex);
+   spin_unlock_irqrestore(>mode_config.idr_lock, flags);

return obj;
 }
@@ -433,9 +438,9 @@ EXPORT_SYMBOL(drm_framebuffer_init);
 static void __drm_framebuffer_unregister(struct drm_device *dev,
 struct drm_framebuffer *fb)
 {
-   mutex_lock(>mode_config.idr_mutex);
+   spin_lock_irq(>mode_config.idr_lock);
idr_remove(>mode_config.crtc_idr, fb->base.id);
-   mutex_unlock(>mode_config.idr_mutex);
+   spin_unlock_irq(>mode_config.idr_lock);

fb->base.id = 0;
 }
@@ -465,14 +470,15 @@ static struct drm_framebuffer 
*__drm_framebuffer_lookup(struct drm_device *dev,
 {
struct drm_mode_object *obj = NULL;
struct drm_framebuffer *fb;
+   unsigned long flags;

-   mutex_lock(>mode_config.idr_mutex);
+   spin_lock_irqsave(>mode_config.idr_lock, flags);
obj = idr_find(>mode_config.crtc_idr, id);
if (!obj || (obj->type != DRM_MODE_OBJECT_FB) || (obj->id != id))
fb = NULL;
else
fb = obj_to_fb(obj);
-   mutex_unlock(>mode_config.idr_mutex);
+   spin_unlock_irqrestore(>mode_config.idr_lock, flags);

return fb;
 }
@@ -5049,7 +5055,7 @@ void drm_mode_config_init(struct drm_device *dev)
 {
mutex_init(>mode_config.mutex);
drm_modeset_lock_init(>mode_config.connection_mutex);
-   mutex_init(>mode_config.idr_mutex);
+   spin_lock_init(>mode_config.idr_lock);
mutex_init(>mode_config.fb_lock);
INIT_LIST_HEAD(>mode_config.fb_list);
INIT_LIST_HEAD(>mode_config.crtc_list);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 77d9763..9c57b56 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -742,7 +742,7 @@ struct drm_mode_group {
 /**
  * drm_mode_config - Mode configuration control structure
  * @mutex: mutex protecting KMS related lists and 

[PATCH 2/2] drm: don't recycle used modeset IDs

[David Herrmann]

With MST, we now have connector hotplugging, yey! Pretty easy to use in
user-space, but introduces some nasty races:
 * If a connector is removed and added again while a compositor is in
   background, it will get the same ID again. If the compositor wakes up,
   it cannot know whether its the same connector or a new one, thus they
   must re-read EDID information, etc.
 * possible_clones, possible_crtcs, etc. depend on indices, not IDs. So if
   an object is removed and a new one is added, those bitmasks are invalid
   and must be refreshed. This currently does not affect connectors, but
   only crtcs and encoders, but it's only a matter of time when this will
   change.

The easiest way to protect against this, is to not recylce modeset IDs.
Instead, always allocate a new, higher ID. All ioctls that affect modeset
objects, take IDs. Thus, during hotplug races, those ioctls will simply
fail if invalid IDs are passed. They will no longer silently run on a
newly hotplugged object.

Furthermore, hotplug-races during state sync can now be easily detected. A
call to GET_RESOURCES returns a list of available IDs atomically.
User-space can now start fetching all those objects via GET_* ioctls. If
any of those fails, they know that the given object was unplugged. Thus,
the "possible_*" bit-fields are invalidated. User-space can now decide
whether to restart the sync all over or wait for the 'change' uevent that
is sent on modeset object modifications (or, well, is supposed to be sent
and probably will be at some point).

With this in place, modeset object hotplugging should work fine for all
modeset objects in the KMS API.

CC'ing stable so we can rely on all kernels with MST to not recycle IDs.

Cc:  # 3.16+
Signed-off-by: David Herrmann 
---
 drivers/gpu/drm/drm_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 97eba56..ab0fe6d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -286,7 +286,7 @@ static int drm_mode_object_get_reg(struct drm_device *dev,
idr_preload(GFP_KERNEL);
spin_lock_irq(>mode_config.idr_lock);

-   ret = idr_alloc(>mode_config.crtc_idr, register_obj ? obj : NULL, 
1, 0, GFP_NOWAIT);
+   ret = idr_alloc_cyclic(>mode_config.crtc_idr, register_obj ? obj : 
NULL, 1, 0, GFP_NOWAIT);
if (ret >= 0) {
/*
 * Set up the object linking under the protection of the idr
-- 
2.1.0

[PATCH 10/20] drm: move __OS_HAS_AGP into drm_agpsupport.h

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:36PM +0200, David Herrmann wrote:
> With drm_memory.h gone, there is no header left that uses __OS_HAS_AGP.
> Move it into drm_agpsupport.h (which is itself included from drmP.h) to
> hide it harder from public eyes.
> 
> Signed-off-by: David Herrmann 
> ---
>  include/drm/drmP.h   | 2 --
>  include/drm/drm_agpsupport.h | 3 +++
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 294f7da..c6f337c 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -67,8 +67,6 @@
>  
>  #include 
>  
> -#define __OS_HAS_AGP (defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) && 
> defined(MODULE)))
> -
>  struct module;
>  
>  struct drm_file;
> diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h
> index 3bebeb4..4f1724c 100644
> --- a/include/drm/drm_agpsupport.h
> +++ b/include/drm/drm_agpsupport.h
> @@ -8,6 +8,9 @@
>  #include 
>  #include 
>  
> +#define __OS_HAS_AGP (defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) && \
> +   defined(MODULE)))

I'm not really sure what the intent was of the final defined(MODULE).
Surely the fact whether a driver is being built as a module or not does
not influence whether or not the OS supports AGP.

And if this is merely meant to make sure that drivers that are built-in
don't break to build if AGP is a module, then that should be a job for
Kconfig rather than some macro.

So I think the above could simply become:

#define __OS_HAS_AGP IS_ENABLED(CONFIG_AGP)

And once we have that I think we could even easily get rid of the custom
__OS_HAS_AGP macro and use IS_ENABLED(CONFIG_AGP) directly.

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/20140829/2f70ce4f/attachment.sig>

[PATCH 11/20] drm: order includes alphabetically in drmP.h

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:37PM +0200, David Herrmann wrote:
> It is hardly possible to review the drmP.h includes, anymore. Order them
> alphabetically, linux/ first, then asm/ and then local drm/ includes.
> 
> Since a long time ago, kernel headers have been converted to include
> required headers themselves. No-one cares whether that means the compiler
> has to include a header multiple times. In fact, GCC already does some
> optimization regarding multiple inclusions if a sorrounding #ifndef is
> present.
> 
> Signed-off-by: David Herrmann 
> ---
>  include/drm/drmP.h | 46 +++---
>  1 file changed, 23 insertions(+), 23 deletions(-)

This looks good, but I don't think I'm going to bother manually checking
each of those. Provided everything still complies, this is at least:

Acked-by: Thierry Reding 
-- 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/20140829/9456d988/attachment.sig>

[PATCH 12/20] drm: drop DRM_DEBUG_CODE

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:38PM +0200, David Herrmann wrote:
[...]
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 83a2461..5cfa574 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -2761,18 +2761,25 @@ void radeon_atombios_fini(struct radeon_device *rdev);
>  /*
>   * RING helpers.
>   */
> -#if DRM_DEBUG_CODE == 0
> +
> +/**
> + * radeon_ring_write - write a value to the ring
> + *
> + * @ring: radeon_ring structure holding ring information
> + * @v: dword (dw) value to write
> + *
> + * Write a value to the requested ring buffer (all asics).
> + */
>  static inline void radeon_ring_write(struct radeon_ring *ring, uint32_t v)
>  {
> + if (ring->count_dw <= 0)
> + DRM_ERROR("radeon: writing more dwords to the ring than 
> expected!\n");
> +
>   ring->ring[ring->wptr++] = v;
>   ring->wptr &= ring->ptr_mask;
>   ring->count_dw--;
>   ring->ring_free_dw--;
>  }
> -#else
> -/* With debugging this is just too big to inline */
> -void radeon_ring_write(struct radeon_ring *ring, uint32_t v);
> -#endif
>  
>  /*
>   * ASICs macro.
> diff --git a/drivers/gpu/drm/radeon/radeon_ring.c 
> b/drivers/gpu/drm/radeon/radeon_ring.c
> index d656079..6f2a9bd 100644
> --- a/drivers/gpu/drm/radeon/radeon_ring.c
> +++ b/drivers/gpu/drm/radeon/radeon_ring.c
> @@ -45,27 +45,6 @@
>  static int radeon_debugfs_ring_init(struct radeon_device *rdev, struct 
> radeon_ring *ring);
>  
>  /**
> - * radeon_ring_write - write a value to the ring
> - *
> - * @ring: radeon_ring structure holding ring information
> - * @v: dword (dw) value to write
> - *
> - * Write a value to the requested ring buffer (all asics).
> - */
> -void radeon_ring_write(struct radeon_ring *ring, uint32_t v)
> -{
> -#if DRM_DEBUG_CODE
> - if (ring->count_dw <= 0) {
> - DRM_ERROR("radeon: writing more dwords to the ring than 
> expected!\n");
> - }
> -#endif
> - ring->ring[ring->wptr++] = v;
> - ring->wptr &= ring->ptr_mask;
> - ring->count_dw--;
> - ring->ring_free_dw--;
> -}

The original was clearly never tested with DRM_DEBUG_CODE != 0. I don't
see how it could've built given that the header protects the complete
function whereas the implementation protects only part of the function.

Reviewed-by: Thierry Reding 
-- 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/20140829/af04788d/attachment.sig>

[PATCH 13/20] drm: inline "struct drm_sigdata"

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:39PM +0200, David Herrmann wrote:
> The sigdata structure is only used to group two fields in drm_device.
> Inline it and make it an unnamed object.

I'm not sure what unnamed object means.

> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
[...]
> @@ -1035,9 +1029,13 @@ struct drm_device {
>  
>   struct drm_sg_mem *sg;  /**< Scatter gather memory */
>   unsigned int num_crtcs;  /**< Number of CRTCs on this 
> device */
> - struct drm_sigdata sigdata;/**< For block_all_signals */
>   sigset_t sigmask;
>  
> + struct {
> + int context;

Your patch doesn't introduce this, but shouldn't context here be
unsigned given that it's compared to drm_hw_lock's .lock field?

I guess struct drm_lock could have that same change. I suppose it might
not be all that important since presumably the code works, but it's
still inconsistent use of types.

Anyway, if you could clarify the "unnamed object" in the commit
description, this patch:

Reviewed-by: Thierry Reding 
-- 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/20140829/46e5e666/attachment-0001.sig>

[PATCH 14/20] drm: move remaining includes in drmP.h to the top

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:40PM +0200, David Herrmann wrote:
[...]
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
[...]
> @@ -64,8 +61,14 @@
>  #include 
>  #include 
>  
> -#include 
> +#include 
> +#include 

The uapi/ prefix is not strictly necessary since include/uapi is in the
include path, but I guess it can't hurt to be explicit, so:

Reviewed-by: Thierry Reding 
-- 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/20140829/2b3362cd/attachment.sig>

[Intel-gfx] [PATCH 4/9] drm/i915: split intel_update_plane into check() and commit()

[Daniel Vetter]

On Fri, Aug 29, 2014 at 10:55:41AM +0300, Ville Syrj?l? wrote:
> On Thu, Aug 28, 2014 at 02:40:08PM -0300, 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.
> > 
> > Signed-off-by: Gustavo Padovan 
> > ---
> >  drivers/gpu/drm/i915/intel_sprite.c | 128 
> > ++--
> >  1 file changed, 93 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index 4cbe286..b1cb606 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -844,22 +844,32 @@ static bool colorkey_enabled(struct intel_plane 
> > *intel_plane)
> > return key.flags != I915_SET_COLORKEY_NONE;
> >  }
> >  
> > +static bool get_visible(struct drm_rect *src, struct drm_rect *dst,
> 
> get_visisble() is not a good name here IMO, also I think this split is at
> a slightly too low level. What we really want is to start creating some
> kind of plane config struct that can be passed to both check and commit,
> and then check can already store all the clipped coordinates etc. to the
> plane config and commit can just look them up w/o recomputing them.
> 
> Initially you could just have one such struct on the stack in
> intel_update_plane() and pass it to both functions. Later we'll need to
> figure out how to pass around the plane configs for all planes during
> the nuclear flip, but there's no need to worry about such things quite yet.

To avoid too much rework I think it would be good to peak at the
drm_plane_state structure in either Rob's or my atomic branch (iirc
they're the same anyway), so that we can align as much as possible with
the atomic interface.

On the crtc side for full modesets we already have intel_crtc_config,
which doesn't really fully align with drm_crtc_state. So lots of work
required there.

Otherwise I think Ville's plan to start out with an on-stack
intel_plane_config sounds sane.
-Daniel

> 
> > +   const struct drm_rect *clip,
> > +   int min_scale, int max_scale)
> > +{
> > +   int hscale, vscale;
> > +
> > +   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);
> > +   BUG_ON(vscale < 0);
> > +
> > +   return drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
> > +}
> > +
> >  static int
> > -intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > +intel_check_sprite_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)
> >  {
> > -   struct drm_device *dev = plane->dev;
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > struct intel_plane *intel_plane = to_intel_plane(plane);
> > -   enum pipe pipe = intel_crtc->pipe;
> > 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 hscale, vscale;
> > int max_scale, min_scale;
> > @@ -882,20 +892,6 @@ intel_update_plane(struct drm_plane *plane, struct 
> > drm_crtc *crtc,
> > .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) {
> > @@ -930,13 +926,7 @@ intel_update_plane(struct drm_plane *plane, struct 
> > drm_crtc *crtc,
> > drm_rect_rotate(, fb->width << 16, fb->height << 16,
> > intel_plane->rotation);
> >  
> > -   hscale = drm_rect_calc_hscale_relaxed(, , min_scale, max_scale);
> > -   BUG_ON(hscale < 0);
> > -
> > -   vscale = drm_rect_calc_vscale_relaxed(, , min_scale, max_scale);
> > -   BUG_ON(vscale < 0);
> > -
> > -   visible = drm_rect_clip_scaled(, , , hscale, vscale);
> > +   

[Bug 83234] Opening Steam crashes X session

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=83234

Maciej  changed:

   What|Removed |Added

   Severity|normal  |major

-- 
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/20140829/e681f085/attachment.html>

[PATCH 06/20] drm: move AGP definitions harder

[Daniel Vetter]

On Fri, Aug 29, 2014 at 01:43:10PM +0200, Thierry Reding wrote:
> On Fri, Aug 29, 2014 at 12:12:32PM +0200, David Herrmann wrote:
> > Move drm_agp_head to drm_agpsupport.h and drm_agp_mem into drm_legacy.h.
> > Unfortunately, drivers still heavily access drm_agp_head so we cannot
> > move it to drm_legacy.h. However, at least it's no longer visible in
> > drmP.h now (it's directly included from it, though).
> > 
> > Signed-off-by: David Herrmann 
> > ---
> >  drivers/gpu/drm/drm_agpsupport.c |  1 +
> >  drivers/gpu/drm/drm_legacy.h | 15 +++
> >  drivers/gpu/drm/drm_memory.c |  1 +
> >  drivers/gpu/drm/drm_vm.c |  1 +
> >  include/drm/drmP.h   | 30 +-
> >  include/drm/drm_agpsupport.h | 13 +
> >  6 files changed, 32 insertions(+), 29 deletions(-)
> 
> I don't know exactly what you mean by "move harder" in the subject, but
> other than that this looks good, so with a better subject this is:

I guess "Try to hide AGP definitions harder" would be more accurate.
-Daniel

> 
> Reviewed-by: Thierry Reding 



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 15/20] drm: simplify drm_*_set_unique()

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:41PM +0200, David Herrmann wrote:
> Lets use kasprintf() to avoid pre-allocating the buffer. This is really
> nothing to optimize for speed and the input is trusted, so kasprintf() is
> just fine.
> 
> Signed-off-by: David Herrmann 
> ---
>  drivers/gpu/drm/drm_pci.c  | 30 --
>  drivers/gpu/drm/drm_platform.c | 31 ---
>  2 files changed, 16 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 020cfd9..8efea6b 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -129,31 +129,17 @@ static int drm_get_pci_domain(struct drm_device *dev)
>  
>  static int drm_pci_set_busid(struct drm_device *dev, struct drm_master 
> *master)
>  {
> - int len, ret;
> - master->unique_len = 40;
> - master->unique_size = master->unique_len;
> - master->unique = kmalloc(master->unique_size, GFP_KERNEL);
> - if (master->unique == NULL)
> + master->unique = kasprintf(GFP_KERNEL, "pci:%04x:%02x:%02x.%d",
> + drm_get_pci_domain(dev),
> + dev->pdev->bus->number,
> + PCI_SLOT(dev->pdev->devfn),
> + PCI_FUNC(dev->pdev->devfn));

I think we've been trying to standardize on aligning parameters on
subsequent lines with the first parameter on the first line.

[...]
> + master->unique_len = strlen(master->unique);
> + master->unique_size = master->unique_len + 1;

[...]
> diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
[...]
>  static int drm_platform_set_busid(struct drm_device *dev, struct drm_master 
> *master)
>  {
> - int len, ret, id;
> -
> - master->unique_len = 13 + strlen(dev->platformdev->name);
> - master->unique_size = master->unique_len;
> - master->unique = kmalloc(master->unique_len + 1, GFP_KERNEL);
> -
> - if (master->unique == NULL)
> - return -ENOMEM;
> + int id;
>  
>   id = dev->platformdev->id;
> -
> - /* if only a single instance of the platform device, id will be
> -  * set to -1.. use 0 instead to avoid a funny looking bus-id:
> -  */
> - if (id == -1)
> + if (id < 0)
>   id = 0;

Perhaps collapse all of the above into:

int id = (dev->platformdev->id < 0) ? 0 : dev->platformdev->id;

? Not that it matters much. I suspect we could easily remove all traces
of this particular function in a next step.

> - len = snprintf(master->unique, master->unique_len,
> - "platform:%s:%02d", dev->platformdev->name, id);
> -
> - if (len > master->unique_len) {
> - DRM_ERROR("Unique buffer overflowed\n");
> - ret = -EINVAL;
> - goto err;
> - }
> + master->unique = kasprintf(GFP_KERNEL, "platform:%s:%02d",
> + dev->platformdev->name, id);
> + if (!master->unique)
> + return -ENOMEM;
>  
> + master->unique_len = strlen(master->unique);
> + master->unique_size = master->unique_len;

unique_size is weird. It seems to me like it should always be unique_len
+ 1. Why drm_platform_bus should be special escapes me. Also, after this
patch it seems to be completely unused, so perhaps we should just drop
it.

All of those comments can either be addressed in a separate patch (or
ignored), though, so:

Reviewed-by: Thierry Reding 
-- 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/20140829/a563d2d5/attachment-0001.sig>

[PATCH 16/20] drm: drop unused drm_master->unique_size

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:42PM +0200, David Herrmann wrote:
> This field is unused and there is really no reason to optimize
> unique-allocations. Drop it.
> 
> Signed-off-by: David Herrmann 
> ---
>  drivers/gpu/drm/drm_ioctl.c| 1 -
>  drivers/gpu/drm/drm_pci.c  | 4 +---
>  drivers/gpu/drm/drm_platform.c | 1 -
>  include/drm/drmP.h | 2 --
>  4 files changed, 1 insertion(+), 7 deletions(-)

Now that's what I get for not reading mails in advance. Anyway I'm
glad we agree on the uselessness of unique_size. =)

Reviewed-by: Thierry Reding 
-- 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/20140829/b3e1618a/attachment.sig>

[PATCH 09/20] drm: merge drm_memory.h into drm_memory.c

[Daniel Vetter]

On Fri, Aug 29, 2014 at 01:56:23PM +0200, Thierry Reding wrote:
> On Fri, Aug 29, 2014 at 12:12:35PM +0200, David Herrmann wrote:
> > The drm_memory.h header is only used to define PAGE_AGP, which is only
> > used in drm_memory.c. Fold the header into drm_memory.c and drop it.
> > 
> > Signed-off-by: David Herrmann 
> > ---
> >  drivers/gpu/drm/drm_memory.c | 11 +
> >  include/drm/drmP.h   |  6 ++---
> >  include/drm/drm_memory.h | 59 
> > 
> >  3 files changed, 13 insertions(+), 63 deletions(-)
> >  delete mode 100644 include/drm/drm_memory.h
> > 
> > diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
> > index 7888dad..62fda6a 100644
> > --- a/drivers/gpu/drm/drm_memory.c
> > +++ b/drivers/gpu/drm/drm_memory.c
> > @@ -39,6 +39,17 @@
> >  #include "drm_legacy.h"
> >  
> >  #if __OS_HAS_AGP
> > +
> > +#ifdef HAVE_PAGE_AGP
> > +# include 
> > +#else
> 
> This check seems to be redundant. All architectures that support AGP
> provide this header.
> 
> > +# ifdef __powerpc__
> > +#  define PAGE_AGP __pgprot(_PAGE_KERNEL | _PAGE_NO_CACHE)
> 
> Is this even necessary? It seems like PowerPC always defines
> HAVE_PAGE_AGP.

Iirc I've tried to untangle this before and I'm not 100% this is actually
the case on all ppc platforms. It looked like there's some crazy include
header depency stuff going on.

But given how popular drm on ppc and that the few platforms where people
actually use gpus are the saner ones (hopefully) I think we can just move
ahead with this change and fixup any compile breakage once it's reported.
If there is any.

> > +# else
> > +#  define PAGE_AGP PAGE_KERNEL
> > +# endif
> > +#endif
> 
> Shouldn't this simply be moved into the asm/agp.h header for each of the
> architectures that has AGP?

Same comment really, I think the include mess is to hard to untangle. At
least I've failed. So with the #ifdef HAVE_PAGE_AGP dropped around the
include as Thierry suggested this is

Reviewed-by: Daniel Vetter 

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 10/20] drm: move __OS_HAS_AGP into drm_agpsupport.h

[Daniel Vetter]

On Fri, Aug 29, 2014 at 02:03:12PM +0200, Thierry Reding wrote:
> On Fri, Aug 29, 2014 at 12:12:36PM +0200, David Herrmann wrote:
> > With drm_memory.h gone, there is no header left that uses __OS_HAS_AGP.
> > Move it into drm_agpsupport.h (which is itself included from drmP.h) to
> > hide it harder from public eyes.
> > 
> > Signed-off-by: David Herrmann 
> > ---
> >  include/drm/drmP.h   | 2 --
> >  include/drm/drm_agpsupport.h | 3 +++
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index 294f7da..c6f337c 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -67,8 +67,6 @@
> >  
> >  #include 
> >  
> > -#define __OS_HAS_AGP (defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) 
> > && defined(MODULE)))
> > -
> >  struct module;
> >  
> >  struct drm_file;
> > diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h
> > index 3bebeb4..4f1724c 100644
> > --- a/include/drm/drm_agpsupport.h
> > +++ b/include/drm/drm_agpsupport.h
> > @@ -8,6 +8,9 @@
> >  #include 
> >  #include 
> >  
> > +#define __OS_HAS_AGP (defined(CONFIG_AGP) || (defined(CONFIG_AGP_MODULE) 
> > && \
> > + defined(MODULE)))
> 
> I'm not really sure what the intent was of the final defined(MODULE).
> Surely the fact whether a driver is being built as a module or not does
> not influence whether or not the OS supports AGP.

I think this was to make sure agp drivers are loaded before the drm
drivers. But since ages we can build the different agp drivers
indidivudally as modules, so this stopped making any sense at all.

> And if this is merely meant to make sure that drivers that are built-in
> don't break to build if AGP is a module, then that should be a job for
> Kconfig rather than some macro.
> 
> So I think the above could simply become:
> 
>   #define __OS_HAS_AGP IS_ENABLED(CONFIG_AGP)
> 
> And once we have that I think we could even easily get rid of the custom
> __OS_HAS_AGP macro and use IS_ENABLED(CONFIG_AGP) directly.

Yeah, I think a simple IS_ENABLED(CONFIG_AGP) sould be good enough.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 2/2] drm: don't recycle used modeset IDs

[Daniel Vetter]

On Fri, Aug 29, 2014 at 02:01:01PM +0200, David Herrmann wrote:
> With MST, we now have connector hotplugging, yey! Pretty easy to use in
> user-space, but introduces some nasty races:
>  * If a connector is removed and added again while a compositor is in
>background, it will get the same ID again. If the compositor wakes up,
>it cannot know whether its the same connector or a new one, thus they
>must re-read EDID information, etc.
>  * possible_clones, possible_crtcs, etc. depend on indices, not IDs. So if
>an object is removed and a new one is added, those bitmasks are invalid
>and must be refreshed. This currently does not affect connectors, but
>only crtcs and encoders, but it's only a matter of time when this will
>change.
> 
> The easiest way to protect against this, is to not recylce modeset IDs.
> Instead, always allocate a new, higher ID. All ioctls that affect modeset
> objects, take IDs. Thus, during hotplug races, those ioctls will simply
> fail if invalid IDs are passed. They will no longer silently run on a
> newly hotplugged object.
> 
> Furthermore, hotplug-races during state sync can now be easily detected. A
> call to GET_RESOURCES returns a list of available IDs atomically.
> User-space can now start fetching all those objects via GET_* ioctls. If
> any of those fails, they know that the given object was unplugged. Thus,
> the "possible_*" bit-fields are invalidated. User-space can now decide
> whether to restart the sync all over or wait for the 'change' uevent that
> is sent on modeset object modifications (or, well, is supposed to be sent
> and probably will be at some point).
> 
> With this in place, modeset object hotplugging should work fine for all
> modeset objects in the KMS API.
> 
> CC'ing stable so we can rely on all kernels with MST to not recycle IDs.
> 
> Cc:  # 3.16+
> Signed-off-by: David Herrmann 

So userspace just needs to cycle through piles of framebuffer objects to
make bad things happen? Doesn't sound like a terribly solid plan.

I guess we could save this by doing normal id allocations for fbs and
monotonically increasing allocations for all other objects.
-Daniel

> ---
>  drivers/gpu/drm/drm_crtc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 97eba56..ab0fe6d 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -286,7 +286,7 @@ static int drm_mode_object_get_reg(struct drm_device *dev,
>   idr_preload(GFP_KERNEL);
>   spin_lock_irq(>mode_config.idr_lock);
>  
> - ret = idr_alloc(>mode_config.crtc_idr, register_obj ? obj : NULL, 
> 1, 0, GFP_NOWAIT);
> + ret = idr_alloc_cyclic(>mode_config.crtc_idr, register_obj ? obj : 
> NULL, 1, 0, GFP_NOWAIT);
>   if (ret >= 0) {
>   /*
>* Set up the object linking under the protection of the idr
> -- 
> 2.1.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 1/2] drm: make idr_mutex a spinlock

[Daniel Vetter]

On Fri, Aug 29, 2014 at 02:01:00PM +0200, David Herrmann wrote:
> There is no reason to use a heavy mutex for idr protection. Use a spinlock
> and make idr-allocation use idr_preload().
> 
> This patch also makes mode-object lookup irq-save, in case you ever wanna
> lookup modeset objects from interrupts. This is just a side-effect of
> avoiding a mutex.
> 
> Signed-off-by: David Herrmann 

I've thought irqsave/restore are terribly serializing instructions, so
this might actually be slower than a plain mutex. And imo if it doesn't
show up in profiles it's not worth to optimize it - generally mutexes are
really fast and in most cases already nicely degenerate to spinlocks
anyway.
-Daniel

> ---
>  drivers/gpu/drm/drm_crtc.c | 34 --
>  include/drm/drm_crtc.h |  4 ++--
>  2 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 61b6978..97eba56 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -283,8 +283,10 @@ static int drm_mode_object_get_reg(struct drm_device 
> *dev,
>  {
>   int ret;
>  
> - mutex_lock(>mode_config.idr_mutex);
> - ret = idr_alloc(>mode_config.crtc_idr, register_obj ? obj : NULL, 
> 1, 0, GFP_KERNEL);
> + idr_preload(GFP_KERNEL);
> + spin_lock_irq(>mode_config.idr_lock);
> +
> + ret = idr_alloc(>mode_config.crtc_idr, register_obj ? obj : NULL, 
> 1, 0, GFP_NOWAIT);
>   if (ret >= 0) {
>   /*
>* Set up the object linking under the protection of the idr
> @@ -293,7 +295,9 @@ static int drm_mode_object_get_reg(struct drm_device *dev,
>   obj->id = ret;
>   obj->type = obj_type;
>   }
> - mutex_unlock(>mode_config.idr_mutex);
> +
> + spin_unlock_irq(>mode_config.idr_lock);
> + idr_preload_end();
>  
>   return ret < 0 ? ret : 0;
>  }
> @@ -322,9 +326,9 @@ int drm_mode_object_get(struct drm_device *dev,
>  static void drm_mode_object_register(struct drm_device *dev,
>struct drm_mode_object *obj)
>  {
> - mutex_lock(>mode_config.idr_mutex);
> + spin_lock_irq(>mode_config.idr_lock);
>   idr_replace(>mode_config.crtc_idr, obj, obj->id);
> - mutex_unlock(>mode_config.idr_mutex);
> + spin_unlock_irq(>mode_config.idr_lock);
>  }
>  
>  /**
> @@ -339,17 +343,18 @@ static void drm_mode_object_register(struct drm_device 
> *dev,
>  void drm_mode_object_put(struct drm_device *dev,
>struct drm_mode_object *object)
>  {
> - mutex_lock(>mode_config.idr_mutex);
> + spin_lock_irq(>mode_config.idr_lock);
>   idr_remove(>mode_config.crtc_idr, object->id);
> - mutex_unlock(>mode_config.idr_mutex);
> + spin_unlock_irq(>mode_config.idr_lock);
>  }
>  
>  static struct drm_mode_object *_object_find(struct drm_device *dev,
>   uint32_t id, uint32_t type)
>  {
>   struct drm_mode_object *obj = NULL;
> + unsigned long flags;
>  
> - mutex_lock(>mode_config.idr_mutex);
> + spin_lock_irqsave(>mode_config.idr_lock, flags);
>   obj = idr_find(>mode_config.crtc_idr, id);
>   if (obj && type != DRM_MODE_OBJECT_ANY && obj->type != type)
>   obj = NULL;
> @@ -358,7 +363,7 @@ static struct drm_mode_object *_object_find(struct 
> drm_device *dev,
>   /* don't leak out unref'd fb's */
>   if (obj && (obj->type == DRM_MODE_OBJECT_FB))
>   obj = NULL;
> - mutex_unlock(>mode_config.idr_mutex);
> + spin_unlock_irqrestore(>mode_config.idr_lock, flags);
>  
>   return obj;
>  }
> @@ -433,9 +438,9 @@ EXPORT_SYMBOL(drm_framebuffer_init);
>  static void __drm_framebuffer_unregister(struct drm_device *dev,
>struct drm_framebuffer *fb)
>  {
> - mutex_lock(>mode_config.idr_mutex);
> + spin_lock_irq(>mode_config.idr_lock);
>   idr_remove(>mode_config.crtc_idr, fb->base.id);
> - mutex_unlock(>mode_config.idr_mutex);
> + spin_unlock_irq(>mode_config.idr_lock);
>  
>   fb->base.id = 0;
>  }
> @@ -465,14 +470,15 @@ static struct drm_framebuffer 
> *__drm_framebuffer_lookup(struct drm_device *dev,
>  {
>   struct drm_mode_object *obj = NULL;
>   struct drm_framebuffer *fb;
> + unsigned long flags;
>  
> - mutex_lock(>mode_config.idr_mutex);
> + spin_lock_irqsave(>mode_config.idr_lock, flags);
>   obj = idr_find(>mode_config.crtc_idr, id);
>   if (!obj || (obj->type != DRM_MODE_OBJECT_FB) || (obj->id != id))
>   fb = NULL;
>   else
>   fb = obj_to_fb(obj);
> - mutex_unlock(>mode_config.idr_mutex);
> + spin_unlock_irqrestore(>mode_config.idr_lock, flags);
>  
>   return fb;
>  }
> @@ -5049,7 +5055,7 @@ void drm_mode_config_init(struct drm_device *dev)
>  {
>   mutex_init(>mode_config.mutex);
>   drm_modeset_lock_init(>mode_config.connection_mutex);
> - mutex_init(>mode_config.idr_mutex);

[PATCH 17/20] drm: add driver->set_busid() callback

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:43PM +0200, David Herrmann wrote:
> One step closer to dropping all the drm_bus_* code:
> Add a driver->set_busid() callback and make all drivers use the generic
> helpers. Nouveau is the only driver that uses two different bus-types with
> the same drm_driver. This is totally broken if both buses are available on
> the same machine (unlikely, but lets be safe).

It's not at all unlikely. There are quite a few people using nouveau to
drive an discrete GPU over PCIe on Tegra K1.

I've also been noticing lately (and I'm not sure why I didn't see it
earlier, possibly because the relevant patches weren't in nouveau yet)
that the nouveau kernel driver crashes when running X on Tegra K1 with
nouveau (and the gk20a GPU) enabled. The reason being that the device is
wrongfully marked as drm_pci_bus.

> Therefore, we create two
> different drivers for each platform during module_init() and set the
> set_busid() callback respectively.

[PATCH 18/20] drm: Goody bye, drm_bus!

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:44PM +0200, David Herrmann wrote:
> ..we will not miss you..
> 
> Signed-off-by: David Herrmann 
> ---
>  drivers/gpu/drm/drm_ioctl.c|  8 +---
>  drivers/gpu/drm/drm_pci.c  |  6 --
>  drivers/gpu/drm/drm_platform.c |  5 -
>  drivers/gpu/drm/drm_usb.c  | 12 
>  include/drm/drmP.h |  5 -
>  5 files changed, 1 insertion(+), 35 deletions(-)

\o/

Reviewed-by: Thierry Reding 
-- 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/20140829/01ab6fc1/attachment.sig>

[PATCH 2/2] drm: don't recycle used modeset IDs

[David Herrmann]

Hi

On Fri, Aug 29, 2014 at 2:51 PM, Daniel Vetter  wrote:
> On Fri, Aug 29, 2014 at 02:01:01PM +0200, David Herrmann wrote:
>> With MST, we now have connector hotplugging, yey! Pretty easy to use in
>> user-space, but introduces some nasty races:
>>  * If a connector is removed and added again while a compositor is in
>>background, it will get the same ID again. If the compositor wakes up,
>>it cannot know whether its the same connector or a new one, thus they
>>must re-read EDID information, etc.
>>  * possible_clones, possible_crtcs, etc. depend on indices, not IDs. So if
>>an object is removed and a new one is added, those bitmasks are invalid
>>and must be refreshed. This currently does not affect connectors, but
>>only crtcs and encoders, but it's only a matter of time when this will
>>change.
>>
>> The easiest way to protect against this, is to not recylce modeset IDs.
>> Instead, always allocate a new, higher ID. All ioctls that affect modeset
>> objects, take IDs. Thus, during hotplug races, those ioctls will simply
>> fail if invalid IDs are passed. They will no longer silently run on a
>> newly hotplugged object.
>>
>> Furthermore, hotplug-races during state sync can now be easily detected. A
>> call to GET_RESOURCES returns a list of available IDs atomically.
>> User-space can now start fetching all those objects via GET_* ioctls. If
>> any of those fails, they know that the given object was unplugged. Thus,
>> the "possible_*" bit-fields are invalidated. User-space can now decide
>> whether to restart the sync all over or wait for the 'change' uevent that
>> is sent on modeset object modifications (or, well, is supposed to be sent
>> and probably will be at some point).
>>
>> With this in place, modeset object hotplugging should work fine for all
>> modeset objects in the KMS API.
>>
>> CC'ing stable so we can rely on all kernels with MST to not recycle IDs.
>>
>> Cc:  # 3.16+
>> Signed-off-by: David Herrmann 
>
> So userspace just needs to cycle through piles of framebuffer objects to
> make bad things happen? Doesn't sound like a terribly solid plan.

IDs will still get recycled, but only once all IDs got used. So this is "safe".

Sure, user-space can create 2 billion framebuffers and destroy them
again, thus causing the ID range to overflow and recycle old IDs. Not
sure how fast you can run 2 billion syscalls.. If that's a real issue,
I'd vote for using the high-range for user-space managed objects,
low-range for kernel-space managed objects ([1...INT_MAX] and
[INT_MAX+1...UINT_MAX] or so).

> I guess we could save this by doing normal id allocations for fbs and
> monotonically increasing allocations for all other objects.

This doesn't work. A connector with ID #n might get unplugged and
another process created a new FB, which will then get ID #n. Sure, I
doubt there's a real conflict as ioctls check the type, but it still
sounds really ugly to me.

Thanks
David

[PATCH 15/20] drm: simplify drm_*_set_unique()

[Daniel Vetter]

On Fri, Aug 29, 2014 at 12:12:41PM +0200, David Herrmann wrote:
> Lets use kasprintf() to avoid pre-allocating the buffer. This is really
> nothing to optimize for speed and the input is trusted, so kasprintf() is
> just fine.
> 
> Signed-off-by: David Herrmann 

Reviewed-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_pci.c  | 30 --
>  drivers/gpu/drm/drm_platform.c | 31 ---
>  2 files changed, 16 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 020cfd9..8efea6b 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -129,31 +129,17 @@ static int drm_get_pci_domain(struct drm_device *dev)
>  
>  static int drm_pci_set_busid(struct drm_device *dev, struct drm_master 
> *master)
>  {
> - int len, ret;
> - master->unique_len = 40;
> - master->unique_size = master->unique_len;
> - master->unique = kmalloc(master->unique_size, GFP_KERNEL);
> - if (master->unique == NULL)
> + master->unique = kasprintf(GFP_KERNEL, "pci:%04x:%02x:%02x.%d",
> + drm_get_pci_domain(dev),
> + dev->pdev->bus->number,
> + PCI_SLOT(dev->pdev->devfn),
> + PCI_FUNC(dev->pdev->devfn));
> + if (!master->unique)
>   return -ENOMEM;
>  
> -
> - len = snprintf(master->unique, master->unique_len,
> -"pci:%04x:%02x:%02x.%d",
> -drm_get_pci_domain(dev),
> -dev->pdev->bus->number,
> -PCI_SLOT(dev->pdev->devfn),
> -PCI_FUNC(dev->pdev->devfn));
> -
> - if (len >= master->unique_len) {
> - DRM_ERROR("buffer overflow");
> - ret = -EINVAL;
> - goto err;
> - } else
> - master->unique_len = len;
> -
> + master->unique_len = strlen(master->unique);
> + master->unique_size = master->unique_len + 1;
>   return 0;
> -err:
> - return ret;
>  }
>  
>  int drm_pci_set_unique(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
> index d5b76f1..0c09ddd 100644
> --- a/drivers/gpu/drm/drm_platform.c
> +++ b/drivers/gpu/drm/drm_platform.c
> @@ -70,35 +70,20 @@ err_free:
>  
>  static int drm_platform_set_busid(struct drm_device *dev, struct drm_master 
> *master)
>  {
> - int len, ret, id;
> -
> - master->unique_len = 13 + strlen(dev->platformdev->name);
> - master->unique_size = master->unique_len;
> - master->unique = kmalloc(master->unique_len + 1, GFP_KERNEL);
> -
> - if (master->unique == NULL)
> - return -ENOMEM;
> + int id;
>  
>   id = dev->platformdev->id;
> -
> - /* if only a single instance of the platform device, id will be
> -  * set to -1.. use 0 instead to avoid a funny looking bus-id:
> -  */
> - if (id == -1)
> + if (id < 0)
>   id = 0;
>  
> - len = snprintf(master->unique, master->unique_len,
> - "platform:%s:%02d", dev->platformdev->name, id);
> -
> - if (len > master->unique_len) {
> - DRM_ERROR("Unique buffer overflowed\n");
> - ret = -EINVAL;
> - goto err;
> - }
> + master->unique = kasprintf(GFP_KERNEL, "platform:%s:%02d",
> + dev->platformdev->name, id);
> + if (!master->unique)
> + return -ENOMEM;
>  
> + master->unique_len = strlen(master->unique);
> + master->unique_size = master->unique_len;
>   return 0;
> -err:
> - return ret;
>  }
>  
>  static struct drm_bus drm_platform_bus = {
> -- 
> 2.1.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 16/20] drm: drop unused drm_master->unique_size

[Daniel Vetter]

On Fri, Aug 29, 2014 at 12:12:42PM +0200, David Herrmann wrote:
> This field is unused and there is really no reason to optimize
> unique-allocations. Drop it.
> 
> Signed-off-by: David Herrmann 

Reviewed-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_ioctl.c| 1 -
>  drivers/gpu/drm/drm_pci.c  | 4 +---
>  drivers/gpu/drm/drm_platform.c | 1 -
>  include/drm/drmP.h | 2 --
>  4 files changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index aa1ac79..cb6b54a 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -189,7 +189,6 @@ drm_unset_busid(struct drm_device *dev,
>   kfree(master->unique);
>   master->unique = NULL;
>   master->unique_len = 0;
> - master->unique_size = 0;
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 8efea6b..e266927 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -138,7 +138,6 @@ static int drm_pci_set_busid(struct drm_device *dev, 
> struct drm_master *master)
>   return -ENOMEM;
>  
>   master->unique_len = strlen(master->unique);
> - master->unique_size = master->unique_len + 1;
>   return 0;
>  }
>  
> @@ -149,8 +148,7 @@ int drm_pci_set_unique(struct drm_device *dev,
>   int domain, bus, slot, func, ret;
>  
>   master->unique_len = u->unique_len;
> - master->unique_size = u->unique_len + 1;
> - master->unique = kmalloc(master->unique_size, GFP_KERNEL);
> + master->unique = kmalloc(master->unique_len + 1, GFP_KERNEL);
>   if (!master->unique) {
>   ret = -ENOMEM;
>   goto err;
> diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
> index 0c09ddd..f197a2b 100644
> --- a/drivers/gpu/drm/drm_platform.c
> +++ b/drivers/gpu/drm/drm_platform.c
> @@ -82,7 +82,6 @@ static int drm_platform_set_busid(struct drm_device *dev, 
> struct drm_master *mas
>   return -ENOMEM;
>  
>   master->unique_len = strlen(master->unique);
> - master->unique_size = master->unique_len;
>   return 0;
>  }
>  
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index a8b24fc..98b1eaf 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -574,7 +574,6 @@ struct drm_gem_object {
>   * @minor: Link back to minor char device we are master for. Immutable.
>   * @unique: Unique identifier: e.g. busid. Protected by drm_global_mutex.
>   * @unique_len: Length of unique field. Protected by drm_global_mutex.
> - * @unique_size: Amount allocated. Protected by drm_global_mutex.
>   * @magiclist: Hash of used authentication tokens. Protected by struct_mutex.
>   * @magicfree: List of used authentication tokens. Protected by struct_mutex.
>   * @lock: DRI lock information.
> @@ -585,7 +584,6 @@ struct drm_master {
>   struct drm_minor *minor;
>   char *unique;
>   int unique_len;
> - int unique_size;
>   struct drm_open_hash magiclist;
>   struct list_head magicfree;
>   struct drm_lock_data lock;
> -- 
> 2.1.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 17/20] drm: add driver->set_busid() callback

[Daniel Vetter]

On Fri, Aug 29, 2014 at 12:12:43PM +0200, David Herrmann wrote:
> One step closer to dropping all the drm_bus_* code:
> Add a driver->set_busid() callback and make all drivers use the generic
> helpers. Nouveau is the only driver that uses two different bus-types with
> the same drm_driver. This is totally broken if both buses are available on
> the same machine (unlikely, but lets be safe). Therefore, we create two
> different drivers for each platform during module_init() and set the
> set_busid() callback respectively.
> 
> Signed-off-by: David Herrmann 

This has a bit a midlayer smell to it tbh, we don't really need a
set_busid callback I think. Instead there's just two cases:
- The crazy implementation for pci devices. But we already have dev->pdev,
  so the core can figure this out itself.
- Everyone else just sets a static name. For those I think we should just
  add a drm_dev_set_busid function which they can call in their
  driver-load function. Well maybe different versions for platform drivers
  and stuff.

Thanks, Daniel

> ---
>  drivers/gpu/drm/armada/armada_drv.c  |  1 +
>  drivers/gpu/drm/ast/ast_drv.c|  1 +
>  drivers/gpu/drm/bochs/bochs_drv.c|  1 +
>  drivers/gpu/drm/cirrus/cirrus_drv.c  |  1 +
>  drivers/gpu/drm/drm_ioctl.c  |  8 +++-
>  drivers/gpu/drm/drm_pci.c|  3 ++-
>  drivers/gpu/drm/drm_platform.c   |  3 ++-
>  drivers/gpu/drm/exynos/exynos_drm_drv.c  |  1 +
>  drivers/gpu/drm/gma500/psb_drv.c |  1 +
>  drivers/gpu/drm/i810/i810_drv.c  |  1 +
>  drivers/gpu/drm/i915/i915_drv.c  |  1 +
>  drivers/gpu/drm/mga/mga_drv.c|  1 +
>  drivers/gpu/drm/mgag200/mgag200_drv.c|  1 +
>  drivers/gpu/drm/msm/msm_drv.c|  1 +
>  drivers/gpu/drm/nouveau/nouveau_drm.c| 19 +--
>  drivers/gpu/drm/omapdrm/omap_drv.c   |  1 +
>  drivers/gpu/drm/qxl/qxl_drv.c|  1 +
>  drivers/gpu/drm/r128/r128_drv.c  |  1 +
>  drivers/gpu/drm/radeon/radeon_drv.c  |  2 ++
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c|  1 +
>  drivers/gpu/drm/savage/savage_drv.c  |  1 +
>  drivers/gpu/drm/shmobile/shmob_drm_drv.c |  1 +
>  drivers/gpu/drm/sis/sis_drv.c|  1 +
>  drivers/gpu/drm/tdfx/tdfx_drv.c  |  1 +
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c  |  1 +
>  drivers/gpu/drm/udl/udl_drv.c|  6 ++
>  drivers/gpu/drm/via/via_drv.c|  1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c  |  1 +
>  drivers/staging/imx-drm/imx-drm-core.c   |  1 +
>  include/drm/drmP.h   |  3 +++
>  30 files changed, 58 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/armada/armada_drv.c 
> b/drivers/gpu/drm/armada/armada_drv.c
> index e2d5792..f672e6a 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -308,6 +308,7 @@ static struct drm_driver armada_drm_driver = {
>   .postclose  = NULL,
>   .lastclose  = armada_drm_lastclose,
>   .unload = armada_drm_unload,
> + .set_busid  = drm_platform_set_busid,
>   .get_vblank_counter = drm_vblank_count,
>   .enable_vblank  = armada_drm_enable_vblank,
>   .disable_vblank = armada_drm_disable_vblank,
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index f19682a..9a32d9d 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -199,6 +199,7 @@ static struct drm_driver driver = {
>  
>   .load = ast_driver_load,
>   .unload = ast_driver_unload,
> + .set_busid = drm_pci_set_busid,
>  
>   .fops = _fops,
>   .name = DRIVER_NAME,
> diff --git a/drivers/gpu/drm/bochs/bochs_drv.c 
> b/drivers/gpu/drm/bochs/bochs_drv.c
> index 9738e9b..98837bd 100644
> --- a/drivers/gpu/drm/bochs/bochs_drv.c
> +++ b/drivers/gpu/drm/bochs/bochs_drv.c
> @@ -82,6 +82,7 @@ static struct drm_driver bochs_driver = {
>   .driver_features= DRIVER_GEM | DRIVER_MODESET,
>   .load   = bochs_load,
>   .unload = bochs_unload,
> + .set_busid  = drm_pci_set_busid,
>   .fops   = _fops,
>   .name   = "bochs-drm",
>   .desc   = "bochs dispi vga interface (qemu stdvga)",
> diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.c 
> b/drivers/gpu/drm/cirrus/cirrus_drv.c
> index 919c73b..e705335 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_drv.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_drv.c
> @@ -128,6 +128,7 @@ static struct drm_driver driver = {
>   .driver_features = DRIVER_MODESET | DRIVER_GEM,
>   .load = cirrus_driver_load,
>   .unload = cirrus_driver_unload,
> + .set_busid = drm_pci_set_busid,
>   .fops = _driver_fops,
>   .name = DRIVER_NAME,
>   .desc = DRIVER_DESC,
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c

[PATCH 19/20] drm: merge drm_usb into udl

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:45PM +0200, David Herrmann wrote:
[...]
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
[...]
>  module_init(udl_init);
>  module_exit(udl_exit);
> +MODULE_LICENSE("GPL");

According to the header file the license is GPL v2 only, so this should
be "GPL v2".

Might also be good to add MODULE_AUTHOR and MODULE_DESCRIPTION here.

Other than that:

Reviewed-by: Thierry Reding 
-- 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/20140829/8960541a/attachment.sig>

[PATCH 18/20] drm: Goody bye, drm_bus!

[Daniel Vetter]

On Fri, Aug 29, 2014 at 12:12:44PM +0200, David Herrmann wrote:
> ..we will not miss you..
> 
> Signed-off-by: David Herrmann 

Since this is independant of how we'll get rid of bus->set_busid:

Reviewed-by: Daniel Vetter 


> ---
>  drivers/gpu/drm/drm_ioctl.c|  8 +---
>  drivers/gpu/drm/drm_pci.c  |  6 --
>  drivers/gpu/drm/drm_platform.c |  5 -
>  drivers/gpu/drm/drm_usb.c  | 12 
>  include/drm/drmP.h |  5 -
>  5 files changed, 1 insertion(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 4770bd7..3a1349f 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -250,15 +250,9 @@ static int drm_set_busid(struct drm_device *dev, struct 
> drm_file *file_priv)
>   drm_unset_busid(dev, master);
>   return ret;
>   }
> - } else if (dev->driver->bus && dev->driver->bus->set_busid) {
> - ret = dev->driver->bus->set_busid(dev, master);
> - if (ret) {
> - drm_unset_busid(dev, master);
> - return ret;
> - }
>   } else {
>   if (WARN(dev->unique == NULL,
> -  "No drm_bus.set_busid() implementation provided by "
> +  "No drm_driver.set_busid() implementation provided by "
>"%ps. Use drm_dev_set_unique() to set the unique "
>"name explicitly.", dev->driver))
>   return -EINVAL;
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 0400c37..7563130 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -254,10 +254,6 @@ void drm_pci_agp_destroy(struct drm_device *dev)
>   }
>  }
>  
> -static struct drm_bus drm_pci_bus = {
> - .set_busid = drm_pci_set_busid,
> -};
> -
>  /**
>   * drm_get_pci_dev - Register a PCI device with the DRM subsystem
>   * @pdev: PCI device
> @@ -338,8 +334,6 @@ int drm_pci_init(struct drm_driver *driver, struct 
> pci_driver *pdriver)
>  
>   DRM_DEBUG("\n");
>  
> - driver->bus = _pci_bus;
> -
>   if (driver->driver_features & DRIVER_MODESET)
>   return pci_register_driver(pdriver);
>  
> diff --git a/drivers/gpu/drm/drm_platform.c b/drivers/gpu/drm/drm_platform.c
> index 939cd22..5314c9d 100644
> --- a/drivers/gpu/drm/drm_platform.c
> +++ b/drivers/gpu/drm/drm_platform.c
> @@ -86,10 +86,6 @@ int drm_platform_set_busid(struct drm_device *dev, struct 
> drm_master *master)
>  }
>  EXPORT_SYMBOL(drm_platform_set_busid);
>  
> -static struct drm_bus drm_platform_bus = {
> - .set_busid = drm_platform_set_busid,
> -};
> -
>  /**
>   * drm_platform_init - Register a platform device with the DRM subsystem
>   * @driver: DRM device driver
> @@ -105,7 +101,6 @@ int drm_platform_init(struct drm_driver *driver, struct 
> platform_device *platfor
>  {
>   DRM_DEBUG("\n");
>  
> - driver->bus = _platform_bus;
>   return drm_get_platform_dev(platform_device, driver);
>  }
>  EXPORT_SYMBOL(drm_platform_init);
> diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c
> index f2fe94a..9c43490 100644
> --- a/drivers/gpu/drm/drm_usb.c
> +++ b/drivers/gpu/drm/drm_usb.c
> @@ -36,16 +36,6 @@ err_free:
>  }
>  EXPORT_SYMBOL(drm_get_usb_dev);
>  
> -static int drm_usb_set_busid(struct drm_device *dev,
> -struct drm_master *master)
> -{
> - return 0;
> -}
> -
> -static struct drm_bus drm_usb_bus = {
> - .set_busid = drm_usb_set_busid,
> -};
> -
>  /**
>   * drm_usb_init - Register matching USB devices with the DRM subsystem
>   * @driver: DRM device driver
> @@ -61,8 +51,6 @@ int drm_usb_init(struct drm_driver *driver, struct 
> usb_driver *udriver)
>   int res;
>   DRM_DEBUG("\n");
>  
> - driver->bus = _usb_bus;
> -
>   res = usb_register(udriver);
>   return res;
>  }
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index c82f292..5ae388a 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -605,10 +605,6 @@ struct drm_master {
>  #define DRM_SCANOUTPOS_INVBL(1 << 1)
>  #define DRM_SCANOUTPOS_ACCURATE (1 << 2)
>  
> -struct drm_bus {
> - int (*set_busid)(struct drm_device *dev, struct drm_master *master);
> -};
> -
>  /**
>   * DRM driver structure. This structure represent the common code for
>   * a family of cards. There will one drm_device for each card present
> @@ -846,7 +842,6 @@ struct drm_driver {
>   const struct drm_ioctl_desc *ioctls;
>   int num_ioctls;
>   const struct file_operations *fops;
> - struct drm_bus *bus;
>  
>   /* List of devices hanging off this driver with stealth attach. */
>   struct list_head legacy_dev_list;
> -- 
> 2.1.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 20/20] drm: move drm-lock API to drm_legacy.h

[Thierry Reding]

On Fri, Aug 29, 2014 at 12:12:46PM +0200, David Herrmann wrote:
> Same as the other legacy APIs, most of this is internal, so prefix it with
> drm_legacy_* and move into drm_legacy.h.
> 
> Signed-off-by: David Herrmann 
> ---
>  drivers/gpu/drm/drm_fops.c  |  6 +++---
>  drivers/gpu/drm/drm_ioctl.c |  4 ++--
>  drivers/gpu/drm/drm_legacy.h|  9 +
>  drivers/gpu/drm/drm_lock.c  | 20 +++-
>  drivers/gpu/drm/i810/i810_dma.c |  4 ++--
>  drivers/gpu/drm/savage/savage_bci.c |  4 ++--
>  drivers/gpu/drm/sis/sis_mm.c|  6 +++---
>  drivers/gpu/drm/via/via_mm.c|  6 +++---
>  include/drm/drmP.h  | 14 +++---
>  9 files changed, 38 insertions(+), 35 deletions(-)

Reviewed-by: Thierry Reding 
-- 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/20140829/a3d2440d/attachment.sig>

[PATCH 1/2] drm: make idr_mutex a spinlock

[David Herrmann]

Hi

On Fri, Aug 29, 2014 at 2:53 PM, Daniel Vetter  wrote:
> On Fri, Aug 29, 2014 at 02:01:00PM +0200, David Herrmann wrote:
>> There is no reason to use a heavy mutex for idr protection. Use a spinlock
>> and make idr-allocation use idr_preload().
>>
>> This patch also makes mode-object lookup irq-save, in case you ever wanna
>> lookup modeset objects from interrupts. This is just a side-effect of
>> avoiding a mutex.
>>
>> Signed-off-by: David Herrmann 
>
> I've thought irqsave/restore are terribly serializing instructions, so
> this might actually be slower than a plain mutex. And imo if it doesn't
> show up in profiles it's not worth to optimize it - generally mutexes are
> really fast and in most cases already nicely degenerate to spinlocks
> anyway.

Honestly, this patch is less about speed than 'correctness'. Sure, a
mutex is just a spin-lock in low-contention cases and there really is
no high-contention here. However, spin-locks are the major lock-type
for pure data. mutexes only make sense when you have to lock data
structures _while_ performing operations that can sleep. Using a
spin-lock here prevents people from doing stupid things while holding
this lock. And really, this lock is about ID registration and
deregistration, nothing else.

Btw., I can happily turn all those lock/unlock sequences into
spin_lock() and spin_unlock() so we ignore irq-contexts completely, if
that's the only issue.

Thanks
David

[PATCH 19/20] drm: merge drm_usb into udl

[Daniel Vetter]

On Fri, Aug 29, 2014 at 12:12:45PM +0200, David Herrmann wrote:
> This merges all the remains of drm_usb into its only user, udl. We can
> then drop all the drm_usb stuff, including dev->usbdev.
> 
> Signed-off-by: David Herrmann 

A bit of (seemingly) unecessary code movement (the probe/disconnect
functions and id table). With or without that fixed this is

Reviewed-by: Daniel Vetter 

And it looks really, really pretty ;-)

> ---
>  Documentation/DocBook/drm.tmpl  |   3 +-
>  drivers/gpu/drm/Kconfig |   6 ---
>  drivers/gpu/drm/Makefile|   3 --
>  drivers/gpu/drm/drm_usb.c   |  76 ---
>  drivers/gpu/drm/udl/Kconfig |   3 +-
>  drivers/gpu/drm/udl/udl_connector.c |   4 +-
>  drivers/gpu/drm/udl/udl_drv.c   | 102 
> +---
>  drivers/gpu/drm/udl/udl_drv.h   |   1 +
>  drivers/gpu/drm/udl/udl_main.c  |   8 +--
>  include/drm/drmP.h  |   1 -
>  include/drm/drm_usb.h   |  15 --
>  11 files changed, 70 insertions(+), 152 deletions(-)
>  delete mode 100644 drivers/gpu/drm/drm_usb.c
>  delete mode 100644 include/drm/drm_usb.h
> 
> diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
> index 5c299fa..99f7ee6 100644
> --- a/Documentation/DocBook/drm.tmpl
> +++ b/Documentation/DocBook/drm.tmpl
> @@ -291,10 +291,9 @@ char *date;
>Device Registration
>
>  A number of functions are provided to help with device registration.
> -The functions deal with PCI, USB and platform devices, respectively.
> +The functions deal with PCI and platform devices, respectively.
>
>  !Edrivers/gpu/drm/drm_pci.c
> -!Edrivers/gpu/drm/drm_usb.c
>  !Edrivers/gpu/drm/drm_platform.c
>
>  New drivers that no longer rely on the services provided by the
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index e3500f9..e3b4b0f 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -25,12 +25,6 @@ config DRM_MIPI_DSI
>   bool
>   depends on DRM
>  
> -config DRM_USB
> - tristate
> - depends on DRM
> - depends on USB_SUPPORT && USB_ARCH_HAS_HCD
> - select USB
> -
>  config DRM_KMS_HELPER
>   tristate
>   depends on DRM
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 9b7cb3f..9292a76 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -22,8 +22,6 @@ drm-$(CONFIG_PCI) += ati_pcigart.o
>  drm-$(CONFIG_DRM_PANEL) += drm_panel.o
>  drm-$(CONFIG_OF) += drm_of.o
>  
> -drm-usb-y   := drm_usb.o
> -
>  drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
>   drm_plane_helper.o drm_dp_mst_topology.o
>  drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> @@ -36,7 +34,6 @@ CFLAGS_drm_trace_points.o := -I$(src)
>  
>  obj-$(CONFIG_DRM)+= drm.o
>  obj-$(CONFIG_DRM_MIPI_DSI) += drm_mipi_dsi.o
> -obj-$(CONFIG_DRM_USB)   += drm_usb.o
>  obj-$(CONFIG_DRM_TTM)+= ttm/
>  obj-$(CONFIG_DRM_TDFX)   += tdfx/
>  obj-$(CONFIG_DRM_R128)   += r128/
> diff --git a/drivers/gpu/drm/drm_usb.c b/drivers/gpu/drm/drm_usb.c
> deleted file mode 100644
> index 9c43490..000
> --- a/drivers/gpu/drm/drm_usb.c
> +++ /dev/null
> @@ -1,76 +0,0 @@
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -int drm_get_usb_dev(struct usb_interface *interface,
> - const struct usb_device_id *id,
> - struct drm_driver *driver)
> -{
> - struct drm_device *dev;
> - int ret;
> -
> - DRM_DEBUG("\n");
> -
> - dev = drm_dev_alloc(driver, >dev);
> - if (!dev)
> - return -ENOMEM;
> -
> - dev->usbdev = interface_to_usbdev(interface);
> - usb_set_intfdata(interface, dev);
> -
> - ret = drm_dev_register(dev, 0);
> - if (ret)
> - goto err_free;
> -
> - DRM_INFO("Initialized %s %d.%d.%d %s on minor %d\n",
> -  driver->name, driver->major, driver->minor, driver->patchlevel,
> -  driver->date, dev->primary->index);
> -
> - return 0;
> -
> -err_free:
> - drm_dev_unref(dev);
> - return ret;
> -
> -}
> -EXPORT_SYMBOL(drm_get_usb_dev);
> -
> -/**
> - * drm_usb_init - Register matching USB devices with the DRM subsystem
> - * @driver: DRM device driver
> - * @udriver: USB device driver
> - *
> - * Registers one or more devices matched by a USB driver with the DRM
> - * subsystem.
> - *
> - * Return: 0 on success or a negative error code on failure.
> - */
> -int drm_usb_init(struct drm_driver *driver, struct usb_driver *udriver)
> -{
> - int res;
> - DRM_DEBUG("\n");
> -
> - res = usb_register(udriver);
> - return res;
> -}
> -EXPORT_SYMBOL(drm_usb_init);
> -
> -/**
> - * drm_usb_exit - Unregister matching USB devices from the DRM subsystem
> - * @driver: DRM device driver
> - * @udriver: USB device driver
> - *
> - * Unregisters one or more devices 

[PATCH 00/20] DRM: Core Cleanups

[Daniel Vetter]

On Fri, Aug 29, 2014 at 12:12:26PM +0200, David Herrmann wrote:
> Hi
> 
> More cleanups of DRM core code. Diffstat says:
>  72 files changed, 832 insertions(+), 1038 deletions(-)
> 
> ..which is already nice, but doesn't reflect that a lot of code is now hidden
> from main headers. Furthermore, with this series applied, drmP.h no longer 
> looks
> as ugly as it is now (widly unstructured). It still ain't a beauty, though:
>  include/drm/drmP.h   | 236 
> ---
> 
> Anyhow, highlights:
>  - drop "drm_memory.h"
>  - drop "drm_usb.c"
>  - drop "struct drm_waitlist"
>  - drop "struct drm_sigdata"
>  - drop "struct drm_bus"
>  - drop "drm_master->unique_size"
> 
> Two patches are driver related:
>  - radeon: Patch #1, moves drm_buffer.c to radeon/
>  - nouveau: Patch #17, splits drm_driver for nouveau
> They're rather trivial. Everything else is DRM-core.
> 
> I've moved some legacy stuff around so it's easier to drop it once unused. If
> someone disagrees with move, let me know and I'll drop the patch.
> 
> I've pushed this to the 0-day testing bots now. I'll comment on the patches in
> case something shows up.
> 
> Comments welcome!
> David
> 
> 
> David Herrmann (20):
>   drm/radeon: move drm_buffer to drm/radeon/
>   drm: mark drm_buf and drm_map as legacy
>   drm: move "struct drm_vma_entry" to drm_vm.c
>   drm: move "struct drm_magic_entry" to drm_auth.c
>   drm: drop unused "struct drm_waitlist"
>   drm: move AGP definitions harder
>   drm: replace weird conditional includes
>   drm: drop __KERNEL__ protection in drmP.h
>   drm: merge drm_memory.h into drm_memory.c
>   drm: move __OS_HAS_AGP into drm_agpsupport.h
>   drm: order includes alphabetically in drmP.h
>   drm: drop DRM_DEBUG_CODE
>   drm: inline "struct drm_sigdata"
>   drm: move remaining includes in drmP.h to the top
>   drm: simplify drm_*_set_unique()
>   drm: drop unused drm_master->unique_size
>   drm: add driver->set_busid() callback
>   drm: Goody bye, drm_bus!
>   drm: merge drm_usb into udl
>   drm: move drm-lock API to drm_legacy.h

Oh well, I've thought I'll be lazy and only review what Thierry didn't
look at yet ;-) I guess should have looked at the time-stamps.

Anyway nothing else but what Thierry spotted, so Ack on all the patches I
didn't comment on. And thanks a lot for doing this.
-Daniel

> 
>  Documentation/DocBook/drm.tmpl   |   3 +-
>  drivers/gpu/drm/Kconfig  |   6 -
>  drivers/gpu/drm/Makefile |   5 +-
>  drivers/gpu/drm/armada/armada_drv.c  |   1 +
>  drivers/gpu/drm/ast/ast_drv.c|   1 +
>  drivers/gpu/drm/bochs/bochs_drv.c|   1 +
>  drivers/gpu/drm/cirrus/cirrus_drv.c  |   1 +
>  drivers/gpu/drm/drm_agpsupport.c |   1 +
>  drivers/gpu/drm/drm_auth.c   |   6 +
>  drivers/gpu/drm/drm_buffer.c | 181 
>  drivers/gpu/drm/drm_bufs.c   |  89 ++--
>  drivers/gpu/drm/drm_debugfs.c|   2 -
>  drivers/gpu/drm/drm_drv.c|  11 +-
>  drivers/gpu/drm/drm_fops.c   |  16 +--
>  drivers/gpu/drm/drm_info.c   |  59 
>  drivers/gpu/drm/drm_ioctl.c  |  27 ++--
>  drivers/gpu/drm/drm_legacy.h |  39 +
>  drivers/gpu/drm/drm_lock.c   |  35 ++---
>  drivers/gpu/drm/drm_memory.c |  12 ++
>  drivers/gpu/drm/drm_pci.c|  41 ++
>  drivers/gpu/drm/drm_platform.c   |  38 ++---
>  drivers/gpu/drm/drm_usb.c|  88 
>  drivers/gpu/drm/drm_vm.c |  74 ++
>  drivers/gpu/drm/exynos/exynos_drm_drv.c  |   1 +
>  drivers/gpu/drm/gma500/psb_drv.c |   1 +
>  drivers/gpu/drm/i810/i810_dma.c  |   4 +-
>  drivers/gpu/drm/i810/i810_drv.c  |   1 +
>  drivers/gpu/drm/i915/i915_dma.c  |   2 +-
>  drivers/gpu/drm/i915/i915_drv.c  |   1 +
>  drivers/gpu/drm/mga/mga_dma.c|  49 +++
>  drivers/gpu/drm/mga/mga_drv.c|   1 +
>  drivers/gpu/drm/mgag200/mgag200_drv.c|   1 +
>  drivers/gpu/drm/msm/msm_drv.c|   1 +
>  drivers/gpu/drm/nouveau/nouveau_drm.c|  19 ++-
>  drivers/gpu/drm/omapdrm/omap_drv.c   |   1 +
>  drivers/gpu/drm/qxl/qxl_drv.c|   1 +
>  drivers/gpu/drm/r128/r128_cce.c  |   2 +-
>  drivers/gpu/drm/r128/r128_drv.c  |   1 +
>  drivers/gpu/drm/radeon/Makefile  |   2 +-
>  drivers/gpu/drm/radeon/drm_buffer.c  | 177 +++
>  drivers/gpu/drm/radeon/drm_buffer.h  | 148 +++
>  drivers/gpu/drm/radeon/r300_cmdbuf.c |   2 +-
>  drivers/gpu/drm/radeon/r600_cp.c |   2 +-
>  drivers/gpu/drm/radeon/radeon.h  |  17 ++-
>  drivers/gpu/drm/radeon/radeon_cp.c   |  22 +--
>  drivers/gpu/drm/radeon/radeon_drv.c  |   2 +
>  drivers/gpu/drm/radeon/radeon_ring.c |  21 ---
>  drivers/gpu/drm/radeon/radeon_state.c|   

[PATCH 1/2] drm: make idr_mutex a spinlock

[Thierry Reding]

On Fri, Aug 29, 2014 at 02:01:00PM +0200, David Herrmann wrote:
> There is no reason to use a heavy mutex for idr protection. Use a spinlock
> and make idr-allocation use idr_preload().
> 
> This patch also makes mode-object lookup irq-save, in case you ever wanna
> lookup modeset objects from interrupts. This is just a side-effect of
> avoiding a mutex.

I don't think that's entirely accurate. idr_preload(GFP_KERNEL) might
sleep since GFP_KERNEL & __GFP_WAIT != 0.

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/20140829/f47786d1/attachment.sig>

[PATCH 2/2] drm: don't recycle used modeset IDs

[David Herrmann]

Hi

On Fri, Aug 29, 2014 at 2:57 PM, David Herrmann  
wrote:
> Hi
>
> On Fri, Aug 29, 2014 at 2:51 PM, Daniel Vetter  wrote:
>> On Fri, Aug 29, 2014 at 02:01:01PM +0200, David Herrmann wrote:
>>> With MST, we now have connector hotplugging, yey! Pretty easy to use in
>>> user-space, but introduces some nasty races:
>>>  * If a connector is removed and added again while a compositor is in
>>>background, it will get the same ID again. If the compositor wakes up,
>>>it cannot know whether its the same connector or a new one, thus they
>>>must re-read EDID information, etc.
>>>  * possible_clones, possible_crtcs, etc. depend on indices, not IDs. So if
>>>an object is removed and a new one is added, those bitmasks are invalid
>>>and must be refreshed. This currently does not affect connectors, but
>>>only crtcs and encoders, but it's only a matter of time when this will
>>>change.
>>>
>>> The easiest way to protect against this, is to not recylce modeset IDs.
>>> Instead, always allocate a new, higher ID. All ioctls that affect modeset
>>> objects, take IDs. Thus, during hotplug races, those ioctls will simply
>>> fail if invalid IDs are passed. They will no longer silently run on a
>>> newly hotplugged object.
>>>
>>> Furthermore, hotplug-races during state sync can now be easily detected. A
>>> call to GET_RESOURCES returns a list of available IDs atomically.
>>> User-space can now start fetching all those objects via GET_* ioctls. If
>>> any of those fails, they know that the given object was unplugged. Thus,
>>> the "possible_*" bit-fields are invalidated. User-space can now decide
>>> whether to restart the sync all over or wait for the 'change' uevent that
>>> is sent on modeset object modifications (or, well, is supposed to be sent
>>> and probably will be at some point).
>>>
>>> With this in place, modeset object hotplugging should work fine for all
>>> modeset objects in the KMS API.
>>>
>>> CC'ing stable so we can rely on all kernels with MST to not recycle IDs.
>>>
>>> Cc:  # 3.16+
>>> Signed-off-by: David Herrmann 
>>
>> So userspace just needs to cycle through piles of framebuffer objects to
>> make bad things happen? Doesn't sound like a terribly solid plan.
>
> IDs will still get recycled, but only once all IDs got used. So this is 
> "safe".
>
> Sure, user-space can create 2 billion framebuffers and destroy them
> again, thus causing the ID range to overflow and recycle old IDs. Not
> sure how fast you can run 2 billion syscalls.. If that's a real issue,
> I'd vote for using the high-range for user-space managed objects,
> low-range for kernel-space managed objects ([1...INT_MAX] and
> [INT_MAX+1...UINT_MAX] or so).
>
>> I guess we could save this by doing normal id allocations for fbs and
>> monotonically increasing allocations for all other objects.
>
> This doesn't work. A connector with ID #n might get unplugged and
> another process created a new FB, which will then get ID #n. Sure, I
> doubt there's a real conflict as ioctls check the type, but it still
> sounds really ugly to me.

On a second thought: maybe your idea isn't as bad as I thought. I
mean, everyone must do type-checking when looking up mode-objects, so
it seems safe to rely on that.

Thanks
David

[PATCH 1/2] drm: make idr_mutex a spinlock

[David Herrmann]

Hi

On Fri, Aug 29, 2014 at 3:10 PM, Thierry Reding
 wrote:
> On Fri, Aug 29, 2014 at 02:01:00PM +0200, David Herrmann wrote:
>> There is no reason to use a heavy mutex for idr protection. Use a spinlock
>> and make idr-allocation use idr_preload().
>>
>> This patch also makes mode-object lookup irq-save, in case you ever wanna
>> lookup modeset objects from interrupts. This is just a side-effect of
>> avoiding a mutex.
>
> I don't think that's entirely accurate. idr_preload(GFP_KERNEL) might
> sleep since GFP_KERNEL & __GFP_WAIT != 0.

idr_preload() is only used in registration, not in lookups. Not sure
what you refer to?

Thanks
David

[PATCH 1/2] drm: make idr_mutex a spinlock

[Thierry Reding]

On Fri, Aug 29, 2014 at 03:11:19PM +0200, David Herrmann wrote:
> Hi
> 
> On Fri, Aug 29, 2014 at 3:10 PM, Thierry Reding
>  wrote:
> > On Fri, Aug 29, 2014 at 02:01:00PM +0200, David Herrmann wrote:
> >> There is no reason to use a heavy mutex for idr protection. Use a spinlock
> >> and make idr-allocation use idr_preload().
> >>
> >> This patch also makes mode-object lookup irq-save, in case you ever wanna
> >> lookup modeset objects from interrupts. This is just a side-effect of
> >> avoiding a mutex.
> >
> > I don't think that's entirely accurate. idr_preload(GFP_KERNEL) might
> > sleep since GFP_KERNEL & __GFP_WAIT != 0.
> 
> idr_preload() is only used in registration, not in lookups. Not sure
> what you refer to?

Yes, you're right. I wasn't paying close enough attention and somehow
applied "makes irq-safe" to the "idr_preload()" change without relating
it to "lookup".

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/20140829/274ccdbf/attachment.sig>

[PATCH 09/20] drm: merge drm_memory.h into drm_memory.c

[Thierry Reding]

On Fri, Aug 29, 2014 at 02:43:10PM +0200, Daniel Vetter wrote:
> On Fri, Aug 29, 2014 at 01:56:23PM +0200, Thierry Reding wrote:
> > On Fri, Aug 29, 2014 at 12:12:35PM +0200, David Herrmann wrote:
> > > The drm_memory.h header is only used to define PAGE_AGP, which is only
> > > used in drm_memory.c. Fold the header into drm_memory.c and drop it.
> > > 
> > > Signed-off-by: David Herrmann 
> > > ---
> > >  drivers/gpu/drm/drm_memory.c | 11 +
> > >  include/drm/drmP.h   |  6 ++---
> > >  include/drm/drm_memory.h | 59 
> > > 
> > >  3 files changed, 13 insertions(+), 63 deletions(-)
> > >  delete mode 100644 include/drm/drm_memory.h
> > > 
> > > diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c
> > > index 7888dad..62fda6a 100644
> > > --- a/drivers/gpu/drm/drm_memory.c
> > > +++ b/drivers/gpu/drm/drm_memory.c
> > > @@ -39,6 +39,17 @@
> > >  #include "drm_legacy.h"
> > >  
> > >  #if __OS_HAS_AGP
> > > +
> > > +#ifdef HAVE_PAGE_AGP
> > > +# include 
> > > +#else
> > 
> > This check seems to be redundant. All architectures that support AGP
> > provide this header.
> > 
> > > +# ifdef __powerpc__
> > > +#  define PAGE_AGP   __pgprot(_PAGE_KERNEL | _PAGE_NO_CACHE)
> > 
> > Is this even necessary? It seems like PowerPC always defines
> > HAVE_PAGE_AGP.
> 
> Iirc I've tried to untangle this before and I'm not 100% this is actually
> the case on all ppc platforms. It looked like there's some crazy include
> header depency stuff going on.

Interestingly, I don't even see _PAGE_KERNEL defined on PowerPC...

> But given how popular drm on ppc and that the few platforms where people
> actually use gpus are the saner ones (hopefully) I think we can just move
> ahead with this change and fixup any compile breakage once it's reported.
> If there is any.

I'm not objecting to this change since it's merely reorganizing code.
This is just more possible future cleanup.

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/20140829/a8010f2d/attachment-0001.sig>

[PATCH 17/20] drm: add driver->set_busid() callback

[Thierry Reding]

On Fri, Aug 29, 2014 at 03:01:00PM +0200, Daniel Vetter wrote:
> On Fri, Aug 29, 2014 at 12:12:43PM +0200, David Herrmann wrote:
> > One step closer to dropping all the drm_bus_* code:
> > Add a driver->set_busid() callback and make all drivers use the generic
> > helpers. Nouveau is the only driver that uses two different bus-types with
> > the same drm_driver. This is totally broken if both buses are available on
> > the same machine (unlikely, but lets be safe). Therefore, we create two
> > different drivers for each platform during module_init() and set the
> > set_busid() callback respectively.
> > 
> > Signed-off-by: David Herrmann 
> 
> This has a bit a midlayer smell to it tbh, we don't really need a
> set_busid callback I think. Instead there's just two cases:
> - The crazy implementation for pci devices. But we already have dev->pdev,
>   so the core can figure this out itself.
> - Everyone else just sets a static name. For those I think we should just
>   add a drm_dev_set_busid function which they can call in their
>   driver-load function. Well maybe different versions for platform drivers
>   and stuff.

We already have drm_dev_set_unique() for exactly this purpose. I think
drivers can just be converted to use that one at a time and when no
users are left we can drop drm_*_set_busid().

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/20140829/d8a5166b/attachment.sig>

[PATCH 2/2] drm: don't recycle used modeset IDs

[Daniel Vetter]

On Fri, Aug 29, 2014 at 02:57:12PM +0200, David Herrmann wrote:
> Hi
> 
> On Fri, Aug 29, 2014 at 2:51 PM, Daniel Vetter  wrote:
> > On Fri, Aug 29, 2014 at 02:01:01PM +0200, David Herrmann wrote:
> >> With MST, we now have connector hotplugging, yey! Pretty easy to use in
> >> user-space, but introduces some nasty races:
> >>  * If a connector is removed and added again while a compositor is in
> >>background, it will get the same ID again. If the compositor wakes up,
> >>it cannot know whether its the same connector or a new one, thus they
> >>must re-read EDID information, etc.
> >>  * possible_clones, possible_crtcs, etc. depend on indices, not IDs. So if
> >>an object is removed and a new one is added, those bitmasks are invalid
> >>and must be refreshed. This currently does not affect connectors, but
> >>only crtcs and encoders, but it's only a matter of time when this will
> >>change.
> >>
> >> The easiest way to protect against this, is to not recylce modeset IDs.
> >> Instead, always allocate a new, higher ID. All ioctls that affect modeset
> >> objects, take IDs. Thus, during hotplug races, those ioctls will simply
> >> fail if invalid IDs are passed. They will no longer silently run on a
> >> newly hotplugged object.
> >>
> >> Furthermore, hotplug-races during state sync can now be easily detected. A
> >> call to GET_RESOURCES returns a list of available IDs atomically.
> >> User-space can now start fetching all those objects via GET_* ioctls. If
> >> any of those fails, they know that the given object was unplugged. Thus,
> >> the "possible_*" bit-fields are invalidated. User-space can now decide
> >> whether to restart the sync all over or wait for the 'change' uevent that
> >> is sent on modeset object modifications (or, well, is supposed to be sent
> >> and probably will be at some point).
> >>
> >> With this in place, modeset object hotplugging should work fine for all
> >> modeset objects in the KMS API.
> >>
> >> CC'ing stable so we can rely on all kernels with MST to not recycle IDs.
> >>
> >> Cc:  # 3.16+
> >> Signed-off-by: David Herrmann 
> >
> > So userspace just needs to cycle through piles of framebuffer objects to
> > make bad things happen? Doesn't sound like a terribly solid plan.
> 
> IDs will still get recycled, but only once all IDs got used. So this is 
> "safe".
> 
> Sure, user-space can create 2 billion framebuffers and destroy them
> again, thus causing the ID range to overflow and recycle old IDs. Not
> sure how fast you can run 2 billion syscalls.. If that's a real issue,
> I'd vote for using the high-range for user-space managed objects,
> low-range for kernel-space managed objects ([1...INT_MAX] and
> [INT_MAX+1...UINT_MAX] or so).
> 
> > I guess we could save this by doing normal id allocations for fbs and
> > monotonically increasing allocations for all other objects.
> 
> This doesn't work. A connector with ID #n might get unplugged and
> another process created a new FB, which will then get ID #n. Sure, I
> doubt there's a real conflict as ioctls check the type, but it still
> sounds really ugly to me.

Oh, ugly for sure, but it should work since userspace doesn't really have
any reason to mix up ids and the kernel does need to check the type
anyway. And with a simple

if (type == OBJ_FB)
idr_alloc
else
idr_alloc_cyclic

I think it should work out with minimal fuss. At least the full separation
of id spaces wouldn't be any more or less complexity.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 1/2] drm: make idr_mutex a spinlock

[Daniel Vetter]

On Fri, Aug 29, 2014 at 03:03:58PM +0200, David Herrmann wrote:
> Hi
> 
> On Fri, Aug 29, 2014 at 2:53 PM, Daniel Vetter  wrote:
> > On Fri, Aug 29, 2014 at 02:01:00PM +0200, David Herrmann wrote:
> >> There is no reason to use a heavy mutex for idr protection. Use a spinlock
> >> and make idr-allocation use idr_preload().
> >>
> >> This patch also makes mode-object lookup irq-save, in case you ever wanna
> >> lookup modeset objects from interrupts. This is just a side-effect of
> >> avoiding a mutex.
> >>
> >> Signed-off-by: David Herrmann 
> >
> > I've thought irqsave/restore are terribly serializing instructions, so
> > this might actually be slower than a plain mutex. And imo if it doesn't
> > show up in profiles it's not worth to optimize it - generally mutexes are
> > really fast and in most cases already nicely degenerate to spinlocks
> > anyway.
> 
> Honestly, this patch is less about speed than 'correctness'. Sure, a
> mutex is just a spin-lock in low-contention cases and there really is
> no high-contention here. However, spin-locks are the major lock-type
> for pure data. mutexes only make sense when you have to lock data
> structures _while_ performing operations that can sleep. Using a
> spin-lock here prevents people from doing stupid things while holding
> this lock. And really, this lock is about ID registration and
> deregistration, nothing else.
> 
> Btw., I can happily turn all those lock/unlock sequences into
> spin_lock() and spin_unlock() so we ignore irq-contexts completely, if
> that's the only issue.

Yeah, if you want I'll r-b plain spinlocks. Imo locks also serve as
documentation, so making it clear that we neither allocate nor sleep while
holding them is good. But making it irq save just because is imo
counterproductive.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 4/9] drm/i915: split intel_update_plane into check() and commit()

[Gustavo Padovan]

2014-08-29 Ville Syrj?l? :

> On Thu, Aug 28, 2014 at 02:40:08PM -0300, 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.
> > 
> > Signed-off-by: Gustavo Padovan 
> > ---
> >  drivers/gpu/drm/i915/intel_sprite.c | 128 
> > ++--
> >  1 file changed, 93 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> > b/drivers/gpu/drm/i915/intel_sprite.c
> > index 4cbe286..b1cb606 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -844,22 +844,32 @@ static bool colorkey_enabled(struct intel_plane 
> > *intel_plane)
> > return key.flags != I915_SET_COLORKEY_NONE;
> >  }
> >  
> > +static bool get_visible(struct drm_rect *src, struct drm_rect *dst,
> 
> get_visisble() is not a good name here IMO, also I think this split is at
> a slightly too low level. What we really want is to start creating some
> kind of plane config struct that can be passed to both check and commit,
> and then check can already store all the clipped coordinates etc. to the
> plane config and commit can just look them up w/o recomputing them.

What do you really mean by too low level here? Would grabbing
struct drm_plane_state from the atomic branch fix this for you?

I'll get a v2 of these patches working with struct drm_plane_state.

Gustavo

[PATCH 2/2] drm: don't recycle used modeset IDs

[Rob Clark]

On Fri, Aug 29, 2014 at 9:10 AM, David Herrmann  
wrote:
> Hi
>
> On Fri, Aug 29, 2014 at 2:57 PM, David Herrmann  
> wrote:
>> Hi
>>
>> On Fri, Aug 29, 2014 at 2:51 PM, Daniel Vetter  wrote:
>>> On Fri, Aug 29, 2014 at 02:01:01PM +0200, David Herrmann wrote:
 With MST, we now have connector hotplugging, yey! Pretty easy to use in
 user-space, but introduces some nasty races:
  * If a connector is removed and added again while a compositor is in
background, it will get the same ID again. If the compositor wakes up,
it cannot know whether its the same connector or a new one, thus they
must re-read EDID information, etc.
  * possible_clones, possible_crtcs, etc. depend on indices, not IDs. So if
an object is removed and a new one is added, those bitmasks are invalid
and must be refreshed. This currently does not affect connectors, but
only crtcs and encoders, but it's only a matter of time when this will
change.

 The easiest way to protect against this, is to not recylce modeset IDs.
 Instead, always allocate a new, higher ID. All ioctls that affect modeset
 objects, take IDs. Thus, during hotplug races, those ioctls will simply
 fail if invalid IDs are passed. They will no longer silently run on a
 newly hotplugged object.

 Furthermore, hotplug-races during state sync can now be easily detected. A
 call to GET_RESOURCES returns a list of available IDs atomically.
 User-space can now start fetching all those objects via GET_* ioctls. If
 any of those fails, they know that the given object was unplugged. Thus,
 the "possible_*" bit-fields are invalidated. User-space can now decide
 whether to restart the sync all over or wait for the 'change' uevent that
 is sent on modeset object modifications (or, well, is supposed to be sent
 and probably will be at some point).

 With this in place, modeset object hotplugging should work fine for all
 modeset objects in the KMS API.

 CC'ing stable so we can rely on all kernels with MST to not recycle IDs.

 Cc:  # 3.16+
 Signed-off-by: David Herrmann 
>>>
>>> So userspace just needs to cycle through piles of framebuffer objects to
>>> make bad things happen? Doesn't sound like a terribly solid plan.
>>
>> IDs will still get recycled, but only once all IDs got used. So this is 
>> "safe".
>>
>> Sure, user-space can create 2 billion framebuffers and destroy them
>> again, thus causing the ID range to overflow and recycle old IDs. Not
>> sure how fast you can run 2 billion syscalls.. If that's a real issue,
>> I'd vote for using the high-range for user-space managed objects,
>> low-range for kernel-space managed objects ([1...INT_MAX] and
>> [INT_MAX+1...UINT_MAX] or so).
>>
>>> I guess we could save this by doing normal id allocations for fbs and
>>> monotonically increasing allocations for all other objects.
>>
>> This doesn't work. A connector with ID #n might get unplugged and
>> another process created a new FB, which will then get ID #n. Sure, I
>> doubt there's a real conflict as ioctls check the type, but it still
>> sounds really ugly to me.
>
> On a second thought: maybe your idea isn't as bad as I thought. I
> mean, everyone must do type-checking when looking up mode-objects, so
> it seems safe to rely on that.

note that for atomic there are cases where we do lookup w/ ANY_TYPE..
*but* the idea might still work since FB's are still handled
specially(ish) do to refcnting..

BR,
-R


> Thanks
> David
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/9] drm/i915: split intel_update_plane into check() and commit()

[Ville Syrjälä]

On Fri, Aug 29, 2014 at 12:09:39PM -0300, Gustavo Padovan wrote:
> 2014-08-29 Ville Syrj?l? :
> 
> > On Thu, Aug 28, 2014 at 02:40:08PM -0300, 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.
> > > 
> > > Signed-off-by: Gustavo Padovan 
> > > ---
> > >  drivers/gpu/drm/i915/intel_sprite.c | 128 
> > > ++--
> > >  1 file changed, 93 insertions(+), 35 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
> > > b/drivers/gpu/drm/i915/intel_sprite.c
> > > index 4cbe286..b1cb606 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -844,22 +844,32 @@ static bool colorkey_enabled(struct intel_plane 
> > > *intel_plane)
> > >   return key.flags != I915_SET_COLORKEY_NONE;
> > >  }
> > >  
> > > +static bool get_visible(struct drm_rect *src, struct drm_rect *dst,
> > 
> > get_visisble() is not a good name here IMO, also I think this split is at
> > a slightly too low level. What we really want is to start creating some
> > kind of plane config struct that can be passed to both check and commit,
> > and then check can already store all the clipped coordinates etc. to the
> > plane config and commit can just look them up w/o recomputing them.
> 
> What do you really mean by too low level here?

I mean you just roughtly cut it in half from the middle and duplicated a
bit of the clipping logic in both halves. That's actually fairly broken
since you seem to have left the extra src coordinate frobbing (align) that
we do after clipping only to the check() part. So you'd need to yank all
that extra code to the "get_visible" function as well.

But with the plane config you can avoid doing all that work twice since
check will do it and just pass the adjusted coordinates to commit.

> Would grabbing
> struct drm_plane_state from the atomic branch fix this for you?

That looks like it's meant to house the user requested coordinates
rather than the clipped ones. What you could do is just shovel the
drm_rects we use during the clipping into a new i915 specific plane
config struct and pass that to both check and commit. We can later
look into moving that stuff into some core struct if seems like a win
for more than one driver.

> 
> I'll get a v2 of these patches working with struct drm_plane_state.
> 
>   Gustavo

-- 
Ville Syrj?l?
Intel OTC

[PATCH 1/2] drm: Resolve many missing-field-initializers warnings

[Jeff Kirsher]

From: Mark Rustad 

Resolve many missing-field-initializers warnings that appear
in W=2 builds by changing nested object initialization syntax.

Signed-off-by: Mark Rustad 
Signed-off-by: Jeff Kirsher 
---
 include/drm/drm_modes.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index 91d0582..0a118f3 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -86,7 +86,7 @@ enum drm_mode_status {
.htotal = (ht), .hskew = (hsk), .vdisplay = (vd), \
.vsync_start = (vss), .vsync_end = (vse), .vtotal = (vt), \
.vscan = (vs), .flags = (f), \
-   .base.type = DRM_MODE_OBJECT_MODE
+   .base = { .type = DRM_MODE_OBJECT_MODE }

 #define CRTC_INTERLACE_HALVE_V (1 << 0) /* halve V values for interlacing */
 #define CRTC_STEREO_DOUBLE (1 << 1) /* adjust timings for stereo modes */
-- 
1.9.3