Nouveau - Jul 2017 - [PATCH] disp/gf119-: avoid creating non-existent heads

We assume that each board has 4 heads for GF119+. However this is not
necessarily true - in the case of a GP108 board, the register indicated
that there were only 2.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101601
Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
---

I'm not extremely happy about the fact that we have to duplicate the head
count
in both the nv50_display logic for creating DRM-side CRTC's, as well as the
nvkm-side logic for creating heads. However I don't immediately see an easy
way
around it without creating a nvif ioctl to get the number of heads.

 drm/nouveau/nv50_display.c               | 8 +++++---
 drm/nouveau/nvkm/engine/disp/gf119.c     | 2 +-
 drm/nouveau/nvkm/engine/disp/headgf119.c | 4 +++-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drm/nouveau/nv50_display.c b/drm/nouveau/nv50_display.c
index e3132a2c..e2170efb 100644
--- a/drm/nouveau/nv50_display.c
+++ b/drm/nouveau/nv50_display.c
@@ -4443,11 +4443,13 @@ nv50_display_create(struct drm_device *dev)
 
 	/* create crtc objects to represent the hw heads */
 	if (disp->disp->oclass >= GF110_DISP)
-		crtcs = nvif_rd32(&device->object, 0x022448);
+		crtcs = nvif_rd32(&device->object, 0x612004) & 0xf;
 	else
-		crtcs = 2;
+		crtcs = 0x3;
 
-	for (i = 0; i < crtcs; i++) {
+	for (i = 0; i < fls(crtcs); i++) {
+		if (!(crtcs & (1 << i)))
+			continue;
 		ret = nv50_head_create(dev, i);
 		if (ret)
 			goto out;
diff --git a/drm/nouveau/nvkm/engine/disp/gf119.c
b/drm/nouveau/nvkm/engine/disp/gf119.c
index d8765b57..415987e9 100644
--- a/drm/nouveau/nvkm/engine/disp/gf119.c
+++ b/drm/nouveau/nvkm/engine/disp/gf119.c
@@ -168,7 +168,7 @@ int
 gf119_disp_new_(const struct nv50_disp_func *func, struct nvkm_device *device,
 		int index, struct nvkm_disp **pdisp)
 {
-	u32 heads = nvkm_rd32(device, 0x022448);
+	u32 heads = fls(nvkm_rd32(device, 0x612004) & 0xf);
 	return nv50_disp_new_(func, device, index, heads, pdisp);
 }
 
diff --git a/drm/nouveau/nvkm/engine/disp/headgf119.c
b/drm/nouveau/nvkm/engine/disp/headgf119.c
index b3355275..8d44bdf6 100644
--- a/drm/nouveau/nvkm/engine/disp/headgf119.c
+++ b/drm/nouveau/nvkm/engine/disp/headgf119.c
@@ -92,5 +92,7 @@ gf119_head = {
 int
 gf119_head_new(struct nvkm_disp *disp, int id)
 {
-	return nvkm_head_new_(&gf119_head, disp, id);
+	if (nvkm_rd32(disp->engine.subdev.device, 0x612004) & 0xf & (1
<< id))
+		return nvkm_head_new_(&gf119_head, disp, id);
+	return 0;
 }
-- 
2.13.0

On 07/04/2017 03:06 AM, Ilia Mirkin wrote:
> We assume that each board has 4 heads for GF119+. However this is not
> necessarily true - in the case of a GP108 board, the register indicated
> that there were only 2.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101601
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
> 
> I'm not extremely happy about the fact that we have to duplicate the
head count
> in both the nv50_display logic for creating DRM-side CRTC's, as well as
the
> nvkm-side logic for creating heads. However I don't immediately see an
easy way
> around it without creating a nvif ioctl to get the number of heads.
Yeah, I have WIP stuff from long ago that would make this nicer, but for
now it's fine to do what you've done here.
> 
>  drm/nouveau/nv50_display.c               | 8 +++++---
>  drm/nouveau/nvkm/engine/disp/gf119.c     | 2 +-
>  drm/nouveau/nvkm/engine/disp/headgf119.c | 4 +++-
>  3 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drm/nouveau/nv50_display.c b/drm/nouveau/nv50_display.c
> index e3132a2c..e2170efb 100644
> --- a/drm/nouveau/nv50_display.c
> +++ b/drm/nouveau/nv50_display.c
> @@ -4443,11 +4443,13 @@ nv50_display_create(struct drm_device *dev)
>  
>  	/* create crtc objects to represent the hw heads */
>  	if (disp->disp->oclass >= GF110_DISP)
> -		crtcs = nvif_rd32(&device->object, 0x022448);
> +		crtcs = nvif_rd32(&device->object, 0x612004) & 0xf;
>  	else
> -		crtcs = 2;
> +		crtcs = 0x3;
>  
> -	for (i = 0; i < crtcs; i++) {
> +	for (i = 0; i < fls(crtcs); i++) {
> +		if (!(crtcs & (1 << i)))
> +			continue;
>  		ret = nv50_head_create(dev, i);
>  		if (ret)
>  			goto out;
> diff --git a/drm/nouveau/nvkm/engine/disp/gf119.c
b/drm/nouveau/nvkm/engine/disp/gf119.c
> index d8765b57..415987e9 100644
> --- a/drm/nouveau/nvkm/engine/disp/gf119.c
> +++ b/drm/nouveau/nvkm/engine/disp/gf119.c
> @@ -168,7 +168,7 @@ int
>  gf119_disp_new_(const struct nv50_disp_func *func, struct nvkm_device
*device,
>  		int index, struct nvkm_disp **pdisp)
>  {
> -	u32 heads = nvkm_rd32(device, 0x022448);
> +	u32 heads = fls(nvkm_rd32(device, 0x612004) & 0xf);
>  	return nv50_disp_new_(func, device, index, heads, pdisp);
I'd drop this change.  I suspect 0x022448 is still the "maximum head
count" register, and that 0x612004 is just a disable mask.
>  }
>  
> diff --git a/drm/nouveau/nvkm/engine/disp/headgf119.c
b/drm/nouveau/nvkm/engine/disp/headgf119.c
> index b3355275..8d44bdf6 100644
> --- a/drm/nouveau/nvkm/engine/disp/headgf119.c
> +++ b/drm/nouveau/nvkm/engine/disp/headgf119.c
> @@ -92,5 +92,7 @@ gf119_head = {
>  int
>  gf119_head_new(struct nvkm_disp *disp, int id)
>  {
> -	return nvkm_head_new_(&gf119_head, disp, id);
> +	if (nvkm_rd32(disp->engine.subdev.device, 0x612004) & 0xf & (1
<< id))
> +		return nvkm_head_new_(&gf119_head, disp, id);
> +	return 0;
>  }
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20170704/3bbe6b85/attachment.sig>

On Mon, Jul 3, 2017 at 6:02 PM, Ben Skeggs <skeggsb at gmail.com>
wrote:
> On 07/04/2017 03:06 AM, Ilia Mirkin wrote:
>> We assume that each board has 4 heads for GF119+. However this is not
>> necessarily true - in the case of a GP108 board, the register indicated
>> that there were only 2.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101601
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>
>> I'm not extremely happy about the fact that we have to duplicate
the head count
>> in both the nv50_display logic for creating DRM-side CRTC's, as
well as the
>> nvkm-side logic for creating heads. However I don't immediately see
an easy way
>> around it without creating a nvif ioctl to get the number of heads.
> Yeah, I have WIP stuff from long ago that would make this nicer, but for
> now it's fine to do what you've done here.
>
>>
>>  drm/nouveau/nv50_display.c               | 8 +++++---
>>  drm/nouveau/nvkm/engine/disp/gf119.c     | 2 +-
>>  drm/nouveau/nvkm/engine/disp/headgf119.c | 4 +++-
>>  3 files changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/drm/nouveau/nv50_display.c b/drm/nouveau/nv50_display.c
>> index e3132a2c..e2170efb 100644
>> --- a/drm/nouveau/nv50_display.c
>> +++ b/drm/nouveau/nv50_display.c
>> @@ -4443,11 +4443,13 @@ nv50_display_create(struct drm_device *dev)
>>
>>       /* create crtc objects to represent the hw heads */
>>       if (disp->disp->oclass >= GF110_DISP)
>> -             crtcs = nvif_rd32(&device->object, 0x022448);
>> +             crtcs = nvif_rd32(&device->object, 0x612004) &
0xf;
>>       else
>> -             crtcs = 2;
>> +             crtcs = 0x3;
>>
>> -     for (i = 0; i < crtcs; i++) {
>> +     for (i = 0; i < fls(crtcs); i++) {
>> +             if (!(crtcs & (1 << i)))
>> +                     continue;
>>               ret = nv50_head_create(dev, i);
>>               if (ret)
>>                       goto out;
>> diff --git a/drm/nouveau/nvkm/engine/disp/gf119.c
b/drm/nouveau/nvkm/engine/disp/gf119.c
>> index d8765b57..415987e9 100644
>> --- a/drm/nouveau/nvkm/engine/disp/gf119.c
>> +++ b/drm/nouveau/nvkm/engine/disp/gf119.c
>> @@ -168,7 +168,7 @@ int
>>  gf119_disp_new_(const struct nv50_disp_func *func, struct nvkm_device
*device,
>>               int index, struct nvkm_disp **pdisp)
>>  {
>> -     u32 heads = nvkm_rd32(device, 0x022448);
>> +     u32 heads = fls(nvkm_rd32(device, 0x612004) & 0xf);
>>       return nv50_disp_new_(func, device, index, heads, pdisp);
> I'd drop this change.  I suspect 0x022448 is still the "maximum
head
> count" register, and that 0x612004 is just a disable mask.
I suspect you're right (it's 4 as expected on the GP108 in question).
I think I was trying to avoid creating phantom heads, but the code in
headgf119.c will prevent that and ultimately have an identical effect.

Feel free to drop this hunk when applying.
>
>>  }
>>
>> diff --git a/drm/nouveau/nvkm/engine/disp/headgf119.c
b/drm/nouveau/nvkm/engine/disp/headgf119.c
>> index b3355275..8d44bdf6 100644
>> --- a/drm/nouveau/nvkm/engine/disp/headgf119.c
>> +++ b/drm/nouveau/nvkm/engine/disp/headgf119.c
>> @@ -92,5 +92,7 @@ gf119_head = {
>>  int
>>  gf119_head_new(struct nvkm_disp *disp, int id)
>>  {
>> -     return nvkm_head_new_(&gf119_head, disp, id);
>> +     if (nvkm_rd32(disp->engine.subdev.device, 0x612004) & 0xf
& (1 << id))
>> +             return nvkm_head_new_(&gf119_head, disp, id);
>> +     return 0;
>>  }
>>
>
>
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
>