Re: [PATCH 1/4 v3] drm: Add support of ARC PGU display controller
On Fri, Mar 18, 2016 at 08:11:49AM +, Alexey Brodkin wrote: > Hi Daniel, > > On Fri, 2016-03-18 at 09:02 +0100, Daniel Vetter wrote: > > On Thu, Mar 17, 2016 at 08:27:10PM +, Alexey Brodkin wrote: > > > > > > Hi Daniel, > > > > > > On Tue, 2016-03-15 at 16:59 +0100, Daniel Vetter wrote: > > > > > > > > On Tue, Mar 15, 2016 at 03:24:46PM +, Alexey Brodkin wrote: > > > > > > > > > > On Tue, 2016-03-15 at 09:10 +0100, Daniel Vetter wrote: > > > > > > > > > > > > On Mon, Mar 14, 2016 at 11:15:59AM +, Alexey Brodkin wrote: > > > > > > > > > > > > > > On Mon, 2016-03-14 at 08:00 +0100, Daniel Vetter 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, > > > > > > > > > + .preclose = arcpgu_preclose, > > > > > > > > > + .lastclose = arcpgu_lastclose, > > > > > > > > > + .name = "drm-arcpgu", > > > > > > > > > + .desc = "ARC PGU Controller", > > > > > > > > > + .date = "20160219", > > > > > > > > > + .major = 1, > > > > > > > > > + .minor = 0, > > > > > > > > > + .patchlevel = 0, > > > > > > > > > + .fops = &arcpgu_drm_ops, > > > > > > > > > + .load = arcpgu_load, > > > > > > > > > + .unload = arcpgu_unload, > > > > > > > > Load and unload hooks are deprecated (it's a classic midlayer > > > > > > > > mistake). > > > > > > > > Please use drm_dev_alloc/register pairs directly instead, and > > > > > > > > put your > > > > > > > > device setup code in-between. Similar for unloading. There's a > > > > > > > > bunch of > > > > > > > > example drivers converted already. > > > > > > > Ok I took "atmel-hlcdc" as example. > > > > > > > And that's interesting. > > > > > > > > > > > > > > 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 > > > > > > > arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab > > > > > > > [ cut here ] > > > > > > > WARNING: CPU: 0 PID: 1 at lib/kobject.c:244 > > > > > > > kobject_add_internal+0x17c/0x498() > > > > > > > kobject_add_internal failed for card0-HDMI-A-1 (error: -2 parent: > > > > > > > card0) > > > > > > > Modules linked in: > > > > > > > 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 > > > > > > > ---[ 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 > > > > > > > arcpgu e0017000.pgu: failed to regiter DRM connector and helper > > > > > > > funcs > > > > > > > arcpgu: probe of e0017000.pgu failed with error -2 > > > > > > > -->8--- > > > > > > > > > > > > > > But if I move arcpgu_load() after drm_dev_register() then > > > > > > > everything > > > > > > > starts properly and I may see HDMI screen works perfectly fine. > > > > > > > > > > > > > > Any thoughts? > > > > > > Oops, yeah missed that detail. If you look at atmel it has a loop to > > > > > > 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. > > > > > > - Adjust kerneldoc of drm_dev_register() to mention > > > > > > 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). > > > > > > > > > > > > I know a bit of work but imo not too much, and by doing some small > > > > > > refac
Re: [PATCH 2/4] drm: Add DT bindings documentation for ARC PGU display controller
On Thu, Mar 3, 2016 at 7:58 AM, Alexey Brodkin wrote: > Hi Rob, > > On Tue, 2016-02-23 at 14:38 -0600, Rob Herring wrote: >> On Fri, Feb 19, 2016 at 04:03:52PM +0300, Alexey Brodkin wrote: >> > >> > This add DT bindings documentation for ARC PGU display controller. >> > >> > Signed-off-by: Alexey Brodkin >> > Cc: Rob Herring >> > Cc: Pawel Moll >> > Cc: Mark Rutland >> > Cc: Ian Campbell >> > Cc: Kumar Gala >> > Cc: devicet...@vger.kernel.org >> > Cc: linux-snps-arc@lists.infradead.org >> > --- >> > .../devicetree/bindings/display/snps,arcpgu.txt| 74 >> > ++ >> > 1 file changed, 74 insertions(+) >> > create mode 100644 >> > Documentation/devicetree/bindings/display/snps,arcpgu.txt >> > >> > diff --git a/Documentation/devicetree/bindings/display/snps,arcpgu.txt >> > b/Documentation/devicetree/bindings/display/snps,arcpgu.txt >> > new file mode 100644 >> > index 000..c8382fb >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/display/snps,arcpgu.txt >> > @@ -0,0 +1,74 @@ >> > +ARC PGU >> > + >> > +This is a display controller found on several development boards produced >> > +by Synopsys. The ARC PGU is an RGB streamer that reads the data from a >> > +framebuffer and sends it to a single digital encoder (usually HDMI). >> > + >> > +Required properties: >> > + - compatible: "snps,arcpgu" >> Seems like this should be more specific. Is there some sort or >> versioning with ARC blocks? > > Well as of today there's only one and only version of PGU. > So is there a real need for "snps,arcpgu-1.0"? > >> > >> > + - reg: Physical base address and length of the controller's registers. >> > + - clocks: A list of phandle + clock-specifier pairs, one for each >> > +entry in 'clock-names'. >> > + - clock-names: A list of clock names. For ARC PGU it should contain: >> > + - "pxlclk" for the clock feeding the output PLL of the controller. >> > + - encoder-slave: Phandle of encoder chip. >> This is unnecessary with the OF graph. > > Do you mean I may drop "encoder-slave" from bindings description? Yes, you should drop it. > I actually thought about that because in case of simulation platform where > this device is also used there's no encoder as well as no connector - we're > dealing with memory area which is read by host and then displayed on host's > display. > ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2] ARC: axs10x - add Ethernet PHY description in .dts
Hi Sergei, On Thu, 2016-03-17 at 13:58 +0300, Sergei Shtylyov wrote: > On 3/17/2016 12:41 PM, Alexey Brodkin wrote: > > > > > Following commit broke DW GMAC functionality on AXS10x boards: > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e34d65696d2ef13dc32f2a162556c86c461ed763 > Note that scripts/checkpatch.pl now enforces certain format for citing > commits: commit <12-digit SHA1> (""). Frankly I haven't run that patch through checkpatch due to patch simplicity. But I'll try to not do any assumptions from now on and will try to use checkpatch for each and every thing I send :) Thanks for spotting all his! -Alexey ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 1/4 v3] drm: Add support of ARC PGU display controller
On Thu, Mar 17, 2016 at 08:27:10PM +, Alexey Brodkin wrote: > Hi Daniel, > > On Tue, 2016-03-15 at 16:59 +0100, Daniel Vetter wrote: > > On Tue, Mar 15, 2016 at 03:24:46PM +, Alexey Brodkin wrote: > > > On Tue, 2016-03-15 at 09:10 +0100, Daniel Vetter wrote: > > > > On Mon, Mar 14, 2016 at 11:15:59AM +, Alexey Brodkin wrote: > > > > > On Mon, 2016-03-14 at 08:00 +0100, Daniel Vetter 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, > > > > > > > + .preclose = arcpgu_preclose, > > > > > > > + .lastclose = arcpgu_lastclose, > > > > > > > + .name = "drm-arcpgu", > > > > > > > + .desc = "ARC PGU Controller", > > > > > > > + .date = "20160219", > > > > > > > + .major = 1, > > > > > > > + .minor = 0, > > > > > > > + .patchlevel = 0, > > > > > > > + .fops = &arcpgu_drm_ops, > > > > > > > + .load = arcpgu_load, > > > > > > > + .unload = arcpgu_unload, > > > > > > Load and unload hooks are deprecated (it's a classic midlayer > > > > > > mistake). > > > > > > Please use drm_dev_alloc/register pairs directly instead, and put > > > > > > your > > > > > > device setup code in-between. Similar for unloading. There's a > > > > > > bunch of > > > > > > example drivers converted already. > > > > > Ok I took "atmel-hlcdc" as example. > > > > > And that's interesting. > > > > > > > > > > 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 > > > > > arcpgu e0017000.pgu: arc_pgu ID: 0xabbabaab > > > > > [ cut here ] > > > > > WARNING: CPU: 0 PID: 1 at lib/kobject.c:244 > > > > > kobject_add_internal+0x17c/0x498() > > > > > kobject_add_internal failed for card0-HDMI-A-1 (error: -2 parent: > > > > > card0) > > > > > Modules linked in: > > > > > 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 > > > > > ---[ 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 > > > > > arcpgu e0017000.pgu: failed to regiter DRM connector and helper funcs > > > > > arcpgu: probe of e0017000.pgu failed with error -2 > > > > > -->8--- > > > > > > > > > > But if I move arcpgu_load() after drm_dev_register() then everything > > > > > starts properly and I may see HDMI screen works perfectly fine. > > > > > > > > > > Any thoughts? > > > > Oops, yeah missed that detail. If you look at atmel it has a loop to > > > > 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. > > > > - Adjust kerneldoc of drm_dev_register() to mention > > > > 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). > > > > > > > > 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 > > > > subsystem clean&easy to understand. You're up for this? Would be a prep > > > > series, I'll happily review it to get it merged fast. Just a few weeks > > > > ago > > > > I merged 20+ patches to make ->mode_fixup hooks optional and remove > > > > dummy > > > > ones all over the subsystem, in other words: You'll have my full > > > > attention > > > > ;-) > > > Sure, I'm ready to pay that price :) > > > Stay tuned and patches will follow. > > 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
Re: [PATCH] ARC: build: Turn off -Wmaybe-uninitialized for ARC gcc 4.8
On Friday 18 March 2016 03:22 PM, Arnd Bergmann wrote: > On Friday 18 March 2016 14:16:23 Vineet Gupta wrote: >> diff --git a/arch/arc/Makefile b/arch/arc/Makefile >> index fed12f39d8ce..aeb101e8e674 100644 >> --- a/arch/arc/Makefile >> +++ b/arch/arc/Makefile >> @@ -48,9 +48,14 @@ endif >> upto_gcc44:= $(call cc-ifversion, -le, 0404, y) >> atleast_gcc44 := $(call cc-ifversion, -ge, 0404, y) >> atleast_gcc48 := $(call cc-ifversion, -ge, 0408, y) >> +is_gcc48 := $(call cc-ifversion, -eq, 0408, y) >> >> cflags-$(atleast_gcc44)+= -fsection-anchors >> >> +# gcc 4.8 spits out false positives for default -O3 >> +# disable these for 4.8 and revisit when we upgrade to newer ver >> +cflags-$(is_gcc48) += $(call >> cc-disable-warning,maybe-uninitialized,) >> + >> cflags-$(CONFIG_ARC_HAS_LLSC) += -mlock >> cflags-$(CONFIG_ARC_HAS_SWAPE) += -mswape > > Is this any better with gcc-4.9 or gcc-5? I don't think there's a production ARC toolchain with gcc 4.9 which we can use yet - Claudiu is still in the the middle of upstreaming the new ARC HS port bits so things are still in flight there. These tools are off of github ! Maybe it's better to add the flag to > the line that adds -O3 for consistency. We do the same thing for -Os in the > global Makefile, as that triggers a similar load of warnings. Sure, but I prefer this to be only for gcc 4.8 as this warning seems to be healthy in small doses :-) At least it keeps the door open for future discussion with gcc guys ! The following nested construct actually works - does that look OK to you ? ARCH_CFLAGS += -O3 $(call cc-ifversion, -lt, 0408, $(call cc-disable-warning,maybe-uninitialized,)) Thx, -Vineet > > Arnd > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc