From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arto Merilainen Subject: Re: [PATCH 3/4] drm/tegra: Add VIC support Date: Mon, 25 May 2015 10:11:28 +0300 Message-ID: <5562CB20.4060700@nvidia.com> References: <1432214425-27137-1-git-send-email-amerilainen@nvidia.com> <1432214425-27137-4-git-send-email-amerilainen@nvidia.com> <555DEE5F.2060100@nvidia.com> <20150522102528.GD16507@ulmo> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20150522102528.GD16507@ulmo> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Thierry Reding , Mikko Perttunen Cc: achew@nvidia.com, dnibade@nvidia.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, srasal@nvidia.com, linux-tegra@vger.kernel.org List-Id: linux-tegra@vger.kernel.org T24gMDUvMjIvMjAxNSAwMToyNSBQTSwgVGhpZXJyeSBSZWRpbmcgd3JvdGU6Cj4gKiBQR1AgU2ln bmVkIGJ5IGFuIHVua25vd24ga2V5Cj4KPiBPbiBUaHUsIE1heSAyMSwgMjAxNSBhdCAwNTo0MDoz MVBNICswMzAwLCBNaWtrbyBQZXJ0dHVuZW4gd3JvdGU6Cj4+IE9uIDA1LzIxLzIwMTUgMDQ6MjAg UE0sIEFydG8gTWVyaWxhaW5lbiB3cm90ZToKPiBbLi4uXQo+Pj4gK3N0YXRpYyBpbnQgdmljX2lz X2FkZHJfcmVnKHN0cnVjdCBkZXZpY2UgKmRldiwgdTMyIGNsYXNzLCB1MzIgb2Zmc2V0LCB1MzIg dmFsKQo+Pj4gK3sKPj4+ICsJc3RydWN0IHZpYyAqdmljID0gZGV2X2dldF9kcnZkYXRhKGRldik7 Cj4+PiArCj4+PiArCS8qIGhhbmRsZSBob3N0IGNsYXNzICovCj4+PiArCWlmIChjbGFzcyA9PSBI T1NUMVhfQ0xBU1NfSE9TVDFYKSB7Cj4+PiArCQlpZiAob2Zmc2V0ID09IDB4MmIpCj4+PiArCQkJ cmV0dXJuIHRydWU7Cj4+PiArCQlyZXR1cm4gZmFsc2U7Cj4+Cj4+ICJyZXR1cm4gKG9mZnNldCA9 PSAweDJiKTsiIHBlcmhhcHM/Cj4KPiBJIHRoaW5rIHRoaXMgc2hvdWxkIHJlYWxseSBiZSBleHRy YWN0ZWQgaW50byBhIHNlcGFyYXRlIGhlbHBlci4gSWYgd2UKPiBldmVyIG5lZWQgdG8gdGFrZSBp bnRvIGFjY291bnQgYWRkaXRpb25hbCBvZmZzZXRzIHdlIHdvdWxkIG90aGVyd2lzZQo+IGhhdmUg dG8gZXh0ZW5kIGV2ZXJ5IGRyaXZlciByYXRoZXIgdGhhbiBqdXN0IHRoZSBoZWxwZXIuCgpJIGFn cmVlLCB0aGF0IHdvdWxkIGJlIGJldHRlci4KCj4KPiBBbHNvIEkgdGhpbmsgdGhlIDB4MmIgc2hv dWxkIGJlIHJlcGxhY2VkIGJ5IHNvbWUgc3ltYm9saWMgbmFtZS4KPiBBY2NvcmRpbmcgdG8gdGhl IFRSTSAweDJiIGlzIHRoZSBob3N0MXggY2xhc3MgbWV0aG9kIG5hbWVkCj4gTlZfQ0xBU1NfSE9T VF9JTkRDVFJMXzAuIE9kZGx5IGVub3VnaCB0aGF0IGRvZXNuJ3Qgc2VlbSB0byBiZSBhbiBhZGRy ZXNzCj4gcmVnaXN0ZXIuIEluc3RlYWQgdGhlIGFkZHJlc3Mgc2VlbXMgdG8gYmUgaW4gdGhlIElO RE9GRjIgYW5kIElORE9GRgo+IG1ldGhvZHMgKDB4MmMgYW5kIDB4MmQpLiBJIGFsc28gY2FuJ3Qg dGVsbCBmcm9tIHRoZSBUUk0gd2hhdCBleGFjdGx5Cj4gdGhlc2UgYXJlIHN1cHBvc2VkIHRvIGRv Lgo+Cj4gQXJ0bywgY2FuIHlvdSBjbGFyaWZ5PwoKVGhpcyBsb29rcyBsaWtlIGFuIHVuZm9ydHVu YXRlIG1pc3Rha2UgdGhhdCBnb3QgcmVwcm9kdWNlZCBmcm9tIGdyMmQgYW5kIApncjNkLgoKVGhl IElORENUUkwgbWV0aG9kIGlzIHVzZWQgZm9yIGluZGlyZWN0IHJlZ2lzdGVyIGFjY2Vzc2luZyBh bmQgaXQgYWxsb3dzIApIb3N0MXggdG8gcmVhZCByZWdpc3RlcnMgb2YgYW4gZW5naW5lIC0gb3Ig d3JpdGUgZGF0YSBkaXJlY3RseSB0byAKbWVtb3J5LiBJdCBhbGxvdyBpbXBsZW1lbnRpbmcgY29u dGV4dCBzd2l0Y2ggZm9yIHRoZSBjbGllbnRzIHdob3NlIHN0YXRlIApzaG91bGQgYmUgbm90IGNo YW5nZSBiZXR3ZWVuIGpvYnMgZnJvbSB0aGUgc2FtZSBhcHBsaWNhdGlvbi4KCj4KPj4+ICsJaWYg KElTX0VSUih2aWMtPnJzdCkpIHsKPj4+ICsJCWRldl9lcnIoJnBkZXYtPmRldiwgImNhbm5vdCBn ZXQgcmVzZXRcbiIpOwo+Pj4gKwkJcmV0dXJuIFBUUl9FUlIodmljLT5yc3QpOwo+Pj4gKwl9Cj4+ PiArCj4+PiArCXBsYXRmb3JtX3NldF9kcnZkYXRhKHBkZXYsIHZpYyk7Cj4+PiArCj4+PiArCUlO SVRfTElTVF9IRUFEKCZ2aWMtPmNsaWVudC5iYXNlLmxpc3QpOwo+Pj4gKwl2aWMtPmNsaWVudC5i YXNlLm9wcyA9ICZ2aWNfY2xpZW50X29wczsKPj4+ICsJdmljLT5jbGllbnQuYmFzZS5kZXYgPSBk ZXY7Cj4+PiArCXZpYy0+Y2xpZW50LmJhc2UuY2xhc3MgPSB2aWNfY29uZmlnLT5jbGFzc19pZDsK Pj4+ICsJdmljLT5jbGllbnQuYmFzZS5zeW5jcHRzID0gc3luY3B0czsKPj4+ICsJdmljLT5jbGll bnQuYmFzZS5udW1fc3luY3B0cyA9IDE7Cj4+PiArCXZpYy0+ZGV2ID0gZGV2Owo+Pj4gKwl2aWMt PmNvbmZpZyA9IHZpY19jb25maWc7Cj4+PiArCj4+PiArCUlOSVRfTElTVF9IRUFEKCZ2aWMtPmNs aWVudC5saXN0KTsKPj4+ICsJdmljLT5jbGllbnQub3BzID0gJnZpY19vcHM7Cj4+PiArCj4+PiAr CWVyciA9IHRlZ3JhX3Bvd2VyZ2F0ZV9zZXF1ZW5jZV9wb3dlcl91cCh2aWMtPmNvbmZpZy0+cG93 ZXJnYXRlX2lkLAo+Pj4gKwkJCQkJCXZpYy0+Y2xrLCB2aWMtPnJzdCk7Cj4+PiArCWlmIChlcnIp IHsKPj4+ICsJCWRldl9lcnIoZGV2LCAiY2Fubm90IHR1cm4gb24gdGhlIGRldmljZVxuIik7Cj4+ PiArCQlyZXR1cm4gZXJyOwo+Pj4gKwl9Cj4+PiArCj4+PiArCWVyciA9IGhvc3QxeF9jbGllbnRf cmVnaXN0ZXIoJnZpYy0+Y2xpZW50LmJhc2UpOwo+Pj4gKwlpZiAoZXJyIDwgMCkgewo+Pgo+PiBZ b3UgdXNlZCAnaWYgKGVycikgeycgcHJldmlvdXNseSwgc28gbWF5YmUgYWxzbyBoZXJlLgo+Cj4g Rm9yIGNvbnNpc3RlbmN5IHdpdGggb3RoZXIgVGVncmEgRFJNIGNvZGUgdGhlc2UgY2hlY2tzIHNo b3VsZCB1c2UgKGF0Cj4gbGVhc3Qgd2hlcmUgcG9zc2libGUpIHRoZSAoZXJyIDwgMCkgbm90YXRp b24uCj4KCldpbGwgZml4LgoKLSBBcnRvCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZy ZWVkZXNrdG9wLm9yZwpodHRwOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGlu Zm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751270AbbEYHLi (ORCPT ); Mon, 25 May 2015 03:11:38 -0400 Received: from hqemgate14.nvidia.com ([216.228.121.143]:2839 "EHLO hqemgate14.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750839AbbEYHLg (ORCPT ); Mon, 25 May 2015 03:11:36 -0400 X-PGP-Universal: processed; by hqnvupgp08.nvidia.com on Mon, 25 May 2015 00:08:26 -0700 Message-ID: <5562CB20.4060700@nvidia.com> Date: Mon, 25 May 2015 10:11:28 +0300 From: Arto Merilainen User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Thierry Reding , Mikko Perttunen CC: , , , , , Subject: Re: [PATCH 3/4] drm/tegra: Add VIC support References: <1432214425-27137-1-git-send-email-amerilainen@nvidia.com> <1432214425-27137-4-git-send-email-amerilainen@nvidia.com> <555DEE5F.2060100@nvidia.com> <20150522102528.GD16507@ulmo> In-Reply-To: <20150522102528.GD16507@ulmo> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.21.26.102] X-ClientProxiedBy: UKMAIL102.nvidia.com (10.26.138.15) To UKMAIL101.nvidia.com (10.26.138.13) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/22/2015 01:25 PM, Thierry Reding wrote: > * PGP Signed by an unknown key > > On Thu, May 21, 2015 at 05:40:31PM +0300, Mikko Perttunen wrote: >> On 05/21/2015 04:20 PM, Arto Merilainen wrote: > [...] >>> +static int vic_is_addr_reg(struct device *dev, u32 class, u32 offset, u32 val) >>> +{ >>> + struct vic *vic = dev_get_drvdata(dev); >>> + >>> + /* handle host class */ >>> + if (class == HOST1X_CLASS_HOST1X) { >>> + if (offset == 0x2b) >>> + return true; >>> + return false; >> >> "return (offset == 0x2b);" perhaps? > > I think this should really be extracted into a separate helper. If we > ever need to take into account additional offsets we would otherwise > have to extend every driver rather than just the helper. I agree, that would be better. > > Also I think the 0x2b should be replaced by some symbolic name. > According to the TRM 0x2b is the host1x class method named > NV_CLASS_HOST_INDCTRL_0. Oddly enough that doesn't seem to be an address > register. Instead the address seems to be in the INDOFF2 and INDOFF > methods (0x2c and 0x2d). I also can't tell from the TRM what exactly > these are supposed to do. > > Arto, can you clarify? This looks like an unfortunate mistake that got reproduced from gr2d and gr3d. The INDCTRL method is used for indirect register accessing and it allows Host1x to read registers of an engine - or write data directly to memory. It allow implementing context switch for the clients whose state should be not change between jobs from the same application. > >>> + if (IS_ERR(vic->rst)) { >>> + dev_err(&pdev->dev, "cannot get reset\n"); >>> + return PTR_ERR(vic->rst); >>> + } >>> + >>> + platform_set_drvdata(pdev, vic); >>> + >>> + INIT_LIST_HEAD(&vic->client.base.list); >>> + vic->client.base.ops = &vic_client_ops; >>> + vic->client.base.dev = dev; >>> + vic->client.base.class = vic_config->class_id; >>> + vic->client.base.syncpts = syncpts; >>> + vic->client.base.num_syncpts = 1; >>> + vic->dev = dev; >>> + vic->config = vic_config; >>> + >>> + INIT_LIST_HEAD(&vic->client.list); >>> + vic->client.ops = &vic_ops; >>> + >>> + err = tegra_powergate_sequence_power_up(vic->config->powergate_id, >>> + vic->clk, vic->rst); >>> + if (err) { >>> + dev_err(dev, "cannot turn on the device\n"); >>> + return err; >>> + } >>> + >>> + err = host1x_client_register(&vic->client.base); >>> + if (err < 0) { >> >> You used 'if (err) {' previously, so maybe also here. > > For consistency with other Tegra DRM code these checks should use (at > least where possible) the (err < 0) notation. > Will fix. - Arto