UDL: screen update breakage

UDL: screen update breakage

[poma]

https://bugzilla.redhat.com/show_bug.cgi?id=1375566

Guys,
do you use such a device - DisplayLink GPU USB2.0 ?

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: UDL: screen update breakage

[Noralf Trønnes]

Den 13.09.2016 16:44, skrev poma:
> https://bugzilla.redhat.com/show_bug.cgi?id=1375566
>
> Guys,
> do you use such a device - DisplayLink GPU USB2.0 ?
>

I haven't got such a display.
You can try to revert just the changes in udl_handle_damage().
The other changes are for fbdev.

Noralf.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/udl: Fix for the X server screen update

[poma]

Fix for DisplayLink GPU USB2.0 device X server screen update

Within X server on top of DisplayLink GPU USB2.0 device,
screen content is not refreshed i.e. updated,
which is the most basic functionality of the screen.

This partially (udl_handle_damage()) reverts commit:

- e375882406d0cc24030746638592004755ed4ae0
  "drm/udl: Use drm_fb_helper deferred_io support"

Thanks Noralf for the tip.

$ modinfo udl
filename:       /lib/modules/4.7.4-234.fc24.x86_64/updates/udl.ko
license:        GPL
alias:          usb:v17E9p*d*dc*dsc*dp*icFFisc00ip00in*
depends:        
intree:         Y
vermagic:       4.7.4-234.fc24.x86_64 SMP mod_unload 
parm:           fb_bpp:int
parm:           fb_defio:int

$ dmesg | grep udl
[   41.888469] udl: module verification failed: signature and/or required key missing - tainting kernel
[   42.156988] udl 1-2:1.0: fb1: udldrmfb frame buffer device
[   42.158940] [drm] Initialized udl on minor 1
[   42.159676] usbcore: registered new interface driver udl

$ grep udl /var/log/Xorg.0.log
[    71.194] (**) |   |-->Device "udl0"

Tested-by: poma <poma@gmail.com>
---
 drivers/gpu/drm/udl/udl_drv.h |  2 ++
 drivers/gpu/drm/udl/udl_fb.c  | 39 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 0b03d34..4a064ef 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -81,6 +81,8 @@ struct udl_framebuffer {
 	struct drm_framebuffer base;
 	struct udl_gem_object *obj;
 	bool active_16; /* active on the 16-bit channel */
+	int x1, y1, x2, y2; /* dirty rect */
+	spinlock_t dirty_lock;
 };
 
 #define to_udl_fb(x) container_of(x, struct udl_framebuffer, base)
diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index d5df555..b2b42d2 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -90,6 +90,9 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
 	struct urb *urb;
 	int aligned_x;
 	int bpp = (fb->base.bits_per_pixel / 8);
+	int x2, y2;
+	bool store_for_later = false;
+	unsigned long flags;
 
 	if (!fb->active_16)
 		return 0;
@@ -115,6 +118,38 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
 	    (y + height > fb->base.height))
 		return -EINVAL;
 
+	/* if we are in atomic just store the info
+	   can't test inside spin lock */
+	if (in_atomic())
+		store_for_later = true;
+
+	x2 = x + width - 1;
+	y2 = y + height - 1;
+
+	spin_lock_irqsave(&fb->dirty_lock, flags);
+
+	if (fb->y1 < y)
+		y = fb->y1;
+	if (fb->y2 > y2)
+		y2 = fb->y2;
+	if (fb->x1 < x)
+		x = fb->x1;
+	if (fb->x2 > x2)
+		x2 = fb->x2;
+
+	if (store_for_later) {
+		fb->x1 = x;
+		fb->x2 = x2;
+		fb->y1 = y;
+		fb->y2 = y2;
+		spin_unlock_irqrestore(&fb->dirty_lock, flags);
+		return 0;
+	}
+
+	fb->x1 = fb->y1 = INT_MAX;
+	fb->x2 = fb->y2 = 0;
+
+	spin_unlock_irqrestore(&fb->dirty_lock, flags);
 	start_cycles = get_cycles();
 
 	urb = udl_get_urb(dev);
@@ -122,14 +157,14 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
 		return 0;
 	cmd = urb->transfer_buffer;
 
-	for (i = y; i < height ; i++) {
+	for (i = y; i <= y2 ; i++) {
 		const int line_offset = fb->base.pitches[0] * i;
 		const int byte_offset = line_offset + (x * bpp);
 		const int dev_byte_offset = (fb->base.width * bpp * i) + (x * bpp);
 		if (udl_render_hline(dev, bpp, &urb,
 				     (char *) fb->obj->vmapping,
 				     &cmd, byte_offset, dev_byte_offset,
-				     width * bpp,
+				     (x2 - x + 1) * bpp,
 				     &bytes_identical, &bytes_sent))
 			goto error;
 	}
-- 
2.7.4


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/udl: Fix for the X server screen update

[Noralf Trønnes]

Den 18.09.2016 15:48, skrev poma:
> Fix for DisplayLink GPU USB2.0 device X server screen update
>
> Within X server on top of DisplayLink GPU USB2.0 device,
> screen content is not refreshed i.e. updated,
> which is the most basic functionality of the screen.
>
> This partially (udl_handle_damage()) reverts commit:
>
> - e375882406d0cc24030746638592004755ed4ae0
>    "drm/udl: Use drm_fb_helper deferred_io support"
>
> Thanks Noralf for the tip.
>
> $ modinfo udl
> filename:       /lib/modules/4.7.4-234.fc24.x86_64/updates/udl.ko
> license:        GPL
> alias:          usb:v17E9p*d*dc*dsc*dp*icFFisc00ip00in*
> depends:
> intree:         Y
> vermagic:       4.7.4-234.fc24.x86_64 SMP mod_unload
> parm:           fb_bpp:int
> parm:           fb_defio:int
>
> $ dmesg | grep udl
> [   41.888469] udl: module verification failed: signature and/or required key missing - tainting kernel
> [   42.156988] udl 1-2:1.0: fb1: udldrmfb frame buffer device
> [   42.158940] [drm] Initialized udl on minor 1
> [   42.159676] usbcore: registered new interface driver udl
>
> $ grep udl /var/log/Xorg.0.log
> [    71.194] (**) |   |-->Device "udl0"
>
> Tested-by: poma <poma@gmail.com>
> ---
>   drivers/gpu/drm/udl/udl_drv.h |  2 ++
>   drivers/gpu/drm/udl/udl_fb.c  | 39 +++++++++++++++++++++++++++++++++++++--
>   2 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
> index 0b03d34..4a064ef 100644
> --- a/drivers/gpu/drm/udl/udl_drv.h
> +++ b/drivers/gpu/drm/udl/udl_drv.h
> @@ -81,6 +81,8 @@ struct udl_framebuffer {
>   	struct drm_framebuffer base;
>   	struct udl_gem_object *obj;
>   	bool active_16; /* active on the 16-bit channel */
> +	int x1, y1, x2, y2; /* dirty rect */
> +	spinlock_t dirty_lock;
>   };
>   
>   #define to_udl_fb(x) container_of(x, struct udl_framebuffer, base)
> diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
> index d5df555..b2b42d2 100644
> --- a/drivers/gpu/drm/udl/udl_fb.c
> +++ b/drivers/gpu/drm/udl/udl_fb.c
> @@ -90,6 +90,9 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
>   	struct urb *urb;
>   	int aligned_x;
>   	int bpp = (fb->base.bits_per_pixel / 8);
> +	int x2, y2;
> +	bool store_for_later = false;
> +	unsigned long flags;
>   
>   	if (!fb->active_16)
>   		return 0;
> @@ -115,6 +118,38 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
>   	    (y + height > fb->base.height))
>   		return -EINVAL;
>   
> +	/* if we are in atomic just store the info
> +	   can't test inside spin lock */
> +	if (in_atomic())
> +		store_for_later = true;
> +
> +	x2 = x + width - 1;
> +	y2 = y + height - 1;
> +
> +	spin_lock_irqsave(&fb->dirty_lock, flags);

You can drop the spinlock and store_for_later since it always runs in
process context.
Which means that there's no need to store x1/x2/y1/y2 in fb.

> +
> +	if (fb->y1 < y)
> +		y = fb->y1;
> +	if (fb->y2 > y2)
> +		y2 = fb->y2;
> +	if (fb->x1 < x)
> +		x = fb->x1;
> +	if (fb->x2 > x2)
> +		x2 = fb->x2;
> +
> +	if (store_for_later) {
> +		fb->x1 = x;
> +		fb->x2 = x2;
> +		fb->y1 = y;
> +		fb->y2 = y2;
> +		spin_unlock_irqrestore(&fb->dirty_lock, flags);
> +		return 0;
> +	}
> +
> +	fb->x1 = fb->y1 = INT_MAX;
> +	fb->x2 = fb->y2 = 0;
> +
> +	spin_unlock_irqrestore(&fb->dirty_lock, flags);
>   	start_cycles = get_cycles();
>   
>   	urb = udl_get_urb(dev);
> @@ -122,14 +157,14 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
>   		return 0;
>   	cmd = urb->transfer_buffer;
>   
> -	for (i = y; i < height ; i++) {
> +	for (i = y; i <= y2 ; i++) {
>   		const int line_offset = fb->base.pitches[0] * i;
>   		const int byte_offset = line_offset + (x * bpp);
>   		const int dev_byte_offset = (fb->base.width * bpp * i) + (x * bpp);
>   		if (udl_render_hline(dev, bpp, &urb,
>   				     (char *) fb->obj->vmapping,
>   				     &cmd, byte_offset, dev_byte_offset,
> -				     width * bpp,

There is obviously something wrong with the use of height/width here
as a substitute for y2 and the x1 formula.
If you add a printk here you can see how they differ.

Noralf.

> +				     (x2 - x + 1) * bpp,
>   				     &bytes_identical, &bytes_sent))
>   			goto error;
>   	}

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/udl: Fix for the X server screen update

[poma]

On 18.09.2016 19:42, Noralf Trønnes wrote:
> 
> Den 18.09.2016 15:48, skrev poma:
>> Fix for DisplayLink GPU USB2.0 device X server screen update
>>
>> Within X server on top of DisplayLink GPU USB2.0 device,
>> screen content is not refreshed i.e. updated,
>> which is the most basic functionality of the screen.
>>
>> This partially (udl_handle_damage()) reverts commit:
>>
>> - e375882406d0cc24030746638592004755ed4ae0
>>    "drm/udl: Use drm_fb_helper deferred_io support"
>>
>> Thanks Noralf for the tip.
>>
>> $ modinfo udl
>> filename:       /lib/modules/4.7.4-234.fc24.x86_64/updates/udl.ko
>> license:        GPL
>> alias:          usb:v17E9p*d*dc*dsc*dp*icFFisc00ip00in*
>> depends:
>> intree:         Y
>> vermagic:       4.7.4-234.fc24.x86_64 SMP mod_unload
>> parm:           fb_bpp:int
>> parm:           fb_defio:int
>>
>> $ dmesg | grep udl
>> [   41.888469] udl: module verification failed: signature and/or required key missing - tainting kernel
>> [   42.156988] udl 1-2:1.0: fb1: udldrmfb frame buffer device
>> [   42.158940] [drm] Initialized udl on minor 1
>> [   42.159676] usbcore: registered new interface driver udl
>>
>> $ grep udl /var/log/Xorg.0.log
>> [    71.194] (**) |   |-->Device "udl0"
>>
>> Tested-by: poma <poma@gmail.com>
>> ---
>>   drivers/gpu/drm/udl/udl_drv.h |  2 ++
>>   drivers/gpu/drm/udl/udl_fb.c  | 39 +++++++++++++++++++++++++++++++++++++--
>>   2 files changed, 39 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
>> index 0b03d34..4a064ef 100644
>> --- a/drivers/gpu/drm/udl/udl_drv.h
>> +++ b/drivers/gpu/drm/udl/udl_drv.h
>> @@ -81,6 +81,8 @@ struct udl_framebuffer {
>>   	struct drm_framebuffer base;
>>   	struct udl_gem_object *obj;
>>   	bool active_16; /* active on the 16-bit channel */
>> +	int x1, y1, x2, y2; /* dirty rect */
>> +	spinlock_t dirty_lock;
>>   };
>>   
>>   #define to_udl_fb(x) container_of(x, struct udl_framebuffer, base)
>> diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
>> index d5df555..b2b42d2 100644
>> --- a/drivers/gpu/drm/udl/udl_fb.c
>> +++ b/drivers/gpu/drm/udl/udl_fb.c
>> @@ -90,6 +90,9 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
>>   	struct urb *urb;
>>   	int aligned_x;
>>   	int bpp = (fb->base.bits_per_pixel / 8);
>> +	int x2, y2;
>> +	bool store_for_later = false;
>> +	unsigned long flags;
>>   
>>   	if (!fb->active_16)
>>   		return 0;
>> @@ -115,6 +118,38 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
>>   	    (y + height > fb->base.height))
>>   		return -EINVAL;
>>   
>> +	/* if we are in atomic just store the info
>> +	   can't test inside spin lock */
>> +	if (in_atomic())
>> +		store_for_later = true;
>> +
>> +	x2 = x + width - 1;
>> +	y2 = y + height - 1;
>> +
>> +	spin_lock_irqsave(&fb->dirty_lock, flags);
> 
> You can drop the spinlock and store_for_later since it always runs in
> process context.
> Which means that there's no need to store x1/x2/y1/y2 in fb.
> 
>> +
>> +	if (fb->y1 < y)
>> +		y = fb->y1;
>> +	if (fb->y2 > y2)
>> +		y2 = fb->y2;
>> +	if (fb->x1 < x)
>> +		x = fb->x1;
>> +	if (fb->x2 > x2)
>> +		x2 = fb->x2;
>> +
>> +	if (store_for_later) {
>> +		fb->x1 = x;
>> +		fb->x2 = x2;
>> +		fb->y1 = y;
>> +		fb->y2 = y2;
>> +		spin_unlock_irqrestore(&fb->dirty_lock, flags);
>> +		return 0;
>> +	}
>> +
>> +	fb->x1 = fb->y1 = INT_MAX;
>> +	fb->x2 = fb->y2 = 0;
>> +
>> +	spin_unlock_irqrestore(&fb->dirty_lock, flags);
>>   	start_cycles = get_cycles();
>>   
>>   	urb = udl_get_urb(dev);
>> @@ -122,14 +157,14 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
>>   		return 0;
>>   	cmd = urb->transfer_buffer;
>>   
>> -	for (i = y; i < height ; i++) {
>> +	for (i = y; i <= y2 ; i++) {
>>   		const int line_offset = fb->base.pitches[0] * i;
>>   		const int byte_offset = line_offset + (x * bpp);
>>   		const int dev_byte_offset = (fb->base.width * bpp * i) + (x * bpp);
>>   		if (udl_render_hline(dev, bpp, &urb,
>>   				     (char *) fb->obj->vmapping,
>>   				     &cmd, byte_offset, dev_byte_offset,
>> -				     width * bpp,
> 

That's right, this is sufficient

---
 drivers/gpu/drm/udl/udl_fb.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index d5df555..05ab114 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -90,6 +90,7 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
 	struct urb *urb;
 	int aligned_x;
 	int bpp = (fb->base.bits_per_pixel / 8);
+	int x2, y2;
 
 	if (!fb->active_16)
 		return 0;
@@ -115,6 +116,9 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
 	    (y + height > fb->base.height))
 		return -EINVAL;
 
+	x2 = x + width - 1;
+	y2 = y + height - 1;
+
 	start_cycles = get_cycles();
 
 	urb = udl_get_urb(dev);
@@ -122,14 +126,14 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
 		return 0;
 	cmd = urb->transfer_buffer;
 
-	for (i = y; i < height ; i++) {
+	for (i = y; i <= y2 ; i++) {
 		const int line_offset = fb->base.pitches[0] * i;
 		const int byte_offset = line_offset + (x * bpp);
 		const int dev_byte_offset = (fb->base.width * bpp * i) + (x * bpp);
 		if (udl_render_hline(dev, bpp, &urb,
 				     (char *) fb->obj->vmapping,
 				     &cmd, byte_offset, dev_byte_offset,
-				     width * bpp,
+				     (x2 - x + 1) * bpp,
 				     &bytes_identical, &bytes_sent))
 			goto error;
 	}
-- 

> There is obviously something wrong with the use of height/width here
> as a substitute for y2 and the x1 formula.
> If you add a printk here you can see how they differ.
> 
> Noralf.
> 
>> +				     (x2 - x + 1) * bpp,
>>   				     &bytes_identical, &bytes_sent))
>>   			goto error;
>>   	}
> 

What exact kernel print expression and where to insert it?
By following instructions: http://elinux.org/Debugging_by_printing
printk content doesn't shows within dmesg output.


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/udl: Fix for the X server screen update v2

[poma]

Fix for DisplayLink GPU USB2.0 device X server screen update

Within X server on top of DisplayLink GPU USB2.0 device,
screen content is not refreshed i.e. updated,
which is the most basic functionality of the screen.

This partially (udl_handle_damage()) reverts commit:

- e375882406d0cc24030746638592004755ed4ae0
  "drm/udl: Use drm_fb_helper deferred_io support"

Thanks Noralf for the comments.

Reported and
Tested-by: poma <poma@gmail.com>
---
 drivers/gpu/drm/udl/udl_fb.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index d5df555..05ab114 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -90,6 +90,7 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
 	struct urb *urb;
 	int aligned_x;
 	int bpp = (fb->base.bits_per_pixel / 8);
+	int x2, y2;
 
 	if (!fb->active_16)
 		return 0;
@@ -115,6 +116,9 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
 	    (y + height > fb->base.height))
 		return -EINVAL;
 
+	x2 = x + width - 1;
+	y2 = y + height - 1;
+
 	start_cycles = get_cycles();
 
 	urb = udl_get_urb(dev);
@@ -122,14 +126,14 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
 		return 0;
 	cmd = urb->transfer_buffer;
 
-	for (i = y; i < height ; i++) {
+	for (i = y; i <= y2 ; i++) {
 		const int line_offset = fb->base.pitches[0] * i;
 		const int byte_offset = line_offset + (x * bpp);
 		const int dev_byte_offset = (fb->base.width * bpp * i) + (x * bpp);
 		if (udl_render_hline(dev, bpp, &urb,
 				     (char *) fb->obj->vmapping,
 				     &cmd, byte_offset, dev_byte_offset,
-				     width * bpp,
+				     (x2 - x + 1) * bpp,
 				     &bytes_identical, &bytes_sent))
 			goto error;
 	}
-- 
2.7.4


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/udl: Fix for the X server screen update v2

[Daniel Vetter]

On Mon, Sep 19, 2016 at 8:27 PM, poma <pomidorabelisima@gmail.com> wrote:
> @@ -122,14 +126,14 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
>                 return 0;
>         cmd = urb->transfer_buffer;
>
> -       for (i = y; i < height ; i++) {
> +       for (i = y; i <= y2 ; i++) {

I think a simpler fix (which retains Noralf's nice cleanup would be to
change the loop condition to i < y + height. At least that seems to be
the underlying bug. Can you pls test that and then submit either that
one-liner (if it works) or your original patch (it's missing the
signed-off-by right now, so can't be merged as-is)? Either one has my
r-b (preemptive since I'm travelling).

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/udl: Fix for the X server screen update v3

[poma]

Within X server, on top of DisplayLink GPU USB2.0 device,
screen content is not refreshed i.e. updated.

This fixes commit:

- e375882406d0cc24030746638592004755ed4ae0
  "drm/udl: Use drm_fb_helper deferred_io support"

Thanks Noralf and Daniel for the comments.

Tested-by: poma <poma@gmail.com>
---
 drivers/gpu/drm/udl/udl_fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index d5df555..cc2daba 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -122,7 +122,7 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
 		return 0;
 	cmd = urb->transfer_buffer;
 
-	for (i = y; i < height ; i++) {
+	for (i = y; i < y + height ; i++) {
 		const int line_offset = fb->base.pitches[0] * i;
 		const int byte_offset = line_offset + (x * bpp);
 		const int dev_byte_offset = (fb->base.width * bpp * i) + (x * bpp);
-- 
2.7.4


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/udl: Fix for the X server screen update v3

[David Herrmann]

Hi

On Wed, Sep 21, 2016 at 6:47 AM, poma <pomidorabelisima@gmail.com> wrote:
> Within X server, on top of DisplayLink GPU USB2.0 device,
> screen content is not refreshed i.e. updated.
>
> This fixes commit:
>
> - e375882406d0cc24030746638592004755ed4ae0
>   "drm/udl: Use drm_fb_helper deferred_io support"
>
> Thanks Noralf and Daniel for the comments.
>
> Tested-by: poma <poma@gmail.com>

Can you provide your Signed-off-by: line?

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

> ---
>  drivers/gpu/drm/udl/udl_fb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
> index d5df555..cc2daba 100644
> --- a/drivers/gpu/drm/udl/udl_fb.c
> +++ b/drivers/gpu/drm/udl/udl_fb.c
> @@ -122,7 +122,7 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
>                 return 0;
>         cmd = urb->transfer_buffer;
>
> -       for (i = y; i < height ; i++) {
> +       for (i = y; i < y + height ; i++) {
>                 const int line_offset = fb->base.pitches[0] * i;
>                 const int byte_offset = line_offset + (x * bpp);
>                 const int dev_byte_offset = (fb->base.width * bpp * i) + (x * bpp);
> --
> 2.7.4
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/udl: Fix for the X server screen update v3

[poma]

On 21.09.2016 09:34, David Herrmann wrote:
> Hi
> 
> On Wed, Sep 21, 2016 at 6:47 AM, poma <pomidorabelisima@gmail.com> wrote:
>> Within X server, on top of DisplayLink GPU USB2.0 device,
>> screen content is not refreshed i.e. updated.
>>
>> This fixes commit:
>>
>> - e375882406d0cc24030746638592004755ed4ae0
>>   "drm/udl: Use drm_fb_helper deferred_io support"
>>
>> Thanks Noralf and Daniel for the comments.
>>
>> Tested-by: poma <poma@gmail.com>
> 
> Can you provide your Signed-off-by: line?
> 
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> 
> Thanks
> David
> 

S-o-b should be performed by the actual kernel developer.
R-b & T-b, I've already written.


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/udl: Fix for the X server screen update v3

[David Herrmann]

Hi

On Wed, Sep 21, 2016 at 1:19 PM, poma <pomidorabelisima@gmail.com> wrote:
> On 21.09.2016 09:34, David Herrmann wrote:
>> Hi
>>
>> On Wed, Sep 21, 2016 at 6:47 AM, poma <pomidorabelisima@gmail.com> wrote:
>>> Within X server, on top of DisplayLink GPU USB2.0 device,
>>> screen content is not refreshed i.e. updated.
>>>
>>> This fixes commit:
>>>
>>> - e375882406d0cc24030746638592004755ed4ae0
>>>   "drm/udl: Use drm_fb_helper deferred_io support"
>>>
>>> Thanks Noralf and Daniel for the comments.
>>>
>>> Tested-by: poma <poma@gmail.com>
>>
>> Can you provide your Signed-off-by: line?
>>
>> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
>>
>> Thanks
>> David
>>
>
> S-o-b should be performed by the actual kernel developer.
> R-b & T-b, I've already written.

The author of a patch must provide the S-o-b (see
Documentation/SubmittingPatches if you want details). So simply reply
with a "S-o-b: foo <bar>" line to this mail. And please include it in
all patches you submit (preferably use `git commit --sign-off`).

Thanks
David
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/udl: Fix for the X server screen update v3

[Jani Nikula]

On Wed, 21 Sep 2016, poma <pomidorabelisima@gmail.com> wrote:
> On 21.09.2016 09:34, David Herrmann wrote:
>> Hi
>> 
>> On Wed, Sep 21, 2016 at 6:47 AM, poma <pomidorabelisima@gmail.com> wrote:
>>> Within X server, on top of DisplayLink GPU USB2.0 device,
>>> screen content is not refreshed i.e. updated.
>>>
>>> This fixes commit:
>>>
>>> - e375882406d0cc24030746638592004755ed4ae0
>>>   "drm/udl: Use drm_fb_helper deferred_io support"
>>>
>>> Thanks Noralf and Daniel for the comments.
>>>
>>> Tested-by: poma <poma@gmail.com>
>> 
>> Can you provide your Signed-off-by: line?
>> 
>> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
>> 
>> Thanks
>> David
>> 
>
> S-o-b should be performed by the actual kernel developer.
> R-b & T-b, I've already written.

Please read section 11 of Documentation/SubmittingPatches.

In short, you need to add your Signed-off-by with your real name.

BR,
Jani.




-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/udl: Fix for the X server screen update v3

[poma]

On 21.09.2016 13:33, David Herrmann wrote:
> Hi
> 
> On Wed, Sep 21, 2016 at 1:19 PM, poma <pomidorabelisima@gmail.com> wrote:
>> On 21.09.2016 09:34, David Herrmann wrote:
>>> Hi
>>>
>>> On Wed, Sep 21, 2016 at 6:47 AM, poma <pomidorabelisima@gmail.com> wrote:
>>>> Within X server, on top of DisplayLink GPU USB2.0 device,
>>>> screen content is not refreshed i.e. updated.
>>>>
>>>> This fixes commit:
>>>>
>>>> - e375882406d0cc24030746638592004755ed4ae0
>>>>   "drm/udl: Use drm_fb_helper deferred_io support"
>>>>
>>>> Thanks Noralf and Daniel for the comments.
>>>>
>>>> Tested-by: poma <poma@gmail.com>
>>>
>>> Can you provide your Signed-off-by: line?
>>>
>>> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
>>>
>>> Thanks
>>> David
>>>
>>
>> S-o-b should be performed by the actual kernel developer.
>> R-b & T-b, I've already written.
> 
> The author of a patch must provide the S-o-b (see
> Documentation/SubmittingPatches if you want details). So simply reply
> with a "S-o-b: foo <bar>" line to this mail. And please include it in
> all patches you submit (preferably use `git commit --sign-off`).
> 
> Thanks
> David
> 

Patches are nothing but direct suggestions from Noralf and Daniel.
I wrote a patch to show what is actually tested, tested successfully,
but I'm not the author of these corrections.


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/udl: Fix for the X server screen update v3

[Jani Nikula]

On Wed, 21 Sep 2016, poma <pomidorabelisima@gmail.com> wrote:
> On 21.09.2016 13:33, David Herrmann wrote:
>> The author of a patch must provide the S-o-b (see
>> Documentation/SubmittingPatches if you want details). So simply reply
>> with a "S-o-b: foo <bar>" line to this mail. And please include it in
>> all patches you submit (preferably use `git commit --sign-off`).
>> 
>> Thanks
>> David
>> 
>
> Patches are nothing but direct suggestions from Noralf and Daniel.
> I wrote a patch to show what is actually tested, tested successfully,
> but I'm not the author of these corrections.

For the third and last time, see Documentation/SubmittingPatches.

The Signed-off-by tag is not about authorship, it's about "Developer's
Certificate of Origin". The text is also available at
http://developercertificate.org/. If you send a patch, no matter whose
patch it is, we can't apply it without your Signed-off-by.

Thanks for your understanding.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/udl: Fix for the X server screen update v2

[poma]

On 19.09.2016 23:12, Daniel Vetter wrote:
> On Mon, Sep 19, 2016 at 8:27 PM, poma <pomidorabelisima@gmail.com> wrote:
>> @@ -122,14 +126,14 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y,
>>                 return 0;
>>         cmd = urb->transfer_buffer;
>>
>> -       for (i = y; i < height ; i++) {
>> +       for (i = y; i <= y2 ; i++) {
> 
> I think a simpler fix (which retains Noralf's nice cleanup would be to
> change the loop condition to i < y + height. At least that seems to be
> the underlying bug. Can you pls test that and then submit either that
> one-liner (if it works) or your original patch (it's missing the
> signed-off-by right now, so can't be merged as-is)? Either one has my
> r-b (preemptive since I'm travelling).
> 
> Thanks, Daniel
> 

Feel free to (re)send patch v3, whenever you can.
After all, the idea is yours.

Respect

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel