Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCH v3 6/7] OMAPDSS: DPI: Add support for multiple instances
From: Archit Taneja @ 2014-06-04  6:53 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1401864063-19196-1-git-send-email-archit@ti.com>

Register DPI outputs, and assign the port_num to them as specified by the
'reg' property in the DPI ports in DT.

To support multiple DPI instances, dpi_get_channel needs to take the DPI
instance's reg-id to get the corresponding channel. Make it take this
argument.We just pass 0 in the non-DT path, since we don't support multiple
instances in the non-DT case.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/fbdev/omap2/dss/dpi.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/omap2/dss/dpi.c b/drivers/video/fbdev/omap2/dss/dpi.c
index b579022..3e204a8 100644
--- a/drivers/video/fbdev/omap2/dss/dpi.c
+++ b/drivers/video/fbdev/omap2/dss/dpi.c
@@ -616,7 +616,7 @@ static void dpi_init_pll(struct dpi_data *dpi)
  * the channel in some more dynamic manner, or get the channel as a user
  * parameter.
  */
-static enum omap_channel dpi_get_channel(void)
+static enum omap_channel dpi_get_channel(int reg)
 {
 	switch (omapdss_get_version()) {
 	case OMAPDSS_VER_OMAP24xx:
@@ -710,7 +710,7 @@ static void dpi_init_output(struct platform_device *pdev)
 	out->id = OMAP_DSS_OUTPUT_DPI;
 	out->output_type = OMAP_DISPLAY_TYPE_DPI;
 	out->name = "dpi.0";
-	out->dispc_channel = dpi_get_channel();
+	out->dispc_channel = dpi_get_channel(0);
 	out->ops.dpi = &dpi_ops;
 	out->owner = THIS_MODULE;
 
@@ -730,11 +730,31 @@ static void dpi_init_output_port(struct platform_device *pdev,
 {
 	struct dpi_data *dpi = port->data;
 	struct omap_dss_device *out = &dpi->output;
+	int r;
+	u32 reg;
+
+	r = of_property_read_u32(port, "reg", &reg);
+	if (r)
+		reg = 0;
+
+	switch (reg) {
+	case 2:
+		out->name = "dpi.2";
+		break;
+	case 1:
+		out->name = "dpi.1";
+		break;
+	case 0:
+	default:
+		out->name = "dpi.0";
+		break;
+	}
 
 	out->dev = &pdev->dev;
 	out->id = OMAP_DSS_OUTPUT_DPI;
 	out->output_type = OMAP_DISPLAY_TYPE_DPI;
-	out->dispc_channel = dpi_get_channel();
+	out->dispc_channel = dpi_get_channel(reg);
+	out->port_num = reg;
 	out->ops.dpi = &dpi_ops;
 	out->owner = THIS_MODULE;
 
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH v3 7/7] omapdss: DSS: add reg-id param to dpi_select_source
From: Archit Taneja @ 2014-06-04  6:53 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1401864063-19196-1-git-send-email-archit@ti.com>

Add an argument which describes which DPI instance we are referring to when
selecting it's overlay manager.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/fbdev/omap2/dss/dpi.c |  2 +-
 drivers/video/fbdev/omap2/dss/dss.c | 12 ++++++------
 drivers/video/fbdev/omap2/dss/dss.h |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/video/fbdev/omap2/dss/dpi.c b/drivers/video/fbdev/omap2/dss/dpi.c
index 3e204a8..776c409 100644
--- a/drivers/video/fbdev/omap2/dss/dpi.c
+++ b/drivers/video/fbdev/omap2/dss/dpi.c
@@ -396,7 +396,7 @@ static int dpi_display_enable(struct omap_dss_device *dssdev)
 	if (r)
 		goto err_get_dispc;
 
-	r = dss_dpi_select_source(out->manager->id);
+	r = dss_dpi_select_source(out->port_num, out->manager->id);
 	if (r)
 		goto err_src_sel;
 
diff --git a/drivers/video/fbdev/omap2/dss/dss.c b/drivers/video/fbdev/omap2/dss/dss.c
index 3b41953..8fd141f 100644
--- a/drivers/video/fbdev/omap2/dss/dss.c
+++ b/drivers/video/fbdev/omap2/dss/dss.c
@@ -72,7 +72,7 @@ struct dss_features {
 	const char *parent_clk_name;
 	enum omap_display_type *ports;
 	int num_ports;
-	int (*dpi_select_source)(enum omap_channel channel);
+	int (*dpi_select_source)(int id, enum omap_channel channel);
 };
 
 static struct {
@@ -566,7 +566,7 @@ enum dss_hdmi_venc_clk_source_select dss_get_hdmi_venc_clk_source(void)
 	return REG_GET(DSS_CONTROL, 15, 15);
 }
 
-static int dss_dpi_select_source_omap2_omap3(enum omap_channel channel)
+static int dss_dpi_select_source_omap2_omap3(int id, enum omap_channel channel)
 {
 	if (channel != OMAP_DSS_CHANNEL_LCD)
 		return -EINVAL;
@@ -574,7 +574,7 @@ static int dss_dpi_select_source_omap2_omap3(enum omap_channel channel)
 	return 0;
 }
 
-static int dss_dpi_select_source_omap4(enum omap_channel channel)
+static int dss_dpi_select_source_omap4(int id, enum omap_channel channel)
 {
 	int val;
 
@@ -594,7 +594,7 @@ static int dss_dpi_select_source_omap4(enum omap_channel channel)
 	return 0;
 }
 
-static int dss_dpi_select_source_omap5(enum omap_channel channel)
+static int dss_dpi_select_source_omap5(int id, enum omap_channel channel)
 {
 	int val;
 
@@ -620,9 +620,9 @@ static int dss_dpi_select_source_omap5(enum omap_channel channel)
 	return 0;
 }
 
-int dss_dpi_select_source(enum omap_channel channel)
+int dss_dpi_select_source(int id, enum omap_channel channel)
 {
-	return dss.feat->dpi_select_source(channel);
+	return dss.feat->dpi_select_source(id, channel);
 }
 
 static int dss_get_clocks(void)
diff --git a/drivers/video/fbdev/omap2/dss/dss.h b/drivers/video/fbdev/omap2/dss/dss.h
index 849ede1..a7a0d8a 100644
--- a/drivers/video/fbdev/omap2/dss/dss.h
+++ b/drivers/video/fbdev/omap2/dss/dss.h
@@ -209,7 +209,7 @@ int dss_init_platform_driver(void) __init;
 void dss_uninit_platform_driver(void);
 
 unsigned long dss_get_dispc_clk_rate(void);
-int dss_dpi_select_source(enum omap_channel channel);
+int dss_dpi_select_source(int id, enum omap_channel channel);
 void dss_select_hdmi_venc_clk_source(enum dss_hdmi_venc_clk_source_select);
 enum dss_hdmi_venc_clk_source_select dss_get_hdmi_venc_clk_source(void);
 const char *dss_get_generic_clk_source_name(enum omap_dss_clk_source clk_src);
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 0/10] use safer test on the result of find_first_zero_bit
From: Julia Lawall @ 2014-06-04  9:07 UTC (permalink / raw)
  To: linux-rdma
  Cc: devel, linux-s390, linux-fbdev, linux-scsi, iss_storagedev,
	linux-sh, kernel-janitors, linux-wireless, linux-kernel, ath10k,
	adi-buildroot-devel, netdev

Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
return a larger number than the maximum position argument if that position
is not a multiple of BITS_PER_LONG.


^ permalink raw reply

* [PATCH 3/10] video: use safer test on the result of find_first_zero_bit
From: Julia Lawall @ 2014-06-04  9:07 UTC (permalink / raw)
  To: Jean-Christophe Plagniol-Villard
  Cc: kernel-janitors, Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <1401872880-23685-1-git-send-email-Julia.Lawall@lip6.fr>

From: Julia Lawall <Julia.Lawall@lip6.fr>

Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
return a larger number than the maximum position argument if that position
is not a multiple of BITS_PER_LONG.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression e1,e2,e3;
statement S1,S2;
@@

e1 = find_first_zero_bit(e2,e3)
...
if (e1 
- =
+ >  e3)
S1 else S2
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/video/fbdev/sh_mobile_meram.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -u -p a/drivers/video/fbdev/sh_mobile_meram.c b/drivers/video/fbdev/sh_mobile_meram.c
--- a/drivers/video/fbdev/sh_mobile_meram.c
+++ b/drivers/video/fbdev/sh_mobile_meram.c
@@ -222,7 +222,7 @@ static int meram_plane_alloc(struct sh_m
 	unsigned long idx;
 
 	idx = find_first_zero_bit(&priv->used_icb, 28);
-	if (idx = 28)
+	if (idx >= 28)
 		return -ENOMEM;
 	plane->cache = &priv->icbs[idx];
 


^ permalink raw reply

* Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit
From: Geert Uytterhoeven @ 2014-06-04  9:35 UTC (permalink / raw)
  To: Julia Lawall
  Cc: driverdevel, linux-s390, Linux Fbdev development list, scsi,
	iss_storagedev, Linux-sh list, linux-rdma, linux-wireless,
	kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
	ath10k, adi-buildroot-devel, netdev@vger.kernel.org
In-Reply-To: <1401872880-23685-1-git-send-email-Julia.Lawall@lip6.fr>

Hi Julia,

On Wed, Jun 4, 2014 at 11:07 AM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
> return a larger number than the maximum position argument if that position
> is not a multiple of BITS_PER_LONG.

Shouldn't this be fixed in find_first_zero_bit() instead?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit
From: Julia Lawall @ 2014-06-04  9:38 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Julia Lawall, linux-rdma,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Fbdev development list, Linux-sh list,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-wireless,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, driverdevel,
	iss_storagedev-VXdhtT5mjnY, scsi, linux-s390,
	adi-buildroot-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <CAMuHMdXsoFUXj4jLMWN6NY7Um19KwWO9Rhx=9=gqWu_K5Ev2kQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>



On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:

> Hi Julia,
>
> On Wed, Jun 4, 2014 at 11:07 AM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> > Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
> > return a larger number than the maximum position argument if that position
> > is not a multiple of BITS_PER_LONG.
>
> Shouldn't this be fixed in find_first_zero_bit() instead?

OK, I could do that as well.  Most of the callers currently test with >=.
Should they be left as is, or changed to use =?

julia

^ permalink raw reply

* Re: [PATCH] drm/i915: Kick out vga console
From: David Herrmann @ 2014-06-04  9:38 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: linux-fbdev@vger.kernel.org, Intel Graphics Development,
	DRI Development, Tomi Valkeinen, Jean-Christophe Plagniol-Villard
In-Reply-To: <1401836249-4922-1-git-send-email-daniel.vetter@ffwll.ch>

Hi

On Wed, Jun 4, 2014 at 12:57 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> Touching the VGA resources on an IVB EFI machine causes hard hangs when
> we then kick out the efifb. Ouch.
>
> Apparently this also prevents unclaimed register errors on hsw and
> hard machine hangs on my i855gm when trying to unbind fbcon.
>
> Also, we want this to make I915_FBDEV=n safe.
>
> v2: Rebase and pimp commit message.
>
> v3: We also need to unregister the vga console, otherwise the unbind
> of the fb console before module unload might resurrect it again.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?idg813
> Cc: David Herrmann <dh.herrmann@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_dma.c  | 34 +++++++++++++++++++++++++++++++++-
>  drivers/video/console/dummycon.c |  1 +
>  drivers/video/console/vgacon.c   |  1 +
>  3 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index b9159ade5e85..a4df80740b37 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -36,6 +36,8 @@
>  #include "i915_drv.h"
>  #include "i915_trace.h"
>  #include <linux/pci.h>
> +#include <linux/console.h>
> +#include <linux/vt.h>
>  #include <linux/vgaarb.h>
>  #include <linux/acpi.h>
>  #include <linux/pnp.h>
> @@ -1450,6 +1452,29 @@ static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv)
>  }
>  #endif
>
> +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> +{
> +#if !defined(CONFIG_VGA_CONSOLE)
> +       return 0;
> +#else
> +       int ret;
> +
> +#if !defined(CONFIG_DUMMY_CONSOLE)
> +       return -ENODEV;
> +#endif
> +
> +       DRM_INFO("Replacing VGA console driver\n");
> +
> +       console_lock();
> +       ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);

You rely on compiler-optimizations here. "dummy_con" is not available
if !CONFIG_DUMMY_CONSOLE, but you use it. This causes linker-failure
if dead-code elimination is not done (-O0).

Thanks
David

> +       if (ret = 0)
> +               ret = do_unregister_con_driver(&vga_con);
> +       console_unlock();
> +
> +       return ret;
> +#endif
> +}
> +
>  static void i915_dump_device_info(struct drm_i915_private *dev_priv)
>  {
>         const struct intel_device_info *info = &dev_priv->info;
> @@ -1623,8 +1648,15 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>         if (ret)
>                 goto out_regs;
>
> -       if (drm_core_check_feature(dev, DRIVER_MODESET))
> +       if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> +               ret = i915_kick_out_vgacon(dev_priv);
> +               if (ret) {
> +                       DRM_ERROR("failed to remove conflicting VGA console\n");
> +                       goto out_gtt;
> +               }
> +
>                 i915_kick_out_firmware_fb(dev_priv);
> +       }
>
>         pci_set_master(dev->pdev);
>
> diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
> index b63860f7beab..40bec8d64b0a 100644
> --- a/drivers/video/console/dummycon.c
> +++ b/drivers/video/console/dummycon.c
> @@ -77,3 +77,4 @@ const struct consw dummy_con = {
>      .con_set_palette = DUMMY,
>      .con_scrolldelta = DUMMY,
>  };
> +EXPORT_SYMBOL_GPL(dummy_con);
> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
> index 9d8feac67637..84acd6223dc5 100644
> --- a/drivers/video/console/vgacon.c
> +++ b/drivers/video/console/vgacon.c
> @@ -1440,5 +1440,6 @@ const struct consw vga_con = {
>         .con_build_attr = vgacon_build_attr,
>         .con_invert_region = vgacon_invert_region,
>  };
> +EXPORT_SYMBOL(vga_con);
>
>  MODULE_LICENSE("GPL");
> --
> 1.8.1.4
>

^ permalink raw reply

* RE: [PATCH 0/10] use safer test on the result of find_first_zero_bit
From: David Laight @ 2014-06-04  9:46 UTC (permalink / raw)
  To: 'Julia Lawall', Geert Uytterhoeven
  Cc: linux-rdma,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Fbdev development list, Linux-sh list,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-wireless, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	driverdevel, iss_storagedev-VXdhtT5mjnY@public.gmane.org, scsi,
	linux-s390,
	adi-buildroot-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
In-Reply-To: <alpine.DEB.2.10.1406041137270.2441@hadrien>

From: Julia Lawall
> On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:
> 
> > Hi Julia,
> >
> > On Wed, Jun 4, 2014 at 11:07 AM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> > > Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
> > > return a larger number than the maximum position argument if that position
> > > is not a multiple of BITS_PER_LONG.
> >
> > Shouldn't this be fixed in find_first_zero_bit() instead?
> 
> OK, I could do that as well.  Most of the callers currently test with >=.
> Should they be left as is, or changed to use =?

Do we want to add an extra test to find_first_zero_bit() and effectively
slow down all the calls - especially those where the length is a
multiple of 8 (probably the most common).

Maybe the documented return code should be changed to allow for the
existing behaviour.

	David




^ permalink raw reply

* RE: [PATCH 0/10] use safer test on the result of find_first_zero_bit
From: Julia Lawall @ 2014-06-04  9:52 UTC (permalink / raw)
  To: David Laight
  Cc: 'Julia Lawall', Geert Uytterhoeven, linux-rdma,
	kernel-janitors@vger.kernel.org, Linux Fbdev development list,
	Linux-sh list, linux-kernel@vger.kernel.org,
	ath10k@lists.infradead.org, linux-wireless,
	netdev@vger.kernel.org, driverdevel, iss_storagedev@hp.com, scsi,
	linux-s390, adi-buildroot-devel@lists.sourceforge.net
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1725705F@AcuExch.aculab.com>



On Wed, 4 Jun 2014, David Laight wrote:

> From: Julia Lawall
> > On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:
> >
> > > Hi Julia,
> > >
> > > On Wed, Jun 4, 2014 at 11:07 AM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> > > > Find_first_zero_bit considers BITS_PER_LONG bits at a time, and thus may
> > > > return a larger number than the maximum position argument if that position
> > > > is not a multiple of BITS_PER_LONG.
> > >
> > > Shouldn't this be fixed in find_first_zero_bit() instead?
> >
> > OK, I could do that as well.  Most of the callers currently test with >=.
> > Should they be left as is, or changed to use =?
>
> Do we want to add an extra test to find_first_zero_bit() and effectively
> slow down all the calls - especially those where the length is a
> multiple of 8 (probably the most common).

Currently, most of the calls test with >=, and most of the others seem to
need to (either the size value did not look like a multiple of anything in
particular, or it was eg read from a device).

Note that it is BITS_PER_LONG, so it seems like it is typically 32 or 64,
not 8.

> Maybe the documented return code should be changed to allow for the
> existing behaviour.

Sorry, I'm not sure to understand what you suggest here.

thanks,
julia

^ permalink raw reply

* Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit
From: Geert Uytterhoeven @ 2014-06-04 10:55 UTC (permalink / raw)
  To: Julia Lawall
  Cc: driverdevel, linux-s390, Linux Fbdev development list, scsi,
	iss_storagedev@hp.com, Linux-sh list, linux-rdma, linux-wireless,
	kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
	ath10k@lists.infradead.org,
	adi-buildroot-devel@lists.sourceforge.net, David Laight,
	Arnd Bergmann, netdev@vger.kernel.org
In-Reply-To: <alpine.DEB.2.10.1406041148590.2441@hadrien>

Hi Julia,

On Wed, Jun 4, 2014 at 11:52 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> Maybe the documented return code should be changed to allow for the
>> existing behaviour.
>
> Sorry, I'm not sure to understand what you suggest here.

include/asm-generic/bitops/find.h:

| /**
|  * find_first_zero_bit - find the first cleared bit in a memory region
|  * @addr: The address to start the search at
|  * @size: The maximum number of bits to search
|  *
|  * Returns the bit number of the first cleared bit.
|  * If no bits are zero, returns @size.

"If no bits are zero, returns @size or a number larger than @size."

|  */
| extern unsigned long find_first_zero_bit(const unsigned long *addr,
|                                          unsigned long size);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit
From: Julia Lawall @ 2014-06-04 11:00 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: driverdevel, linux-s390, Linux Fbdev development list, scsi,
	iss_storagedev@hp.com, Linux-sh list, linux-rdma, linux-wireless,
	kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
	ath10k@lists.infradead.org,
	adi-buildroot-devel@lists.sourceforge.net, Julia Lawall,
	David Laight, Arnd Bergmann, netdev@vger.kernel.org
In-Reply-To: <CAMuHMdXPBQTYA0c=1hkC7943-_J59TVSFaEKsfY1F2ogV=Ld+A@mail.gmail.com>



On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:

> Hi Julia,
>
> On Wed, Jun 4, 2014 at 11:52 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >> Maybe the documented return code should be changed to allow for the
> >> existing behaviour.
> >
> > Sorry, I'm not sure to understand what you suggest here.
>
> include/asm-generic/bitops/find.h:
>
> | /**
> |  * find_first_zero_bit - find the first cleared bit in a memory region
> |  * @addr: The address to start the search at
> |  * @size: The maximum number of bits to search
> |  *
> |  * Returns the bit number of the first cleared bit.
> |  * If no bits are zero, returns @size.
>
> "If no bits are zero, returns @size or a number larger than @size."

OK, thanks.  I was only looking at the C code.

But the C code contains a loop that is followed by:

        if (!size)
                return result;
        tmp = *p;

found_first:
        tmp |= ~0UL << size;
        if (tmp = ~0UL)        /* Are any bits zero? */
                return result + size;   /* Nope. */

In the first return, it would seem that result = size.  Could the second
one be changed to just return size?  It should not hurt performance.

julia

>
> |  */
> | extern unsigned long find_first_zero_bit(const unsigned long *addr,
> |                                          unsigned long size);
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
>

^ permalink raw reply

* Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit
From: Geert Uytterhoeven @ 2014-06-04 11:07 UTC (permalink / raw)
  To: Julia Lawall
  Cc: David Laight, linux-rdma,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux Fbdev development list, Linux-sh list,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-wireless, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	driverdevel, iss_storagedev-VXdhtT5mjnY@public.gmane.org, scsi,
	linux-s390,
	adi-buildroot-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	Arnd Bergmann
In-Reply-To: <alpine.DEB.2.10.1406041258200.2441@hadrien>

Hi Julia,

On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> OK, thanks.  I was only looking at the C code.
>
> But the C code contains a loop that is followed by:
>
>         if (!size)
>                 return result;
>         tmp = *p;
>
> found_first:
>         tmp |= ~0UL << size;
>         if (tmp = ~0UL)        /* Are any bits zero? */
>                 return result + size;   /* Nope. */
>
> In the first return, it would seem that result = size.  Could the second
> one be changed to just return size?  It should not hurt performance.

"size" may have been changed between function entry and this line.
So you have to store it in a temporary.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH] drm/i915: Kick out vga console
From: Jani Nikula @ 2014-06-04 12:20 UTC (permalink / raw)
  To: David Herrmann, Daniel Vetter
  Cc: linux-fbdev@vger.kernel.org, Tomi Valkeinen,
	Jean-Christophe Plagniol-Villard, Intel Graphics Development,
	DRI Development
In-Reply-To: <CANq1E4Qxpu7UecPHxdu_VzU=kADR_hijFXxd48ag_rbx7jo8mg@mail.gmail.com>

On Wed, 04 Jun 2014, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Wed, Jun 4, 2014 at 12:57 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> From: Chris Wilson <chris@chris-wilson.co.uk>
>>
>> Touching the VGA resources on an IVB EFI machine causes hard hangs when
>> we then kick out the efifb. Ouch.
>>
>> Apparently this also prevents unclaimed register errors on hsw and
>> hard machine hangs on my i855gm when trying to unbind fbcon.
>>
>> Also, we want this to make I915_FBDEV=n safe.
>>
>> v2: Rebase and pimp commit message.
>>
>> v3: We also need to unregister the vga console, otherwise the unbind
>> of the fb console before module unload might resurrect it again.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?idg813
>> Cc: David Herrmann <dh.herrmann@gmail.com>
>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: linux-fbdev@vger.kernel.org
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>  drivers/gpu/drm/i915/i915_dma.c  | 34 +++++++++++++++++++++++++++++++++-
>>  drivers/video/console/dummycon.c |  1 +
>>  drivers/video/console/vgacon.c   |  1 +
>>  3 files changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index b9159ade5e85..a4df80740b37 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -36,6 +36,8 @@
>>  #include "i915_drv.h"
>>  #include "i915_trace.h"
>>  #include <linux/pci.h>
>> +#include <linux/console.h>
>> +#include <linux/vt.h>
>>  #include <linux/vgaarb.h>
>>  #include <linux/acpi.h>
>>  #include <linux/pnp.h>
>> @@ -1450,6 +1452,29 @@ static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv)
>>  }
>>  #endif
>>
>> +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
>> +{
>> +#if !defined(CONFIG_VGA_CONSOLE)
>> +       return 0;
>> +#else
>> +       int ret;
>> +
>> +#if !defined(CONFIG_DUMMY_CONSOLE)
>> +       return -ENODEV;
>> +#endif
>> +
>> +       DRM_INFO("Replacing VGA console driver\n");
>> +
>> +       console_lock();
>> +       ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
>
> You rely on compiler-optimizations here. "dummy_con" is not available
> if !CONFIG_DUMMY_CONSOLE, but you use it. This causes linker-failure
> if dead-code elimination is not done (-O0).

Nested #ifs too. How about

#if !defined(CONFIG_VGA_CONSOLE)
static inline int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
{
	return 0;
}
#elif !defined(CONFIG_DUMMY_CONSOLE)
static inline int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
{
	return -ENODEV;
}
#else
static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
{
	/* ... */
}
#endif

in proper kernel style?

BR,
Jani.


>
> Thanks
> David
>
>> +       if (ret = 0)
>> +               ret = do_unregister_con_driver(&vga_con);
>> +       console_unlock();
>> +
>> +       return ret;
>> +#endif
>> +}
>> +
>>  static void i915_dump_device_info(struct drm_i915_private *dev_priv)
>>  {
>>         const struct intel_device_info *info = &dev_priv->info;
>> @@ -1623,8 +1648,15 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>>         if (ret)
>>                 goto out_regs;
>>
>> -       if (drm_core_check_feature(dev, DRIVER_MODESET))
>> +       if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>> +               ret = i915_kick_out_vgacon(dev_priv);
>> +               if (ret) {
>> +                       DRM_ERROR("failed to remove conflicting VGA console\n");
>> +                       goto out_gtt;
>> +               }
>> +
>>                 i915_kick_out_firmware_fb(dev_priv);
>> +       }
>>
>>         pci_set_master(dev->pdev);
>>
>> diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
>> index b63860f7beab..40bec8d64b0a 100644
>> --- a/drivers/video/console/dummycon.c
>> +++ b/drivers/video/console/dummycon.c
>> @@ -77,3 +77,4 @@ const struct consw dummy_con = {
>>      .con_set_palette = DUMMY,
>>      .con_scrolldelta = DUMMY,
>>  };
>> +EXPORT_SYMBOL_GPL(dummy_con);
>> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
>> index 9d8feac67637..84acd6223dc5 100644
>> --- a/drivers/video/console/vgacon.c
>> +++ b/drivers/video/console/vgacon.c
>> @@ -1440,5 +1440,6 @@ const struct consw vga_con = {
>>         .con_build_attr = vgacon_build_attr,
>>         .con_invert_region = vgacon_invert_region,
>>  };
>> +EXPORT_SYMBOL(vga_con);
>>
>>  MODULE_LICENSE("GPL");
>> --
>> 1.8.1.4
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply

* [GIT PULL] main fbdev changes for 3.16
From: Tomi Valkeinen @ 2014-06-04 12:27 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, Linux Fbdev development list

[-- Attachment #1: Type: text/plain, Size: 4606 bytes --]

Hi Linus,

Please pull the main fbdev changes for 3.16.

 Tomi

The following changes since commit a798c10faf62a505d24e5f6213fbaf904a39623f:

  Linux 3.15-rc2 (2014-04-20 11:08:50 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git tags/fbdev-main-3.16

for you to fetch changes up to fbc6c4a13bbfb420eedfdb26a0a859f9c07e8a7b:

  drivers/video/fbdev/fb-puv3.c: Add header files for function unifb_mmap (2014-05-23 13:51:10 +0300)

----------------------------------------------------------------
fbdev changes for 3.16 (main part)

Mainly fixes and small improvements. The biggest change seems to be backlight
control support for mx3fb.

----------------------------------------------------------------
Aaro Koskinen (1):
      video: omap: delete support for early fbmem allocation

Alexander Stein (1):
      video: mx3fb: Add backlight control support

Arnd Bergmann (7):
      video: sh_mobile_lcdcfb depends on meram
      video: clarify I2C dependencies
      video/omap: fix modular build
      video/mbx: fix building debugfs support
      video: export fb_prepare_logo
      video: atmel needs FB_BACKLIGHT
      video/nuc900: allow modular build

Boris BREZILLON (1):
      video: of: display_timing: fix default native-mode setting

Brian W Hart (1):
      fbdev/fb.h: silence warning with -Wsign-compare

Dan Carpenter (1):
      video: mmpfb: cleanup some static checker warnings in probe()

David Ung (1):
      video: Check EDID for HDMI connection

Denis Carikli (1):
      video: mx3fb: Use devm_kzalloc

Fabian Frederick (1):
      fbdev: fbmem: remove positive test on unsigned values

Jean Delvare (3):
      fbdev: Fix tmiofb driver dependencies
      video: exynos: Add a dependency to the menu
      video: Kconfig: Add a dependency to the Goldfish framebuffer driver

Jingoo Han (1):
      video: pxa3xx-gcu: use devm_ioremap_resource()

Jon Ringle (1):
      video: da8xx-fb: Add support for Densitron 84-0023-001T

Julia Lawall (1):
      video: delete unneeded call to platform_get_drvdata

Lucas Stach (1):
      video: of: display_timing: remove two unsafe error messages

Masami Ichikawa (1):
      fbcon: Fix memory leak in con2fb_release_oldinfo()

Masanari Iida (2):
      video: fbdev: Fix format string mismatch in wm8505fb.c
      video: fbdev: Fix format string mismatch in gbefb.c

Mikulas Patocka (1):
      matroxfb: perform a dummy read of M_STATUS

Richard Weinberger (2):
      video: mmp: Remove references to CPU_MMP3
      video: mmp: Remove references to CPU_PXA988

Rickard Strandqvist (2):
      video: fbdev: grvga.c: Fix for possible null pointer dereference
      video: fbdev: s3fb.c: Fix for possible null pointer dereference

Zhichuang SUN (1):
      drivers/video/fbdev/fb-puv3.c: Add header files for function unifb_mmap

 drivers/video/console/fbcon.c              |  1 +
 drivers/video/fbdev/Kconfig                | 11 ++--
 drivers/video/fbdev/bf54x-lq043fb.c        |  2 -
 drivers/video/fbdev/core/fbmem.c           |  7 +--
 drivers/video/fbdev/core/fbmon.c           |  9 +++-
 drivers/video/fbdev/da8xx-fb.c             | 14 +++++
 drivers/video/fbdev/exynos/Kconfig         |  2 +-
 drivers/video/fbdev/fb-puv3.c              |  2 +
 drivers/video/fbdev/gbefb.c                |  2 +-
 drivers/video/fbdev/grvga.c                |  3 +-
 drivers/video/fbdev/matrox/matroxfb_base.h |  2 +-
 drivers/video/fbdev/mbx/Makefile           |  3 +-
 drivers/video/fbdev/mbx/mbxdebugfs.c       |  2 +-
 drivers/video/fbdev/mbx/mbxfb.c            |  2 +
 drivers/video/fbdev/mmp/Kconfig            |  2 +-
 drivers/video/fbdev/mmp/fb/mmpfb.c         |  9 +---
 drivers/video/fbdev/mmp/hw/Kconfig         |  6 +--
 drivers/video/fbdev/mmp/hw/mmp_ctrl.h      | 32 -----------
 drivers/video/fbdev/mx3fb.c                | 85 ++++++++++++++++++++++++++++--
 drivers/video/fbdev/omap/Kconfig           |  9 ++++
 drivers/video/fbdev/omap/Makefile          | 23 ++++----
 drivers/video/fbdev/omap/lcdc.c            | 67 +----------------------
 drivers/video/fbdev/omap/omapfb_main.c     |  1 +
 drivers/video/fbdev/pxa3xx-gcu.c           |  6 +--
 drivers/video/fbdev/s3fb.c                 |  3 +-
 drivers/video/fbdev/wm8505fb.c             |  2 +-
 drivers/video/of_display_timing.c          | 10 ++--
 include/linux/fb.h                         |  3 +-
 include/linux/omap-dma.h                   |  2 +-
 29 files changed, 169 insertions(+), 153 deletions(-)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [GIT PULL] omap fbdev changes for 3.16
From: Tomi Valkeinen @ 2014-06-04 12:30 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, Linux Fbdev development list

[-- Attachment #1: Type: text/plain, Size: 6587 bytes --]

Hi Linus,

Please pull the omap fbdev changes for 3.16.

 Tomi

The following changes since commit 89ca3b881987f5a4be4c5dbaa7f0df12bbdde2fd:

  Linux 3.15-rc4 (2014-05-04 18:14:42 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git tags/fbdev-omap-3.16

for you to fetch changes up to f2dd36ac9974cc2353bcbb8e6b643fb96030564c:

  OMAPDSS: move 'compatible' converter to omapdss driver (2014-05-28 09:25:15 +0300)

----------------------------------------------------------------
fbdev changes for 3.16 (omap)

* DT support for the panel drivers that were still missing it
* TI AM43xx support
* TI OMAP5 support

----------------------------------------------------------------
Archit Taneja (4):
      omapdss: remove check for simpler port/endpoint binding
      OMAPDSS: HDMI: support larger register offsets for OMAP5 HDMI core
      OMAPDSS: HDMI: PHY changes for OMAP5
      OMAPDSS: HDMI: PLL changes for OMAP5

Arnd Bergmann (1):
      video: omap2dss: fix LPAE warnings

Fabian Frederick (1):
      OMAPDSS: add __exit to dss_uninit_ports

Jyri Sarha (1):
      OMAPDSS: hdmi5_core: Fix compilation with OMAP5_DSS_HDMI_AUDIO

Marek Belisko (2):
      omapdss: panel-tpo-td028ec1: Add DT support.
      omapdss: panel-tpo-td028ec1: Add module alias

Sathya Prakash M R (1):
      OMAPDSS: Add DSS features for AM43xx

Tomi Valkeinen (33):
      OMAPDSS: panel-dpi: use gpiod for enable gpio
      OMAPDSS: panel-dpi: Add DT support
      Doc/DT: Add DT binding documentation for MIPI DPI Panel
      OMAPDSS: connector-hdmi: hpd support
      Doc/DT: hdmi-connector: add HPD GPIO documentation
      OMAPDSS: HDMI: lane config support
      Doc/DT: ti,omap4-dss: hdmi lanes
      OMAPDSS: HDMI4: set regulator voltage to 1.8V
      OMAPDSS: DSI: set regulator voltage to 1.8V
      OMAPDSS: remove venc_panel.c
      OMAPDSS: remove unused macros
      ARM: OMAP: add detection of omap5-dss
      OMAPDSS: DSS & DISPC DT support for OMAP5
      OMAPDSS: features: fix OMAP5 features
      OMAPDSS: DPI: fix LCD3 DSI source
      OMAPDSS: DSI: Add OMAP5 DSI module IDs
      OMAPDSS: HDMI: improve Makefile
      OMAPDSS: HDMI: move irq & phy pwr handling
      OMAPDSS: HDMI: Add OMAP5 HDMI support
      OMAPDSS: HDMI: cleanup ioremaps
      Doc/DT: Add OMAP5 DSS DT bindings
      OMAPDSS: Fix writes to DISPC_POL_FREQ
      OMAPDSS: panel-dpi: enable-gpio
      OMAPDSS: panel-lgphilips-lb035q02: use gpiod for enable gpio
      OMAPDSS: panel-lgphilips-lb035q02: Add DT support
      Doc/DT: Add binding doc for lgphilips,lb035q02.txt
      OMAPDSS: Panel TPO-TD043MTEA1 DT support
      Doc/DT: Add DT binding documentation for TPO td043mtea1 panel
      OMAPDSS: panel NEC-NL8048HL11 DT support
      OMAPDSS: HDMI: cleanup WP ioremaps
      OMAPDSS: HDMI: remove unused defines
      OMAPDSS: HDMI: fix devm_ioremap_resource error checks
      OMAPDSS: move 'compatible' converter to omapdss driver

Tony Lindgren (3):
      OMAPDSS: panel-sharp-ls037v7dw01: update to use gpiod
      OMAPDSS: panel sharp-ls037v7dw01 DT support
      Doc/DT: Add DT binding documentation for SHARP LS037V7DW01

 .../devicetree/bindings/video/hdmi-connector.txt   |   1 +
 .../bindings/video/lgphilips,lb035q02.txt          |  33 +
 .../devicetree/bindings/video/panel-dpi.txt        |  45 +
 .../bindings/video/sharp,ls037v7dw01.txt           |  43 +
 .../devicetree/bindings/video/ti,omap4-dss.txt     |   4 +
 .../devicetree/bindings/video/ti,omap5-dss.txt     |  96 +++
 .../bindings/video/toppoly,td028ttec1.txt          |  30 +
 .../devicetree/bindings/video/tpo,td043mtea1.txt   |  33 +
 arch/arm/mach-omap2/display.c                      |  62 +-
 drivers/video/fbdev/omap2/Makefile                 |   2 +-
 .../fbdev/omap2/displays-new/connector-hdmi.c      |  25 +-
 drivers/video/fbdev/omap2/displays-new/panel-dpi.c |  95 ++-
 .../omap2/displays-new/panel-lgphilips-lb035q02.c  |  77 +-
 .../omap2/displays-new/panel-nec-nl8048hl11.c      |  45 +-
 .../omap2/displays-new/panel-sharp-ls037v7dw01.c   | 210 +++--
 .../omap2/displays-new/panel-tpo-td028ttec1.c      |  33 +-
 .../omap2/displays-new/panel-tpo-td043mtea1.c      |  42 +-
 drivers/video/fbdev/omap2/dss/Kconfig              |  26 +-
 drivers/video/fbdev/omap2/dss/Makefile             |   7 +-
 drivers/video/fbdev/omap2/dss/core.c               |   6 +
 drivers/video/fbdev/omap2/dss/dispc.c              |  22 +-
 drivers/video/fbdev/omap2/dss/dpi.c                |   4 +
 drivers/video/fbdev/omap2/dss/dsi.c                |  18 +
 drivers/video/fbdev/omap2/dss/dss.c                |  20 +-
 drivers/video/fbdev/omap2/dss/dss.h                |   3 +
 drivers/video/fbdev/omap2/dss/dss_features.c       |  70 +-
 drivers/video/fbdev/omap2/dss/hdmi.h               |  17 +-
 drivers/video/fbdev/omap2/dss/hdmi4.c              | 113 ++-
 drivers/video/fbdev/omap2/dss/hdmi4_core.c         |  28 +-
 drivers/video/fbdev/omap2/dss/hdmi5.c              | 829 ++++++++++++++++++
 drivers/video/fbdev/omap2/dss/hdmi5_core.c         | 922 +++++++++++++++++++++
 drivers/video/fbdev/omap2/dss/hdmi5_core.h         | 306 +++++++
 drivers/video/fbdev/omap2/dss/hdmi_common.c        |  41 +
 drivers/video/fbdev/omap2/dss/hdmi_phy.c           | 265 ++++--
 drivers/video/fbdev/omap2/dss/hdmi_pll.c           | 107 ++-
 drivers/video/fbdev/omap2/dss/hdmi_wp.c            |  29 +-
 drivers/video/fbdev/omap2/dss/omapdss-boot-init.c  | 229 +++++
 drivers/video/fbdev/omap2/dss/venc_panel.c         | 232 ------
 include/video/omapdss.h                            |   8 +-
 39 files changed, 3604 insertions(+), 574 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/lgphilips,lb035q02.txt
 create mode 100644 Documentation/devicetree/bindings/video/panel-dpi.txt
 create mode 100644 Documentation/devicetree/bindings/video/sharp,ls037v7dw01.txt
 create mode 100644 Documentation/devicetree/bindings/video/ti,omap5-dss.txt
 create mode 100644 Documentation/devicetree/bindings/video/toppoly,td028ttec1.txt
 create mode 100644 Documentation/devicetree/bindings/video/tpo,td043mtea1.txt
 create mode 100644 drivers/video/fbdev/omap2/dss/hdmi5.c
 create mode 100644 drivers/video/fbdev/omap2/dss/hdmi5_core.c
 create mode 100644 drivers/video/fbdev/omap2/dss/hdmi5_core.h
 create mode 100644 drivers/video/fbdev/omap2/dss/omapdss-boot-init.c
 delete mode 100644 drivers/video/fbdev/omap2/dss/venc_panel.c


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH 0/10] use safer test on the result of find_first_zero_bit
From: Julia Lawall @ 2014-06-04 13:12 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Julia Lawall, David Laight, linux-rdma,
	kernel-janitors@vger.kernel.org, Linux Fbdev development list,
	Linux-sh list, linux-kernel@vger.kernel.org,
	ath10k@lists.infradead.org, linux-wireless,
	netdev@vger.kernel.org, driverdevel, iss_storagedev@hp.com, scsi,
	linux-s390, adi-buildroot-devel@lists.sourceforge.net,
	Arnd Bergmann, sebott
In-Reply-To: <CAMuHMdV2wP43Lxf-UtCum37HyMPSboRb6RLNTHzm_ZNOBeJR0g@mail.gmail.com>



On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:

> Hi Julia,
>
> On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > OK, thanks.  I was only looking at the C code.
> >
> > But the C code contains a loop that is followed by:
> >
> >         if (!size)
> >                 return result;
> >         tmp = *p;
> >
> > found_first:
> >         tmp |= ~0UL << size;
> >         if (tmp = ~0UL)        /* Are any bits zero? */
> >                 return result + size;   /* Nope. */
> >
> > In the first return, it would seem that result = size.  Could the second
> > one be changed to just return size?  It should not hurt performance.
>
> "size" may have been changed between function entry and this line.
> So you have to store it in a temporary.

Sorry, after reflection it seems that indeed size + result is always the
original size, so it is actually all of the code that uses >= that is
doing something unnecessary.  = for the failure test is fine.

julia

^ permalink raw reply

* Re: [PATCH] drm/i915: Kick out vga console
From: David Herrmann @ 2014-06-04 13:18 UTC (permalink / raw)
  To: Jani Nikula
  Cc: linux-fbdev@vger.kernel.org, Daniel Vetter,
	Intel Graphics Development, DRI Development, Tomi Valkeinen,
	Jean-Christophe Plagniol-Villard
In-Reply-To: <87sinkeuiu.fsf@intel.com>

Hi

On Wed, Jun 4, 2014 at 2:20 PM, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Wed, 04 Jun 2014, David Herrmann <dh.herrmann@gmail.com> wrote:
>> You rely on compiler-optimizations here. "dummy_con" is not available
>> if !CONFIG_DUMMY_CONSOLE, but you use it. This causes linker-failure
>> if dead-code elimination is not done (-O0).
>
> Nested #ifs too. How about
>
> #if !defined(CONFIG_VGA_CONSOLE)
> static inline int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> {
>         return 0;
> }
> #elif !defined(CONFIG_DUMMY_CONSOLE)
> static inline int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> {
>         return -ENODEV;
> }
> #else
> static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> {
>         /* ... */
> }
> #endif
>
> in proper kernel style?

Or even shorter:

#if defined(CONFIG_VGA_CONSOLE) && defined(CONFIG_DUMMY_CONSOLE)
static inline int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
{
        ...
}
#else
static inline int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
{
        return IS_ENABLED(CONFIG_VGA_CONSOLE) ? -ENODEV : 0;
}
#endif

Thanks
David

On Wed, Jun 4, 2014 at 2:20 PM, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Wed, 04 Jun 2014, David Herrmann <dh.herrmann@gmail.com> wrote:
>> Hi
>>
>> On Wed, Jun 4, 2014 at 12:57 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>> From: Chris Wilson <chris@chris-wilson.co.uk>
>>>
>>> Touching the VGA resources on an IVB EFI machine causes hard hangs when
>>> we then kick out the efifb. Ouch.
>>>
>>> Apparently this also prevents unclaimed register errors on hsw and
>>> hard machine hangs on my i855gm when trying to unbind fbcon.
>>>
>>> Also, we want this to make I915_FBDEV=n safe.
>>>
>>> v2: Rebase and pimp commit message.
>>>
>>> v3: We also need to unregister the vga console, otherwise the unbind
>>> of the fb console before module unload might resurrect it again.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?idg813
>>> Cc: David Herrmann <dh.herrmann@gmail.com>
>>> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> Cc: linux-fbdev@vger.kernel.org
>>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
>>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> ---
>>>  drivers/gpu/drm/i915/i915_dma.c  | 34 +++++++++++++++++++++++++++++++++-
>>>  drivers/video/console/dummycon.c |  1 +
>>>  drivers/video/console/vgacon.c   |  1 +
>>>  3 files changed, 35 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>>> index b9159ade5e85..a4df80740b37 100644
>>> --- a/drivers/gpu/drm/i915/i915_dma.c
>>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>>> @@ -36,6 +36,8 @@
>>>  #include "i915_drv.h"
>>>  #include "i915_trace.h"
>>>  #include <linux/pci.h>
>>> +#include <linux/console.h>
>>> +#include <linux/vt.h>
>>>  #include <linux/vgaarb.h>
>>>  #include <linux/acpi.h>
>>>  #include <linux/pnp.h>
>>> @@ -1450,6 +1452,29 @@ static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv)
>>>  }
>>>  #endif
>>>
>>> +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
>>> +{
>>> +#if !defined(CONFIG_VGA_CONSOLE)
>>> +       return 0;
>>> +#else
>>> +       int ret;
>>> +
>>> +#if !defined(CONFIG_DUMMY_CONSOLE)
>>> +       return -ENODEV;
>>> +#endif
>>> +
>>> +       DRM_INFO("Replacing VGA console driver\n");
>>> +
>>> +       console_lock();
>>> +       ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
>>
>> You rely on compiler-optimizations here. "dummy_con" is not available
>> if !CONFIG_DUMMY_CONSOLE, but you use it. This causes linker-failure
>> if dead-code elimination is not done (-O0).
>
> Nested #ifs too. How about
>
> #if !defined(CONFIG_VGA_CONSOLE)
> static inline int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> {
>         return 0;
> }
> #elif !defined(CONFIG_DUMMY_CONSOLE)
> static inline int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> {
>         return -ENODEV;
> }
> #else
> static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> {
>         /* ... */
> }
> #endif
>
> in proper kernel style?
>
> BR,
> Jani.
>
>
>>
>> Thanks
>> David
>>
>>> +       if (ret = 0)
>>> +               ret = do_unregister_con_driver(&vga_con);
>>> +       console_unlock();
>>> +
>>> +       return ret;
>>> +#endif
>>> +}
>>> +
>>>  static void i915_dump_device_info(struct drm_i915_private *dev_priv)
>>>  {
>>>         const struct intel_device_info *info = &dev_priv->info;
>>> @@ -1623,8 +1648,15 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>>>         if (ret)
>>>                 goto out_regs;
>>>
>>> -       if (drm_core_check_feature(dev, DRIVER_MODESET))
>>> +       if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>>> +               ret = i915_kick_out_vgacon(dev_priv);
>>> +               if (ret) {
>>> +                       DRM_ERROR("failed to remove conflicting VGA console\n");
>>> +                       goto out_gtt;
>>> +               }
>>> +
>>>                 i915_kick_out_firmware_fb(dev_priv);
>>> +       }
>>>
>>>         pci_set_master(dev->pdev);
>>>
>>> diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
>>> index b63860f7beab..40bec8d64b0a 100644
>>> --- a/drivers/video/console/dummycon.c
>>> +++ b/drivers/video/console/dummycon.c
>>> @@ -77,3 +77,4 @@ const struct consw dummy_con = {
>>>      .con_set_palette = DUMMY,
>>>      .con_scrolldelta = DUMMY,
>>>  };
>>> +EXPORT_SYMBOL_GPL(dummy_con);
>>> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
>>> index 9d8feac67637..84acd6223dc5 100644
>>> --- a/drivers/video/console/vgacon.c
>>> +++ b/drivers/video/console/vgacon.c
>>> @@ -1440,5 +1440,6 @@ const struct consw vga_con = {
>>>         .con_build_attr = vgacon_build_attr,
>>>         .con_invert_region = vgacon_invert_region,
>>>  };
>>> +EXPORT_SYMBOL(vga_con);
>>>
>>>  MODULE_LICENSE("GPL");
>>> --
>>> 1.8.1.4
>>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Jani Nikula, Intel Open Source Technology Center

^ permalink raw reply

* RE: [PATCH 0/10] use safer test on the result of find_first_zero_bit
From: David Laight @ 2014-06-04 13:34 UTC (permalink / raw)
  To: 'Julia Lawall', Geert Uytterhoeven
  Cc: linux-rdma, kernel-janitors@vger.kernel.org,
	Linux Fbdev development list, Linux-sh list,
	linux-kernel@vger.kernel.org, ath10k@lists.infradead.org,
	linux-wireless, netdev@vger.kernel.org, driverdevel,
	iss_storagedev@hp.com, scsi, linux-s390,
	adi-buildroot-devel@lists.sourceforge.net, Arnd Bergmann,
	sebott@linux.vnet.ibm.com
In-Reply-To: <alpine.DEB.2.10.1406041510520.2441@hadrien>

From: Julia Lawall
> On Wed, 4 Jun 2014, Geert Uytterhoeven wrote:
> 
> > Hi Julia,
> >
> > On Wed, Jun 4, 2014 at 1:00 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > > OK, thanks.  I was only looking at the C code.
> > >
> > > But the C code contains a loop that is followed by:
> > >
> > >         if (!size)
> > >                 return result;
> > >         tmp = *p;
> > >
> > > found_first:
> > >         tmp |= ~0UL << size;
> > >         if (tmp = ~0UL)        /* Are any bits zero? */
> > >                 return result + size;   /* Nope. */
> > >
> > > In the first return, it would seem that result = size.  Could the second
> > > one be changed to just return size?  It should not hurt performance.
> >
> > "size" may have been changed between function entry and this line.
> > So you have to store it in a temporary.
> 
> Sorry, after reflection it seems that indeed size + result is always the
> original size, so it is actually all of the code that uses >= that is
> doing something unnecessary.  = for the failure test is fine.

There is nothing wrong with defensive coding.
The 'tmp |= ~0UL << size' ensures that the return value is 'correct'
when there are no bits set.
The function could have been defined so that this wasn't needed.

If you assume that the 'no zero bits' is unlikely, then checking the
return value from ffz() could well be slightly faster.
Not that anything is likely to notice.

	David




^ permalink raw reply

* [PATCH 5/5] drm/i915: Kick out vga console
From: Daniel Vetter @ 2014-06-05 14:58 UTC (permalink / raw)
  To: Intel Graphics Development
  Cc: linux-fbdev, Daniel Vetter, LKML, DRI Development, Tomi Valkeinen,
	Jean-Christophe Plagniol-Villard
In-Reply-To: <1401980308-5116-1-git-send-email-daniel.vetter@ffwll.ch>

Touching the VGA resources on an IVB EFI machine causes hard hangs when
we then kick out the efifb. Ouch.

Apparently this also prevents unclaimed register errors on hsw and
hard machine hangs on my i855gm when trying to unbind fbcon.

Also, we want this to make I915_FBDEV=n safe.

v2: Rebase and pimp commit message.

v3: We also need to unregister the vga console, otherwise the unbind
of the fb console before module unload might resurrect it again.

v4: Ignore errors when the vga console is already unregistered - this
can happen when e.g. reloading i915.ko.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?idg813
Cc: David Herrmann <dh.herrmann@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_dma.c  | 43 +++++++++++++++++++++++++++++++++++++++-
 drivers/video/console/dummycon.c |  1 +
 drivers/video/console/vgacon.c   |  1 +
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 27fe65ac5940..bcb66ddd649e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -36,6 +36,8 @@
 #include "i915_drv.h"
 #include "i915_trace.h"
 #include <linux/pci.h>
+#include <linux/console.h>
+#include <linux/vt.h>
 #include <linux/vgaarb.h>
 #include <linux/acpi.h>
 #include <linux/pnp.h>
@@ -1449,6 +1451,38 @@ static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv)
 }
 #endif
 
+#if !defined(CONFIG_VGA_CONSOLE)
+static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
+{
+	return 0;
+}
+#elif !defined(CONFIG_DUMMY_CONSOLE)
+static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
+{
+	return -ENODEV;
+}
+#else
+static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
+{
+	int ret;
+
+	DRM_INFO("Replacing VGA console driver\n");
+
+	console_lock();
+	ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
+	if (ret = 0) {
+		ret = do_unregister_con_driver(&vga_con);
+
+		/* Ignore "already unregistered". */
+		if (ret = -ENODEV)
+			ret = 0;
+	}
+	console_unlock();
+
+	return ret;
+}
+#endif
+
 static void i915_dump_device_info(struct drm_i915_private *dev_priv)
 {
 	const struct intel_device_info *info = &dev_priv->info;
@@ -1622,8 +1656,15 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	if (ret)
 		goto out_regs;
 
-	if (drm_core_check_feature(dev, DRIVER_MODESET))
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		ret = i915_kick_out_vgacon(dev_priv);
+		if (ret) {
+			DRM_ERROR("failed to remove conflicting VGA console\n");
+			goto out_gtt;
+		}
+
 		i915_kick_out_firmware_fb(dev_priv);
+	}
 
 	pci_set_master(dev->pdev);
 
diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
index b63860f7beab..40bec8d64b0a 100644
--- a/drivers/video/console/dummycon.c
+++ b/drivers/video/console/dummycon.c
@@ -77,3 +77,4 @@ const struct consw dummy_con = {
     .con_set_palette =	DUMMY,
     .con_scrolldelta =	DUMMY,
 };
+EXPORT_SYMBOL_GPL(dummy_con);
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 9d8feac67637..84acd6223dc5 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -1440,5 +1440,6 @@ const struct consw vga_con = {
 	.con_build_attr = vgacon_build_attr,
 	.con_invert_region = vgacon_invert_region,
 };
+EXPORT_SYMBOL(vga_con);
 
 MODULE_LICENSE("GPL");
-- 
1.8.1.4


^ permalink raw reply related

* Re: [PATCH 5/5] drm/i915: Kick out vga console
From: David Herrmann @ 2014-06-06  7:28 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: linux-fbdev@vger.kernel.org, Intel Graphics Development, LKML,
	DRI Development, Tomi Valkeinen, Jean-Christophe Plagniol-Villard
In-Reply-To: <1401980308-5116-5-git-send-email-daniel.vetter@ffwll.ch>

Hi

On Thu, Jun 5, 2014 at 4:58 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Touching the VGA resources on an IVB EFI machine causes hard hangs when
> we then kick out the efifb. Ouch.
>
> Apparently this also prevents unclaimed register errors on hsw and
> hard machine hangs on my i855gm when trying to unbind fbcon.
>
> Also, we want this to make I915_FBDEV=n safe.
>
> v2: Rebase and pimp commit message.
>
> v3: We also need to unregister the vga console, otherwise the unbind
> of the fb console before module unload might resurrect it again.
>
> v4: Ignore errors when the vga console is already unregistered - this
> can happen when e.g. reloading i915.ko.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?idg813
> Cc: David Herrmann <dh.herrmann@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_dma.c  | 43 +++++++++++++++++++++++++++++++++++++++-
>  drivers/video/console/dummycon.c |  1 +
>  drivers/video/console/vgacon.c   |  1 +
>  3 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 27fe65ac5940..bcb66ddd649e 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -36,6 +36,8 @@
>  #include "i915_drv.h"
>  #include "i915_trace.h"
>  #include <linux/pci.h>
> +#include <linux/console.h>
> +#include <linux/vt.h>
>  #include <linux/vgaarb.h>
>  #include <linux/acpi.h>
>  #include <linux/pnp.h>
> @@ -1449,6 +1451,38 @@ static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv)
>  }
>  #endif
>
> +#if !defined(CONFIG_VGA_CONSOLE)
> +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> +{
> +       return 0;
> +}
> +#elif !defined(CONFIG_DUMMY_CONSOLE)

Why not "select DUMMY_CONSOLE if VT"? It's really stupid to disable
DUMMY_CONSOLE.. Furthermore, we already rely on HW_CONSOLE_BINDING so
this should be safe.

Patch looks good to me:

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

Thanks
David

> +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> +{
> +       return -ENODEV;
> +}
> +#else
> +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> +{
> +       int ret;
> +
> +       DRM_INFO("Replacing VGA console driver\n");
> +
> +       console_lock();
> +       ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> +       if (ret = 0) {
> +               ret = do_unregister_con_driver(&vga_con);
> +
> +               /* Ignore "already unregistered". */
> +               if (ret = -ENODEV)
> +                       ret = 0;
> +       }
> +       console_unlock();
> +
> +       return ret;
> +}
> +#endif
> +
>  static void i915_dump_device_info(struct drm_i915_private *dev_priv)
>  {
>         const struct intel_device_info *info = &dev_priv->info;
> @@ -1622,8 +1656,15 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>         if (ret)
>                 goto out_regs;
>
> -       if (drm_core_check_feature(dev, DRIVER_MODESET))
> +       if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> +               ret = i915_kick_out_vgacon(dev_priv);
> +               if (ret) {
> +                       DRM_ERROR("failed to remove conflicting VGA console\n");
> +                       goto out_gtt;
> +               }
> +
>                 i915_kick_out_firmware_fb(dev_priv);
> +       }
>
>         pci_set_master(dev->pdev);
>
> diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
> index b63860f7beab..40bec8d64b0a 100644
> --- a/drivers/video/console/dummycon.c
> +++ b/drivers/video/console/dummycon.c
> @@ -77,3 +77,4 @@ const struct consw dummy_con = {
>      .con_set_palette = DUMMY,
>      .con_scrolldelta = DUMMY,
>  };
> +EXPORT_SYMBOL_GPL(dummy_con);
> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
> index 9d8feac67637..84acd6223dc5 100644
> --- a/drivers/video/console/vgacon.c
> +++ b/drivers/video/console/vgacon.c
> @@ -1440,5 +1440,6 @@ const struct consw vga_con = {
>         .con_build_attr = vgacon_build_attr,
>         .con_invert_region = vgacon_invert_region,
>  };
> +EXPORT_SYMBOL(vga_con);
>
>  MODULE_LICENSE("GPL");
> --
> 1.8.1.4
>

^ permalink raw reply

* Re: [PATCH 5/5] drm/i915: Kick out vga console
From: Daniel Vetter @ 2014-06-06  7:47 UTC (permalink / raw)
  To: David Herrmann
  Cc: linux-fbdev@vger.kernel.org, Daniel Vetter,
	Intel Graphics Development, LKML, DRI Development, Tomi Valkeinen,
	Jean-Christophe Plagniol-Villard
In-Reply-To: <CANq1E4Si5+DVVz5xUcp0FXaX2rSH0XJTCa_TBD5PtRs0cJwo0A@mail.gmail.com>

On Fri, Jun 06, 2014 at 09:28:03AM +0200, David Herrmann wrote:
> Hi
> 
> On Thu, Jun 5, 2014 at 4:58 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Touching the VGA resources on an IVB EFI machine causes hard hangs when
> > we then kick out the efifb. Ouch.
> >
> > Apparently this also prevents unclaimed register errors on hsw and
> > hard machine hangs on my i855gm when trying to unbind fbcon.
> >
> > Also, we want this to make I915_FBDEV=n safe.
> >
> > v2: Rebase and pimp commit message.
> >
> > v3: We also need to unregister the vga console, otherwise the unbind
> > of the fb console before module unload might resurrect it again.
> >
> > v4: Ignore errors when the vga console is already unregistered - this
> > can happen when e.g. reloading i915.ko.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?idg813
> > Cc: David Herrmann <dh.herrmann@gmail.com>
> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: linux-fbdev@vger.kernel.org
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c  | 43 +++++++++++++++++++++++++++++++++++++++-
> >  drivers/video/console/dummycon.c |  1 +
> >  drivers/video/console/vgacon.c   |  1 +
> >  3 files changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 27fe65ac5940..bcb66ddd649e 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -36,6 +36,8 @@
> >  #include "i915_drv.h"
> >  #include "i915_trace.h"
> >  #include <linux/pci.h>
> > +#include <linux/console.h>
> > +#include <linux/vt.h>
> >  #include <linux/vgaarb.h>
> >  #include <linux/acpi.h>
> >  #include <linux/pnp.h>
> > @@ -1449,6 +1451,38 @@ static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv)
> >  }
> >  #endif
> >
> > +#if !defined(CONFIG_VGA_CONSOLE)
> > +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> > +{
> > +       return 0;
> > +}
> > +#elif !defined(CONFIG_DUMMY_CONSOLE)
> 
> Why not "select DUMMY_CONSOLE if VT"? It's really stupid to disable
> DUMMY_CONSOLE.. Furthermore, we already rely on HW_CONSOLE_BINDING so
> this should be safe.

Iirc this lead kconfig to complain about dep loops ... And I've tried to
figure it out, but I think it's actually impossible. So I let it be with
the -ENODEV to make sure if it's not impossible any more we'll catch it.
-Daniel

> 
> Patch looks good to me:
> 
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> 
> Thanks
> David
> 
> > +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> > +{
> > +       return -ENODEV;
> > +}
> > +#else
> > +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> > +{
> > +       int ret;
> > +
> > +       DRM_INFO("Replacing VGA console driver\n");
> > +
> > +       console_lock();
> > +       ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> > +       if (ret = 0) {
> > +               ret = do_unregister_con_driver(&vga_con);
> > +
> > +               /* Ignore "already unregistered". */
> > +               if (ret = -ENODEV)
> > +                       ret = 0;
> > +       }
> > +       console_unlock();
> > +
> > +       return ret;
> > +}
> > +#endif
> > +
> >  static void i915_dump_device_info(struct drm_i915_private *dev_priv)
> >  {
> >         const struct intel_device_info *info = &dev_priv->info;
> > @@ -1622,8 +1656,15 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> >         if (ret)
> >                 goto out_regs;
> >
> > -       if (drm_core_check_feature(dev, DRIVER_MODESET))
> > +       if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> > +               ret = i915_kick_out_vgacon(dev_priv);
> > +               if (ret) {
> > +                       DRM_ERROR("failed to remove conflicting VGA console\n");
> > +                       goto out_gtt;
> > +               }
> > +
> >                 i915_kick_out_firmware_fb(dev_priv);
> > +       }
> >
> >         pci_set_master(dev->pdev);
> >
> > diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
> > index b63860f7beab..40bec8d64b0a 100644
> > --- a/drivers/video/console/dummycon.c
> > +++ b/drivers/video/console/dummycon.c
> > @@ -77,3 +77,4 @@ const struct consw dummy_con = {
> >      .con_set_palette = DUMMY,
> >      .con_scrolldelta = DUMMY,
> >  };
> > +EXPORT_SYMBOL_GPL(dummy_con);
> > diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
> > index 9d8feac67637..84acd6223dc5 100644
> > --- a/drivers/video/console/vgacon.c
> > +++ b/drivers/video/console/vgacon.c
> > @@ -1440,5 +1440,6 @@ const struct consw vga_con = {
> >         .con_build_attr = vgacon_build_attr,
> >         .con_invert_region = vgacon_invert_region,
> >  };
> > +EXPORT_SYMBOL(vga_con);
> >
> >  MODULE_LICENSE("GPL");
> > --
> > 1.8.1.4
> >

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply

* [PATCH] pwm-backlight:
From: Lothar Waßmann @ 2014-06-06 11:02 UTC (permalink / raw)
  To: Bryan Wu, Jean-Christophe Plagniol-Villard, Jingoo Han, Lee Jones,
	Thierry Reding, Tomi Valkeinen, linux-fbdev, linux-kernel,
	linux-pwm
  Cc: Lothar Waßmann

commit 257462dbf3ed pwm-backlight: switch to gpiod interface
introduced a regression leading to acquiring a bogus GPIO-0 when
configured from DT without an 'enable-gpios' property.
The driver will happily accept the 0 initialized 'enable_gpio' member
of the struct platform_pwm_backlight_data as valid gpio number, and
request this GPIO as enable pin. In case of multiple driver instances,
the second will fail to register with the error message:
pwm-backlight backlight1.23: failed to request GPIO#0: -16

Fix this by setting enable_gpio in the pdata struct to -EINVAL.

Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
 drivers/video/backlight/pwm_bl.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 38ca88b..3d265c4 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -178,7 +178,7 @@ static int pwm_backlight_parse_dt(struct device *dev,
 		data->dft_brightness = value;
 		data->max_brightness--;
 	}
-
+	data->enable_gpio = -EINVAL;
 	return 0;
 }
 
-- 
1.7.10.4


^ permalink raw reply related

* Re: [PATCH] pwm-backlight:
From: Lee Jones @ 2014-06-06 12:42 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: Bryan Wu, Jean-Christophe Plagniol-Villard, Jingoo Han,
	Thierry Reding, Tomi Valkeinen, linux-fbdev, linux-kernel,
	linux-pwm
In-Reply-To: <1402052529-22761-1-git-send-email-LW@KARO-electronics.de>

On Fri, 06 Jun 2014, Lothar Waßmann wrote:

> commit 257462dbf3ed pwm-backlight: switch to gpiod interface
> introduced a regression leading to acquiring a bogus GPIO-0 when
> configured from DT without an 'enable-gpios' property.
> The driver will happily accept the 0 initialized 'enable_gpio' member
> of the struct platform_pwm_backlight_data as valid gpio number, and
> request this GPIO as enable pin. In case of multiple driver instances,
> the second will fail to register with the error message:
> pwm-backlight backlight1.23: failed to request GPIO#0: -16
> 
> Fix this by setting enable_gpio in the pdata struct to -EINVAL.
> 
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
> ---
>  drivers/video/backlight/pwm_bl.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> index 38ca88b..3d265c4 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -178,7 +178,7 @@ static int pwm_backlight_parse_dt(struct device *dev,
>  		data->dft_brightness = value;
>  		data->max_brightness--;
>  	}
> -
> +	data->enable_gpio = -EINVAL;
>  	return 0;
>  }

This patch doesn't look like it will apply.  I suggest you pull
v3.17-rc1 when it lands and rebase your patch.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH 5/5] drm/i915: Kick out vga console
From: Daniel Vetter @ 2014-06-06 15:20 UTC (permalink / raw)
  To: Intel Graphics Development
  Cc: Linux Fbdev development list, Daniel Vetter, LKML,
	DRI Development, Tomi Valkeinen, David Herrmann,
	Jean-Christophe Plagniol-Villard
In-Reply-To: <1401980308-5116-5-git-send-email-daniel.vetter@ffwll.ch>

Tomi/Jean can you please ack merging this through drm-intel trees? It
just exports the vga and dummy consoles so that i915 can do what it
needs to do.

Thanks, Daniel

On Thu, Jun 5, 2014 at 4:58 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Touching the VGA resources on an IVB EFI machine causes hard hangs when
> we then kick out the efifb. Ouch.
>
> Apparently this also prevents unclaimed register errors on hsw and
> hard machine hangs on my i855gm when trying to unbind fbcon.
>
> Also, we want this to make I915_FBDEV=n safe.
>
> v2: Rebase and pimp commit message.
>
> v3: We also need to unregister the vga console, otherwise the unbind
> of the fb console before module unload might resurrect it again.
>
> v4: Ignore errors when the vga console is already unregistered - this
> can happen when e.g. reloading i915.ko.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?idg813
> Cc: David Herrmann <dh.herrmann@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_dma.c  | 43 +++++++++++++++++++++++++++++++++++++++-
>  drivers/video/console/dummycon.c |  1 +
>  drivers/video/console/vgacon.c   |  1 +
>  3 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 27fe65ac5940..bcb66ddd649e 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -36,6 +36,8 @@
>  #include "i915_drv.h"
>  #include "i915_trace.h"
>  #include <linux/pci.h>
> +#include <linux/console.h>
> +#include <linux/vt.h>
>  #include <linux/vgaarb.h>
>  #include <linux/acpi.h>
>  #include <linux/pnp.h>
> @@ -1449,6 +1451,38 @@ static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv)
>  }
>  #endif
>
> +#if !defined(CONFIG_VGA_CONSOLE)
> +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> +{
> +       return 0;
> +}
> +#elif !defined(CONFIG_DUMMY_CONSOLE)
> +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> +{
> +       return -ENODEV;
> +}
> +#else
> +static int i915_kick_out_vgacon(struct drm_i915_private *dev_priv)
> +{
> +       int ret;
> +
> +       DRM_INFO("Replacing VGA console driver\n");
> +
> +       console_lock();
> +       ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, 1);
> +       if (ret = 0) {
> +               ret = do_unregister_con_driver(&vga_con);
> +
> +               /* Ignore "already unregistered". */
> +               if (ret = -ENODEV)
> +                       ret = 0;
> +       }
> +       console_unlock();
> +
> +       return ret;
> +}
> +#endif
> +
>  static void i915_dump_device_info(struct drm_i915_private *dev_priv)
>  {
>         const struct intel_device_info *info = &dev_priv->info;
> @@ -1622,8 +1656,15 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>         if (ret)
>                 goto out_regs;
>
> -       if (drm_core_check_feature(dev, DRIVER_MODESET))
> +       if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> +               ret = i915_kick_out_vgacon(dev_priv);
> +               if (ret) {
> +                       DRM_ERROR("failed to remove conflicting VGA console\n");
> +                       goto out_gtt;
> +               }
> +
>                 i915_kick_out_firmware_fb(dev_priv);
> +       }
>
>         pci_set_master(dev->pdev);
>
> diff --git a/drivers/video/console/dummycon.c b/drivers/video/console/dummycon.c
> index b63860f7beab..40bec8d64b0a 100644
> --- a/drivers/video/console/dummycon.c
> +++ b/drivers/video/console/dummycon.c
> @@ -77,3 +77,4 @@ const struct consw dummy_con = {
>      .con_set_palette = DUMMY,
>      .con_scrolldelta = DUMMY,
>  };
> +EXPORT_SYMBOL_GPL(dummy_con);
> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
> index 9d8feac67637..84acd6223dc5 100644
> --- a/drivers/video/console/vgacon.c
> +++ b/drivers/video/console/vgacon.c
> @@ -1440,5 +1440,6 @@ const struct consw vga_con = {
>         .con_build_attr = vgacon_build_attr,
>         .con_invert_region = vgacon_invert_region,
>  };
> +EXPORT_SYMBOL(vga_con);
>
>  MODULE_LICENSE("GPL");
> --
> 1.8.1.4
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply

* Re: [PATCH 5/5] drm/i915: Kick out vga console
From: Tomi Valkeinen @ 2014-06-09 13:22 UTC (permalink / raw)
  To: Daniel Vetter, Intel Graphics Development
  Cc: Linux Fbdev development list, LKML, DRI Development,
	David Herrmann, Jean-Christophe Plagniol-Villard
In-Reply-To: <CAKMK7uH7GWspr03NBi7rVY+2UN-vdthMV7eixXA-xc3t2JdzPg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 273 bytes --]

Hi,

On 06/06/14 18:20, Daniel Vetter wrote:
> Tomi/Jean can you please ack merging this through drm-intel trees? It
> just exports the vga and dummy consoles so that i915 can do what it
> needs to do.

Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox