dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Christian König" <christian.koenig@amd.com>
To: Thomas Zimmermann <tzimmermann@suse.de>,
	ray.huang@amd.com, Jerry.Zhang@amd.com,
	dri-devel@lists.freedesktop.org
Cc: thellstrom@vmware.com, nouveau@lists.freedesktop.org,
	airlied@linux.ie, puck.chen@hisilicon.com,
	amd-gfx@lists.freedesktop.org,
	virtualization@lists.linux-foundation.org,
	z.liuxinliang@hisilicon.com, zourongrong@gmail.com,
	kong.kongxinwei@hisilicon.com,
	linux-graphics-maintainer@vmware.com, kraxel@redhat.com,
	gregkh@linuxfoundation.org, alexander.deucher@amd.com,
	bskeggs@redhat.com
Subject: Re: [PATCH 0/2] Provide init/release functions for struct ttm_bo_global
Date: Thu, 30 Aug 2018 08:45:05 +0200	[thread overview]
Message-ID: <73eea61d-e913-4f62-69b5-c4f482724236@amd.com> (raw)
In-Reply-To: <5e815e50-d0be-5069-cf1d-aa881a18f094@suse.de>

Hi Thomas,

Am 30.08.2018 um 08:34 schrieb Thomas Zimmermann:
> Hi Christian,
>
> I just wanted to ask if there's something else required to get this
> patch set reviewed and landed.

I just need to find some time to review them.

> On top of these two patches, I have a patch set that replaces the
> driver-specific global-bo-and-mem combos with a single struct ttm_global
> structure. It further merges struct drm_global into struct ttm_global
> and implements it such that drivers can either share the global memory

That sounds good.

> or create their private instance.

That doesn't sounds good. Drivers should not be allowed to create their 
own private instance of that.

Thanks for doing this,
Christian.

>   It's also a step towards drm device
> hotplug, which someone just asked.
>
> Best regards
> Thomas
>
>
> Am 13.08.2018 um 12:33 schrieb Christian König:
>> Yes, please! I had it on my TODO list to clean that up for an eternity.
>>
>> Actually I never understood why that should be driver work to setup TTM?
>>
>> I mean can't we just have a module_init/module_exit for TTM?
>>
>> Thanks,
>> Christian.
>>
>> Am 13.08.2018 um 12:24 schrieb Thomas Zimmermann:
>>> TTM uses global memory and BO for backing graphics buffers. These are
>>> represented by struct ttm_mem_global and struct ttm_bo_global.
>>>
>>> Currently, struct ttm_bo_global can only be initialized and released
>>> through
>>> struct ttm_bo_global_ref. This is a workaround for passing an instance of
>>> ttm_mem_global to the BO global initialization code.
>>>
>>> The use of struct ttm_bo_global_ref makes driver code unnecessary hard to
>>> understand. At the same time drivers can use any combination of memory
>>> and
>>> BO for initializing the global instances. This can result in subtle bugs
>>> when the order of initializing and releasing drivers changes.
>>>
>>> As a first step for resolving these problems, the provided patch set
>>> separates initialization and release of struct ttm_bo_global from
>>> struct ttm_bo_global_ref.
>>>
>>> The first patch only renames ttm_bo_global_{init/release}. Hopefully this
>>> change can be applied at once for all drivers.
>>>
>>> Future directions: All TTM-based drivers follow the same pattern for
>>> setting
>>> up the TTM. In a follow-up patch, this code can be moved into a single
>>> place
>>> and shared among drivers.
>>>
>>> Thomas Zimmermann (2):
>>>     drm/ttm: Rename ttm_bo_global_{init,release}() to
>>>       ttm_bo_global_ref_*()
>>>     drm/ttm: Provide ttm_bo_global_{init/release}() for struct
>>>       ttm_bo_global
>>>
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  4 +-
>>>    drivers/gpu/drm/ast/ast_ttm.c               |  4 +-
>>>    drivers/gpu/drm/bochs/bochs_mm.c            |  4 +-
>>>    drivers/gpu/drm/cirrus/cirrus_ttm.c         |  4 +-
>>>    drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c |  4 +-
>>>    drivers/gpu/drm/mgag200/mgag200_ttm.c       |  4 +-
>>>    drivers/gpu/drm/nouveau/nouveau_ttm.c       |  4 +-
>>>    drivers/gpu/drm/qxl/qxl_ttm.c               |  4 +-
>>>    drivers/gpu/drm/radeon/radeon_ttm.c         |  4 +-
>>>    drivers/gpu/drm/ttm/ttm_bo.c                | 12 ++---
>>>    drivers/gpu/drm/virtio/virtgpu_ttm.c        |  4 +-
>>>    drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c    |  4 +-
>>>    drivers/staging/vboxvideo/vbox_ttm.c        |  4 +-
>>>    include/drm/ttm/ttm_bo_driver.h             | 53 ++++++++++++++++-----
>>>    14 files changed, 70 insertions(+), 43 deletions(-)
>>>
>>> -- 
>>> 2.18.0
>>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2018-08-30  6:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-13 10:24 [PATCH 0/2] Provide init/release functions for struct ttm_bo_global Thomas Zimmermann
     [not found] ` <20180813102443.12662-1-tzimmermann-l3A5Bk7waGM@public.gmane.org>
2018-08-13 10:24   ` [PATCH 1/2] drm/ttm: Rename ttm_bo_global_{init, release}() to ttm_bo_global_ref_*() Thomas Zimmermann
2018-08-13 10:33   ` [PATCH 0/2] Provide init/release functions for struct ttm_bo_global Christian König
2018-08-13 12:28     ` Thomas Zimmermann
     [not found]       ` <87d57d1e-4ce3-cde7-5e05-798b6738ae6c-l3A5Bk7waGM@public.gmane.org>
2018-08-13 12:54         ` Thomas Hellstrom
     [not found]     ` <b77c649e-4c5d-a19d-8c46-d91c69859293-5C7GfCeVMHo@public.gmane.org>
2018-08-30  6:34       ` Thomas Zimmermann
2018-08-30  6:45         ` Christian König [this message]
2018-08-30 10:35           ` Thomas Zimmermann
2018-08-13 10:24 ` [PATCH 2/2] drm/ttm: Provide ttm_bo_global_{init/release}() " Thomas Zimmermann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=73eea61d-e913-4f62-69b5-c4f482724236@amd.com \
    --to=christian.koenig@amd.com \
    --cc=Jerry.Zhang@amd.com \
    --cc=airlied@linux.ie \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=bskeggs@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=kong.kongxinwei@hisilicon.com \
    --cc=kraxel@redhat.com \
    --cc=linux-graphics-maintainer@vmware.com \
    --cc=nouveau@lists.freedesktop.org \
    --cc=puck.chen@hisilicon.com \
    --cc=ray.huang@amd.com \
    --cc=thellstrom@vmware.com \
    --cc=tzimmermann@suse.de \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=z.liuxinliang@hisilicon.com \
    --cc=zourongrong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox