diff for duplicates of <1458288709.12484.15.camel@synopsys.com> diff --git a/a/1.txt b/N1/1.txt index 975f2ca..df0912e 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,26 +1,26 @@ Hi Daniel, -On Fri, 2016-03-18@09:02 +0100, Daniel Vetter wrote: -> On Thu, Mar 17, 2016@08:27:10PM +0000, Alexey Brodkin wrote: +On Fri, 2016-03-18 at 09:02 +0100, Daniel Vetter wrote: +> On Thu, Mar 17, 2016 at 08:27:10PM +0000, Alexey Brodkin wrote: > > > > Hi Daniel, > > -> > On Tue, 2016-03-15@16:59 +0100, Daniel Vetter wrote: +> > On Tue, 2016-03-15 at 16:59 +0100, Daniel Vetter wrote: > > > -> > > On Tue, Mar 15, 2016@03:24:46PM +0000, Alexey Brodkin wrote: +> > > On Tue, Mar 15, 2016 at 03:24:46PM +0000, Alexey Brodkin wrote: > > > > -> > > > On Tue, 2016-03-15@09:10 +0100, Daniel Vetter wrote: +> > > > On Tue, 2016-03-15 at 09:10 +0100, Daniel Vetter wrote: > > > > > -> > > > > On Mon, Mar 14, 2016@11:15:59AM +0000, Alexey Brodkin wrote: +> > > > > On Mon, Mar 14, 2016 at 11:15:59AM +0000, Alexey Brodkin wrote: > > > > > > -> > > > > > On Mon, 2016-03-14@08:00 +0100, Daniel Vetter wrote: +> > > > > > On Mon, 2016-03-14 at 08:00 +0100, Daniel Vetter wrote: > > > > > > > -> > > > > > > On Fri, Mar 11, 2016@06:42:36PM +0300, Alexey Brodkin wrote: +> > > > > > > On Fri, Mar 11, 2016 at 06:42:36PM +0300, Alexey Brodkin wrote: > > > > > > > > -> > > > > > > > ? +> > > > > > > > > > > > > > > > +static struct drm_driver arcpgu_drm_driver = { > > > > > > > > + .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | -> > > > > > > > + ???DRIVER_ATOMIC, +> > > > > > > > + DRIVER_ATOMIC, > > > > > > > > + .preclose = arcpgu_preclose, > > > > > > > > + .lastclose = arcpgu_lastclose, > > > > > > > > + .name = "drm-arcpgu", @@ -39,7 +39,7 @@ On Fri, 2016-03-18@09:02 +0100, Daniel Vetter wrote: > > > > > > Ok I took "atmel-hlcdc" as example. > > > > > > And that's interesting. > > > > > > -> > > > > > If I put my?arcpgu_load() in between?drm_dev_alloc() and +> > > > > > If I put my arcpgu_load() in between drm_dev_alloc() and > > > > > > drm_dev_register() then I'm getting this on the driver probe: > > > > > > ---------------------------------->8------------------------------- > > > > > > [drm] Initialized drm 1.1.0 20060810 @@ -51,23 +51,23 @@ On Fri, 2016-03-18@09:02 +0100, Daniel Vetter wrote: > > > > > > CPU: 0 PID: 1 Comm: swapper Not tainted 4.5.0-rc3-01062-ga447822-dirty #17 > > > > > > > > > > > > Stack Trace: -> > > > > > ? arc_unwind_core.constprop.1+0xa4/0x110 -> > > > > > ? warn_slowpath_fmt+0x6e/0xfc -> > > > > > ? kobject_add_internal+0x17c/0x498 -> > > > > > ? kobject_add+0x98/0xe4 -> > > > > > ? device_add+0xc6/0x734 -> > > > > > ? device_create_with_groups+0x12a/0x144 -> > > > > > ? drm_sysfs_connector_add+0x54/0xe8 -> > > > > > ? arcpgu_drm_hdmi_init+0xd4/0x17c -> > > > > > ? arcpgu_probe+0x138/0x24c -> > > > > > ? platform_drv_probe+0x2e/0x6c -> > > > > > ? really_probe+0x212/0x35c -> > > > > > ? __driver_attach+0x90/0x94 -> > > > > > ? bus_for_each_dev+0x46/0x80 -> > > > > > ? bus_add_driver+0x14e/0x1b4 -> > > > > > ? driver_register+0x64/0x108 -> > > > > > ? do_one_initcall+0x86/0x194 -> > > > > > ? kernel_init_freeable+0xf0/0x188 +> > > > > > arc_unwind_core.constprop.1+0xa4/0x110 +> > > > > > warn_slowpath_fmt+0x6e/0xfc +> > > > > > kobject_add_internal+0x17c/0x498 +> > > > > > kobject_add+0x98/0xe4 +> > > > > > device_add+0xc6/0x734 +> > > > > > device_create_with_groups+0x12a/0x144 +> > > > > > drm_sysfs_connector_add+0x54/0xe8 +> > > > > > arcpgu_drm_hdmi_init+0xd4/0x17c +> > > > > > arcpgu_probe+0x138/0x24c +> > > > > > platform_drv_probe+0x2e/0x6c +> > > > > > really_probe+0x212/0x35c +> > > > > > __driver_attach+0x90/0x94 +> > > > > > bus_for_each_dev+0x46/0x80 +> > > > > > bus_add_driver+0x14e/0x1b4 +> > > > > > driver_register+0x64/0x108 +> > > > > > do_one_initcall+0x86/0x194 +> > > > > > kernel_init_freeable+0xf0/0x188 > > > > > > ---[ end trace c67166ad43ddcce2 ]--- > > > > > > [drm:drm_sysfs_connector_add] adding "HDMI-A-1" to sysfs > > > > > > [drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -2 @@ -83,12 +83,12 @@ On Fri, 2016-03-18@09:02 +0100, Daniel Vetter wrote: > > > > > register all the drm connectors _after_ calling drm_dev_register(). > > > > > Totally forgot about that. Can you pls > > > > > - Extract a new drm_connector_register_all() function -> > > > > ? (atmel_hlcdc_dc_connector_plug_all seems to be the best template), -> > > > > ? including kerneldoc. +> > > > > (atmel_hlcdc_dc_connector_plug_all seems to be the best template), +> > > > > including kerneldoc. > > > > > - Adjust kerneldoc of drm_dev_register() to mention -> > > > > ? drm_connector_register_all() and that ordering constraint. +> > > > > drm_connector_register_all() and that ordering constraint. > > > > > - Roll that helper out to all the drivers that currently hand-roll it (one -> > > > > ? patch per driver). +> > > > > patch per driver). > > > > > > > > > > I know a bit of work but imo not too much, and by doing some small > > > > > refactoring every time someone stumbles over a drm pitfall we keep the @@ -102,20 +102,20 @@ On Fri, 2016-03-18@09:02 +0100, Daniel Vetter wrote: > > > Awesome, looking forward to your patches. > > Sorry it took longer for me to finally put my hands on that work but anyways. > > -> > I'm looking now at how drivers use existing?drm_connector_unplug_all() and -> > their implementation of what would be?drm_connector_plug_all() and see +> > I'm looking now at how drivers use existing drm_connector_unplug_all() and +> > their implementation of what would be drm_connector_plug_all() and see > > in some implementations people wraps both helpers with > > mutex_{lock|unlock}(&dev->mode_config.mutex). But not everybody does this. > > > > So essentially my questions are: -> > ?[1] If it's necessary to get hold of that mutex before execution of either helper? +> > [1] If it's necessary to get hold of that mutex before execution of either helper? > In plug_all I think so, unplug_all has a FIXME comment about how locking > against sysfs is horrible and it's all going to blow up. But we did > recently change the connector sysfs files, so maybe that's fixed now. Not > sure. > -> > ?[2] If this is really necessary then IMHO it makes sense to move mutex_lock/unlock -> > ? ? ?in helpers itself, right? +> > [2] If this is really necessary then IMHO it makes sense to move mutex_lock/unlock +> > in helpers itself, right? > Yeah, locking in the helper makes imo sense. Hm, now I'm even more confused :) diff --git a/a/content_digest b/N1/content_digest index efaf820..65d94c3 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -7,35 +7,39 @@ "ref\020160315155940.GL14170@phenom.ffwll.local\0" "ref\01458246430.3134.14.camel@synopsys.com\0" "ref\020160318080257.GD14170@phenom.ffwll.local\0" - "From\0Alexey.Brodkin@synopsys.com (Alexey Brodkin)\0" - "Subject\0[PATCH 1/4 v3] drm: Add support of ARC PGU display controller\0" + "From\0Alexey Brodkin <Alexey.Brodkin@synopsys.com>\0" + "Subject\0Re: [PATCH 1/4 v3] drm: Add support of ARC PGU display controller\0" "Date\0Fri, 18 Mar 2016 08:11:49 +0000\0" - "To\0linux-snps-arc@lists.infradead.org\0" + "To\0daniel@ffwll.ch <daniel@ffwll.ch>\0" + "Cc\0dri-devel@lists.freedesktop.org <dri-devel@lists.freedesktop.org>" + linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org> + Jose.Abreu@synopsys.com <Jose.Abreu@synopsys.com> + " linux-snps-arc@lists.infradead.org <linux-snps-arc@lists.infradead.org>\0" "\00:1\0" "b\0" "Hi Daniel,\n" "\n" - "On Fri, 2016-03-18@09:02 +0100, Daniel Vetter wrote:\n" - "> On Thu, Mar 17, 2016@08:27:10PM +0000, Alexey Brodkin wrote:\n" + "On Fri, 2016-03-18 at 09:02 +0100, Daniel Vetter wrote:\n" + "> On Thu, Mar 17, 2016 at 08:27:10PM +0000, Alexey Brodkin wrote:\n" "> > \n" "> > Hi Daniel,\n" "> > \n" - "> > On Tue, 2016-03-15@16:59 +0100, Daniel Vetter wrote:\n" + "> > On Tue, 2016-03-15 at 16:59 +0100, Daniel Vetter wrote:\n" "> > > \n" - "> > > On Tue, Mar 15, 2016@03:24:46PM +0000, Alexey Brodkin wrote:\n" + "> > > On Tue, Mar 15, 2016 at 03:24:46PM +0000, Alexey Brodkin wrote:\n" "> > > > \n" - "> > > > On Tue, 2016-03-15@09:10 +0100, Daniel Vetter wrote:\n" + "> > > > On Tue, 2016-03-15 at 09:10 +0100, Daniel Vetter wrote:\n" "> > > > > \n" - "> > > > > On Mon, Mar 14, 2016@11:15:59AM +0000, Alexey Brodkin wrote:\n" + "> > > > > On Mon, Mar 14, 2016 at 11:15:59AM +0000, Alexey Brodkin wrote:\n" "> > > > > > \n" - "> > > > > > On Mon, 2016-03-14@08:00 +0100, Daniel Vetter wrote:\n" + "> > > > > > On Mon, 2016-03-14 at 08:00 +0100, Daniel Vetter wrote:\n" "> > > > > > > \n" - "> > > > > > > On Fri, Mar 11, 2016@06:42:36PM +0300, Alexey Brodkin wrote:\n" + "> > > > > > > On Fri, Mar 11, 2016 at 06:42:36PM +0300, Alexey Brodkin wrote:\n" "> > > > > > > > \n" - "> > > > > > > > ?\n" + "> > > > > > > > \302\240\n" "> > > > > > > > +static struct drm_driver arcpgu_drm_driver = {\n" "> > > > > > > > +\t.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |\n" - "> > > > > > > > +\t\t\t???DRIVER_ATOMIC,\n" + "> > > > > > > > +\t\t\t\302\240\302\240\302\240DRIVER_ATOMIC,\n" "> > > > > > > > +\t.preclose = arcpgu_preclose,\n" "> > > > > > > > +\t.lastclose = arcpgu_lastclose,\n" "> > > > > > > > +\t.name = \"drm-arcpgu\",\n" @@ -54,7 +58,7 @@ "> > > > > > Ok I took \"atmel-hlcdc\" as example.\n" "> > > > > > And that's interesting.\n" "> > > > > > \n" - "> > > > > > If I put my?arcpgu_load() in between?drm_dev_alloc() and\n" + "> > > > > > If I put my\302\240arcpgu_load() in between\302\240drm_dev_alloc() and\n" "> > > > > > drm_dev_register() then I'm getting this on the driver probe:\n" "> > > > > > ---------------------------------->8-------------------------------\n" "> > > > > > [drm] Initialized drm 1.1.0 20060810\n" @@ -66,23 +70,23 @@ "> > > > > > CPU: 0 PID: 1 Comm: swapper Not tainted 4.5.0-rc3-01062-ga447822-dirty #17\n" "> > > > > > \n" "> > > > > > Stack Trace:\n" - "> > > > > > ? arc_unwind_core.constprop.1+0xa4/0x110\n" - "> > > > > > ? warn_slowpath_fmt+0x6e/0xfc\n" - "> > > > > > ? kobject_add_internal+0x17c/0x498\n" - "> > > > > > ? kobject_add+0x98/0xe4\n" - "> > > > > > ? device_add+0xc6/0x734\n" - "> > > > > > ? device_create_with_groups+0x12a/0x144\n" - "> > > > > > ? drm_sysfs_connector_add+0x54/0xe8\n" - "> > > > > > ? arcpgu_drm_hdmi_init+0xd4/0x17c\n" - "> > > > > > ? arcpgu_probe+0x138/0x24c\n" - "> > > > > > ? platform_drv_probe+0x2e/0x6c\n" - "> > > > > > ? really_probe+0x212/0x35c\n" - "> > > > > > ? __driver_attach+0x90/0x94\n" - "> > > > > > ? bus_for_each_dev+0x46/0x80\n" - "> > > > > > ? bus_add_driver+0x14e/0x1b4\n" - "> > > > > > ? driver_register+0x64/0x108\n" - "> > > > > > ? do_one_initcall+0x86/0x194\n" - "> > > > > > ? kernel_init_freeable+0xf0/0x188\n" + "> > > > > > \302\240 arc_unwind_core.constprop.1+0xa4/0x110\n" + "> > > > > > \302\240 warn_slowpath_fmt+0x6e/0xfc\n" + "> > > > > > \302\240 kobject_add_internal+0x17c/0x498\n" + "> > > > > > \302\240 kobject_add+0x98/0xe4\n" + "> > > > > > \302\240 device_add+0xc6/0x734\n" + "> > > > > > \302\240 device_create_with_groups+0x12a/0x144\n" + "> > > > > > \302\240 drm_sysfs_connector_add+0x54/0xe8\n" + "> > > > > > \302\240 arcpgu_drm_hdmi_init+0xd4/0x17c\n" + "> > > > > > \302\240 arcpgu_probe+0x138/0x24c\n" + "> > > > > > \302\240 platform_drv_probe+0x2e/0x6c\n" + "> > > > > > \302\240 really_probe+0x212/0x35c\n" + "> > > > > > \302\240 __driver_attach+0x90/0x94\n" + "> > > > > > \302\240 bus_for_each_dev+0x46/0x80\n" + "> > > > > > \302\240 bus_add_driver+0x14e/0x1b4\n" + "> > > > > > \302\240 driver_register+0x64/0x108\n" + "> > > > > > \302\240 do_one_initcall+0x86/0x194\n" + "> > > > > > \302\240 kernel_init_freeable+0xf0/0x188\n" "> > > > > > ---[ end trace c67166ad43ddcce2 ]---\n" "> > > > > > [drm:drm_sysfs_connector_add] adding \"HDMI-A-1\" to sysfs\n" "> > > > > > [drm:drm_sysfs_connector_add] *ERROR* failed to register connector device: -2\n" @@ -98,12 +102,12 @@ "> > > > > register all the drm connectors _after_ calling drm_dev_register().\n" "> > > > > Totally forgot about that. Can you pls\n" "> > > > > - Extract a new drm_connector_register_all() function\n" - "> > > > > ? (atmel_hlcdc_dc_connector_plug_all seems to be the best template),\n" - "> > > > > ? including kerneldoc.\n" + "> > > > > \302\240 (atmel_hlcdc_dc_connector_plug_all seems to be the best template),\n" + "> > > > > \302\240 including kerneldoc.\n" "> > > > > - Adjust kerneldoc of drm_dev_register() to mention\n" - "> > > > > ? drm_connector_register_all() and that ordering constraint.\n" + "> > > > > \302\240 drm_connector_register_all() and that ordering constraint.\n" "> > > > > - Roll that helper out to all the drivers that currently hand-roll it (one\n" - "> > > > > ? patch per driver).\n" + "> > > > > \302\240 patch per driver).\n" "> > > > > \n" "> > > > > I know a bit of work but imo not too much, and by doing some small\n" "> > > > > refactoring every time someone stumbles over a drm pitfall we keep the\n" @@ -117,20 +121,20 @@ "> > > Awesome, looking forward to your patches.\n" "> > Sorry it took longer for me to finally put my hands on that work but anyways.\n" "> > \n" - "> > I'm looking now at how drivers use existing?drm_connector_unplug_all() and\n" - "> > their implementation of what would be?drm_connector_plug_all() and see\n" + "> > I'm looking now at how drivers use existing\302\240drm_connector_unplug_all() and\n" + "> > their implementation of what would be\302\240drm_connector_plug_all() and see\n" "> > in some implementations people wraps both helpers with\n" "> > mutex_{lock|unlock}(&dev->mode_config.mutex). But not everybody does this.\n" "> > \n" "> > So essentially my questions are:\n" - "> > ?[1] If it's necessary to get hold of that mutex before execution of either helper?\n" + "> > \302\240[1] If it's necessary to get hold of that mutex before execution of either helper?\n" "> In plug_all I think so, unplug_all has a FIXME comment about how locking\n" "> against sysfs is horrible and it's all going to blow up. But we did\n" "> recently change the connector sysfs files, so maybe that's fixed now. Not\n" "> sure.\n" ">\n" - "> > ?[2] If this is really necessary then IMHO it makes sense to move mutex_lock/unlock\n" - "> > ? ? ?in helpers itself, right?\n" + "> > \302\240[2] If this is really necessary then IMHO it makes sense to move mutex_lock/unlock\n" + "> > \302\240 \302\240 \302\240in helpers itself, right?\n" "> Yeah, locking in the helper makes imo sense.\n" "\n" "Hm, now I'm even more confused :)\n" @@ -151,4 +155,4 @@ "\n" -Alexey -3366d394f7958941a9d6fbe4b28ae62651c6d1120de564c636e932d982479665 +ebe5e1f5ce8286099532c576ea1e5e3d6055f8c66c060e301c8fced01ea3ca1c
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.