From patchwork Fri May 18 12:39:39 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thierry Reding X-Patchwork-Id: 10410429 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 91D50602C2 for ; Fri, 18 May 2018 12:39:47 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7AE03288F1 for ; Fri, 18 May 2018 12:39:47 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6FA1328952; Fri, 18 May 2018 12:39:47 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.1 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 8BD07288F1 for ; Fri, 18 May 2018 12:39:46 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3FC356ED5D; Fri, 18 May 2018 12:39:45 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wr0-x242.google.com (mail-wr0-x242.google.com [IPv6:2a00:1450:400c:c0c::242]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5F54E6ED5D for ; Fri, 18 May 2018 12:39:43 +0000 (UTC) Received: by mail-wr0-x242.google.com with SMTP id t16-v6so5882546wrm.9 for ; Fri, 18 May 2018 05:39:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=3EaBhTQIAmrL6ttdh2nINLv2SbVXjrfYypMu8sMJr10=; b=JDIVHi5cg6RO6Uwsz+BZL9bN28t6qfAyxYMKGiRaK6QIM63WFuxwtBOwesFIkd8gT3 lUmtKtFMZwOc31OqhhnI33Use9kP0IMoljMvLYRujrO1ScHxeuohRjrCRTYcTyHvtDz4 W/8ZTelJgIXQvIvm+HSc051YEWPgDGU2QRPPR+AJFwvOxiGN7yqNsioDSx4YVV7VFMCI GDlRpY46A5cUSzuV46LZVk7xcCM1K2a5vD289OrqKcQEyFQ4TQAEOfTLtBkPhDC5431U uXK6qoxYxlNpdBIm2QokjYYeuyBdbJaKqFZwW15NY2N4xXPfKdQHYsP8SNEXzgm4azez JNHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=3EaBhTQIAmrL6ttdh2nINLv2SbVXjrfYypMu8sMJr10=; b=Z0+MUqIZNvpFB/HIEiM3uWaz2PGHab4vstDg2O9iAVZ+xb1N5l34l2diJpo+YuIoAK DfmXT/JEPK6sSRhgKSaNHWURvNwRUJFWcw/rD3lXR+hioIhlspkbD5s4ecMY+lPtIQZC oVlh/i2OvSLkng7d8q0XQxcLl9OtG4mTTObtxbhdzDBnpSbfq6yVeO4BVyVl2MIfrad2 psln/mtvbAzzDRKcyKZviVMryo9HReHyZX3nWCcW61AdrI4/YjZWf4JD7vmc0EeuCQsG 2CEQgi39ge4vQFnhjqlTXru5BncJOjF1xXZ1nZN7/QFSpVg5imJzhXpwvYnuxjXZb/jK gzlw== X-Gm-Message-State: ALKqPwdqycqtzfraNFQmNrD6FQb8Akl6jjbjRFxy6I2UL+JFuTxZ2V2s uIh5Npo+h9Zhj2W482G57Og= X-Google-Smtp-Source: AB8JxZqIdRkeluzWrDolFrp/ZjmQrdzZx8BeemG5vCNNtrvJUFeU/0Z/kMP+9zBwGG2TSjK9UDOxag== X-Received: by 2002:adf:9658:: with SMTP id c24-v6mr6962216wra.190.1526647181618; Fri, 18 May 2018 05:39:41 -0700 (PDT) Received: from localhost (p200300E41F19FC00D958180F3872A5C5.dip0.t-ipconnect.de. [2003:e4:1f19:fc00:d958:180f:3872:a5c5]) by smtp.gmail.com with ESMTPSA id z66-v6sm8004228wme.13.2018.05.18.05.39.40 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 18 May 2018 05:39:40 -0700 (PDT) Date: Fri, 18 May 2018 14:39:39 +0200 From: Thierry Reding To: Mikko Perttunen Subject: Re: [PATCH 7/7] gpu: host1x: Track client version Message-ID: <20180518123939.GA9594@ulmo> References: <20180517153430.9602-1-thierry.reding@gmail.com> <20180517153430.9602-8-thierry.reding@gmail.com> <0a6d355f-1173-61e5-af38-97f65ae41145@kapsi.fi> MIME-Version: 1.0 In-Reply-To: <0a6d355f-1173-61e5-af38-97f65ae41145@kapsi.fi> User-Agent: Mutt/1.9.5 (2018-04-13) X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-tegra@vger.kernel.org, Dmitry Osipenko , dri-devel@lists.freedesktop.org, Mikko Perttunen Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP On Fri, May 18, 2018 at 03:21:11PM +0300, Mikko Perttunen wrote: > On 05/17/2018 06:34 PM, Thierry Reding wrote: > > From: Thierry Reding > > > > Userspace needs to know the version of the interface implemented by a > > client so it can create the proper command streams. Allow individual > > drivers to store this version along with the client so that it can be > > returned to userspace upon opening a channel. > > > > Signed-off-by: Thierry Reding > > --- > > include/linux/host1x.h | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/include/linux/host1x.h b/include/linux/host1x.h > > index 89110d896d72..57d26406bdfd 100644 > > --- a/include/linux/host1x.h > > +++ b/include/linux/host1x.h > > @@ -49,6 +49,7 @@ struct host1x_client_ops { > > * @dev: pointer to struct device backing this host1x client > > * @ops: host1x client operations > > * @class: host1x class represented by this client > > + * @version: interface version implemented by this client > > * @channel: host1x channel associated with this client > > * @syncpts: array of syncpoints requested for this client > > * @num_syncpts: number of syncpoints requested for this client > > @@ -61,6 +62,8 @@ struct host1x_client { > > const struct host1x_client_ops *ops; > > enum host1x_class class; > > + unsigned int version; > > + > > It doesn't seem to me that this fits here - Host1x doesn't provide any > userspace interface, TegraDRM does. We will (hopefully) have clients in the > future that use a different userspace interface, or don't have one at all. > So this property should be on TegraDRM side instead. I think the line is somewhat blurry. Technically it is the host1x clients that define the version of the interface that they provide. By interface I mean the "class" of the command stream supported. So gr2d will define what methods are valid to use on a specific version of the module. The same goes for gr3d and VIC. The hardware module is where that specification lives. So if we ever had a client that didn't provide a userspace interface via Tegra DRM we'd still want to represent what version of the (command stream) interface is expected. By moving this to drm/tegra, we move the hardware specific bits into the userspace related parts, so I'm not sure it is a really good fit. However, I do understand where you're coming from. Ultimately where userspace gets involved is at the ABI level that drm/tegra exposes. And for drivers that don't expose a userspace ABI, they'd likely need to deal with the version number internally, so storing this in host1x clients wouldn't fit very well either. I don't really care all that much how exactly we store the version information so that it can be reported back to userspace, and moving it to drm/tegra makes the code somewhat more elegant. I came up with the below, which is completely untested but should work. Thierry --- >8 --- diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c index 4330d5d48801..5dc02340007e 100644 Acked-by: Dmitry Osipenko --- a/drivers/gpu/drm/tegra/drm.c +++ b/drivers/gpu/drm/tegra/drm.c @@ -1389,7 +1389,7 @@ static int tegra_open_channel(struct drm_device *drm, void *data, break; args->syncpts = client->base.num_syncpts; - args->version = client->base.version; + args->version = client->version; args->context = context->id; break; } diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h index 13660f6225a2..f493e0bf2172 100644 --- a/drivers/gpu/drm/tegra/drm.h +++ b/drivers/gpu/drm/tegra/drm.h @@ -105,6 +105,7 @@ struct tegra_drm_client { struct host1x_client base; struct list_head list; + unsigned int version; const struct tegra_drm_client_ops *ops; }; diff --git a/drivers/gpu/drm/tegra/gr2d.c b/drivers/gpu/drm/tegra/gr2d.c index 7cae8252f0fa..165a1cece039 100644 --- a/drivers/gpu/drm/tegra/gr2d.c +++ b/drivers/gpu/drm/tegra/gr2d.c @@ -196,11 +196,11 @@ static int gr2d_probe(struct platform_device *pdev) gr2d->client.base.ops = &gr2d_client_ops; gr2d->client.base.dev = dev; gr2d->client.base.class = HOST1X_CLASS_GR2D; - gr2d->client.base.version = gr2d->soc->version; gr2d->client.base.syncpts = syncpts; gr2d->client.base.num_syncpts = 1; INIT_LIST_HEAD(&gr2d->client.list); + gr2d->client.version = gr2d->soc->version; gr2d->client.ops = &gr2d_ops; err = host1x_client_register(&gr2d->client.base); diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c index 9bd89916d2ef..0d72e78bd6e8 100644 --- a/drivers/gpu/drm/tegra/gr3d.c +++ b/drivers/gpu/drm/tegra/gr3d.c @@ -325,11 +325,11 @@ static int gr3d_probe(struct platform_device *pdev) gr3d->client.base.ops = &gr3d_client_ops; gr3d->client.base.dev = &pdev->dev; gr3d->client.base.class = HOST1X_CLASS_GR3D; - gr3d->client.base.version = gr3d->soc->version; gr3d->client.base.syncpts = syncpts; gr3d->client.base.num_syncpts = 1; INIT_LIST_HEAD(&gr3d->client.list); + gr3d->client.version = gr3d->soc->version; gr3d->client.ops = &gr3d_ops; err = host1x_client_register(&gr3d->client.base); diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c index b9df467ca71e..9fa77405db01 100644 --- a/drivers/gpu/drm/tegra/vic.c +++ b/drivers/gpu/drm/tegra/vic.c @@ -342,12 +342,12 @@ static int vic_probe(struct platform_device *pdev) vic->client.base.ops = &vic_client_ops; vic->client.base.dev = dev; vic->client.base.class = HOST1X_CLASS_VIC; - vic->client.base.version = vic->config->version; vic->client.base.syncpts = syncpts; vic->client.base.num_syncpts = 1; vic->dev = dev; INIT_LIST_HEAD(&vic->client.list); + vic->client.version = vic->config->version; vic->client.ops = &vic_ops; err = host1x_client_register(&vic->client.base); diff --git a/include/linux/host1x.h b/include/linux/host1x.h index 6dc819b1ab64..518864cfbd16 100644 --- a/include/linux/host1x.h +++ b/include/linux/host1x.h @@ -62,7 +62,6 @@ struct host1x_client { const struct host1x_client_ops *ops; enum host1x_class class; - unsigned int version; struct host1x_channel *channel;