[PATCH 11/12] drm/nouveau: support GK20A in nouveau_accel_init()
On Wed, Mar 26, 2014 at 1:27 PM, Ben Skeggs wrote:
> On Tue, Mar 25, 2014 at 9:10 AM, Thierry Reding
> 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?
Do you know how to query this in a chipset-independant way? I have
failed to find any information for this.
The code does continue without any issue after reporting the error, so
indeed that check is not strictly necessary. But I was just mimicking
what follows right after:
if (device->chipset >= 0xa3 &&
device->chipset != 0xaa &&
device->chipset != 0xac) {
ret = nouveau_channel_new(drm, >client, NVDRM_DEVICE,
NVDRM_CHAN + 1, NvDmaFB, NvDmaTT,
>cechan);
if (ret)
NV_ERROR(drm, "failed to create ce channel, %d\n", ret);
arg0 = NvDmaFB;
arg1 = NvDmaTT;
} else {
arg0 = NvDmaFB;
arg1 = NvDmaTT;
}
So if we are trying to avoid showing this error for 0xa0 class
devices, why not for NV_E0?
[Nouveau] [PATCH 11/12] drm/nouveau: support GK20A in nouveau_accel_init()
On Thu, Apr 3, 2014 at 12:23 AM, Ilia Mirkin wrote: > On Wed, Apr 2, 2014 at 10:14 AM, Alexandre Courbot > wrote: > + /* 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. >> >> Correct. As far as I have looked (that is, not very far), SW methods >> are used for display-related functions, but there might be other >> use-cases too. Maybe someone who knows better can confirm? > > http://cgit.freedesktop.org/~darktama/nouveau/tree/nvkm/engine/software/nvc0.c > > Take a look at nvc0_software_mthd_mp_control -- that's used in the > mesa driver to... well, control those things :) They're related to > PGRAPH, which should be available on the GK20A. [Not sure what they do > though. One of them is about turning off error reporting.] Hm, I don't know why I didn't notice that was there for now.. I wouldn't be entirely surprised if nvidia had reserved method ids in the 3d object class that were supposed to be used for this, rather than from a sw object class.. > > -ilia
[PATCH 11/12] drm/nouveau: support GK20A in nouveau_accel_init()
On Wed, Mar 26, 2014 at 1:27 PM, Ben Skeggs wrote:
> On Tue, Mar 25, 2014 at 9:10 AM, Thierry Reding
> 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?
Sure, that would be more future-proof as well.
>
>>
>>> + 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.
Yeah, I just followed the style of the file here. Whether it needs to
change or not is not my call. :P
>>> + /* 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.
Correct. As far as I have looked (that is, not very far), SW methods
are used for display-related functions, but there might be other
use-cases too. Maybe someone who knows better can confirm?
[Nouveau] [PATCH 11/12] drm/nouveau: support GK20A in nouveau_accel_init()
On Wed, Apr 2, 2014 at 10:14 AM, Alexandre Courbot wrote: + /* 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. > > Correct. As far as I have looked (that is, not very far), SW methods > are used for display-related functions, but there might be other > use-cases too. Maybe someone who knows better can confirm? http://cgit.freedesktop.org/~darktama/nouveau/tree/nvkm/engine/software/nvc0.c Take a look at nvc0_software_mthd_mp_control -- that's used in the mesa driver to... well, control those things :) They're related to PGRAPH, which should be available on the GK20A. [Not sure what they do though. One of them is about turning off error reporting.] -ilia
[PATCH 11/12] drm/nouveau: support GK20A in nouveau_accel_init()
On Tue, Mar 25, 2014 at 9:10 AM, Thierry Reding
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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
[PATCH 11/12] drm/nouveau: support GK20A in nouveau_accel_init()
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.
> + 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 (...) {
> + /* 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
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL:
[PATCH 11/12] drm/nouveau: support GK20A in nouveau_accel_init()
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.
Signed-off-by: Alexandre Courbot
---
drivers/gpu/drm/nouveau/nouveau_drm.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index ef27949057c3..f2394e84eae6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -171,6 +171,11 @@ nouveau_accel_init(struct nouveau_drm *drm)
return;
}
+ if (device->chipset == 0xea) {
+ /* gk20a does not have CE0/CE1 */
+ arg0 = NVE0_CHANNEL_IND_ENGINE_GR;
+ arg1 = 1;
+ } else
if (device->card_type >= NV_E0) {
ret = nouveau_channel_new(drm, >client, NVDRM_DEVICE,
NVDRM_CHAN + 1,
@@ -207,6 +212,10 @@ nouveau_accel_init(struct nouveau_drm *drm)
return;
}
+ /* Need to figure out how to handle sw for gk20a */
+ if (device->chipset == 0xea)
+ goto skip_sw_init;
+
ret = nouveau_object_new(nv_object(drm), NVDRM_CHAN, NVDRM_NVSW,
nouveau_abi16_swclass(drm), NULL, 0, );
if (ret == 0) {
@@ -233,6 +242,7 @@ nouveau_accel_init(struct nouveau_drm *drm)
return;
}
+skip_sw_init:
if (device->card_type < NV_C0) {
ret = nouveau_gpuobj_new(drm->device, NULL, 32, 0, 0,
>notify);
--
1.9.1
