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 at 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.
