linux-kernel - Re: [PATCH 11/12] drm/nouveau: support GK20A in nouveau_accel_init()

Open Source and information security mailing list archives

Date:	Wed, 26 Mar 2014 14:27:20 +1000
From:	Ben Skeggs <skeggsb@...il.com>
To:	Thierry Reding <thierry.reding@...il.com>
Cc:	Alexandre Courbot <acourbot@...dia.com>,
	Alexandre Courbot <gnurou@...il.com>,
	"nouveau@...ts.freedesktop.org" <nouveau@...ts.freedesktop.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	Ben Skeggs <bskeggs@...hat.com>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH 11/12] drm/nouveau: support GK20A in nouveau_accel_init()

On Tue, Mar 25, 2014 at 9:10 AM, Thierry Reding
<thierry.reding@...il.com> wrote:
> On Mon, Mar 24, 2014 at 05:42:33PM +0900, Alexandre Courbot wrote:
>> GK20A does not embed a dedicated COPY engine and thus cannot allocate
>> the copy channel that nouveau_accel_init() attempts to create. It also
>> lacks any display hardware, so the creation of a software channel does
>> not apply neither.
>
> Perhaps this should be two separate patches?
>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> [...]
>> +     if (device->chipset == 0xea) {
>> +             /* gk20a does not have CE0/CE1 */
>
> This would be another good candidate for a feature flag.
There are ways to query this in a chipset-independent way.  However,
despite reporting it as an error if no copy engines are available, the
code should continue on without the channel happily.  Perhaps we can
just punt the relevent error messages to a debug loglevel for now?

>
>> +             arg0 = NVE0_CHANNEL_IND_ENGINE_GR;
>> +             arg1 = 1;
>> +     } else
>>       if (device->card_type >= NV_E0) {
>
> The formatting here is somewhat weird. From a brief look I couldn't find
> any indication that nouveau deviates from the standard coding style, so
> this should be:
>
>         } else if (...) {
I use the former in a few places, despite it not entirely being
"correct".. It looks nicer though :)  I don't mind either way though.

>
>> +     /* Need to figure out how to handle sw for gk20a */
>> +     if (device->chipset == 0xea)
>> +             goto skip_sw_init;
>
> The commit message makes it sound like SW isn't needed since gk20a
> "lacks any display hardware". In that case the comment here doesn't make
> much sense.
>
> Thierry
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives