From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ilija Hadzic Subject: Re: [PATCH 05/19] drm: move dev_mapping to the minor node Date: Mon, 30 Apr 2012 09:48:38 -0500 (CDT) Message-ID: References: <1334254784-3200-1-git-send-email-ihadzic@research.bell-labs.com> <1334254784-3200-6-git-send-email-ihadzic@research.bell-labs.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from ihemail2.lucent.com (ihemail2.lucent.com [135.245.0.35]) by gabe.freedesktop.org (Postfix) with ESMTP id 928349E7A7 for ; Mon, 30 Apr 2012 07:48:40 -0700 (PDT) In-Reply-To: Content-ID: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Dave Airlie Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Fri, 20 Apr 2012, Dave Airlie wrote: > On Thu, Apr 12, 2012 at 7:19 PM, Ilija Hadzic > wrote: >> Make dev_mapping per-minor instead of per device. This is >> a preparatory patch for introducing render nodes. This >> will allow per-node instead of per-device mapping range, >> once we introduce render nodes. > > One problem is this introduces a ttm/drm dependency that we don't > really have so far. > Sorry for a belated follow-up (I was busy with something else). I understand the concern and I think that the patch can be reworked such that it does not introduce the dependency. What introduces the dependency is a call to drm_unmap_mapping in ttm_bo_unmap_virtual. If the patch is reworked such that the minor tracks i_mapping pointer instead of the whole drm_device structure, the need for drm_unmap_mapping may be eliminated, which in turn will take care of the dependency. That brings me to a question for you. The patch from which I derived this is your patch 7c5cc4f63556e351e9e5980ed22accad410e3fdc (on your original render-nodes branch). That's where you introduced drm_unmap_mapping function, which calls unmap_mapping_range for every minor known to the system. Why was that necessary at first place ? I mean we don't do the eqivalent of that currently when there are multiple physical GPUs in the system, so I don't quite get why introduction of multiple minors would require to go through the whole list of minors. If that's a "just in case"/"just to be safe" kind of thing, then I think we may be able to just call drm_unmap_mapping from the driver that needs to call it and only for the minor in question. If we can do that, then the patch will in principle look the same to what the current code does, except that dev_mapping will be in the minor structure and there will ne no new dependencies introduced. The patch will also be simpler that way. -- Ilija