[xf86-video-nouveau] Properly cleanup fb for reverse-prime-offload

Submitted by Hans de Goede on June 3, 2016, 12:46 p.m.

Details

Message ID 1464957970-19906-1-git-send-email-hdegoede@redhat.com
State New
Headers show
Series "Properly cleanup fb for reverse-prime-offload" ( rev: 1 ) in Nouveau

Not browsing as part of any series.

Commit Message

Hans de Goede June 3, 2016, 12:46 p.m.
drmmode_set_scanout_pixmap(pix) adds drmmod->fb_id through a call
to drmmode_xf86crtc_resize(), but on a subsequent
drmmode_set_scanout_pixmap(NULL) it would not remove the fb.

This keeps the crtc marked as busy, which causes the dgpu to not
being able to runtime suspend, after an output attached to the dgpu
has been used once. Which causes burning through an additional 10W
of power and the laptop to run quite hot.

This commit adds the missing remove fb call, allowing the dgpu to runtime
suspend after an external monitor has been plugged into the laptop.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 src/drmmode_display.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index b950f42..f326e46 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -680,10 +680,16 @@  drmmode_set_scanout_pixmap(xf86CrtcPtr crtc, PixmapPtr ppix)
 	PixmapPtr screenpix = screen->GetScreenPixmap(screen);
 	xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(crtc->scrn);
 	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	drmmode_ptr drmmode = drmmode_crtc->drmmode;
 	int c, total_width = 0, max_height = 0, this_x = 0;
 	if (!ppix) {
-		if (crtc->randr_crtc->scanout_pixmap)
+		if (crtc->randr_crtc->scanout_pixmap) {
 			PixmapStopDirtyTracking(crtc->randr_crtc->scanout_pixmap, screenpix);
+			if (drmmode && drmmode->fb_id) {
+				drmModeRmFB(drmmode->fd, drmmode->fb_id);
+				drmmode->fb_id = 0;
+			}
+		}
 		drmmode_crtc->scanout_pixmap_x = 0;
 		return TRUE;
 	}

Comments

Hi Ilia, Ben, et al.

Can I please get a review of this patch, and assuming the review is favorable,
can someone please push this ?

Regards,

Hans


p.s.

Is it ok if I request push rights to the nouveau repos / group
at freedesktop.org ?


On 03-06-16 14:46, Hans De Goede wrote:
> drmmode_set_scanout_pixmap(pix) adds drmmod->fb_id through a call
> to drmmode_xf86crtc_resize(), but on a subsequent
> drmmode_set_scanout_pixmap(NULL) it would not remove the fb.
>
> This keeps the crtc marked as busy, which causes the dgpu to not
> being able to runtime suspend, after an output attached to the dgpu
> has been used once. Which causes burning through an additional 10W
> of power and the laptop to run quite hot.
>
> This commit adds the missing remove fb call, allowing the dgpu to runtime
> suspend after an external monitor has been plugged into the laptop.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  src/drmmode_display.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
> index b950f42..f326e46 100644
> --- a/src/drmmode_display.c
> +++ b/src/drmmode_display.c
> @@ -680,10 +680,16 @@ drmmode_set_scanout_pixmap(xf86CrtcPtr crtc, PixmapPtr ppix)
>  	PixmapPtr screenpix = screen->GetScreenPixmap(screen);
>  	xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(crtc->scrn);
>  	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
> +	drmmode_ptr drmmode = drmmode_crtc->drmmode;
>  	int c, total_width = 0, max_height = 0, this_x = 0;
>  	if (!ppix) {
> -		if (crtc->randr_crtc->scanout_pixmap)
> +		if (crtc->randr_crtc->scanout_pixmap) {
>  			PixmapStopDirtyTracking(crtc->randr_crtc->scanout_pixmap, screenpix);
> +			if (drmmode && drmmode->fb_id) {
> +				drmModeRmFB(drmmode->fd, drmmode->fb_id);
> +				drmmode->fb_id = 0;
> +			}
> +		}
>  		drmmode_crtc->scanout_pixmap_x = 0;
>  		return TRUE;
>  	}
>
On 06/27/2016 04:24 PM, Hans de Goede wrote:
> Hi Ilia, Ben, et al.
> 
> Can I please get a review of this patch, and assuming the review is
> favorable,
> can someone please push this ?
> 
> Regards,
> 
> Hans
> 
> 
> p.s.
> 
> Is it ok if I request push rights to the nouveau repos / group
> at freedesktop.org ?
Hey Hans,

Patch looks good to me, and I've pushed it.  And yeah, go ahead and
request that!

Ben.

> 
> 
> On 03-06-16 14:46, Hans De Goede wrote:
>> drmmode_set_scanout_pixmap(pix) adds drmmod->fb_id through a call
>> to drmmode_xf86crtc_resize(), but on a subsequent
>> drmmode_set_scanout_pixmap(NULL) it would not remove the fb.
>>
>> This keeps the crtc marked as busy, which causes the dgpu to not
>> being able to runtime suspend, after an output attached to the dgpu
>> has been used once. Which causes burning through an additional 10W
>> of power and the laptop to run quite hot.
>>
>> This commit adds the missing remove fb call, allowing the dgpu to runtime
>> suspend after an external monitor has been plugged into the laptop.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  src/drmmode_display.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/drmmode_display.c b/src/drmmode_display.c
>> index b950f42..f326e46 100644
>> --- a/src/drmmode_display.c
>> +++ b/src/drmmode_display.c
>> @@ -680,10 +680,16 @@ drmmode_set_scanout_pixmap(xf86CrtcPtr crtc,
>> PixmapPtr ppix)
>>      PixmapPtr screenpix = screen->GetScreenPixmap(screen);
>>      xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(crtc->scrn);
>>      drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
>> +    drmmode_ptr drmmode = drmmode_crtc->drmmode;
>>      int c, total_width = 0, max_height = 0, this_x = 0;
>>      if (!ppix) {
>> -        if (crtc->randr_crtc->scanout_pixmap)
>> +        if (crtc->randr_crtc->scanout_pixmap) {
>>              PixmapStopDirtyTracking(crtc->randr_crtc->scanout_pixmap,
>> screenpix);
>> +            if (drmmode && drmmode->fb_id) {
>> +                drmModeRmFB(drmmode->fd, drmmode->fb_id);
>> +                drmmode->fb_id = 0;
>> +            }
>> +        }
>>          drmmode_crtc->scanout_pixmap_x = 0;
>>          return TRUE;
>>      }
>>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau