From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PULL] SH Mobile DRM driver for v3.7 Date: Sun, 16 Sep 2012 17:38:41 +0200 Message-ID: <1379357.9O94Nb1bxd@avalon> References: <13614899.b5G7FiYMn5@avalon> <50547D45.4010309@metafoo.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [95.142.166.194]) by gabe.freedesktop.org (Postfix) with ESMTP id 7220C9E8DD for ; Sun, 16 Sep 2012 08:38:08 -0700 (PDT) In-Reply-To: <50547D45.4010309@metafoo.de> 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: Lars-Peter Clausen Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Saturday 15 September 2012 15:06:13 Lars-Peter Clausen wrote: > On 09/15/2012 01:28 AM, Dave Airlie wrote: > > On Fri, Sep 14, 2012 at 10:38 PM, Laurent Pinchart wrote: > >> Hi Dave, > >> > >> The SH Mobile DRM driver is now (in my opinion) ready for mainline. It > >> requires GEM and KMS/FB helpers that have been reviewed on the list and > >> tested. Sascha is waiting for them to reach your tree to send a pull > >> request for another new driver. > > > > Just a quick review before I pull, > > > > Why does include/drm/shmob_drm.h exist? this file is meant to define > > the userspace API to the driver, if you don't have any userspace API > > or driver specific ioctls, this file shouldn't be required. You might > > want to create include/drm/shmob_internal.h maybe, if this is used as > > an interface to other places in the kernel. I probably need to check > > other have been doing the right thing here as well. (driver_drm.h > > should be user facing only) > > > > Uggh drm_fb_cma_helper.c is pure midlayer mistake, are you 100% sure > > no driver is ever going to want to tweak the drm_fbdev_cma_ops? > > Obviously we can't, for the same reasons we can't know whether there will > ever be a driver which needs to use custom fb_ops. It works fine as it is > now for three different drivers. And other drivers making use of the cma > buffer helpers are likely to have similar requirements, but if we ever get > to a point where a driver needs custom fb_ops it is fairly easy to change > it then. Dave, are you fine with that ? If so I'll submit a new pull request with include/drm/shmob_drm.h moved to include/linux/platform_data/shmob_drm.h. -- Regards, Laurent Pinchart