[PATCH v4] drm/panel: simple: Add support for AUO t215hvn01

2016-10-11 Thread Haixia Shi
The AUO t215hvn01 is a 21.5" FHD (1920x1080) color TFT LCD panel.

This panel is used on the Acer Chromebase 21.5-inch All-in-One (DC221HQ).

Link to spec: http://www.udmgroup.com/ftp/T215HVN01.0.pdf

v2: fix alphabetical order
v3: remove minor revision suffix ".0" and add link to spec
v4: add dt-binding documentation

Signed-off-by: Haixia Shi 
Tested-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
Cc: Emil Velikov 
Cc: Thierry Reding 
Cc: David Airlie 
Cc: Rob Herring <robh+dt at kernel.org>
Cc: Mark Rutland 
Cc: devicetree at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
---
 .../bindings/display/panel/auo,t215hvn01.txt   |  7 +
 drivers/gpu/drm/panel/panel-simple.c   | 30 ++
 2 files changed, 37 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/auo,t215hvn01.txt

diff --git a/Documentation/devicetree/bindings/display/panel/auo,t215hvn01.txt 
b/Documentation/devicetree/bindings/display/panel/auo,t215hvn01.txt
new file mode 100644
index 000..cbd9da3
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/auo,t215hvn01.txt
@@ -0,0 +1,7 @@
+AU Optronics Corporation 21.5" FHD (1920x1080) color TFT LCD panel
+
+Required properties:
+- compatible: should be "auo,t215hvn01"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 113db3c..256c201 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -555,6 +555,33 @@ static const struct panel_desc auo_b133htn01 = {
},
 };

+static const struct drm_display_mode auo_t215hvn01_mode = {
+   .clock = 148800,
+   .hdisplay = 1920,
+   .hsync_start = 1920 + 88,
+   .hsync_end = 1920 + 88 + 44,
+   .htotal = 1920 + 88 + 44 + 148,
+   .vdisplay = 1080,
+   .vsync_start = 1080 + 4,
+   .vsync_end = 1080 + 4 + 5,
+   .vtotal = 1080 + 4 + 5 + 36,
+   .vrefresh = 60,
+};
+
+static const struct panel_desc auo_t215hvn01 = {
+   .modes = _t215hvn01_mode,
+   .num_modes = 1,
+   .bpc = 8,
+   .size = {
+   .width = 430,
+   .height = 270,
+   },
+   .delay = {
+   .disable = 5,
+   .unprepare = 1000,
+   }
+};
+
 static const struct drm_display_mode avic_tm070ddh03_mode = {
.clock = 51200,
.hdisplay = 1024,
@@ -1575,6 +1602,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "auo,b133xtn01",
.data = _b133xtn01,
}, {
+   .compatible = "auo,t215hvn01",
+   .data = _t215hvn01,
+   }, {
.compatible = "avic,tm070ddh03",
.data = _tm070ddh03,
}, {
-- 
2.8.0.rc3.226.g39d4020



[PATCH v3] drm/panel: simple: Add support for AUO t215hvn01

2016-10-10 Thread Haixia Shi
The AUO t215hvn01 is a 21.5" 1920x1080 panel.

Link to spec: http://www.udmgroup.com/ftp/T215HVN01.0.pdf

v2: fix alphabetical order
v3: remove minor revision suffix ".0" and add link to spec

Signed-off-by: Haixia Shi 
Tested-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
Cc: Emil Velikov 
Cc: Thierry Reding 
---
 drivers/gpu/drm/panel/panel-simple.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 113db3c..256c201 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -555,6 +555,33 @@ static const struct panel_desc auo_b133htn01 = {
},
 };

+static const struct drm_display_mode auo_t215hvn01_mode = {
+   .clock = 148800,
+   .hdisplay = 1920,
+   .hsync_start = 1920 + 88,
+   .hsync_end = 1920 + 88 + 44,
+   .htotal = 1920 + 88 + 44 + 148,
+   .vdisplay = 1080,
+   .vsync_start = 1080 + 4,
+   .vsync_end = 1080 + 4 + 5,
+   .vtotal = 1080 + 4 + 5 + 36,
+   .vrefresh = 60,
+};
+
+static const struct panel_desc auo_t215hvn01 = {
+   .modes = _t215hvn01_mode,
+   .num_modes = 1,
+   .bpc = 8,
+   .size = {
+   .width = 430,
+   .height = 270,
+   },
+   .delay = {
+   .disable = 5,
+   .unprepare = 1000,
+   }
+};
+
 static const struct drm_display_mode avic_tm070ddh03_mode = {
.clock = 51200,
.hdisplay = 1024,
@@ -1575,6 +1602,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "auo,b133xtn01",
.data = _b133xtn01,
}, {
+   .compatible = "auo,t215hvn01",
+   .data = _t215hvn01,
+   }, {
.compatible = "avic,tm070ddh03",
.data = _tm070ddh03,
}, {
-- 
2.8.0.rc3.226.g39d4020



[PATCH v2] drm/panel: simple: Add support for AUO t215hvn01

2016-10-10 Thread Haixia Shi
The AUO t215hvn01 is a 21.5" 1920x1080 panel.

v2: fix alphabetical order

Signed-off-by: Haixia Shi 
Tested-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/panel/panel-simple.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 113db3c..54dbb98 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -555,6 +555,33 @@ static const struct panel_desc auo_b133htn01 = {
},
 };

+static const struct drm_display_mode auo_t215hvn01_mode = {
+   .clock = 148800,
+   .hdisplay = 1920,
+   .hsync_start = 1920 + 88,
+   .hsync_end = 1920 + 88 + 44,
+   .htotal = 1920 + 88 + 44 + 148,
+   .vdisplay = 1080,
+   .vsync_start = 1080 + 4,
+   .vsync_end = 1080 + 4 + 5,
+   .vtotal = 1080 + 4 + 5 + 36,
+   .vrefresh = 60,
+};
+
+static const struct panel_desc auo_t215hvn01 = {
+   .modes = _t215hvn01_mode,
+   .num_modes = 1,
+   .bpc = 8,
+   .size = {
+   .width = 430,
+   .height = 270,
+   },
+   .delay = {
+   .disable = 5,
+   .unprepare = 1000,
+   }
+};
+
 static const struct drm_display_mode avic_tm070ddh03_mode = {
.clock = 51200,
.hdisplay = 1024,
@@ -1575,6 +1602,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "auo,b133xtn01",
.data = _b133xtn01,
}, {
+   .compatible = "auo,t215hvn01.0",
+   .data = _t215hvn01,
+   }, {
.compatible = "avic,tm070ddh03",
.data = _tm070ddh03,
}, {
-- 
2.8.0.rc3.226.g39d4020



[PATCH] CHROMIUM: drm/panel: simple: Add support for AUO t215hvn01

2016-10-10 Thread Haixia Shi
The AUO t215hvn01 is a 21.5" 1920x1080 panel.

Signed-off-by: Haixia Shi 
Reviewed-on: https://chromium-review.googlesource.com/394328
Tested-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/panel/panel-simple.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 113db3c..bab1951 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -583,6 +583,33 @@ static const struct panel_desc avic_tm070ddh03 = {
},
 };

+static const struct drm_display_mode auo_t215hvn01_mode = {
+   .clock = 148800,
+   .hdisplay = 1920,
+   .hsync_start = 1920 + 88,
+   .hsync_end = 1920 + 88 + 44,
+   .htotal = 1920 + 88 + 44 + 148,
+   .vdisplay = 1080,
+   .vsync_start = 1080 + 4,
+   .vsync_end = 1080 + 4 + 5,
+   .vtotal = 1080 + 4 + 5 + 36,
+   .vrefresh = 60,
+};
+
+static const struct panel_desc auo_t215hvn01 = {
+   .modes = _t215hvn01_mode,
+   .num_modes = 1,
+   .bpc = 8,
+   .size = {
+   .width = 430,
+   .height = 270,
+   },
+   .delay = {
+   .disable = 5,
+   .unprepare = 1000,
+   }
+};
+
 static const struct drm_display_mode chunghwa_claa101wa01a_mode = {
.clock = 72070,
.hdisplay = 1366,
@@ -1575,6 +1602,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "auo,b133xtn01",
.data = _b133xtn01,
}, {
+   .compatible = "auo,t215hvn01.0",
+   .data = _t215hvn01,
+   }, {
.compatible = "avic,tm070ddh03",
.data = _tm070ddh03,
}, {
-- 
2.8.0.rc3.226.g39d4020



[PATCH 1/2] drm/udl: fix a NULL pointer reference in udl_gem_free_object().

2016-08-31 Thread Haixia Shi
Daniel

Thanks! I agree the PATCH 1/2 needs some more work.

What do you think about the PATCH 2/2 (suspend/resume) -- would it
make sense to review it as a single standalone patch?

Regards, Haixia

On Wed, Aug 31, 2016 at 2:17 PM, Daniel Vetter  wrote:
> On Wed, Aug 31, 2016 at 11:05 PM, Daniel Vetter  wrote:
>> On Wed, Aug 31, 2016 at 10:45 PM, Haixia Shi  wrote:
>>> For details see https://bugs.chromium.org/p/chromium/issues/detail?id=468050
>>>
>>> So drm_mode_config_cleanup() is called from udl_driver_unload() in
>>> which we found there's still a framebuffer left, hence the WARN in
>>> drm_crtc.c:5495. This also forcefully releases all the buffers.
>>>
>>> A bit later the actual drm_buf_release comes in which attempts to
>>> release the buffer again.
>>
>> Leaving a drm_framebuffer behind on unload is definitely a bug, but
>> not fixed with this patch here I think.
>>
>> The dma-buf lifetime issue is far worse, since we simply don't
>> handling those leaking past the lifetime of the exporting drm_device
>> at all. The dma-buf has references to a lot more than just the vma
>> manager. What we probably need is that every exported dma-buf holds a
>> ref on the underlying drm_device, but that means untangling the
>> refcounting of that vs unplugging it.
>
> Just noticed that these problems started only when dma-buf export
> support was added (by you) to udl. That dma-buf support is a bit
> hack-ish (e.g. it leaks sg mappings since udl_unmap_dma_buf isn't
> implemented. Rough sketch of a fix:
>
> - fix up udl_dmabuf.c. We could/should probably put most of these into
> the core as a set of helpers for drivers which use plain shmem gem
> objects.
>
> - fix udl load/unload to no longer be midlayered, i.e. get rid of the
> ->load/unload hooks. There's tons of examples and drivers out there
> for templates.
>
> - fix up the unplug hook to correctly unregister everything. With the
> fixed load/unload all the unplugged tracking is probably no longer
> neeeded. Also with this you can drop the drm_global_mutex dance from
> drm_unplug_dev. Also the sequence in drm_unplug_dev is wrong like the
> midlayered ->unload. First it should do all the unregistering, then
> release internal resources. Atm it's the other way round.
>
> - make sure _all_ public objects (open files, counted by
> dev->open_count, dma-bufs, ...) hold a full reference onto the
> drm_device. For the dma-buf case this probably needs changes in
> drm_prime.c.
>
> - Fix that little ordering issue with the leaking fb. It's probably
> not getting cleanup up as it should in udl_fbdev_unplug.
>
> tada, bug fixed for real!
>
> Cheers, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch


[PATCH 1/2] drm/udl: fix a NULL pointer reference in udl_gem_free_object().

2016-08-31 Thread Haixia Shi
For details see https://bugs.chromium.org/p/chromium/issues/detail?id=468050

So drm_mode_config_cleanup() is called from udl_driver_unload() in
which we found there's still a framebuffer left, hence the WARN in
drm_crtc.c:5495. This also forcefully releases all the buffers.

A bit later the actual drm_buf_release comes in which attempts to
release the buffer again.

On Wed, Aug 31, 2016 at 12:55 AM, Daniel Vetter  wrote:
>
> On Tue, Aug 30, 2016 at 02:50:20PM -0700, Haixia Shi wrote:
> > Previously this function had a NULL pointer check for gem->map_list.map, but
> > that line was refactored after commit 
> > 0de23977cfeb5b357ec884ba15417ae118ff9e9bb
> > ("drm/gem: convert to new unified vma manager").
> >
> > After the refactor it is still necessasry to check that the vma manager is 
> > not
> > NULL because udl_gem_free_object() may come in after the vma manager is 
> > destroyed.
>
> When/how does this happen? Backtrace? Destroying the vma manager before
> the objects are all gone sounds a bit fishy.
> -Daniel
>
> >
> > Signed-off-by: Haixia Shi 
> > Reviewed-by: Stéphane Marchesin 
> > ---
> >  drivers/gpu/drm/udl/udl_gem.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
> > index 818e707..72e1bd4 100644
> > --- a/drivers/gpu/drm/udl/udl_gem.c
> > +++ b/drivers/gpu/drm/udl/udl_gem.c
> > @@ -204,7 +204,8 @@ void udl_gem_free_object(struct drm_gem_object *gem_obj)
> >   if (obj->pages)
> >   udl_gem_put_pages(obj);
> >
> > - drm_gem_free_mmap_offset(gem_obj);
> > + if (gem_obj->dev->vma_offset_manager)
> > + drm_gem_free_mmap_offset(gem_obj);
> >  }
> >
> >  /* the dumb interface doesn't work with the GEM straight MMAP
> > --
> > 2.8.0.rc3.226.g39d4020
> >
> > ___
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch


[PATCH 2/2] drm/udl: implement usb_driver suspend/resume.

2016-08-30 Thread Haixia Shi
The usb_driver suspend and resume function pointers must be populated
to prevent forced unbinding of USB interface driver. See usb/core/driver.c:
unbind_no_pm_drivers_interfaces().

Restore mode and damage the entire frame buffer upon resume.

TEST=suspend and resume with the same UDL device connected
TEST=suspend with UDL, unplug UDL and resume
TEST=suspend with UDL, unplug and connect another UDL device then resume

Signed-off-by: Haixia Shi 
Reviewed-by: Stphane Marchesin 
---
 drivers/gpu/drm/udl/udl_drv.c | 17 +
 drivers/gpu/drm/udl/udl_drv.h |  2 ++
 drivers/gpu/drm/udl/udl_modeset.c | 13 +
 3 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index 17d34e0..e5dc73b 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -16,6 +16,21 @@ static int udl_driver_set_busid(struct drm_device *d, struct 
drm_master *m)
return 0;
 }

+static int udl_usb_suspend(struct usb_interface *interface,
+  pm_message_t message)
+{
+   return 0;
+}
+
+static int udl_usb_resume(struct usb_interface *interface)
+{
+   struct drm_device *dev = usb_get_intfdata(interface);
+
+   udl_modeset_restore(dev);
+   return 0;
+}
+
+
 static const struct vm_operations_struct udl_gem_vm_ops = {
.fault = udl_gem_fault,
.open = drm_gem_vm_open,
@@ -122,6 +137,8 @@ static struct usb_driver udl_driver = {
.name = "udl",
.probe = udl_usb_probe,
.disconnect = udl_usb_disconnect,
+   .suspend = udl_usb_suspend,
+   .resume = udl_usb_resume,
.id_table = id_table,
 };
 module_usb_driver(udl_driver);
diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 0b03d34..f338a57 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -52,6 +52,7 @@ struct udl_device {
struct device *dev;
struct drm_device *ddev;
struct usb_device *udev;
+   struct drm_crtc *crtc;

int sku_pixel_limit;

@@ -87,6 +88,7 @@ struct udl_framebuffer {

 /* modeset */
 int udl_modeset_init(struct drm_device *dev);
+void udl_modeset_restore(struct drm_device *dev);
 void udl_modeset_cleanup(struct drm_device *dev);
 int udl_connector_init(struct drm_device *dev, struct drm_encoder *encoder);

diff --git a/drivers/gpu/drm/udl/udl_modeset.c 
b/drivers/gpu/drm/udl/udl_modeset.c
index 7369512..69d6a4f 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -309,6 +309,8 @@ static int udl_crtc_mode_set(struct drm_crtc *crtc,
char *wrptr;
int color_depth = 0;

+   udl->crtc = crtc;
+
buf = (char *)udl->mode_buf;

/* for now we just clip 24 -> 16 - if we fix that fix this */
@@ -450,6 +452,17 @@ int udl_modeset_init(struct drm_device *dev)
return 0;
 }

+void udl_modeset_restore(struct drm_device *dev)
+{
+   struct udl_device *udl = dev->dev_private;
+   struct udl_framebuffer *ufb;
+   if (!udl->crtc || !udl->crtc->primary->fb)
+   return;
+   udl_crtc_commit(udl->crtc);
+   ufb = to_udl_fb(udl->crtc->primary->fb);
+   udl_handle_damage(ufb, 0, 0, ufb->base.width, ufb->base.height);
+}
+
 void udl_modeset_cleanup(struct drm_device *dev)
 {
drm_mode_config_cleanup(dev);
-- 
2.8.0.rc3.226.g39d4020



[PATCH 1/2] drm/udl: fix a NULL pointer reference in udl_gem_free_object().

2016-08-30 Thread Haixia Shi
Previously this function had a NULL pointer check for gem->map_list.map, but
that line was refactored after commit 0de23977cfeb5b357ec884ba15417ae118ff9e9bb
("drm/gem: convert to new unified vma manager").

After the refactor it is still necessasry to check that the vma manager is not
NULL because udl_gem_free_object() may come in after the vma manager is 
destroyed.

Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/udl/udl_gem.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
index 818e707..72e1bd4 100644
--- a/drivers/gpu/drm/udl/udl_gem.c
+++ b/drivers/gpu/drm/udl/udl_gem.c
@@ -204,7 +204,8 @@ void udl_gem_free_object(struct drm_gem_object *gem_obj)
if (obj->pages)
udl_gem_put_pages(obj);

-   drm_gem_free_mmap_offset(gem_obj);
+   if (gem_obj->dev->vma_offset_manager)
+   drm_gem_free_mmap_offset(gem_obj);
 }

 /* the dumb interface doesn't work with the GEM straight MMAP
-- 
2.8.0.rc3.226.g39d4020



[PATCH v3 2/2] drm: remove drm_device_is_unplugged and related code

2016-02-16 Thread Haixia Shi
David - is the v3 patch acceptable to you? Thanks.

On Fri, Feb 12, 2016 at 11:50 AM, Haixia Shi  wrote:

> v1: Remove the general drm_device_is_unplugged() checks, and move the
> unplugged
> flag handling logic into drm/udl. In general we want to keep
> driver-specific
> logic out of common drm code.
>
> v2: Based on discussion with Stephane and David, drop most of the unplugged
> flag handling logic in drm/udl except for udl_detect() and udl_fb_open().
> The intention is to treat the device removal as a connector-unplug, and kep
> the UDL device fully functional.
>
> v3: Based on feedback from David, entirely drop the "unplugged" flag and
> all
> related code. There is no need to check for the unplugged flag as the
> existing
> udl_usb_disconnect() behavior already ensures the controller is removed,
> and
> all code paths that uses the usb-device are not reachable after unplug.
>
> When a UDL monitor is unplugged, we need to still treat it as a fully
> functional device which just happens to have its connector unplugged.
> This allows user-space to properly deallocate fbs and dumb buffers
> before closing the device.
>
> This drops the "unplugged" flag hack, which puts the device in a
> non-functional state after USB unplug and rejects most operations on
> the device such as ioctls with error -ENODEV.
>
> Signed-off-by: Haixia Shi 
> Reviewed-by: Stéphane Marchesin 
> Cc: David Herrmann 
> Cc: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_drv.c   |  6 --
>  drivers/gpu/drm/drm_fops.c  |  2 --
>  drivers/gpu/drm/drm_gem.c   |  3 ---
>  drivers/gpu/drm/drm_ioctl.c |  3 ---
>  drivers/gpu/drm/drm_vm.c|  3 ---
>  drivers/gpu/drm/udl/udl_connector.c |  2 --
>  drivers/gpu/drm/udl/udl_fb.c|  6 --
>  include/drm/drmP.h  | 14 --
>  8 files changed, 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 167c8d3..f93ee12 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -376,9 +376,6 @@ struct drm_minor *drm_minor_acquire(unsigned int
> minor_id)
>
> if (!minor) {
> return ERR_PTR(-ENODEV);
> -   } else if (drm_device_is_unplugged(minor->dev)) {
> -   drm_dev_unref(minor->dev);
> -   return ERR_PTR(-ENODEV);
> }
>
> return minor;
> @@ -464,9 +461,6 @@ void drm_unplug_dev(struct drm_device *dev)
> drm_minor_unregister(dev, DRM_MINOR_CONTROL);
>
> mutex_lock(_global_mutex);
> -
> -   drm_device_set_unplugged(dev);
> -
> if (dev->open_count == 0) {
> drm_put_dev(dev);
> }
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index 1ea8790..b4332d4 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -497,8 +497,6 @@ int drm_release(struct inode *inode, struct file *filp)
>
> if (!--dev->open_count) {
> retcode = drm_lastclose(dev);
> -   if (drm_device_is_unplugged(dev))
> -   drm_put_dev(dev);
> }
> mutex_unlock(_global_mutex);
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 2e8c77e..c622e32 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -900,9 +900,6 @@ int drm_gem_mmap(struct file *filp, struct
> vm_area_struct *vma)
> struct drm_vma_offset_node *node;
> int ret;
>
> -   if (drm_device_is_unplugged(dev))
> -   return -ENODEV;
> -
> drm_vma_offset_lock_lookup(dev->vma_offset_manager);
> node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
>   vma->vm_pgoff,
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 8ce2a0c..f959074 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -695,9 +695,6 @@ long drm_ioctl(struct file *filp,
>
> dev = file_priv->minor->dev;
>
> -   if (drm_device_is_unplugged(dev))
> -   return -ENODEV;
> -
> is_driver_ioctl = nr >= DRM_COMMAND_BASE && nr < DRM_COMMAND_END;
>
> if (is_driver_ioctl) {
> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
> index f90bd5f..3a68be4 100644
> --- a/drivers/gpu/drm/drm_vm.c
> +++ b/drivers/gpu/drm/drm_vm.c
> @@ -657,9 +657,6 @@ int drm_legacy_mmap(struct file *filp, struct
> vm_area_struct *vma)
> struct drm_device *dev = priv->minor->dev;
> in

[PATCH v3 2/2] drm: remove drm_device_is_unplugged and related code

2016-02-12 Thread Haixia Shi
v1: Remove the general drm_device_is_unplugged() checks, and move the unplugged
flag handling logic into drm/udl. In general we want to keep driver-specific
logic out of common drm code.

v2: Based on discussion with Stephane and David, drop most of the unplugged
flag handling logic in drm/udl except for udl_detect() and udl_fb_open().
The intention is to treat the device removal as a connector-unplug, and kep
the UDL device fully functional.

v3: Based on feedback from David, entirely drop the "unplugged" flag and all
related code. There is no need to check for the unplugged flag as the existing
udl_usb_disconnect() behavior already ensures the controller is removed, and
all code paths that uses the usb-device are not reachable after unplug.

When a UDL monitor is unplugged, we need to still treat it as a fully
functional device which just happens to have its connector unplugged.
This allows user-space to properly deallocate fbs and dumb buffers
before closing the device.

This drops the "unplugged" flag hack, which puts the device in a
non-functional state after USB unplug and rejects most operations on
the device such as ioctls with error -ENODEV.

Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
Cc: David Herrmann 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/drm_drv.c   |  6 --
 drivers/gpu/drm/drm_fops.c  |  2 --
 drivers/gpu/drm/drm_gem.c   |  3 ---
 drivers/gpu/drm/drm_ioctl.c |  3 ---
 drivers/gpu/drm/drm_vm.c|  3 ---
 drivers/gpu/drm/udl/udl_connector.c |  2 --
 drivers/gpu/drm/udl/udl_fb.c|  6 --
 include/drm/drmP.h  | 14 --
 8 files changed, 39 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 167c8d3..f93ee12 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -376,9 +376,6 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)

if (!minor) {
return ERR_PTR(-ENODEV);
-   } else if (drm_device_is_unplugged(minor->dev)) {
-   drm_dev_unref(minor->dev);
-   return ERR_PTR(-ENODEV);
}

return minor;
@@ -464,9 +461,6 @@ void drm_unplug_dev(struct drm_device *dev)
drm_minor_unregister(dev, DRM_MINOR_CONTROL);

mutex_lock(_global_mutex);
-
-   drm_device_set_unplugged(dev);
-
if (dev->open_count == 0) {
drm_put_dev(dev);
}
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 1ea8790..b4332d4 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -497,8 +497,6 @@ int drm_release(struct inode *inode, struct file *filp)

if (!--dev->open_count) {
retcode = drm_lastclose(dev);
-   if (drm_device_is_unplugged(dev))
-   drm_put_dev(dev);
}
mutex_unlock(_global_mutex);

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 2e8c77e..c622e32 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -900,9 +900,6 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct 
*vma)
struct drm_vma_offset_node *node;
int ret;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
drm_vma_offset_lock_lookup(dev->vma_offset_manager);
node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
  vma->vm_pgoff,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 8ce2a0c..f959074 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -695,9 +695,6 @@ long drm_ioctl(struct file *filp,

dev = file_priv->minor->dev;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
is_driver_ioctl = nr >= DRM_COMMAND_BASE && nr < DRM_COMMAND_END;

if (is_driver_ioctl) {
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index f90bd5f..3a68be4 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -657,9 +657,6 @@ int drm_legacy_mmap(struct file *filp, struct 
vm_area_struct *vma)
struct drm_device *dev = priv->minor->dev;
int ret;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
mutex_lock(>struct_mutex);
ret = drm_mmap_locked(filp, vma);
mutex_unlock(>struct_mutex);
diff --git a/drivers/gpu/drm/udl/udl_connector.c 
b/drivers/gpu/drm/udl/udl_connector.c
index 4709b54..a6d5e21 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -96,8 +96,6 @@ static int udl_mode_valid(struct drm_connector *connector,
 static enum drm_connector_status
 udl_detect(struct drm_connector *connector, bool force)
 {
-   if (drm_device_is_unplugged(connector->dev))
-   return connector_status_disconnected;

[PATCH v3 2/2] drm: remove drm_device_is_unplugged and related code

2016-02-11 Thread Haixia Shi
When a UDL monitor is unplugged, we need to still treat it as a fully
functional device which just happens to have its connector unplugged.
This allows user-space to properly deallocate fbs and dumb buffers
before closing the device.

This drops the "unplugged" flag hack, which puts the device in a
non-functional state after USB unplug and rejects most operations on
the device such as ioctls with error -ENODEV.

Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
Cc: David Herrmann 
---
 drivers/gpu/drm/drm_drv.c   |  6 --
 drivers/gpu/drm/drm_fops.c  |  2 --
 drivers/gpu/drm/drm_gem.c   |  3 ---
 drivers/gpu/drm/drm_ioctl.c |  3 ---
 drivers/gpu/drm/drm_vm.c|  3 ---
 drivers/gpu/drm/udl/udl_connector.c |  2 --
 drivers/gpu/drm/udl/udl_fb.c|  6 --
 include/drm/drmP.h  | 14 --
 8 files changed, 39 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 167c8d3..f93ee12 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -376,9 +376,6 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)

if (!minor) {
return ERR_PTR(-ENODEV);
-   } else if (drm_device_is_unplugged(minor->dev)) {
-   drm_dev_unref(minor->dev);
-   return ERR_PTR(-ENODEV);
}

return minor;
@@ -464,9 +461,6 @@ void drm_unplug_dev(struct drm_device *dev)
drm_minor_unregister(dev, DRM_MINOR_CONTROL);

mutex_lock(_global_mutex);
-
-   drm_device_set_unplugged(dev);
-
if (dev->open_count == 0) {
drm_put_dev(dev);
}
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 1ea8790..b4332d4 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -497,8 +497,6 @@ int drm_release(struct inode *inode, struct file *filp)

if (!--dev->open_count) {
retcode = drm_lastclose(dev);
-   if (drm_device_is_unplugged(dev))
-   drm_put_dev(dev);
}
mutex_unlock(_global_mutex);

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 2e8c77e..c622e32 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -900,9 +900,6 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct 
*vma)
struct drm_vma_offset_node *node;
int ret;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
drm_vma_offset_lock_lookup(dev->vma_offset_manager);
node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
  vma->vm_pgoff,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 8ce2a0c..f959074 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -695,9 +695,6 @@ long drm_ioctl(struct file *filp,

dev = file_priv->minor->dev;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
is_driver_ioctl = nr >= DRM_COMMAND_BASE && nr < DRM_COMMAND_END;

if (is_driver_ioctl) {
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index f90bd5f..3a68be4 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -657,9 +657,6 @@ int drm_legacy_mmap(struct file *filp, struct 
vm_area_struct *vma)
struct drm_device *dev = priv->minor->dev;
int ret;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
mutex_lock(>struct_mutex);
ret = drm_mmap_locked(filp, vma);
mutex_unlock(>struct_mutex);
diff --git a/drivers/gpu/drm/udl/udl_connector.c 
b/drivers/gpu/drm/udl/udl_connector.c
index 4709b54..a6d5e21 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -96,8 +96,6 @@ static int udl_mode_valid(struct drm_connector *connector,
 static enum drm_connector_status
 udl_detect(struct drm_connector *connector, bool force)
 {
-   if (drm_device_is_unplugged(connector->dev))
-   return connector_status_disconnected;
return connector_status_connected;
 }

diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
index 200419d..29aca6c 100644
--- a/drivers/gpu/drm/udl/udl_fb.c
+++ b/drivers/gpu/drm/udl/udl_fb.c
@@ -321,12 +321,6 @@ static void udl_fb_imageblit(struct fb_info *info, const 
struct fb_image *image)
 static int udl_fb_open(struct fb_info *info, int user)
 {
struct udl_fbdev *ufbdev = info->par;
-   struct drm_device *dev = ufbdev->ufb.base.dev;
-   struct udl_device *udl = dev->dev_private;
-
-   /* If the USB device is gone, we don't accept new opens */
-   if (drm_device_is_unplugged(udl->ddev))
-   return -ENODEV;

ufbdev->fb_count++;

diff --git a/include/drm/drmP.h b/include/drm/drmP.

[PATCH v2 2/2] drm: remove drm_device_is_unplugged checks in common drm code.

2016-02-11 Thread Haixia Shi
Well, in that case it would be even simpler. We don't even need the
"unplugged" flag check in udl_detect because all connectors are already
unplugged in udl_usb_disconnect (in drm_connector_unplug_all()).

It is not necessary to check the flag in udl_fb_open() either, if the
intention is to keep the device alive.

As for code paths that uses udl->udev, I only see the following 3 places:

(1) udl_parse_vendor_description and udl_alloc_urb_list: both are only
called at udl_driver_load(), so not a problem
(2) udl_usb_probe: won't happen after USB device is unplugged
(3) udl_get_edid: only called from udl_get_modes, won't be an issue because
connectors are already unplugged

So, it seems that I *really* just need to drop the "unplugged" flag and
update the commit message.

On Thu, Feb 11, 2016 at 2:30 AM, David Herrmann 
wrote:

> Hi
>
> On Thu, Feb 11, 2016 at 12:18 AM, Haixia Shi  wrote:
> > When USB cable is disconnected, we mark udl device as unplugged so that
> > udl_detect reports connector status as disconnected, but still keep
> > the drm device alive until user-space closes it.
> >
> > Signed-off-by: Haixia Shi 
> > Reviewed-by: Stéphane Marchesin 
>
> I assume this is based on the discussion I had with Stephane on IRC.
> I'm fine with going this way and keeping the device fully alive, and
> just treat the device removal as a connector-unplug. However, you
> really must document all this in your commit message.
>
> Anyway, if you want to keep the device alive, then please change the
> code to entirely drop the "unplugged" flag and all related code. Then
> make sure you somehow reset "udl->udev" to NULL and make sure no
> code-path uses the usb-device after it was unplugged. I guess there
> should be appropriate locks already.
>
> This way, you end up with a fully functional UDL device, which just
> happens to have to connector plugged. But you *really* have to be
> careful that no-one touches the usb device, as the interface might be
> re-used by some other device any time (or in case the usb-core
> provides protection against this, please document it).
>
> Thanks
> David
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160211/e9e0d932/attachment-0001.html>


[PATCH v2 2/2] drm: remove drm_device_is_unplugged checks in common drm code.

2016-02-10 Thread Haixia Shi
When USB cable is disconnected, we mark udl device as unplugged so that
udl_detect reports connector status as disconnected, but still keep
the drm device alive until user-space closes it.

Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/drm_drv.c   |  6 --
 drivers/gpu/drm/drm_fops.c  |  2 --
 drivers/gpu/drm/drm_gem.c   |  3 ---
 drivers/gpu/drm/drm_ioctl.c |  3 ---
 drivers/gpu/drm/drm_vm.c|  3 ---
 drivers/gpu/drm/udl/udl_connector.c |  2 +-
 drivers/gpu/drm/udl/udl_drv.c   |  1 +
 drivers/gpu/drm/udl/udl_drv.h   |  3 +++
 drivers/gpu/drm/udl/udl_fb.c|  2 +-
 drivers/gpu/drm/udl/udl_main.c  | 15 +++
 include/drm/drmP.h  | 14 --
 11 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 167c8d3..f93ee12 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -376,9 +376,6 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)

if (!minor) {
return ERR_PTR(-ENODEV);
-   } else if (drm_device_is_unplugged(minor->dev)) {
-   drm_dev_unref(minor->dev);
-   return ERR_PTR(-ENODEV);
}

return minor;
@@ -464,9 +461,6 @@ void drm_unplug_dev(struct drm_device *dev)
drm_minor_unregister(dev, DRM_MINOR_CONTROL);

mutex_lock(_global_mutex);
-
-   drm_device_set_unplugged(dev);
-
if (dev->open_count == 0) {
drm_put_dev(dev);
}
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 1ea8790..b4332d4 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -497,8 +497,6 @@ int drm_release(struct inode *inode, struct file *filp)

if (!--dev->open_count) {
retcode = drm_lastclose(dev);
-   if (drm_device_is_unplugged(dev))
-   drm_put_dev(dev);
}
mutex_unlock(_global_mutex);

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 2e8c77e..c622e32 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -900,9 +900,6 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct 
*vma)
struct drm_vma_offset_node *node;
int ret;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
drm_vma_offset_lock_lookup(dev->vma_offset_manager);
node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
  vma->vm_pgoff,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 8ce2a0c..f959074 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -695,9 +695,6 @@ long drm_ioctl(struct file *filp,

dev = file_priv->minor->dev;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
is_driver_ioctl = nr >= DRM_COMMAND_BASE && nr < DRM_COMMAND_END;

if (is_driver_ioctl) {
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index f90bd5f..3a68be4 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -657,9 +657,6 @@ int drm_legacy_mmap(struct file *filp, struct 
vm_area_struct *vma)
struct drm_device *dev = priv->minor->dev;
int ret;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
mutex_lock(>struct_mutex);
ret = drm_mmap_locked(filp, vma);
mutex_unlock(>struct_mutex);
diff --git a/drivers/gpu/drm/udl/udl_connector.c 
b/drivers/gpu/drm/udl/udl_connector.c
index 4709b54..b168f2c 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -96,7 +96,7 @@ static int udl_mode_valid(struct drm_connector *connector,
 static enum drm_connector_status
 udl_detect(struct drm_connector *connector, bool force)
 {
-   if (drm_device_is_unplugged(connector->dev))
+   if (udl_device_is_unplugged(connector->dev))
return connector_status_disconnected;
return connector_status_connected;
 }
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index d5728ec..3cbafe7 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -97,6 +97,7 @@ static void udl_usb_disconnect(struct usb_interface 
*interface)
drm_connector_unplug_all(dev);
udl_fbdev_unplug(dev);
udl_drop_usb(dev);
+   udl_device_set_unplugged(dev);
drm_unplug_dev(dev);
 }

diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 4a064ef..61e117a 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -65,6 +65,7 @@ struct udl_device {
atomic_t bytes_identical; /* saved effort with backbuffer comparison */
atomic_t bytes_sent; /* to us

[PATCH v2 2/2] drm: remove drm_device_is_unplugged checks in common drm code.

2016-02-10 Thread Haixia Shi
When USB cable is disconnected, we mark udl device as unplugged so that
udl_detect reports connector status as disconnected, but still keep
the drm device alive until user-space closes it.

Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/drm_drv.c   |  6 --
 drivers/gpu/drm/drm_fops.c  |  2 --
 drivers/gpu/drm/drm_gem.c   |  3 ---
 drivers/gpu/drm/drm_ioctl.c |  3 ---
 drivers/gpu/drm/drm_vm.c|  3 ---
 drivers/gpu/drm/udl/udl_connector.c |  2 +-
 drivers/gpu/drm/udl/udl_drv.c   |  1 +
 drivers/gpu/drm/udl/udl_drv.h   |  3 +++
 drivers/gpu/drm/udl/udl_fb.c|  2 +-
 drivers/gpu/drm/udl/udl_main.c  | 15 +++
 include/drm/drmP.h  | 14 --
 11 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 167c8d3..f93ee12 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -376,9 +376,6 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)

if (!minor) {
return ERR_PTR(-ENODEV);
-   } else if (drm_device_is_unplugged(minor->dev)) {
-   drm_dev_unref(minor->dev);
-   return ERR_PTR(-ENODEV);
}

return minor;
@@ -464,9 +461,6 @@ void drm_unplug_dev(struct drm_device *dev)
drm_minor_unregister(dev, DRM_MINOR_CONTROL);

mutex_lock(_global_mutex);
-
-   drm_device_set_unplugged(dev);
-
if (dev->open_count == 0) {
drm_put_dev(dev);
}
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 1ea8790..b4332d4 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -497,8 +497,6 @@ int drm_release(struct inode *inode, struct file *filp)

if (!--dev->open_count) {
retcode = drm_lastclose(dev);
-   if (drm_device_is_unplugged(dev))
-   drm_put_dev(dev);
}
mutex_unlock(_global_mutex);

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 2e8c77e..c622e32 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -900,9 +900,6 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct 
*vma)
struct drm_vma_offset_node *node;
int ret;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
drm_vma_offset_lock_lookup(dev->vma_offset_manager);
node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
  vma->vm_pgoff,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 8ce2a0c..f959074 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -695,9 +695,6 @@ long drm_ioctl(struct file *filp,

dev = file_priv->minor->dev;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
is_driver_ioctl = nr >= DRM_COMMAND_BASE && nr < DRM_COMMAND_END;

if (is_driver_ioctl) {
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index f90bd5f..3a68be4 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -657,9 +657,6 @@ int drm_legacy_mmap(struct file *filp, struct 
vm_area_struct *vma)
struct drm_device *dev = priv->minor->dev;
int ret;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
mutex_lock(>struct_mutex);
ret = drm_mmap_locked(filp, vma);
mutex_unlock(>struct_mutex);
diff --git a/drivers/gpu/drm/udl/udl_connector.c 
b/drivers/gpu/drm/udl/udl_connector.c
index 4709b54..b168f2c 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -96,7 +96,7 @@ static int udl_mode_valid(struct drm_connector *connector,
 static enum drm_connector_status
 udl_detect(struct drm_connector *connector, bool force)
 {
-   if (drm_device_is_unplugged(connector->dev))
+   if (udl_device_is_unplugged(connector->dev))
return connector_status_disconnected;
return connector_status_connected;
 }
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index d5728ec..3cbafe7 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -97,6 +97,7 @@ static void udl_usb_disconnect(struct usb_interface 
*interface)
drm_connector_unplug_all(dev);
udl_fbdev_unplug(dev);
udl_drop_usb(dev);
+   udl_device_set_unplugged(dev);
drm_unplug_dev(dev);
 }

diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 4a064ef..61e117a 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -65,6 +65,7 @@ struct udl_device {
atomic_t bytes_identical; /* saved effort with backbuffer comparison */
atomic_t bytes_sent; /* to us

[PATCH 2/2] drm: make unplugged flag specific to udl driver

2016-02-10 Thread Haixia Shi
David

I am having trouble getting the reference to "drm_global_mutex" to link
correctly in drm/udl. The error is

ERROR: "drm_global_mutex" [drivers/gpu/drm/udl/udl.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2

This seems to be only accessed in the common drm code. Do you have a
suggestion how to get around it?

On Wed, Feb 10, 2016 at 1:35 PM, David Herrmann 
wrote:

> Hi
>
> On Wed, Feb 10, 2016 at 9:51 PM, Haixia Shi  wrote:
> >> This should rather be:
> >>
> >> drm_release(inode, filp);
> >> mutex_lock(_global_mutex);
> >> if (!dev->open_count && udl_device_is_unplugged(dev))
> >> drm_put_dev(dev);
> >> mutex_unlock(_global_mutex);
> >>
> >> return 0;
> >>
> >> There is no reason to look at the return code of drm_release(), ever.
> >
> > But drm_release() does return a retcode. It would still make sense to
> return
> > that as-is in case any existing code relies on it.
>
> Nobody should ever return error codes from fops.release(). It is
> completely bogus. You rather confuse generic user-space that calls
> close(), than getting any benefit out of it.
>
> But TBH, I don't care. Feel free to forward the return value. But
> still, please change the order of the calls as I did.
>
> Thanks
> David
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160210/ccab7639/attachment.html>


[PATCH 2/2] drm: make unplugged flag specific to udl driver

2016-02-10 Thread Haixia Shi
Also note that it is not currently feasible for UDL code to access
drm_global_mutex.

Please also refer to the similar comments ("FIXME: open_count is protected
by drm_global_mutex but that would lead to locking inversion with the
driver load path.") in nouveau, i915 and amdgpu.

On Wed, Feb 10, 2016 at 12:51 PM, Haixia Shi  wrote:

> > This should rather be:
> >
> > drm_release(inode, filp);
> > mutex_lock(_global_mutex);
> > if (!dev->open_count && udl_device_is_unplugged(dev))
> > drm_put_dev(dev);
> > mutex_unlock(_global_mutex);
> >
> > return 0;
> >
> > There is no reason to look at the return code of drm_release(), ever.
>
> But drm_release() does return a retcode. It would still make sense to
> return that as-is in case any existing code relies on it.
>
> On Wed, Feb 10, 2016 at 4:24 AM, David Herrmann 
> wrote:
>
>> Hi
>>
>> On Tue, Feb 9, 2016 at 10:05 PM, Haixia Shi  wrote:
>> > Remove the general drm_device_is_unplugged() checks, and move the
>> unplugged
>> > flag handling logic into drm/udl. In general we want to keep
>> driver-specific
>> > logic out of common drm code.
>> >
>> > Signed-off-by: Haixia Shi 
>> > Reviewed-by: Stéphane Marchesin 
>> > ---
>> >  drivers/gpu/drm/drm_drv.c   |  6 
>> >  drivers/gpu/drm/drm_fops.c  |  2 --
>> >  drivers/gpu/drm/drm_gem.c   |  3 --
>> >  drivers/gpu/drm/drm_ioctl.c |  3 --
>> >  drivers/gpu/drm/drm_vm.c|  3 --
>> >  drivers/gpu/drm/udl/udl_connector.c |  2 +-
>> >  drivers/gpu/drm/udl/udl_drv.c   |  7 +++--
>> >  drivers/gpu/drm/udl/udl_drv.h   |  6 
>> >  drivers/gpu/drm/udl/udl_fb.c|  2 +-
>> >  drivers/gpu/drm/udl/udl_gem.c   |  5 +++
>> >  drivers/gpu/drm/udl/udl_main.c  | 61
>> +
>> >  include/drm/drmP.h  | 14 -
>> >  12 files changed, 78 insertions(+), 36 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> > index 167c8d3..f93ee12 100644
>> > --- a/drivers/gpu/drm/drm_drv.c
>> > +++ b/drivers/gpu/drm/drm_drv.c
>> > @@ -376,9 +376,6 @@ struct drm_minor *drm_minor_acquire(unsigned int
>> minor_id)
>> >
>> > if (!minor) {
>> > return ERR_PTR(-ENODEV);
>> > -   } else if (drm_device_is_unplugged(minor->dev)) {
>> > -   drm_dev_unref(minor->dev);
>> > -   return ERR_PTR(-ENODEV);
>> > }
>> >
>> > return minor;
>> > @@ -464,9 +461,6 @@ void drm_unplug_dev(struct drm_device *dev)
>> > drm_minor_unregister(dev, DRM_MINOR_CONTROL);
>> >
>> > mutex_lock(_global_mutex);
>> > -
>> > -   drm_device_set_unplugged(dev);
>> > -
>> > if (dev->open_count == 0) {
>> > drm_put_dev(dev);
>> > }
>> > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
>> > index 1ea8790..b4332d4 100644
>> > --- a/drivers/gpu/drm/drm_fops.c
>> > +++ b/drivers/gpu/drm/drm_fops.c
>> > @@ -497,8 +497,6 @@ int drm_release(struct inode *inode, struct file
>> *filp)
>> >
>> > if (!--dev->open_count) {
>> > retcode = drm_lastclose(dev);
>> > -   if (drm_device_is_unplugged(dev))
>> > -   drm_put_dev(dev);
>> > }
>> > mutex_unlock(_global_mutex);
>> >
>> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>> > index 2e8c77e..c622e32 100644
>> > --- a/drivers/gpu/drm/drm_gem.c
>> > +++ b/drivers/gpu/drm/drm_gem.c
>> > @@ -900,9 +900,6 @@ int drm_gem_mmap(struct file *filp, struct
>> vm_area_struct *vma)
>> > struct drm_vma_offset_node *node;
>> > int ret;
>> >
>> > -   if (drm_device_is_unplugged(dev))
>> > -   return -ENODEV;
>> > -
>> > drm_vma_offset_lock_lookup(dev->vma_offset_manager);
>> > node =
>> drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
>> >   vma->vm_pgoff,
>> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>> > index 8ce2a0c..

[PATCH 2/2] drm: make unplugged flag specific to udl driver

2016-02-10 Thread Haixia Shi
> This should rather be:
>
> drm_release(inode, filp);
> mutex_lock(_global_mutex);
> if (!dev->open_count && udl_device_is_unplugged(dev))
> drm_put_dev(dev);
> mutex_unlock(_global_mutex);
>
> return 0;
>
> There is no reason to look at the return code of drm_release(), ever.

But drm_release() does return a retcode. It would still make sense to
return that as-is in case any existing code relies on it.

On Wed, Feb 10, 2016 at 4:24 AM, David Herrmann 
wrote:

> Hi
>
> On Tue, Feb 9, 2016 at 10:05 PM, Haixia Shi  wrote:
> > Remove the general drm_device_is_unplugged() checks, and move the
> unplugged
> > flag handling logic into drm/udl. In general we want to keep
> driver-specific
> > logic out of common drm code.
> >
> > Signed-off-by: Haixia Shi 
> > Reviewed-by: Stéphane Marchesin 
> > ---
> >  drivers/gpu/drm/drm_drv.c   |  6 
> >  drivers/gpu/drm/drm_fops.c  |  2 --
> >  drivers/gpu/drm/drm_gem.c   |  3 --
> >  drivers/gpu/drm/drm_ioctl.c |  3 --
> >  drivers/gpu/drm/drm_vm.c|  3 --
> >  drivers/gpu/drm/udl/udl_connector.c |  2 +-
> >  drivers/gpu/drm/udl/udl_drv.c   |  7 +++--
> >  drivers/gpu/drm/udl/udl_drv.h   |  6 
> >  drivers/gpu/drm/udl/udl_fb.c|  2 +-
> >  drivers/gpu/drm/udl/udl_gem.c   |  5 +++
> >  drivers/gpu/drm/udl/udl_main.c  | 61
> +
> >  include/drm/drmP.h  | 14 -
> >  12 files changed, 78 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 167c8d3..f93ee12 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -376,9 +376,6 @@ struct drm_minor *drm_minor_acquire(unsigned int
> minor_id)
> >
> > if (!minor) {
> > return ERR_PTR(-ENODEV);
> > -   } else if (drm_device_is_unplugged(minor->dev)) {
> > -   drm_dev_unref(minor->dev);
> > -   return ERR_PTR(-ENODEV);
> > }
> >
> > return minor;
> > @@ -464,9 +461,6 @@ void drm_unplug_dev(struct drm_device *dev)
> > drm_minor_unregister(dev, DRM_MINOR_CONTROL);
> >
> > mutex_lock(_global_mutex);
> > -
> > -   drm_device_set_unplugged(dev);
> > -
> > if (dev->open_count == 0) {
> > drm_put_dev(dev);
> > }
> > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> > index 1ea8790..b4332d4 100644
> > --- a/drivers/gpu/drm/drm_fops.c
> > +++ b/drivers/gpu/drm/drm_fops.c
> > @@ -497,8 +497,6 @@ int drm_release(struct inode *inode, struct file
> *filp)
> >
> > if (!--dev->open_count) {
> > retcode = drm_lastclose(dev);
> > -   if (drm_device_is_unplugged(dev))
> > -   drm_put_dev(dev);
> > }
> > mutex_unlock(_global_mutex);
> >
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > index 2e8c77e..c622e32 100644
> > --- a/drivers/gpu/drm/drm_gem.c
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -900,9 +900,6 @@ int drm_gem_mmap(struct file *filp, struct
> vm_area_struct *vma)
> > struct drm_vma_offset_node *node;
> > int ret;
> >
> > -   if (drm_device_is_unplugged(dev))
> > -   return -ENODEV;
> > -
> > drm_vma_offset_lock_lookup(dev->vma_offset_manager);
> > node =
> drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
> >   vma->vm_pgoff,
> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > index 8ce2a0c..f959074 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -695,9 +695,6 @@ long drm_ioctl(struct file *filp,
> >
> > dev = file_priv->minor->dev;
> >
> > -   if (drm_device_is_unplugged(dev))
> > -   return -ENODEV;
> > -
> > is_driver_ioctl = nr >= DRM_COMMAND_BASE && nr < DRM_COMMAND_END;
> >
> > if (is_driver_ioctl) {
> > diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
> > index f90bd5f..3a68be4 100644
> > --- a/drivers/gpu/drm/drm_vm.c
> > +++ b/drivers/gpu/drm/drm_vm.c
> > @@ -657,9 +657,6 @@ int drm_legacy_mmap(struct file *filp, struct
> vm_area_struc

[PATCH 2/2] drm: make unplugged flag specific to udl driver

2016-02-10 Thread Haixia Shi
> +   if (udl_device_is_unplugged(dev) &&
> +   nr != DRM_IOCTL_NR(DRM_IOCTL_MODE_SETCRTC) &&
> +   nr != DRM_IOCTL_NR(DRM_IOCTL_MODE_RMFB) &&
> +   nr != DRM_IOCTL_NR(DRM_IOCTL_MODE_DESTROY_DUMB))
> +   return -ENODEV;
>
>Why?
>
>Just do:
>
>if (udl_device_is_unplugged(dev))
>return -ENODEV;
>
>Why this complex logic here?

Because there are legitimate ioctl calls after UDL is unplugged. See
crbug.com/583508 and crbug.com/583758 for some background.

The userspace (Chrome in this case) has allocated FBs and Dumb buffers on
the drm device and needs to be given a chance to properly deallocate them
(via RMFB and DESTROY_DUMB). In addition, it needs to call SETCRTC with
fb_id = 0 to properly release the last refcount on the primary fb.

I initially proposed adding an "UNPLUG_DISALLOW" flag to ioctls so that we
can whitelist them on a case-by-case basis but that proposal got shot down
as being unnecessary, but you can see my original patch at
https://chromium-review.googlesource.com/#/c/326160/


On Wed, Feb 10, 2016 at 4:24 AM, David Herrmann 
wrote:

> Hi
>
> On Tue, Feb 9, 2016 at 10:05 PM, Haixia Shi  wrote:
> > Remove the general drm_device_is_unplugged() checks, and move the
> unplugged
> > flag handling logic into drm/udl. In general we want to keep
> driver-specific
> > logic out of common drm code.
> >
> > Signed-off-by: Haixia Shi 
> > Reviewed-by: Stéphane Marchesin 
> > ---
> >  drivers/gpu/drm/drm_drv.c   |  6 
> >  drivers/gpu/drm/drm_fops.c  |  2 --
> >  drivers/gpu/drm/drm_gem.c   |  3 --
> >  drivers/gpu/drm/drm_ioctl.c |  3 --
> >  drivers/gpu/drm/drm_vm.c|  3 --
> >  drivers/gpu/drm/udl/udl_connector.c |  2 +-
> >  drivers/gpu/drm/udl/udl_drv.c   |  7 +++--
> >  drivers/gpu/drm/udl/udl_drv.h   |  6 
> >  drivers/gpu/drm/udl/udl_fb.c|  2 +-
> >  drivers/gpu/drm/udl/udl_gem.c   |  5 +++
> >  drivers/gpu/drm/udl/udl_main.c  | 61
> +
> >  include/drm/drmP.h  | 14 -
> >  12 files changed, 78 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 167c8d3..f93ee12 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -376,9 +376,6 @@ struct drm_minor *drm_minor_acquire(unsigned int
> minor_id)
> >
> > if (!minor) {
> > return ERR_PTR(-ENODEV);
> > -   } else if (drm_device_is_unplugged(minor->dev)) {
> > -   drm_dev_unref(minor->dev);
> > -   return ERR_PTR(-ENODEV);
> > }
> >
> > return minor;
> > @@ -464,9 +461,6 @@ void drm_unplug_dev(struct drm_device *dev)
> > drm_minor_unregister(dev, DRM_MINOR_CONTROL);
> >
> > mutex_lock(_global_mutex);
> > -
> > -   drm_device_set_unplugged(dev);
> > -
> > if (dev->open_count == 0) {
> > drm_put_dev(dev);
> > }
> > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> > index 1ea8790..b4332d4 100644
> > --- a/drivers/gpu/drm/drm_fops.c
> > +++ b/drivers/gpu/drm/drm_fops.c
> > @@ -497,8 +497,6 @@ int drm_release(struct inode *inode, struct file
> *filp)
> >
> > if (!--dev->open_count) {
> > retcode = drm_lastclose(dev);
> > -   if (drm_device_is_unplugged(dev))
> > -   drm_put_dev(dev);
> > }
> > mutex_unlock(_global_mutex);
> >
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > index 2e8c77e..c622e32 100644
> > --- a/drivers/gpu/drm/drm_gem.c
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -900,9 +900,6 @@ int drm_gem_mmap(struct file *filp, struct
> vm_area_struct *vma)
> > struct drm_vma_offset_node *node;
> > int ret;
> >
> > -   if (drm_device_is_unplugged(dev))
> > -   return -ENODEV;
> > -
> > drm_vma_offset_lock_lookup(dev->vma_offset_manager);
> > node =
> drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
> >   vma->vm_pgoff,
> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > index 8ce2a0c..f959074 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -695,9 +695,6 @@ long d

[PATCH 2/2] drm: make unplugged flag specific to udl driver

2016-02-09 Thread Haixia Shi
Remove the general drm_device_is_unplugged() checks, and move the unplugged
flag handling logic into drm/udl. In general we want to keep driver-specific
logic out of common drm code.

Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/drm_drv.c   |  6 
 drivers/gpu/drm/drm_fops.c  |  2 --
 drivers/gpu/drm/drm_gem.c   |  3 --
 drivers/gpu/drm/drm_ioctl.c |  3 --
 drivers/gpu/drm/drm_vm.c|  3 --
 drivers/gpu/drm/udl/udl_connector.c |  2 +-
 drivers/gpu/drm/udl/udl_drv.c   |  7 +++--
 drivers/gpu/drm/udl/udl_drv.h   |  6 
 drivers/gpu/drm/udl/udl_fb.c|  2 +-
 drivers/gpu/drm/udl/udl_gem.c   |  5 +++
 drivers/gpu/drm/udl/udl_main.c  | 61 +
 include/drm/drmP.h  | 14 -
 12 files changed, 78 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 167c8d3..f93ee12 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -376,9 +376,6 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)

if (!minor) {
return ERR_PTR(-ENODEV);
-   } else if (drm_device_is_unplugged(minor->dev)) {
-   drm_dev_unref(minor->dev);
-   return ERR_PTR(-ENODEV);
}

return minor;
@@ -464,9 +461,6 @@ void drm_unplug_dev(struct drm_device *dev)
drm_minor_unregister(dev, DRM_MINOR_CONTROL);

mutex_lock(_global_mutex);
-
-   drm_device_set_unplugged(dev);
-
if (dev->open_count == 0) {
drm_put_dev(dev);
}
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 1ea8790..b4332d4 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -497,8 +497,6 @@ int drm_release(struct inode *inode, struct file *filp)

if (!--dev->open_count) {
retcode = drm_lastclose(dev);
-   if (drm_device_is_unplugged(dev))
-   drm_put_dev(dev);
}
mutex_unlock(_global_mutex);

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 2e8c77e..c622e32 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -900,9 +900,6 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct 
*vma)
struct drm_vma_offset_node *node;
int ret;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
drm_vma_offset_lock_lookup(dev->vma_offset_manager);
node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
  vma->vm_pgoff,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 8ce2a0c..f959074 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -695,9 +695,6 @@ long drm_ioctl(struct file *filp,

dev = file_priv->minor->dev;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
is_driver_ioctl = nr >= DRM_COMMAND_BASE && nr < DRM_COMMAND_END;

if (is_driver_ioctl) {
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index f90bd5f..3a68be4 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -657,9 +657,6 @@ int drm_legacy_mmap(struct file *filp, struct 
vm_area_struct *vma)
struct drm_device *dev = priv->minor->dev;
int ret;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
mutex_lock(>struct_mutex);
ret = drm_mmap_locked(filp, vma);
mutex_unlock(>struct_mutex);
diff --git a/drivers/gpu/drm/udl/udl_connector.c 
b/drivers/gpu/drm/udl/udl_connector.c
index 4709b54..b168f2c 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -96,7 +96,7 @@ static int udl_mode_valid(struct drm_connector *connector,
 static enum drm_connector_status
 udl_detect(struct drm_connector *connector, bool force)
 {
-   if (drm_device_is_unplugged(connector->dev))
+   if (udl_device_is_unplugged(connector->dev))
return connector_status_disconnected;
return connector_status_connected;
 }
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index d5728ec..f5c2a97 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -24,12 +24,12 @@ static const struct vm_operations_struct udl_gem_vm_ops = {

 static const struct file_operations udl_driver_fops = {
.owner = THIS_MODULE,
-   .open = drm_open,
+   .open = udl_drm_open,
.mmap = udl_drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
-   .unlocked_ioctl = drm_ioctl,
-   .release = drm_release,
+   .unlocked_ioctl = udl_drm_ioctl,
+   .release = udl_drm_release,
 #ifdef CONFIG_COMPAT
.compat_ioctl = drm_compat_ioctl,
 #endif
@@ -97,6 +

[PATCH 2/2] drm: make unplugged flag specific to udl driver

2016-02-09 Thread Haixia Shi
Regarding the following comment:

> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index 1ea8790..b4332d4 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -497,8 +497,6 @@ int drm_release(struct inode *inode, struct file
*filp)
>
> if (!--dev->open_count) {
> retcode = drm_lastclose(dev);
> -   if (drm_device_is_unplugged(dev))
> -   drm_put_dev(dev);

>Again, you cannot drop this without replacement! In this case, you
>really should wrap fops.release() from UDL and call into drm_release()
>before copying this unplug-logic.

Does this really work? drm_release() will release minor at the end,
regardless of dev->open_count.

If minor is released, can I still safely get dev and call drm_put_dev(dev)
afterwards?

On Tue, Feb 9, 2016 at 4:44 AM, David Herrmann 
wrote:

> Hi
>
> On Fri, Feb 5, 2016 at 10:57 PM, Haixia Shi  wrote:
> > Remove the general drm_device_is_unplugged() checks, and move the
> unplugged
> > flag handling logic into drm/udl. In general we want to keep
> driver-specific
> > logic out of common drm code.
>
> I like the idea of moving this hack into udl. However, I don't think
> this patch is sufficient, see below for comments.
>
> Btw., hotplug might happen on other buses as well. It just happens
> that no-one tried pci/platform unplugging so far.. We used to have
> some patches flying around to make it work properly (with enter/leave
> markers), but no-one picked those up. But I like the idea of moving
> this unplugged marker into UDL, to make clear if someone else needs
> it, they better do it properly.
>
> > Signed-off-by: Haixia Shi 
> > Reviewed-by: Stéphane Marchesin 
> > ---
> >  drivers/gpu/drm/drm_drv.c   |  6 --
> >  drivers/gpu/drm/drm_fops.c  |  2 --
> >  drivers/gpu/drm/drm_gem.c   |  3 ---
> >  drivers/gpu/drm/drm_ioctl.c |  3 ---
> >  drivers/gpu/drm/drm_vm.c|  3 ---
> >  drivers/gpu/drm/udl/udl_connector.c |  2 +-
> >  drivers/gpu/drm/udl/udl_drv.c   |  1 +
> >  drivers/gpu/drm/udl/udl_drv.h   |  3 +++
> >  drivers/gpu/drm/udl/udl_fb.c|  2 +-
> >  drivers/gpu/drm/udl/udl_main.c  | 15 +++
> >  include/drm/drmP.h  | 14 --
> >  11 files changed, 21 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 167c8d3..f93ee12 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -376,9 +376,6 @@ struct drm_minor *drm_minor_acquire(unsigned int
> minor_id)
> >
> > if (!minor) {
> > return ERR_PTR(-ENODEV);
> > -   } else if (drm_device_is_unplugged(minor->dev)) {
> > -   drm_dev_unref(minor->dev);
> > -   return ERR_PTR(-ENODEV);
> > }
>
> You cannot just drop this without a replacement. You should add a
> drm_driver.open callback to UDL which checks for this.
> drm_minor_acquire() is only ever called from drm_stub_open(), and as
> such only from drm_open().
>
> Alternatively, you can provide fops.open from UDL and wrap drm_open().
>
> >
> > return minor;
> > @@ -464,9 +461,6 @@ void drm_unplug_dev(struct drm_device *dev)
> > drm_minor_unregister(dev, DRM_MINOR_CONTROL);
> >
> > mutex_lock(_global_mutex);
> > -
> > -   drm_device_set_unplugged(dev);
> > -
>
> You replace this by moving it into the caller of drm_unplug_dev().
> Sounds good to me. There has never been a real reason to put it
> underneath the global mutex, anyway.
>
> > if (dev->open_count == 0) {
> > drm_put_dev(dev);
> > }
> > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> > index 1ea8790..b4332d4 100644
> > --- a/drivers/gpu/drm/drm_fops.c
> > +++ b/drivers/gpu/drm/drm_fops.c
> > @@ -497,8 +497,6 @@ int drm_release(struct inode *inode, struct file
> *filp)
> >
> > if (!--dev->open_count) {
> > retcode = drm_lastclose(dev);
> > -   if (drm_device_is_unplugged(dev))
> > -   drm_put_dev(dev);
>
> Again, you cannot drop this without replacement! In this case, you
> really should wrap fops.release() from UDL and call into drm_release()
> before copying this unplug-logic.
>
> > }
> > mutex_unlock(_global_mutex);
> >
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> >

[PATCH 2/2] drm: make unplugged flag specific to udl driver

2016-02-05 Thread Haixia Shi
Remove the general drm_device_is_unplugged() checks, and move the unplugged
flag handling logic into drm/udl. In general we want to keep driver-specific
logic out of common drm code.

Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/drm_drv.c   |  6 --
 drivers/gpu/drm/drm_fops.c  |  2 --
 drivers/gpu/drm/drm_gem.c   |  3 ---
 drivers/gpu/drm/drm_ioctl.c |  3 ---
 drivers/gpu/drm/drm_vm.c|  3 ---
 drivers/gpu/drm/udl/udl_connector.c |  2 +-
 drivers/gpu/drm/udl/udl_drv.c   |  1 +
 drivers/gpu/drm/udl/udl_drv.h   |  3 +++
 drivers/gpu/drm/udl/udl_fb.c|  2 +-
 drivers/gpu/drm/udl/udl_main.c  | 15 +++
 include/drm/drmP.h  | 14 --
 11 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 167c8d3..f93ee12 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -376,9 +376,6 @@ struct drm_minor *drm_minor_acquire(unsigned int minor_id)

if (!minor) {
return ERR_PTR(-ENODEV);
-   } else if (drm_device_is_unplugged(minor->dev)) {
-   drm_dev_unref(minor->dev);
-   return ERR_PTR(-ENODEV);
}

return minor;
@@ -464,9 +461,6 @@ void drm_unplug_dev(struct drm_device *dev)
drm_minor_unregister(dev, DRM_MINOR_CONTROL);

mutex_lock(_global_mutex);
-
-   drm_device_set_unplugged(dev);
-
if (dev->open_count == 0) {
drm_put_dev(dev);
}
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 1ea8790..b4332d4 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -497,8 +497,6 @@ int drm_release(struct inode *inode, struct file *filp)

if (!--dev->open_count) {
retcode = drm_lastclose(dev);
-   if (drm_device_is_unplugged(dev))
-   drm_put_dev(dev);
}
mutex_unlock(_global_mutex);

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 2e8c77e..c622e32 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -900,9 +900,6 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct 
*vma)
struct drm_vma_offset_node *node;
int ret;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
drm_vma_offset_lock_lookup(dev->vma_offset_manager);
node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
  vma->vm_pgoff,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 8ce2a0c..f959074 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -695,9 +695,6 @@ long drm_ioctl(struct file *filp,

dev = file_priv->minor->dev;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
is_driver_ioctl = nr >= DRM_COMMAND_BASE && nr < DRM_COMMAND_END;

if (is_driver_ioctl) {
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index f90bd5f..3a68be4 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -657,9 +657,6 @@ int drm_legacy_mmap(struct file *filp, struct 
vm_area_struct *vma)
struct drm_device *dev = priv->minor->dev;
int ret;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
mutex_lock(>struct_mutex);
ret = drm_mmap_locked(filp, vma);
mutex_unlock(>struct_mutex);
diff --git a/drivers/gpu/drm/udl/udl_connector.c 
b/drivers/gpu/drm/udl/udl_connector.c
index 4709b54..b168f2c 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -96,7 +96,7 @@ static int udl_mode_valid(struct drm_connector *connector,
 static enum drm_connector_status
 udl_detect(struct drm_connector *connector, bool force)
 {
-   if (drm_device_is_unplugged(connector->dev))
+   if (udl_device_is_unplugged(connector->dev))
return connector_status_disconnected;
return connector_status_connected;
 }
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index d5728ec..3cbafe7 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -97,6 +97,7 @@ static void udl_usb_disconnect(struct usb_interface 
*interface)
drm_connector_unplug_all(dev);
udl_fbdev_unplug(dev);
udl_drop_usb(dev);
+   udl_device_set_unplugged(dev);
drm_unplug_dev(dev);
 }

diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 4a064ef..61e117a 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -65,6 +65,7 @@ struct udl_device {
atomic_t bytes_identical; /* saved effort with backbuffer comparison */
atomic_t bytes_sent; /* to usb, after compress

[PATCH 1/2] drm/msm: remove the drm_device_is_unplugged check

2016-02-05 Thread Haixia Shi
This flag is only used for drm/udl.

Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/msm/msm_fbdev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index d95af6e..e119c29 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -65,9 +65,6 @@ static int msm_fbdev_mmap(struct fb_info *info, struct 
vm_area_struct *vma)
struct drm_device *dev = helper->dev;
int ret = 0;

-   if (drm_device_is_unplugged(dev))
-   return -ENODEV;
-
ret = drm_gem_mmap_obj(drm_obj, drm_obj->size, vma);
if (ret) {
pr_err("%s:drm_gem_mmap_obj fail\n", __func__);
-- 
2.7.0.rc3.207.g0ac5344



[PATCH] drm/udl: implement usb_driver suspend/resume.

2015-04-01 Thread Haixia Shi
The usb_driver suspend and resume function pointers must be populated
to prevent forced unbinding of USB interface driver. See usb/core/driver.c:
unbind_no_pm_drivers_interfaces().

Restore mode and damage the entire frame buffer upon resume.

Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/udl/udl_drv.c | 17 +
 drivers/gpu/drm/udl/udl_drv.h |  2 ++
 drivers/gpu/drm/udl/udl_modeset.c | 13 +
 3 files changed, 32 insertions(+)

diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
index d5728ec..5b37aad 100644
--- a/drivers/gpu/drm/udl/udl_drv.c
+++ b/drivers/gpu/drm/udl/udl_drv.c
@@ -16,6 +16,21 @@ static int udl_driver_set_busid(struct drm_device *d, struct 
drm_master *m)
return 0;
 }

+static int udl_usb_suspend(struct usb_interface *interface,
+  pm_message_t message)
+{
+   return 0;
+}
+
+static int udl_usb_resume(struct usb_interface *interface)
+{
+   struct drm_device *dev = usb_get_intfdata(interface);
+
+   udl_modeset_restore(dev);
+   return 0;
+}
+
+
 static const struct vm_operations_struct udl_gem_vm_ops = {
.fault = udl_gem_fault,
.open = drm_gem_vm_open,
@@ -123,6 +138,8 @@ static struct usb_driver udl_driver = {
.name = "udl",
.probe = udl_usb_probe,
.disconnect = udl_usb_disconnect,
+   .suspend = udl_usb_suspend,
+   .resume = udl_usb_resume,
.id_table = id_table,
 };

diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 80adbac..19cb75d 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -52,6 +52,7 @@ struct udl_device {
struct device *dev;
struct drm_device *ddev;
struct usb_device *udev;
+   struct drm_crtc *crtc;

int sku_pixel_limit;

@@ -89,6 +90,7 @@ struct udl_framebuffer {

 /* modeset */
 int udl_modeset_init(struct drm_device *dev);
+void udl_modeset_restore(struct drm_device *dev);
 void udl_modeset_cleanup(struct drm_device *dev);
 int udl_connector_init(struct drm_device *dev, struct drm_encoder *encoder);

diff --git a/drivers/gpu/drm/udl/udl_modeset.c 
b/drivers/gpu/drm/udl/udl_modeset.c
index 677190a6..e4a986c 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -317,6 +317,8 @@ static int udl_crtc_mode_set(struct drm_crtc *crtc,
char *wrptr;
int color_depth = 0;

+   udl->crtc = crtc;
+
buf = (char *)udl->mode_buf;

/* for now we just clip 24 -> 16 - if we fix that fix this */
@@ -461,6 +463,17 @@ int udl_modeset_init(struct drm_device *dev)
return 0;
 }

+void udl_modeset_restore(struct drm_device *dev)
+{
+   struct udl_device *udl = dev->dev_private;
+   struct udl_framebuffer *ufb;
+   if (!udl->crtc || !udl->crtc->primary->fb)
+   return;
+   udl_crtc_commit(udl->crtc);
+   ufb = to_udl_fb(udl->crtc->primary->fb);
+   udl_handle_damage(ufb, 0, 0, ufb->base.width, ufb->base.height);
+}
+
 void udl_modeset_cleanup(struct drm_device *dev)
 {
drm_mode_config_cleanup(dev);
-- 
2.2.0.rc0.207.ga3a616c



[PATCH 2/2] drm/udl: implement cursor.

2015-03-05 Thread Haixia Shi
On Thu, Mar 5, 2015 at 3:54 AM, David Herrmann  wrote:
> Hi
>
> Why pretend the hw supports cursors if it clearly does not? I don't
> see why we cannot leave this to user-space (which can apply
> optimizations for software-only devices, which the kernel cannot). And
> a commit-message would also be nice.

IMHO implementing the drm cursor functions allows user-space to be
platform agnostic.

Will address the other comments and send updated patch in this thread, thanks.

>
> Thanks
> David


[PATCH 1/2] drm/udl: make page flips asynchronous.

2015-03-05 Thread Haixia Shi
On Thu, Mar 5, 2015 at 2:33 AM, Chris Wilson  
wrote:
> On Wed, Mar 04, 2015 at 06:26:23PM -0800, Haixia Shi wrote:
>> This also limits the maximum frequency of page flips by the vrefresh rate.
>>
>> Signed-off-by: Haixia Shi 
>> Reviewed-by: Stéphane Marchesin 
>> Tested-by: Haixia Shi 
>
> I think the better approach would have been to push the task down to
> udl_handle_damage(). Almost all of the callers would prefer deferred
> updates (with the notable exception of modesetting).
> -Chris

I'm not sure this is correct. While there's reasonable expectation for
page flips to be asynchronous (when flip completes it would send the
vblank event), my understanding is that fb_fillrect, fb_copyarea and
fb_imageblit are synchronous.

>
> --
> Chris Wilson, Intel Open Source Technology Centre


[PATCH 2/2] drm/udl: implement cursor.

2015-03-04 Thread Haixia Shi
Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
Tested-by: Haixia Shi 
---
 drivers/gpu/drm/udl/Makefile   |   2 +-
 drivers/gpu/drm/udl/udl_cursor.c   | 156 +
 drivers/gpu/drm/udl/udl_cursor.h   |  46 +++
 drivers/gpu/drm/udl/udl_drv.h  |   4 +
 drivers/gpu/drm/udl/udl_fb.c   |  31 
 drivers/gpu/drm/udl/udl_gem.c  |   3 +
 drivers/gpu/drm/udl/udl_main.c |  10 +++
 drivers/gpu/drm/udl/udl_modeset.c  |  45 +++
 drivers/gpu/drm/udl/udl_transfer.c |  36 -
 9 files changed, 317 insertions(+), 16 deletions(-)
 create mode 100644 drivers/gpu/drm/udl/udl_cursor.c
 create mode 100644 drivers/gpu/drm/udl/udl_cursor.h

diff --git a/drivers/gpu/drm/udl/Makefile b/drivers/gpu/drm/udl/Makefile
index 195bcac..67ccab4 100644
--- a/drivers/gpu/drm/udl/Makefile
+++ b/drivers/gpu/drm/udl/Makefile
@@ -1,6 +1,6 @@

 ccflags-y := -Iinclude/drm

-udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_encoder.o udl_main.o 
udl_fb.o udl_transfer.o udl_gem.o udl_dmabuf.o
+udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_encoder.o udl_main.o 
udl_fb.o udl_transfer.o udl_gem.o udl_dmabuf.o udl_cursor.o

 obj-$(CONFIG_DRM_UDL) := udl.o
diff --git a/drivers/gpu/drm/udl/udl_cursor.c b/drivers/gpu/drm/udl/udl_cursor.c
new file mode 100644
index 000..3b38ef5
--- /dev/null
+++ b/drivers/gpu/drm/udl/udl_cursor.c
@@ -0,0 +1,156 @@
+/*
+ * udl_cursor.c
+ *
+ * Copyright (c) 2015 The Chromium OS Authors
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include 
+#include 
+
+#include "udl_cursor.h"
+#include "udl_drv.h"
+
+#define UDL_CURSOR_W 64
+#define UDL_CURSOR_H 64
+#define UDL_CURSOR_BUF (UDL_CURSOR_W * UDL_CURSOR_H)
+
+/*
+ * UDL drm cursor private structure.
+ */
+struct udl_cursor {
+   uint32_t buffer[UDL_CURSOR_BUF];
+   bool enabled;
+   int x;
+   int y;
+};
+
+int udl_cursor_alloc(struct udl_cursor **cursor)
+{
+   struct udl_cursor *new_cursor = kzalloc(sizeof(struct udl_cursor),
+   GFP_KERNEL);
+   if (!new_cursor)
+   return -ENOMEM;
+   *cursor = new_cursor;
+   return 0;
+}
+
+void udl_cursor_free(struct udl_cursor *cursor)
+{
+   BUG_ON(!cursor);
+   kfree(cursor);
+}
+
+void udl_cursor_copy(struct udl_cursor *dst, struct udl_cursor *src)
+{
+   memcpy(dst, src, sizeof(struct udl_cursor));
+}
+
+bool udl_cursor_enabled(struct udl_cursor *cursor)
+{
+   return cursor->enabled;
+}
+
+void udl_cursor_get_hline(struct udl_cursor *cursor, int x, int y,
+   struct udl_cursor_hline *hline)
+{
+   if (!cursor || !cursor->enabled ||
+   x >= cursor->x + UDL_CURSOR_W ||
+   y < cursor->y || y >= cursor->y + UDL_CURSOR_H) {
+   hline->buffer = NULL;
+   return;
+   }
+
+   hline->buffer = >buffer[UDL_CURSOR_W * (y - cursor->y)];
+   hline->width = UDL_CURSOR_W;
+   hline->offset = x - cursor->x;
+}
+
+/*
+ * Return pre-computed cursor blend value defined as:
+ * R: 5 bits (bit 0:4)
+ * G: 6 bits (bit 5:10)
+ * B: 5 bits (bit 11:15)
+ * A: 7 bits (bit 16:22)
+ */
+static uint32_t cursor_blend_val32(uint32_t pix)
+{
+   /* range of alpha_scaled is 0..64 */
+   uint32_t alpha_scaled = ((pix >> 24) * 65) >> 8;
+   return ((pix >> 3) & 0x1f) |
+   ((pix >> 5) & 0x7e0) |
+   ((pix >> 8) & 0xf800) |
+   (alpha_scaled << 16);
+}
+
+static int udl_cursor_download(struct udl_cursor *cursor,
+   struct drm_gem_object *obj)
+{
+   struct udl_gem_object *udl_gem_obj = to_udl_bo(obj);
+   uint32_t *src_ptr, *dst_ptr;
+   size_t i;
+
+   int ret = udl_gem_vmap(udl_gem_obj);
+   if (ret != 0) {
+   DRM_ERROR("failed to vmap cursor\n");
+   return ret;
+   }
+
+   src_ptr = udl_gem_obj->vmapping;
+   dst_ptr = cursor->buffer;
+   for (i = 0; i < UDL_CURSOR_BUF; ++i)
+   dst_ptr[i] = cursor_blend_val32(le32_to_cpu(src_ptr[i]));
+   return 0;
+}
+
+int udl_cursor_set(struct drm_crtc *crtc, struct drm_file *file,
+   uint32_t handle, uint32_t width, uint32_t height,
+   struct udl_cursor *

[PATCH 1/2] drm/udl: make page flips asynchronous.

2015-03-04 Thread Haixia Shi
This also limits the maximum frequency of page flips by the vrefresh rate.

Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
Tested-by: Haixia Shi 
---
 drivers/gpu/drm/udl/udl_drv.h |   3 +
 drivers/gpu/drm/udl/udl_modeset.c | 129 ++
 2 files changed, 119 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 80adbac..b1e9fae 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -47,6 +47,7 @@ struct urb_list {
 };

 struct udl_fbdev;
+struct udl_flip_queue;

 struct udl_device {
struct device *dev;
@@ -65,6 +66,8 @@ struct udl_device {
atomic_t bytes_identical; /* saved effort with backbuffer comparison */
atomic_t bytes_sent; /* to usb, after compression including overhead */
atomic_t cpu_kcycles_used; /* transpired during pixel processing */
+
+   struct udl_flip_queue *flip_queue;
 };

 struct udl_gem_object {
diff --git a/drivers/gpu/drm/udl/udl_modeset.c 
b/drivers/gpu/drm/udl/udl_modeset.c
index 677190a6..f3804b4 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -303,6 +303,16 @@ udl_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 }
 #endif

+struct udl_flip_queue {
+   struct mutex lock;
+   struct workqueue_struct *wq;
+   struct delayed_work work;
+   struct drm_crtc *crtc;
+   struct drm_pending_vblank_event *event;
+   u64 flip_time;  /* in jiffies */
+   u64 vblank_interval;  /* in jiffies */
+};
+
 static int udl_crtc_mode_set(struct drm_crtc *crtc,
   struct drm_display_mode *mode,
   struct drm_display_mode *adjusted_mode,
@@ -313,6 +323,7 @@ static int udl_crtc_mode_set(struct drm_crtc *crtc,
struct drm_device *dev = crtc->dev;
struct udl_framebuffer *ufb = to_udl_fb(crtc->primary->fb);
struct udl_device *udl = dev->dev_private;
+   struct udl_flip_queue *flip_queue = udl->flip_queue;
char *buf;
char *wrptr;
int color_depth = 0;
@@ -347,6 +358,13 @@ static int udl_crtc_mode_set(struct drm_crtc *crtc,
ufb->active_16 = true;
udl->mode_buf_len = wrptr - buf;

+   /* update flip queue vblank interval */
+   if (flip_queue) {
+   mutex_lock(_queue->lock);
+   flip_queue->vblank_interval = HZ / mode->vrefresh;
+   mutex_unlock(_queue->lock);
+   }
+
/* damage all of it */
udl_handle_damage(ufb, 0, 0, ufb->base.width, ufb->base.height);
return 0;
@@ -364,29 +382,82 @@ static void udl_crtc_destroy(struct drm_crtc *crtc)
kfree(crtc);
 }

+static void udl_sched_page_flip(struct work_struct *work)
+{
+   struct udl_flip_queue *flip_queue =
+   container_of(container_of(work, struct delayed_work, work),
+   struct udl_flip_queue, work);
+   struct drm_crtc *crtc;
+   struct drm_device *dev;
+   struct drm_pending_vblank_event *event;
+   struct drm_framebuffer *fb;
+
+   mutex_lock(_queue->lock);
+   crtc = flip_queue->crtc;
+   dev = crtc->dev;
+   event = flip_queue->event;
+   fb = crtc->primary->fb;
+   flip_queue->event = NULL;
+   mutex_unlock(_queue->lock);
+
+   if (fb)
+   udl_handle_damage(to_udl_fb(fb), 0, 0, fb->width, fb->height);
+   if (event) {
+   unsigned long flags;
+   spin_lock_irqsave(>event_lock, flags);
+   drm_send_vblank_event(dev, 0, event);
+   spin_unlock_irqrestore(>event_lock, flags);
+   }
+}
+
 static int udl_crtc_page_flip(struct drm_crtc *crtc,
  struct drm_framebuffer *fb,
  struct drm_pending_vblank_event *event,
  uint32_t page_flip_flags)
 {
-   struct udl_framebuffer *ufb = to_udl_fb(fb);
struct drm_device *dev = crtc->dev;
-   unsigned long flags;
+   struct udl_device *udl = dev->dev_private;
+   struct udl_flip_queue *flip_queue = udl->flip_queue;

-   struct drm_framebuffer *old_fb = crtc->primary->fb;
-   if (old_fb) {
-   struct udl_framebuffer *uold_fb = to_udl_fb(old_fb);
-   uold_fb->active_16 = false;
+   if (!flip_queue || !flip_queue->wq) {
+   DRM_ERROR("Uninitialized page flip queue\n");
+   return -ENOMEM;
}
-   ufb->active_16 = true;

-   udl_handle_damage(ufb, 0, 0, fb->width, fb->height);
+   mutex_lock(_queue->lock);
+
+   flip_queue->crtc = crtc;
+   if (fb) {
+   struct drm_framebuffer *old_fb;
+   struct udl_framebuffer *ufb;
+   ufb = to_udl_fb(fb);
+   old_fb = cr

[PATCH] drm/udl: properly set active_16 flag in udl_crtc_page_flip(). (v2)

2015-01-30 Thread Haixia Shi
When page flipping, we need to mark the new fb as active and unmark the active
flag for the old fb (if different).

Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/udl/udl_modeset.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_modeset.c 
b/drivers/gpu/drm/udl/udl_modeset.c
index 1701f1d..677190a6 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -340,11 +340,11 @@ static int udl_crtc_mode_set(struct drm_crtc *crtc,

wrptr = udl_dummy_render(wrptr);

-   ufb->active_16 = true;
if (old_fb) {
struct udl_framebuffer *uold_fb = to_udl_fb(old_fb);
uold_fb->active_16 = false;
}
+   ufb->active_16 = true;
udl->mode_buf_len = wrptr - buf;

/* damage all of it */
@@ -373,6 +373,13 @@ static int udl_crtc_page_flip(struct drm_crtc *crtc,
struct drm_device *dev = crtc->dev;
unsigned long flags;

+   struct drm_framebuffer *old_fb = crtc->primary->fb;
+   if (old_fb) {
+   struct udl_framebuffer *uold_fb = to_udl_fb(old_fb);
+   uold_fb->active_16 = false;
+   }
+   ufb->active_16 = true;
+
udl_handle_damage(ufb, 0, 0, fb->width, fb->height);

spin_lock_irqsave(>event_lock, flags);
-- 
2.2.0.rc0.207.ga3a616c



[PATCH 2/2] drm/udl: fix excessive prefetch_range (v2)

2015-01-30 Thread Haixia Shi
The prefetch_range amount is already in number of bytes. Multiplying again by
bpp is unnecessary.

Signed-off-by: Haixia Shi 
Reviewed-by: Daniel Kurtz 
Tested-by: Haixia Shi 
---
 drivers/gpu/drm/udl/udl_transfer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_transfer.c 
b/drivers/gpu/drm/udl/udl_transfer.c
index 917dcb9..45f459f 100644
--- a/drivers/gpu/drm/udl/udl_transfer.c
+++ b/drivers/gpu/drm/udl/udl_transfer.c
@@ -156,7 +156,7 @@ static void udl_compress_hline16(
min((int)(pixel_end - pixel) / bpp,
(int)(cmd_buffer_end - cmd) / 2))) * bpp;

-   prefetch_range((void *) pixel, (cmd_pixel_end - pixel) * bpp);
+   prefetch_range((void *) pixel, cmd_pixel_end - pixel);
pixel_val16 = get_pixel_val16(pixel, bpp);

while (pixel < cmd_pixel_end) {
-- 
2.2.0.rc0.207.ga3a616c



[PATCH 1/2] drm/udl: optimize udl_compress_hline16 (v2)

2015-01-30 Thread Haixia Shi
The run-length encoding algorithm should compare 16-bit encoded pixel
values instead of comparing raw pixel values. It allows pixels
with similar but different colors to be encoded as repeat pixels, and
thus potentially save USB bandwidth.

Signed-off-by: Haixia Shi 
Reviewed-by: Daniel Kurtz 
Tested-by: Haixia Shi 
---
 drivers/gpu/drm/udl/udl_transfer.c | 39 +++---
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_transfer.c 
b/drivers/gpu/drm/udl/udl_transfer.c
index f343db7..917dcb9 100644
--- a/drivers/gpu/drm/udl/udl_transfer.c
+++ b/drivers/gpu/drm/udl/udl_transfer.c
@@ -82,12 +82,14 @@ static inline u16 pixel32_to_be16(const uint32_t pixel)
((pixel >> 8) & 0xf800));
 }

-static bool pixel_repeats(const void *pixel, const uint32_t repeat, int bpp)
+static inline u16 get_pixel_val16(const uint8_t *pixel, int bpp)
 {
+   u16 pixel_val16 = 0;
if (bpp == 2)
-   return *(const uint16_t *)pixel == repeat;
-   else
-   return *(const uint32_t *)pixel == repeat;
+   pixel_val16 = *(const uint16_t *)pixel;
+   else if (bpp == 4)
+   pixel_val16 = pixel32_to_be16(*(const uint32_t *)pixel);
+   return pixel_val16;
 }

 /*
@@ -134,6 +136,7 @@ static void udl_compress_hline16(
uint8_t *cmd_pixels_count_byte = NULL;
const u8 *raw_pixel_start = NULL;
const u8 *cmd_pixel_start, *cmd_pixel_end = NULL;
+   uint16_t pixel_val16;

prefetchw((void *) cmd); /* pull in one cache line at least */

@@ -154,33 +157,29 @@ static void udl_compress_hline16(
(int)(cmd_buffer_end - cmd) / 2))) * bpp;

prefetch_range((void *) pixel, (cmd_pixel_end - pixel) * bpp);
+   pixel_val16 = get_pixel_val16(pixel, bpp);

while (pixel < cmd_pixel_end) {
const u8 *const start = pixel;
-   u32 repeating_pixel;
-
-   if (bpp == 2) {
-   repeating_pixel = *(uint16_t *)pixel;
-   *(uint16_t *)cmd = cpu_to_be16(repeating_pixel);
-   } else {
-   repeating_pixel = *(uint32_t *)pixel;
-   *(uint16_t *)cmd = 
cpu_to_be16(pixel32_to_be16(repeating_pixel));
-   }
+   const uint16_t repeating_pixel_val16 = pixel_val16;
+
+   *(uint16_t *)cmd = cpu_to_be16(pixel_val16);

cmd += 2;
pixel += bpp;

-   if (unlikely((pixel < cmd_pixel_end) &&
-(pixel_repeats(pixel, repeating_pixel, 
bpp {
+   while (pixel < cmd_pixel_end) {
+   pixel_val16 = get_pixel_val16(pixel, bpp);
+   if (pixel_val16 != repeating_pixel_val16)
+   break;
+   pixel += bpp;
+   }
+
+   if (unlikely(pixel > start + bpp)) {
/* go back and fill in raw pixel count */
*raw_pixels_count_byte = (((start -
raw_pixel_start) / bpp) + 1) & 
0xFF;

-   while ((pixel < cmd_pixel_end) &&
-  (pixel_repeats(pixel, repeating_pixel, 
bpp))) {
-   pixel += bpp;
-   }
-
/* immediately after raw data is repeat byte */
*cmd++ = (((pixel - start) / bpp) - 1) & 0xFF;

-- 
2.2.0.rc0.207.ga3a616c



[PATCH 2/2] drm/udl: fix excessive prefetch_range

2015-01-30 Thread Haixia Shi
The prefetch_range amount is already in number of bytes. Multiplying again by
bpp is unnecessary.

Signed-off-by: Haixia Shi 
Reviewed-by: Daniel Kurtz 
Tested-by: Haixia Shi 
---
 drivers/gpu/drm/udl/udl_transfer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_transfer.c 
b/drivers/gpu/drm/udl/udl_transfer.c
index 917dcb9..45f459f 100644
--- a/drivers/gpu/drm/udl/udl_transfer.c
+++ b/drivers/gpu/drm/udl/udl_transfer.c
@@ -156,7 +156,7 @@ static void udl_compress_hline16(
min((int)(pixel_end - pixel) / bpp,
(int)(cmd_buffer_end - cmd) / 2))) * bpp;

-   prefetch_range((void *) pixel, (cmd_pixel_end - pixel) * bpp);
+   prefetch_range((void *) pixel, cmd_pixel_end - pixel);
pixel_val16 = get_pixel_val16(pixel, bpp);

while (pixel < cmd_pixel_end) {
-- 
2.2.0.rc0.207.ga3a616c



[PATCH 1/2] drm/udl: optimize udl_compress_hline16

2015-01-30 Thread Haixia Shi
The run-length encoding algorithm should compare 16-bit encoded pixel
values instead of comparing raw pixel values. It allows pixels
with similar but different colors to be encoded as repeat pixels, and
thus potentially save USB bandwidth.

Signed-off-by: Haixia Shi 
Reviewed-by: Daniel Kurtz 
Tested-by: Haixia Shi 
---
 drivers/gpu/drm/udl/udl_transfer.c | 39 +++---
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_transfer.c 
b/drivers/gpu/drm/udl/udl_transfer.c
index f343db7..917dcb9 100644
--- a/drivers/gpu/drm/udl/udl_transfer.c
+++ b/drivers/gpu/drm/udl/udl_transfer.c
@@ -82,12 +82,14 @@ static inline u16 pixel32_to_be16(const uint32_t pixel)
((pixel >> 8) & 0xf800));
 }

-static bool pixel_repeats(const void *pixel, const uint32_t repeat, int bpp)
+static inline u16 get_pixel_val16(const uint8_t *pixel, int bpp)
 {
+   u16 pixel_val16 = 0;
if (bpp == 2)
-   return *(const uint16_t *)pixel == repeat;
-   else
-   return *(const uint32_t *)pixel == repeat;
+   pixel_val16 = *(const uint16_t *)pixel;
+   else if (bpp == 4)
+   pixel_val16 = pixel32_to_be16(*(const uint32_t *)pixel);
+   return pixel_val16;
 }

 /*
@@ -134,6 +136,7 @@ static void udl_compress_hline16(
uint8_t *cmd_pixels_count_byte = NULL;
const u8 *raw_pixel_start = NULL;
const u8 *cmd_pixel_start, *cmd_pixel_end = NULL;
+   uint16_t pixel_val16;

prefetchw((void *) cmd); /* pull in one cache line at least */

@@ -154,33 +157,29 @@ static void udl_compress_hline16(
(int)(cmd_buffer_end - cmd) / 2))) * bpp;

prefetch_range((void *) pixel, (cmd_pixel_end - pixel) * bpp);
+   pixel_val16 = get_pixel_val16(pixel, bpp);

while (pixel < cmd_pixel_end) {
const u8 *const start = pixel;
-   u32 repeating_pixel;
-
-   if (bpp == 2) {
-   repeating_pixel = *(uint16_t *)pixel;
-   *(uint16_t *)cmd = cpu_to_be16(repeating_pixel);
-   } else {
-   repeating_pixel = *(uint32_t *)pixel;
-   *(uint16_t *)cmd = 
cpu_to_be16(pixel32_to_be16(repeating_pixel));
-   }
+   const uint16_t repeating_pixel_val16 = pixel_val16;
+
+   *(uint16_t *)cmd = cpu_to_be16(pixel_val16);

cmd += 2;
pixel += bpp;

-   if (unlikely((pixel < cmd_pixel_end) &&
-(pixel_repeats(pixel, repeating_pixel, 
bpp {
+   while (pixel < cmd_pixel_end) {
+   pixel_val16 = get_pixel_val16(pixel, bpp);
+   if (pixel_val16 != repeating_pixel_val16)
+   break;
+   pixel += bpp;
+   }
+
+   if (unlikely(pixel > start + bpp)) {
/* go back and fill in raw pixel count */
*raw_pixels_count_byte = (((start -
raw_pixel_start) / bpp) + 1) & 
0xFF;

-   while ((pixel < cmd_pixel_end) &&
-  (pixel_repeats(pixel, repeating_pixel, 
bpp))) {
-   pixel += bpp;
-   }
-
/* immediately after raw data is repeat byte */
*cmd++ = (((pixel - start) / bpp) - 1) & 0xFF;

-- 
2.2.0.rc0.207.ga3a616c



[PATCH 1/2] drm/udl: optimize udl_compress_hline16

2015-01-30 Thread Haixia Shi
Dave

Sorry it seems that my patch was rebased on top of
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

I'm going to re-send the patch on top of the drm-next branch of your
tree (git://people.freedesktop.org/~airlied/linux)

On Thu, Jan 29, 2015 at 7:45 PM, Dave Airlie  wrote:
> On 29 January 2015 at 07:41, Haixia Shi  wrote:
>> The run-length encoding algorithm should compare 16-bit encoded pixel
>> values instead of comparing raw pixel values. It allows pixels
>> with similar but different colors to be encoded as repeat pixels, and
>> thus potentially save USB bandwidth.
>
> This fails to build here, are we missing some precursor patches?
>
>  CC [M]  drivers/gpu/drm/udl/udl_transfer.o
> /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/udl/udl_transfer.c:
> In function ‘get_pixel_val16’:
> /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/udl/udl_transfer.c:91:3:
> error: implicit declaration of function ‘pixel32_to_be16p’
> [-Werror=implicit-function-declaration]
>pixel_val16 = pixel32_to_be16p(pixel);
>^
> /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/udl/udl_transfer.c:
> In function ‘udl_compress_hline16’:
> /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/udl/udl_transfer.c:180:33:
> error: ‘start’ undeclared (first use in this function)
>  *raw_pixels_count_byte = (((start -
>  ^
> /home/airlied/devel/kernel/drm-next/drivers/gpu/drm/udl/udl_transfer.c:180:33:
> note: each undeclared identifier is reported only once for each
> function it appears in
> cc1: some warnings being treated as errors
>
> Dave.


[PATCH 1/2] drm/udl: optimize udl_compress_hline16

2015-01-28 Thread Haixia Shi
Sorry about that; I have just re-sent the patches based on upstream code.

On Wed, Jan 28, 2015 at 1:12 PM, Chris Wilson  
wrote:
> On Wed, Jan 28, 2015 at 10:15:29AM -0800, Haixia Shi wrote:
>> The run-length encoding algorithm should compare 16-bit encoded pixel
>> values instead of comparing raw pixel values. It allows pixels
>> with similar but different colors to be encoded as repeat pixels, and
>> thus potentially save USB bandwidth.
>>
>> Signed-off-by: Haixia Shi 
>> Reviewed-by: Daniel Kurtz 
>> Tested-by: Haixia Shi 
>
> This is not based on upstream code, similar but it won't apply.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre


[PATCH 2/2] drm/udl: fix excessive prefetch_range

2015-01-28 Thread Haixia Shi
The prefetch_range amount is already in number of bytes. Multiplying again by
bpp is unnecessary.

Signed-off-by: Haixia Shi 
Reviewed-by: Daniel Kurtz 
Tested-by: Haixia Shi 
---
 drivers/gpu/drm/udl/udl_transfer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_transfer.c 
b/drivers/gpu/drm/udl/udl_transfer.c
index eadddf9..91e4ae2 100644
--- a/drivers/gpu/drm/udl/udl_transfer.c
+++ b/drivers/gpu/drm/udl/udl_transfer.c
@@ -156,7 +156,7 @@ static void udl_compress_hline16(
min((int)(pixel_end - pixel) / bpp,
(int)(cmd_buffer_end - cmd) / 2))) * bpp;

-   prefetch_range((void *) pixel, (cmd_pixel_end - pixel) * bpp);
+   prefetch_range((void *) pixel, cmd_pixel_end - pixel);
pixel_val16 = get_pixel_val16(pixel, bpp);

while (pixel < cmd_pixel_end) {
-- 
2.2.0.rc0.207.ga3a616c



[PATCH 1/2] drm/udl: optimize udl_compress_hline16

2015-01-28 Thread Haixia Shi
The run-length encoding algorithm should compare 16-bit encoded pixel
values instead of comparing raw pixel values. It allows pixels
with similar but different colors to be encoded as repeat pixels, and
thus potentially save USB bandwidth.

Signed-off-by: Haixia Shi 
Reviewed-by: Daniel Kurtz 
Tested-by: Haixia Shi 
---
 drivers/gpu/drm/udl/udl_transfer.c | 41 +++---
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_transfer.c 
b/drivers/gpu/drm/udl/udl_transfer.c
index f343db7..eadddf9 100644
--- a/drivers/gpu/drm/udl/udl_transfer.c
+++ b/drivers/gpu/drm/udl/udl_transfer.c
@@ -82,12 +82,14 @@ static inline u16 pixel32_to_be16(const uint32_t pixel)
((pixel >> 8) & 0xf800));
 }

-static bool pixel_repeats(const void *pixel, const uint32_t repeat, int bpp)
+static inline u16 get_pixel_val16(const uint8_t *pixel, int bpp)
 {
+   u16 pixel_val16 = 0;
if (bpp == 2)
-   return *(const uint16_t *)pixel == repeat;
-   else
-   return *(const uint32_t *)pixel == repeat;
+   pixel_val16 = *(uint16_t *)pixel;
+   else if (bpp == 4)
+   pixel_val16 = pixel32_to_be16p(pixel);
+   return pixel_val16;
 }

 /*
@@ -134,6 +136,7 @@ static void udl_compress_hline16(
uint8_t *cmd_pixels_count_byte = NULL;
const u8 *raw_pixel_start = NULL;
const u8 *cmd_pixel_start, *cmd_pixel_end = NULL;
+   uint16_t pixel_val16;

prefetchw((void *) cmd); /* pull in one cache line at least */

@@ -154,33 +157,29 @@ static void udl_compress_hline16(
(int)(cmd_buffer_end - cmd) / 2))) * bpp;

prefetch_range((void *) pixel, (cmd_pixel_end - pixel) * bpp);
+   pixel_val16 = get_pixel_val16(pixel, bpp);

while (pixel < cmd_pixel_end) {
-   const u8 *const start = pixel;
-   u32 repeating_pixel;
-
-   if (bpp == 2) {
-   repeating_pixel = *(uint16_t *)pixel;
-   *(uint16_t *)cmd = cpu_to_be16(repeating_pixel);
-   } else {
-   repeating_pixel = *(uint32_t *)pixel;
-   *(uint16_t *)cmd = 
cpu_to_be16(pixel32_to_be16(repeating_pixel));
-   }
+   const u8 * const repeating_pixel = pixel;
+   const uint16_t repeating_pixel_val16 = pixel_val16;
+
+   *(uint16_t *)cmd = cpu_to_be16(pixel_val16);

cmd += 2;
pixel += bpp;

-   if (unlikely((pixel < cmd_pixel_end) &&
-(pixel_repeats(pixel, repeating_pixel, 
bpp {
+   while (pixel < cmd_pixel_end) {
+   pixel_val16 = get_pixel_val16(pixel, bpp);
+   if (pixel_val16 != repeating_pixel_val16)
+   break;
+   pixel += bpp;
+   }
+
+   if (unlikely(pixel > repeating_pixel + bpp)) {
/* go back and fill in raw pixel count */
*raw_pixels_count_byte = (((start -
raw_pixel_start) / bpp) + 1) & 
0xFF;

-   while ((pixel < cmd_pixel_end) &&
-  (pixel_repeats(pixel, repeating_pixel, 
bpp))) {
-   pixel += bpp;
-   }
-
/* immediately after raw data is repeat byte */
*cmd++ = (((pixel - start) / bpp) - 1) & 0xFF;

-- 
2.2.0.rc0.207.ga3a616c



[PATCH 2/2] drm/udl: fix excessive prefetch_range

2015-01-28 Thread Haixia Shi
The prefetch_range amount is already in number of bytes. Multiplying again by
bpp is unnecessary.

Signed-off-by: Haixia Shi 
Reviewed-by: Daniel Kurtz 
Tested-by: Haixia Shi 
---
 drivers/gpu/drm/udl/udl_transfer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_transfer.c 
b/drivers/gpu/drm/udl/udl_transfer.c
index eadddf9..91e4ae2 100644
--- a/drivers/gpu/drm/udl/udl_transfer.c
+++ b/drivers/gpu/drm/udl/udl_transfer.c
@@ -156,7 +156,7 @@ static void udl_compress_hline16(
min((int)(pixel_end - pixel) / bpp,
(int)(cmd_buffer_end - cmd) / 2))) * bpp;

-   prefetch_range((void *) pixel, (cmd_pixel_end - pixel) * bpp);
+   prefetch_range((void *) pixel, cmd_pixel_end - pixel);
pixel_val16 = get_pixel_val16(pixel, bpp);

while (pixel < cmd_pixel_end) {
-- 
2.2.0.rc0.207.ga3a616c



[PATCH 1/2] drm/udl: optimize udl_compress_hline16

2015-01-28 Thread Haixia Shi
The run-length encoding algorithm should compare 16-bit encoded pixel
values instead of comparing raw pixel values. It allows pixels
with similar but different colors to be encoded as repeat pixels, and
thus potentially save USB bandwidth.

Signed-off-by: Haixia Shi 
Reviewed-by: Daniel Kurtz 
Tested-by: Haixia Shi 
---
 drivers/gpu/drm/udl/udl_transfer.c | 41 +++---
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_transfer.c 
b/drivers/gpu/drm/udl/udl_transfer.c
index f343db7..eadddf9 100644
--- a/drivers/gpu/drm/udl/udl_transfer.c
+++ b/drivers/gpu/drm/udl/udl_transfer.c
@@ -82,12 +82,14 @@ static inline u16 pixel32_to_be16(const uint32_t pixel)
((pixel >> 8) & 0xf800));
 }

-static bool pixel_repeats(const void *pixel, const uint32_t repeat, int bpp)
+static inline u16 get_pixel_val16(const uint8_t *pixel, int bpp)
 {
+   u16 pixel_val16 = 0;
if (bpp == 2)
-   return *(const uint16_t *)pixel == repeat;
-   else
-   return *(const uint32_t *)pixel == repeat;
+   pixel_val16 = *(uint16_t *)pixel;
+   else if (bpp == 4)
+   pixel_val16 = pixel32_to_be16p(pixel);
+   return pixel_val16;
 }

 /*
@@ -134,6 +136,7 @@ static void udl_compress_hline16(
uint8_t *cmd_pixels_count_byte = NULL;
const u8 *raw_pixel_start = NULL;
const u8 *cmd_pixel_start, *cmd_pixel_end = NULL;
+   uint16_t pixel_val16;

prefetchw((void *) cmd); /* pull in one cache line at least */

@@ -154,33 +157,29 @@ static void udl_compress_hline16(
(int)(cmd_buffer_end - cmd) / 2))) * bpp;

prefetch_range((void *) pixel, (cmd_pixel_end - pixel) * bpp);
+   pixel_val16 = get_pixel_val16(pixel, bpp);

while (pixel < cmd_pixel_end) {
-   const u8 *const start = pixel;
-   u32 repeating_pixel;
-
-   if (bpp == 2) {
-   repeating_pixel = *(uint16_t *)pixel;
-   *(uint16_t *)cmd = cpu_to_be16(repeating_pixel);
-   } else {
-   repeating_pixel = *(uint32_t *)pixel;
-   *(uint16_t *)cmd = 
cpu_to_be16(pixel32_to_be16(repeating_pixel));
-   }
+   const u8 * const repeating_pixel = pixel;
+   const uint16_t repeating_pixel_val16 = pixel_val16;
+
+   *(uint16_t *)cmd = cpu_to_be16(pixel_val16);

cmd += 2;
pixel += bpp;

-   if (unlikely((pixel < cmd_pixel_end) &&
-(pixel_repeats(pixel, repeating_pixel, 
bpp {
+   while (pixel < cmd_pixel_end) {
+   pixel_val16 = get_pixel_val16(pixel, bpp);
+   if (pixel_val16 != repeating_pixel_val16)
+   break;
+   pixel += bpp;
+   }
+
+   if (unlikely(pixel > repeating_pixel + bpp)) {
/* go back and fill in raw pixel count */
*raw_pixels_count_byte = (((start -
raw_pixel_start) / bpp) + 1) & 
0xFF;

-   while ((pixel < cmd_pixel_end) &&
-  (pixel_repeats(pixel, repeating_pixel, 
bpp))) {
-   pixel += bpp;
-   }
-
/* immediately after raw data is repeat byte */
*cmd++ = (((pixel - start) / bpp) - 1) & 0xFF;

-- 
2.2.0.rc0.207.ga3a616c



[PATCH] drm/udl: properly set active_16 flag in udl_crtc_page_flip().

2015-01-22 Thread Haixia Shi
When page flipping, we need to mark the new fb as active and unmark the active
flag for the old fb (if different).

Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/udl/udl_modeset.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_modeset.c 
b/drivers/gpu/drm/udl/udl_modeset.c
index 1701f1d..a9c611a 100644
--- a/drivers/gpu/drm/udl/udl_modeset.c
+++ b/drivers/gpu/drm/udl/udl_modeset.c
@@ -340,11 +340,11 @@ static int udl_crtc_mode_set(struct drm_crtc *crtc,

wrptr = udl_dummy_render(wrptr);

-   ufb->active_16 = true;
if (old_fb) {
struct udl_framebuffer *uold_fb = to_udl_fb(old_fb);
uold_fb->active_16 = false;
}
+   ufb->active_16 = true;
udl->mode_buf_len = wrptr - buf;

/* damage all of it */
@@ -373,6 +373,13 @@ static int udl_crtc_page_flip(struct drm_crtc *crtc,
struct drm_device *dev = crtc->dev;
unsigned long flags;

+   struct drm_framebuffer *old_fb = crtc->fb;
+   if (old_fb) {
+   struct udl_framebuffer *uold_fb = to_udl_fb(old_fb);
+   uold_fb->active_16 = false;
+   }
+   ufb->active_16 = true;
+
udl_handle_damage(ufb, 0, 0, fb->width, fb->height);

spin_lock_irqsave(>event_lock, flags);
-- 
2.2.0.rc0.207.ga3a616c



[PATCH 2/2] drm/udl: properly check for error pointers

2014-11-25 Thread Haixia Shi
The drm_prime_pages_to_sg() function never returns NULL pointers, only
error pointers and valid pointers.

Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/udl/udl_dmabuf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_dmabuf.c b/drivers/gpu/drm/udl/udl_dmabuf.c
index 2425b76..ac8a66b 100644
--- a/drivers/gpu/drm/udl/udl_dmabuf.c
+++ b/drivers/gpu/drm/udl/udl_dmabuf.c
@@ -99,9 +99,9 @@ static struct sg_table *udl_map_dma_buf(struct 
dma_buf_attachment *attach,

page_count = obj->base.size / PAGE_SIZE;
obj->sg = drm_prime_pages_to_sg(obj->pages, page_count);
-   if (!obj->sg) {
-   DRM_ERROR("sg is null.\n");
-   return ERR_PTR(-ENOMEM);
+   if (IS_ERR(obj->sg)) {
+   DRM_ERROR("failed to allocate sgt.\n");
+   return ERR_CAST(obj->sg);
}

sgt = _attach->sgt;
-- 
2.2.0.rc0.207.ga3a616c



[PATCH 1/2] drm/udl: handle page mapping in dmabuf export.

2014-11-25 Thread Haixia Shi
Fixes dmabuf export failure with -E_NOMEM when the page is not mapped.

Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/udl/udl_dmabuf.c | 7 +--
 drivers/gpu/drm/udl/udl_drv.h| 2 ++
 drivers/gpu/drm/udl/udl_gem.c| 4 ++--
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_dmabuf.c b/drivers/gpu/drm/udl/udl_dmabuf.c
index 1d85c3a..2425b76 100644
--- a/drivers/gpu/drm/udl/udl_dmabuf.c
+++ b/drivers/gpu/drm/udl/udl_dmabuf.c
@@ -90,8 +90,11 @@ static struct sg_table *udl_map_dma_buf(struct 
dma_buf_attachment *attach,
return _attach->sgt;

if (!obj->pages) {
-   DRM_ERROR("pages is null.\n");
-   return ERR_PTR(-ENOMEM);
+   ret = udl_gem_get_pages(obj);
+   if (ret) {
+   DRM_ERROR("failed to map pages.\n");
+   return ERR_PTR(ret);
+   }
}

page_count = obj->base.size / PAGE_SIZE;
diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index 1b132d7..80adbac 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -129,6 +129,8 @@ struct dma_buf *udl_gem_prime_export(struct drm_device *dev,
 struct drm_gem_object *udl_gem_prime_import(struct drm_device *dev,
struct dma_buf *dma_buf);

+int udl_gem_get_pages(struct udl_gem_object *obj);
+void udl_gem_put_pages(struct udl_gem_object *obj);
 int udl_gem_vmap(struct udl_gem_object *obj);
 void udl_gem_vunmap(struct udl_gem_object *obj);
 int udl_drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
index cd3482d..2a0a784 100644
--- a/drivers/gpu/drm/udl/udl_gem.c
+++ b/drivers/gpu/drm/udl/udl_gem.c
@@ -127,7 +127,7 @@ int udl_gem_fault(struct vm_area_struct *vma, struct 
vm_fault *vmf)
}
 }

-static int udl_gem_get_pages(struct udl_gem_object *obj)
+int udl_gem_get_pages(struct udl_gem_object *obj)
 {
struct page **pages;

@@ -143,7 +143,7 @@ static int udl_gem_get_pages(struct udl_gem_object *obj)
return 0;
 }

-static void udl_gem_put_pages(struct udl_gem_object *obj)
+void udl_gem_put_pages(struct udl_gem_object *obj)
 {
if (obj->base.import_attach) {
drm_free_large(obj->pages);
-- 
2.2.0.rc0.207.ga3a616c



[PATCH 2/2] drm/udl: add support to export a handle to a FD on UDL.

2014-11-12 Thread Haixia Shi
Sorry I forgot to remove the Change-Id lines for these 2 patches. Please
review and I'll send updated patches with the Change-Id lines removed.

On Wed, Nov 12, 2014 at 6:33 PM, Haixia Shi  wrote:

> Only importing an FD to a handle is currently supported on UDL,
> but the exporting functionality is equally useful.
>
> Change-Id: If4983041875ebf3bd2ecf996d0771eb77b0cf1dc
> Signed-off-by: Haixia Shi 
> Reviewed-by: Stéphane Marchesin 
> ---
>  drivers/gpu/drm/udl/Makefile |   2 +-
>  drivers/gpu/drm/udl/udl_dmabuf.c | 273
> +++
>  drivers/gpu/drm/udl/udl_drv.c|   2 +
>  drivers/gpu/drm/udl/udl_drv.h|   2 +
>  drivers/gpu/drm/udl/udl_gem.c|  71 --
>  5 files changed, 278 insertions(+), 72 deletions(-)
>  create mode 100644 drivers/gpu/drm/udl/udl_dmabuf.c
>
> diff --git a/drivers/gpu/drm/udl/Makefile b/drivers/gpu/drm/udl/Makefile
> index 05c7481..195bcac 100644
> --- a/drivers/gpu/drm/udl/Makefile
> +++ b/drivers/gpu/drm/udl/Makefile
> @@ -1,6 +1,6 @@
>
>  ccflags-y := -Iinclude/drm
>
> -udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_encoder.o udl_main.o
> udl_fb.o udl_transfer.o udl_gem.o
> +udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_encoder.o udl_main.o
> udl_fb.o udl_transfer.o udl_gem.o udl_dmabuf.o
>
>  obj-$(CONFIG_DRM_UDL) := udl.o
> diff --git a/drivers/gpu/drm/udl/udl_dmabuf.c
> b/drivers/gpu/drm/udl/udl_dmabuf.c
> new file mode 100644
> index 000..1d85c3a
> --- /dev/null
> +++ b/drivers/gpu/drm/udl/udl_dmabuf.c
> @@ -0,0 +1,273 @@
> +/*
> + * udl_dmabuf.c
> + *
> + * Copyright (c) 2014 The Chromium OS Authors
> + *
> + * This program is free software; you can redistribute  it and/or modify
> it
> + * under  the terms of  the GNU General  Public License as published by
> the
> + * Free Software Foundation;  either version 2 of the  License, or (at
> your
> + * option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include 
> +#include "udl_drv.h"
> +#include 
> +#include 
> +
> +struct udl_drm_dmabuf_attachment {
> +   struct sg_table sgt;
> +   enum dma_data_direction dir;
> +   bool is_mapped;
> +};
> +
> +static int udl_attach_dma_buf(struct dma_buf *dmabuf,
> + struct device *dev,
> + struct dma_buf_attachment *attach)
> +{
> +   struct udl_drm_dmabuf_attachment *udl_attach;
> +
> +   DRM_DEBUG_PRIME("[DEV:%s] size:%zd\n", dev_name(attach->dev),
> +   attach->dmabuf->size);
> +
> +   udl_attach = kzalloc(sizeof(*udl_attach), GFP_KERNEL);
> +   if (!udl_attach)
> +   return -ENOMEM;
> +
> +   udl_attach->dir = DMA_NONE;
> +   attach->priv = udl_attach;
> +
> +   return 0;
> +}
> +
> +static void udl_detach_dma_buf(struct dma_buf *dmabuf,
> +  struct dma_buf_attachment *attach)
> +{
> +   struct udl_drm_dmabuf_attachment *udl_attach = attach->priv;
> +   struct sg_table *sgt;
> +
> +   if (!udl_attach)
> +   return;
> +
> +   DRM_DEBUG_PRIME("[DEV:%s] size:%zd\n", dev_name(attach->dev),
> +   attach->dmabuf->size);
> +
> +   sgt = _attach->sgt;
> +
> +   if (udl_attach->dir != DMA_NONE)
> +   dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents,
> +   udl_attach->dir);
> +
> +   sg_free_table(sgt);
> +   kfree(udl_attach);
> +   attach->priv = NULL;
> +}
> +
> +static struct sg_table *udl_map_dma_buf(struct dma_buf_attachment *attach,
> +   enum dma_data_direction dir)
> +{
> +   struct udl_drm_dmabuf_attachment *udl_attach = attach->priv;
> +   struct udl_gem_object *obj = to_udl_bo(attach->dmabuf->priv);
> +   struct drm_device *dev = obj->base.dev;
> +   struct scatterlist *rd, *wr;
> +   struct sg_table *sgt = NULL;
> +   unsigned int i;
> +   int page_count;
> +   int nents, ret;
> +
> +   DRM_DEBUG_PRIME("[DEV:%s] size:%zd dir=%d\n",
> dev_name(attach->dev),
> +   attach->dmabuf->

[PATCH 2/2] drm/udl: add support to export a handle to a FD on UDL.

2014-11-12 Thread Haixia Shi
Only importing an FD to a handle is currently supported on UDL,
but the exporting functionality is equally useful.

Change-Id: If4983041875ebf3bd2ecf996d0771eb77b0cf1dc
Signed-off-by: Haixia Shi 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/udl/Makefile |   2 +-
 drivers/gpu/drm/udl/udl_dmabuf.c | 273 +++
 drivers/gpu/drm/udl/udl_drv.c|   2 +
 drivers/gpu/drm/udl/udl_drv.h|   2 +
 drivers/gpu/drm/udl/udl_gem.c|  71 --
 5 files changed, 278 insertions(+), 72 deletions(-)
 create mode 100644 drivers/gpu/drm/udl/udl_dmabuf.c

diff --git a/drivers/gpu/drm/udl/Makefile b/drivers/gpu/drm/udl/Makefile
index 05c7481..195bcac 100644
--- a/drivers/gpu/drm/udl/Makefile
+++ b/drivers/gpu/drm/udl/Makefile
@@ -1,6 +1,6 @@

 ccflags-y := -Iinclude/drm

-udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_encoder.o udl_main.o 
udl_fb.o udl_transfer.o udl_gem.o
+udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_encoder.o udl_main.o 
udl_fb.o udl_transfer.o udl_gem.o udl_dmabuf.o

 obj-$(CONFIG_DRM_UDL) := udl.o
diff --git a/drivers/gpu/drm/udl/udl_dmabuf.c b/drivers/gpu/drm/udl/udl_dmabuf.c
new file mode 100644
index 000..1d85c3a
--- /dev/null
+++ b/drivers/gpu/drm/udl/udl_dmabuf.c
@@ -0,0 +1,273 @@
+/*
+ * udl_dmabuf.c
+ *
+ * Copyright (c) 2014 The Chromium OS Authors
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include 
+#include "udl_drv.h"
+#include 
+#include 
+
+struct udl_drm_dmabuf_attachment {
+   struct sg_table sgt;
+   enum dma_data_direction dir;
+   bool is_mapped;
+};
+
+static int udl_attach_dma_buf(struct dma_buf *dmabuf,
+ struct device *dev,
+ struct dma_buf_attachment *attach)
+{
+   struct udl_drm_dmabuf_attachment *udl_attach;
+
+   DRM_DEBUG_PRIME("[DEV:%s] size:%zd\n", dev_name(attach->dev),
+   attach->dmabuf->size);
+
+   udl_attach = kzalloc(sizeof(*udl_attach), GFP_KERNEL);
+   if (!udl_attach)
+   return -ENOMEM;
+
+   udl_attach->dir = DMA_NONE;
+   attach->priv = udl_attach;
+
+   return 0;
+}
+
+static void udl_detach_dma_buf(struct dma_buf *dmabuf,
+  struct dma_buf_attachment *attach)
+{
+   struct udl_drm_dmabuf_attachment *udl_attach = attach->priv;
+   struct sg_table *sgt;
+
+   if (!udl_attach)
+   return;
+
+   DRM_DEBUG_PRIME("[DEV:%s] size:%zd\n", dev_name(attach->dev),
+   attach->dmabuf->size);
+
+   sgt = _attach->sgt;
+
+   if (udl_attach->dir != DMA_NONE)
+   dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents,
+   udl_attach->dir);
+
+   sg_free_table(sgt);
+   kfree(udl_attach);
+   attach->priv = NULL;
+}
+
+static struct sg_table *udl_map_dma_buf(struct dma_buf_attachment *attach,
+   enum dma_data_direction dir)
+{
+   struct udl_drm_dmabuf_attachment *udl_attach = attach->priv;
+   struct udl_gem_object *obj = to_udl_bo(attach->dmabuf->priv);
+   struct drm_device *dev = obj->base.dev;
+   struct scatterlist *rd, *wr;
+   struct sg_table *sgt = NULL;
+   unsigned int i;
+   int page_count;
+   int nents, ret;
+
+   DRM_DEBUG_PRIME("[DEV:%s] size:%zd dir=%d\n", dev_name(attach->dev),
+   attach->dmabuf->size, dir);
+
+   /* just return current sgt if already requested. */
+   if (udl_attach->dir == dir && udl_attach->is_mapped)
+   return _attach->sgt;
+
+   if (!obj->pages) {
+   DRM_ERROR("pages is null.\n");
+   return ERR_PTR(-ENOMEM);
+   }
+
+   page_count = obj->base.size / PAGE_SIZE;
+   obj->sg = drm_prime_pages_to_sg(obj->pages, page_count);
+   if (!obj->sg) {
+   DRM_ERROR("sg is null.\n");
+   return ERR_PTR(-ENOMEM);
+   }
+
+   sgt = _attach->sgt;
+
+   ret = sg_alloc_table(sgt, obj->sg->orig_nents, GFP_KERNEL);
+   if (ret) {
+   DRM_ERROR("failed to alloc sgt.\n");
+   return ERR_PTR(-ENOMEM);
+   

[PATCH 1/2] drm/udl: add cache flags definitions for udl_gem_object

2014-11-12 Thread Haixia Shi
By default set udl_gem_object as cacheable, but set WC flag when attaching
dmabuf. In udl_gem_mmap() update cache attributes based on the flags, similar
to exynos_drm_gem_mmap().

Change-Id: I00e5e67f2285d66adcf2ae652a9d59f12af64541
Signed-off-by: Haixia Shi 
Reviewed-by: Sonny Rao 
Reviewed-by: Olof Johansson 
Reviewed-by: Stéphane Marchesin 
---
 drivers/gpu/drm/udl/udl_drv.h |  4 
 drivers/gpu/drm/udl/udl_gem.c | 21 +
 2 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
index c7490a2..3082780 100644
--- a/drivers/gpu/drm/udl/udl_drv.h
+++ b/drivers/gpu/drm/udl/udl_drv.h
@@ -25,6 +25,9 @@
 #define DRIVER_MINOR   0
 #define DRIVER_PATCHLEVEL  1

+#define UDL_BO_CACHEABLE   (1 << 0)
+#define UDL_BO_WC  (1 << 1)
+
 struct udl_device;

 struct urb_node {
@@ -69,6 +72,7 @@ struct udl_gem_object {
struct page **pages;
void *vmapping;
struct sg_table *sg;
+   unsigned int flags;
 };

 #define to_udl_bo(x) container_of(x, struct udl_gem_object, base)
diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c
index 8044f5f..e00459d 100644
--- a/drivers/gpu/drm/udl/udl_gem.c
+++ b/drivers/gpu/drm/udl/udl_gem.c
@@ -25,6 +25,7 @@ struct udl_gem_object *udl_gem_alloc_object(struct drm_device 
*dev,
return NULL;
}

+   obj->flags = UDL_BO_CACHEABLE;
return obj;
 }

@@ -56,6 +57,23 @@ udl_gem_create(struct drm_file *file,
return 0;
 }

+static void update_vm_cache_attr(struct udl_gem_object *obj,
+struct vm_area_struct *vma)
+{
+   DRM_DEBUG_KMS("flags = 0x%x\n", obj->flags);
+
+   /* non-cacheable as default. */
+   if (obj->flags & UDL_BO_CACHEABLE) {
+   vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
+   } else if (obj->flags & UDL_BO_WC) {
+   vma->vm_page_prot =
+   pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
+   } else {
+   vma->vm_page_prot =
+   pgprot_noncached(vm_get_page_prot(vma->vm_flags));
+   }
+}
+
 int udl_dumb_create(struct drm_file *file,
struct drm_device *dev,
struct drm_mode_create_dumb *args)
@@ -77,6 +95,8 @@ int udl_drm_gem_mmap(struct file *filp, struct vm_area_struct 
*vma)
vma->vm_flags &= ~VM_PFNMAP;
vma->vm_flags |= VM_MIXEDMAP;

+   update_vm_cache_attr(to_udl_bo(vma->vm_private_data), vma);
+
return ret;
 }

@@ -279,6 +299,7 @@ struct drm_gem_object *udl_gem_prime_import(struct 
drm_device *dev,
}

uobj->base.import_attach = attach;
+   uobj->flags = UDL_BO_WC;

return >base;

-- 
2.1.0.rc2.206.gedb03e5