* [PATCH] video: mb862xx-i2c: fix for reliable decoder register access
From: Anatolij Gustschin @ 2011-09-01 15:53 UTC (permalink / raw)
To: linux-fbdev
Increase delay when polling for tx status. This fixes
the unreliable video decoder i2c register access.
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
drivers/video/mb862xx/mb862xx-i2c.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/mb862xx/mb862xx-i2c.c b/drivers/video/mb862xx/mb862xx-i2c.c
index b953099..934081d 100644
--- a/drivers/video/mb862xx/mb862xx-i2c.c
+++ b/drivers/video/mb862xx/mb862xx-i2c.c
@@ -23,7 +23,7 @@ static int mb862xx_i2c_wait_event(struct i2c_adapter *adap)
u32 reg;
do {
- udelay(1);
+ udelay(10);
reg = inreg(i2c, GC_I2C_BCR);
if (reg & (I2C_INT | I2C_BER))
break;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] fb: avoid possible deadlock caused by fb_set_suspend
From: Guennadi Liakhovetski @ 2011-09-01 16:28 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: Bruno Prémont, lethal, linux-fbdev, francis.moro, torvalds,
linux-kernel, Herton Ronaldo Krzesinski, stable
In-Reply-To: <4E5FA7F9.9030802@gmx.de>
Hi Florian
On Thu, 1 Sep 2011, Florian Tobias Schandinat wrote:
> ping
>
> Guennadi, I really want this issue fixed. Please have a look at Bruno's patch
> otherwise your driver might remain or get even more broken...
Sorry, somehow this patch excaped my attention. Will test tomorrow.
Thanks
Guennadi
>
> I am scheduling Herton's patch for the next merge window.
>
>
> Regards,
>
> Florian Tobias Schandinat
>
>
> On 06/18/2011 09:19 AM, Bruno Prémont wrote:
> > Guennadi, could you have a look at (completely untested) patch which avoids
> > possible deadlock due to inverted lock taking order on hotplug as well
> > as "readding" lock_fb_info() for fb_set_suspend() call after Herton's
> > patch to fb_set_suspend().
> >
> > Thanks,
> > Bruno
> >
> >
> > On Sat, 18 June 2011 Bruno Prémont <bonbons@linux-vserver.org> wrote:
> >> On Fri, 17 June 2011 Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:
> >>> From: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
> >>>
> >>> A lock ordering issue can cause deadlocks: in framebuffer/console code,
> >>> all needed struct fb_info locks are taken before acquire_console_sem(),
> >>> in places which need to take console semaphore.
> >>>
> >>> But fb_set_suspend is always called with console semaphore held, and
> >>> inside it we call lock_fb_info which gets the fb_info lock, inverse
> >>> locking order of what the rest of the code does. This causes a real
> >>> deadlock issue, when we write to state fb sysfs attribute (which calls
> >>> fb_set_suspend) while a framebuffer is being unregistered by
> >>> remove_conflicting_framebuffers, as can be shown by following show
> >>> blocked state trace on a test program which loads i915 and runs another
> >>> forked processes writing to state attribute:
> >>>
> >>> Test process with semaphore held and trying to get fb_info lock:
> >>
> >> ...
> >>
> >>> fb-test2 which reproduces above is available on kernel.org bug #26232.
> >>> To solve this issue, avoid calling lock_fb_info inside fb_set_suspend,
> >>> and move it out to where needed (callers of fb_set_suspend must call
> >>> lock_fb_info before if needed). So far, the only place which needs to
> >>> call lock_fb_info is store_fbstate, all other places which calls
> >>> fb_set_suspend are suspend/resume hooks that should not need the lock as
> >>> they should be run only when processes are already frozen in
> >>> suspend/resume.
> >>
> >> From a quick look through FB drivers in 2.6.39 I've found one that would need
> >> more work:
> >> - drivers/video/sh_mobile_hdmi.c: sh_hdmi_edid_work_fn()
> >> Should get changed to
> >> a) right locking order in case (hdmi->hp_state = HDMI_HOTPLUG_CONNECTED)
> >> b) lock fb_info in the other case
> >> For this one fb_set_suspend() does get call in a hotplug worker,
> >> thus independently on suspend/resume process.
> >>
> >> The rest does match the suspend/resume hook pattern mentioned.
> >>
> >> Bruno
> >>
> >>
> >>> References: https://bugzilla.kernel.org/show_bug.cgi?id&232
> >>> Signed-off-by: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
> >>> Signed-off-by: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> >>> Cc: stable@kernel.org
> >>> ---
> >>> drivers/video/fbmem.c | 3 ---
> >>> drivers/video/fbsysfs.c | 3 +++
> >>> 2 files changed, 3 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> >>> index 5aac00e..ad93629 100644
> >>> --- a/drivers/video/fbmem.c
> >>> +++ b/drivers/video/fbmem.c
> >>> @@ -1738,8 +1738,6 @@ void fb_set_suspend(struct fb_info *info, int state)
> >>> {
> >>> struct fb_event event;
> >>>
> >>> - if (!lock_fb_info(info))
> >>> - return;
> >>> event.info = info;
> >>> if (state) {
> >>> fb_notifier_call_chain(FB_EVENT_SUSPEND, &event);
> >>> @@ -1748,7 +1746,6 @@ void fb_set_suspend(struct fb_info *info, int state)
> >>> info->state = FBINFO_STATE_RUNNING;
> >>> fb_notifier_call_chain(FB_EVENT_RESUME, &event);
> >>> }
> >>> - unlock_fb_info(info);
> >>> }
> >>>
> >>> /**
> >>> diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
> >>> index 04251ce..67afa9c 100644
> >>> --- a/drivers/video/fbsysfs.c
> >>> +++ b/drivers/video/fbsysfs.c
> >>> @@ -399,9 +399,12 @@ static ssize_t store_fbstate(struct device *device,
> >>>
> >>> state = simple_strtoul(buf, &last, 0);
> >>>
> >>> + if (!lock_fb_info(fb_info))
> >>> + return -ENODEV;
> >>> console_lock();
> >>> fb_set_suspend(fb_info, (int)state);
> >>> console_unlock();
> >>> + unlock_fb_info(fb_info);
> >>>
> >>> return count;
> >>> }
> >
> >
> > diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
> > index 2b9e56a..b1a13ab 100644
> > --- a/drivers/video/sh_mobile_hdmi.c
> > +++ b/drivers/video/sh_mobile_hdmi.c
> > @@ -1151,27 +1151,27 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
> >
> > ch = info->par;
> >
> > - console_lock();
> > + if (lock_fb_info(info)) {
> > + console_lock();
> >
> > - /* HDMI plug in */
> > - if (!sh_hdmi_must_reconfigure(hdmi) &&
> > - info->state = FBINFO_STATE_RUNNING) {
> > - /*
> > - * First activation with the default monitor - just turn
> > - * on, if we run a resume here, the logo disappears
> > - */
> > - if (lock_fb_info(info)) {
> > + /* HDMI plug in */
> > + if (!sh_hdmi_must_reconfigure(hdmi) &&
> > + info->state = FBINFO_STATE_RUNNING) {
> > + /*
> > + * First activation with the default monitor - just turn
> > + * on, if we run a resume here, the logo disappears
> > + */
> > info->var.width = hdmi->var.width;
> > info->var.height = hdmi->var.height;
> > sh_hdmi_display_on(hdmi, info);
> > - unlock_fb_info(info);
> > + } else {
> > + /* New monitor or have to wake up */
> > + fb_set_suspend(info, 0);
> > }
> > - } else {
> > - /* New monitor or have to wake up */
> > - fb_set_suspend(info, 0);
> > - }
> >
> > - console_unlock();
> > + console_unlock();
> > + unlock_fb_info(info);
> > + }
> > } else {
> > ret = 0;
> > if (!hdmi->info)
> > @@ -1181,12 +1181,15 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
> > fb_destroy_modedb(hdmi->monspec.modedb);
> > hdmi->monspec.modedb = NULL;
> >
> > - console_lock();
> > + if (lock_fb_info(info)) {
> > + console_lock();
> >
> > - /* HDMI disconnect */
> > - fb_set_suspend(hdmi->info, 1);
> > + /* HDMI disconnect */
> > + fb_set_suspend(hdmi->info, 1);
> >
> > - console_unlock();
> > + console_unlock();
> > + unlock_fb_info(info);
> > + }
> > pm_runtime_put(hdmi->dev);
> > }
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 0/2] video: s3c-fb: Add window positioning support
From: Florian Tobias Schandinat @ 2011-09-01 16:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1314301917-9938-1-git-send-email-ajaykumar.rs@samsung.com>
Hi all,
On 08/25/2011 07:51 PM, Ajay Kumar wrote:
> Just as a note, there are many drivers like mx3fb.c, au1200fb.c and OMAP
> seem to be doing window/plane positioning in their driver code.
> Is it possible to have this window positioning support at a common place?
Good point. Congratulations for figuring out that I like to standardize things.
But I think your suggestion is far from being enough to be useful for userspace
(which is our goal so that applications can be reused along drivers and don't
need to know about individual drivers).
So let me at first summarize how I understand you implemented those things after
having a brief look at some of the drivers:
Windows are rectangular screen areas whose pixel data come from other locations.
The other locations are accessible via other framebuffer devices (e.g. fb1). So
in this area the data of fb1 is shown and not the data of fb0 that would be
normally shown.
So in addition to your proposed positioning I think we should also have the
following to give userspace a useful set of functionality:
- a way to discover how the screen is composited (how many windows are there,
how they are stacked and how to access those)
- a way to enable/disable windows (make them (in)visible)
- reporting and selecting how the window content can be mixed with the root
screen (overwrite, source or destination color keying)
- things like window size and color format could be handled by the usual fb API
used on the window. However there might be restrictions which cause them to be
not 100% API compatible (for example when changing the color format if the
windows are required to have the same format as the root screen)
- do we need to worry about hardware (up/down) scaling of the window content?
So is that what you need for a standardized window implementation?
Any additional things that were useful/needed in this context?
Would you consider adding support for this API in your drivers? (as
standardizing wouldn't be useful if nobody would implement it)
Best regards,
Florian Tobias Schandinat
>
> For instance, we can have a common struture and ioctl number in
> include/linux/fb.h as below:
>
> #define FBIOPOS_OVERLAY_WIN _IOW('F', 0x21, struct fb_overlay_win_pos)
>
> struct fb_overlay_win_pos {
> __u32 win_pos_x; /* x-offset of window from LCD(0,0) */
> __u32 win_pos_y; /* y-offset of window from LCD(0,0) */
> };
>
> where LCD(0,0) means the first pixel of the LCD screen.
> Individual drivers can have implementation for this ioctl.
>
> To Kukjin Kim,
> [PATCH 1/2] ARM: SAMSUNG: Add Window Positioning Support for s3c-fb driver
>
> To Paul Mundt, Florian Tobias Schandinat
> [PATCH 2/2] video: s3c-fb: Modify s3c-fb driver to support window positioning
>
> arch/arm/plat-samsung/include/plat/fb.h | 14 +++++++++++
> drivers/video/s3c-fb.c | 37 ++++++++++++++++++++++++++----
> 2 files changed, 46 insertions(+), 5 deletions(-)
>
>
^ permalink raw reply
* Re: [PATCH 3/4] OMAP: DSS2: Handle manager change in apply
From: Archit Taneja @ 2011-09-02 6:51 UTC (permalink / raw)
To: Valkeinen, Tomi; +Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org
In-Reply-To: <1314001636-18036-4-git-send-email-tomi.valkeinen@ti.com>
On Monday 22 August 2011 01:57 PM, Valkeinen, Tomi wrote:
> Currently when changing the manager of an overlay, set_manager() directly
> calls dispc to set the overlay's destination.
>
> Change this to be more in line with other overlay configurations, and
> this will also remove the need to have dispc clocks enabled when calling
> set_manager().
>
> A new field is added to overlay struct, "manager_changed". This is
> similar to "display_changed" field in manager struct, and is used to
> inform apply that the manager has changed and thus write to the
> registers is needed.
I was wondering if it would be better to create an overlay_info member
called 'channel_out' rather than having 'manager_enabled' at a higher
level? This way, we won't need to do some of the things below(I have
pointed them out):
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/dispc.c | 4 +++-
> drivers/video/omap2/dss/dss.h | 2 --
> drivers/video/omap2/dss/manager.c | 5 +++++
> drivers/video/omap2/dss/overlay.c | 9 ++-------
> include/video/omapdss.h | 1 +
> 5 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index 9d9fbeb..003227c 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -841,7 +841,7 @@ static void _dispc_set_color_mode(enum omap_plane plane,
> REG_FLD_MOD(DISPC_OVL_ATTRIBUTES(plane), m, 4, 1);
> }
>
> -void dispc_set_channel_out(enum omap_plane plane,
> +static void dispc_set_channel_out(enum omap_plane plane,
> enum omap_channel channel)
> {
> int shift;
> @@ -1860,6 +1860,8 @@ int dispc_setup_plane(enum omap_plane plane,
> _dispc_set_pre_mult_alpha(plane, pre_mult_alpha);
> _dispc_setup_global_alpha(plane, global_alpha);
>
> + dispc_set_channel_out(plane, channel);
> +
> return 0;
> }
>
> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> index adeff04..ff7ac35 100644
> --- a/drivers/video/omap2/dss/dss.h
> +++ b/drivers/video/omap2/dss/dss.h
> @@ -399,8 +399,6 @@ void dispc_set_plane_ba0(enum omap_plane plane, u32 paddr);
> void dispc_set_plane_ba1(enum omap_plane plane, u32 paddr);
> void dispc_set_plane_pos(enum omap_plane plane, u16 x, u16 y);
> void dispc_set_plane_size(enum omap_plane plane, u16 width, u16 height);
> -void dispc_set_channel_out(enum omap_plane plane,
> - enum omap_channel channel_out);
>
> void dispc_enable_gamma_table(bool enable);
> int dispc_setup_plane(enum omap_plane plane,
> diff --git a/drivers/video/omap2/dss/manager.c b/drivers/video/omap2/dss/manager.c
> index 63674b0..a6a909a 100644
> --- a/drivers/video/omap2/dss/manager.c
> +++ b/drivers/video/omap2/dss/manager.c
> @@ -1338,6 +1338,11 @@ static int omap_dss_mgr_apply(struct omap_overlay_manager *mgr)
>
> oc =&dss_cache.overlay_cache[ovl->id];
>
> + if (ovl->manager_changed) {
> + ovl->manager_changed = false;
> + ovl->info_dirty = true;
> + }
> +
We won't need to do this if we created channel_out as an overlay_info
member. The info will get dirty automatically.
> if (!overlay_enabled(ovl)) {
> if (oc->enabled) {
> oc->enabled = false;
> diff --git a/drivers/video/omap2/dss/overlay.c b/drivers/video/omap2/dss/overlay.c
> index c84380c..ab44403 100644
> --- a/drivers/video/omap2/dss/overlay.c
> +++ b/drivers/video/omap2/dss/overlay.c
> @@ -516,6 +516,7 @@ static int omap_dss_set_manager(struct omap_overlay *ovl,
> }
>
> ovl->manager = mgr;
> + ovl->manager_changed = true;
>
> /* XXX: When there is an overlay on a DSI manual update display, and
> * the overlay is first disabled, then moved to tv, and enabled, we
> @@ -529,15 +530,12 @@ static int omap_dss_set_manager(struct omap_overlay *ovl,
> * Userspace workaround for this is to update the LCD after disabling
> * the overlay, but before moving the overlay to TV.
> */
> - dispc_set_channel_out(ovl->id, mgr->id);
We would need to do a get_overlay_info/set_overlay_info here, like it is
done for other overlay sysfs attributes.
>
> return 0;
> }
>
> static int omap_dss_unset_manager(struct omap_overlay *ovl)
> {
> - int r;
> -
> if (!ovl->manager) {
> DSSERR("failed to detach overlay: manager not set\n");
> return -EINVAL;
> @@ -548,11 +546,8 @@ static int omap_dss_unset_manager(struct omap_overlay *ovl)
> return -EINVAL;
> }
>
> - r = ovl->wait_for_go(ovl);
> - if (r)
> - return r;
> -
> ovl->manager = NULL;
> + ovl->manager_changed = true;
>
> return 0;
> }
> diff --git a/include/video/omapdss.h b/include/video/omapdss.h
> index 9301805..b965f5a 100644
> --- a/include/video/omapdss.h
> +++ b/include/video/omapdss.h
> @@ -326,6 +326,7 @@ struct omap_overlay {
> struct omap_overlay_manager *manager;
> struct omap_overlay_info info;
>
> + bool manager_changed;
> /* if true, info has been changed, but not applied() yet */
> bool info_dirty;
>
^ permalink raw reply
* Re: [PATCH 2/4] OMAP4: TWL: Add common omapdss supplies
From: Archit Taneja @ 2011-09-02 6:52 UTC (permalink / raw)
To: Valkeinen, Tomi; +Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org
In-Reply-To: <1314001599-17951-3-git-send-email-tomi.valkeinen@ti.com>
Hi,
On Monday 22 August 2011 01:56 PM, Valkeinen, Tomi wrote:
> OMAP DSS normally gets power from VCXIO on OMAP4. Add configuration for
> this into twl-common.c
>
> Mark VCXIO as always_on, as VCXIO is used by multiple components,
> including the MPU, and turning it off when DSS doesn't need it would
> lead the device to halt.
I had a query. Is this the right place to set always_on to true? Won't
this lead to VCXIO being always on all omap4 boards? Is it necessary
that VCXIO will be used to power on MPU for all omap4 boards?
Archit
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
> arch/arm/mach-omap2/twl-common.c | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
> index 4f6d216..52243577 100644
> --- a/arch/arm/mach-omap2/twl-common.c
> +++ b/arch/arm/mach-omap2/twl-common.c
> @@ -235,6 +235,12 @@ static struct regulator_init_data omap4_vana_idata = {
> },
> };
>
> +static struct regulator_consumer_supply omap4_vcxio_supply[] = {
> + REGULATOR_SUPPLY("vdds_dsi", "omapdss_dss"),
> + REGULATOR_SUPPLY("vdds_dsi", "omapdss_dsi.0"),
> + REGULATOR_SUPPLY("vdds_dsi", "omapdss_dsi.1"),
> +};
> +
> static struct regulator_init_data omap4_vcxio_idata = {
> .constraints = {
> .min_uV = 1800000,
> @@ -243,7 +249,10 @@ static struct regulator_init_data omap4_vcxio_idata = {
> | REGULATOR_MODE_STANDBY,
> .valid_ops_mask = REGULATOR_CHANGE_MODE
> | REGULATOR_CHANGE_STATUS,
> + .always_on = true,
> },
> + .num_consumer_supplies = ARRAY_SIZE(omap4_vcxio_supply),
> + .consumer_supplies = omap4_vcxio_supply,
> };
>
> static struct regulator_init_data omap4_vusb_idata = {
^ permalink raw reply
* Re: [PATCH 1/8] OMAP: DSS2: DISPC: Fix minimum PCD value
From: Archit Taneja @ 2011-09-02 6:54 UTC (permalink / raw)
To: Valkeinen, Tomi
Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org,
K, Mythri P
In-Reply-To: <1314796908-17354-2-git-send-email-tomi.valkeinen@ti.com>
On Wednesday 31 August 2011 06:51 PM, Valkeinen, Tomi wrote:
> The current driver had a hardcoded minimum value of 2 for pixel clock
> divisor (PCD). This doesn't seem to be right.
>
> OMAP4 TRM says that PCD can be 1 when not downscaling, and inverted
> pixel clock (IPC) is off.
>
> OMAP3 TRM says the same, but also in the register descriptions that PCD
> value 1 is invalid.
>
> OMAP2 TRM says PCD 2 is the minimum.
>
> OMAP2 is still untested, but for both OMAP3 and OMAP4 PCD of 1 seems to
> work fine.
>
> This patch adds a new DSS feature, FEAT_PARAM_DSS_PCD, which is used to
> find the minimum and maximum PCD. The minimum is set to 2 for OMAP2, and
> 1 for OMAP3/4.
This looks good to me.
Archit
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/dispc.c | 14 ++++++++++----
> drivers/video/omap2/dss/dss_features.c | 3 +++
> drivers/video/omap2/dss/dss_features.h | 1 +
> 3 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
> index de20936..7f3d847 100644
> --- a/drivers/video/omap2/dss/dispc.c
> +++ b/drivers/video/omap2/dss/dispc.c
> @@ -2339,7 +2339,7 @@ static void dispc_mgr_set_lcd_divisor(enum omap_channel channel, u16 lck_div,
> u16 pck_div)
> {
> BUG_ON(lck_div< 1);
> - BUG_ON(pck_div< 2);
> + BUG_ON(pck_div< 1);
>
> dispc_write_reg(DISPC_DIVISORo(channel),
> FLD_VAL(lck_div, 23, 16) | FLD_VAL(pck_div, 7, 0));
> @@ -2726,11 +2726,17 @@ void dispc_mgr_set_pol_freq(enum omap_channel channel,
> void dispc_find_clk_divs(bool is_tft, unsigned long req_pck, unsigned long fck,
> struct dispc_clock_info *cinfo)
> {
> - u16 pcd_min = is_tft ? 2 : 3;
> + u16 pcd_min, pcd_max;
> unsigned long best_pck;
> u16 best_ld, cur_ld;
> u16 best_pd, cur_pd;
>
> + pcd_min = dss_feat_get_param_min(FEAT_PARAM_DSS_PCD);
> + pcd_max = dss_feat_get_param_max(FEAT_PARAM_DSS_PCD);
> +
> + if (!is_tft)
> + pcd_min = 3;
> +
> best_pck = 0;
> best_ld = 0;
> best_pd = 0;
> @@ -2738,7 +2744,7 @@ void dispc_find_clk_divs(bool is_tft, unsigned long req_pck, unsigned long fck,
> for (cur_ld = 1; cur_ld<= 255; ++cur_ld) {
> unsigned long lck = fck / cur_ld;
>
> - for (cur_pd = pcd_min; cur_pd<= 255; ++cur_pd) {
> + for (cur_pd = pcd_min; cur_pd<= pcd_max; ++cur_pd) {
> unsigned long pck = lck / cur_pd;
> long old_delta = abs(best_pck - req_pck);
> long new_delta = abs(pck - req_pck);
> @@ -2773,7 +2779,7 @@ int dispc_calc_clock_rates(unsigned long dispc_fclk_rate,
> {
> if (cinfo->lck_div> 255 || cinfo->lck_div = 0)
> return -EINVAL;
> - if (cinfo->pck_div< 2 || cinfo->pck_div> 255)
> + if (cinfo->pck_div< 1 || cinfo->pck_div> 255)
> return -EINVAL;
>
> cinfo->lck = dispc_fclk_rate / cinfo->lck_div;
> diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
> index b63c5f8..85b07ca 100644
> --- a/drivers/video/omap2/dss/dss_features.c
> +++ b/drivers/video/omap2/dss/dss_features.c
> @@ -281,6 +281,7 @@ static const char * const omap4_dss_clk_source_names[] = {
>
> static const struct dss_param_range omap2_dss_param_range[] = {
> [FEAT_PARAM_DSS_FCK] = { 0, 173000000 },
> + [FEAT_PARAM_DSS_PCD] = { 2, 255 },
> [FEAT_PARAM_DSIPLL_REGN] = { 0, 0 },
> [FEAT_PARAM_DSIPLL_REGM] = { 0, 0 },
> [FEAT_PARAM_DSIPLL_REGM_DISPC] = { 0, 0 },
> @@ -291,6 +292,7 @@ static const struct dss_param_range omap2_dss_param_range[] = {
>
> static const struct dss_param_range omap3_dss_param_range[] = {
> [FEAT_PARAM_DSS_FCK] = { 0, 173000000 },
> + [FEAT_PARAM_DSS_PCD] = { 1, 255 },
> [FEAT_PARAM_DSIPLL_REGN] = { 0, (1<< 7) - 1 },
> [FEAT_PARAM_DSIPLL_REGM] = { 0, (1<< 11) - 1 },
> [FEAT_PARAM_DSIPLL_REGM_DISPC] = { 0, (1<< 4) - 1 },
> @@ -301,6 +303,7 @@ static const struct dss_param_range omap3_dss_param_range[] = {
>
> static const struct dss_param_range omap4_dss_param_range[] = {
> [FEAT_PARAM_DSS_FCK] = { 0, 186000000 },
> + [FEAT_PARAM_DSS_PCD] = { 1, 255 },
> [FEAT_PARAM_DSIPLL_REGN] = { 0, (1<< 8) - 1 },
> [FEAT_PARAM_DSIPLL_REGM] = { 0, (1<< 12) - 1 },
> [FEAT_PARAM_DSIPLL_REGM_DISPC] = { 0, (1<< 5) - 1 },
> diff --git a/drivers/video/omap2/dss/dss_features.h b/drivers/video/omap2/dss/dss_features.h
> index 4271e96..158d922 100644
> --- a/drivers/video/omap2/dss/dss_features.h
> +++ b/drivers/video/omap2/dss/dss_features.h
> @@ -73,6 +73,7 @@ enum dss_feat_reg_field {
>
> enum dss_range_param {
> FEAT_PARAM_DSS_FCK,
> + FEAT_PARAM_DSS_PCD,
> FEAT_PARAM_DSIPLL_REGN,
> FEAT_PARAM_DSIPLL_REGM,
> FEAT_PARAM_DSIPLL_REGM_DISPC,
^ permalink raw reply
* Re: [PATCH 3/4] OMAP: DSS2: Handle manager change in apply
From: Tomi Valkeinen @ 2011-09-02 7:25 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org
In-Reply-To: <4E607CC3.2000407@ti.com>
On Fri, 2011-09-02 at 12:20 +0530, Archit Taneja wrote:
> On Monday 22 August 2011 01:57 PM, Valkeinen, Tomi wrote:
> > Currently when changing the manager of an overlay, set_manager()
> directly
> > calls dispc to set the overlay's destination.
> >
> > Change this to be more in line with other overlay configurations,
> and
> > this will also remove the need to have dispc clocks enabled when
> calling
> > set_manager().
> >
> > A new field is added to overlay struct, "manager_changed". This is
> > similar to "display_changed" field in manager struct, and is used to
> > inform apply that the manager has changed and thus write to the
> > registers is needed.
>
> I was wondering if it would be better to create an overlay_info
> member
> called 'channel_out' rather than having 'manager_enabled' at a higher
> level? This way, we won't need to do some of the things below(I have
> pointed them out):
The overlay_info is written by the users of the DSS. So if we had
channel_out there, we'd need to remove the set/get_manager() functions.
I made those functions in the first place as I felt changing the manager
is a bit bigger operation than the normal overlay attributes. Changing
the manager does effect both the old and the new managers. While I don't
think we currently do anything related to that, I believe it would be
needed for optimizations like FIFO merge.
It could perhaps be possible to change this so that the overlay_info has
the channel_out parameter, but that would be a bit bigger change, and
would needs lots of testing. So I feel this is a safer change, and it
fixes a problem we had with DRM.
Tomi
^ permalink raw reply
* Re: [PATCH 2/4] OMAP4: TWL: Add common omapdss supplies
From: Tomi Valkeinen @ 2011-09-02 7:30 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org
In-Reply-To: <4E607A40.8000707@ti.com>
On Fri, 2011-09-02 at 12:10 +0530, Archit Taneja wrote:
> Hi,
>
> On Monday 22 August 2011 01:56 PM, Valkeinen, Tomi wrote:
> > OMAP DSS normally gets power from VCXIO on OMAP4. Add configuration
> for
> > this into twl-common.c
> >
> > Mark VCXIO as always_on, as VCXIO is used by multiple components,
> > including the MPU, and turning it off when DSS doesn't need it would
> > lead the device to halt.
>
> I had a query. Is this the right place to set always_on to true?
> Won't
> this lead to VCXIO being always on all omap4 boards? Is it necessary
> that VCXIO will be used to power on MPU for all omap4 boards?
I don't really know, but TRM's 27.2.1 Power Requirements chapter shows
vcxio connected to multiple places. It's an example, but I would guess
it is the common setup, thus setting it to always on in twl-common.c
sounds ok.
If there will be an omap4 board with different setup, then we need to
revisit this.
Tomi
^ permalink raw reply
* Re: [PATCH 3/4] OMAP: DSS2: Handle manager change in apply
From: Archit Taneja @ 2011-09-02 7:44 UTC (permalink / raw)
To: Valkeinen, Tomi; +Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org
In-Reply-To: <1314948322.3374.21.camel@deskari>
On Friday 02 September 2011 12:55 PM, Valkeinen, Tomi wrote:
> On Fri, 2011-09-02 at 12:20 +0530, Archit Taneja wrote:
>> On Monday 22 August 2011 01:57 PM, Valkeinen, Tomi wrote:
>>> Currently when changing the manager of an overlay, set_manager()
>> directly
>>> calls dispc to set the overlay's destination.
>>>
>>> Change this to be more in line with other overlay configurations,
>> and
>>> this will also remove the need to have dispc clocks enabled when
>> calling
>>> set_manager().
>>>
>>> A new field is added to overlay struct, "manager_changed". This is
>>> similar to "display_changed" field in manager struct, and is used to
>>> inform apply that the manager has changed and thus write to the
>>> registers is needed.
>>
>> I was wondering if it would be better to create an overlay_info
>> member
>> called 'channel_out' rather than having 'manager_enabled' at a higher
>> level? This way, we won't need to do some of the things below(I have
>> pointed them out):
>
> The overlay_info is written by the users of the DSS. So if we had
> channel_out there, we'd need to remove the set/get_manager() functions.
> I made those functions in the first place as I felt changing the manager
> is a bit bigger operation than the normal overlay attributes. Changing
> the manager does effect both the old and the new managers. While I don't
> think we currently do anything related to that, I believe it would be
> needed for optimizations like FIFO merge.
Right, I forgot users of DSS2 will also get the opportunity to change
channel_out, and we would need to do extra stuff in that case.
>
> It could perhaps be possible to change this so that the overlay_info has
> the channel_out parameter, but that would be a bit bigger change, and
> would needs lots of testing. So I feel this is a safer change, and it
> fixes a problem we had with DRM.
Okay, we could think about this later then.
Archit
>
> Tomi
>
>
^ permalink raw reply
* Re: [PATCH 0/2] video: s3c-fb: Add window positioning support
From: Tomi Valkeinen @ 2011-09-02 9:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4E5FB69E.2060907@gmx.de>
Hi,
On Thu, 2011-09-01 at 16:45 +0000, Florian Tobias Schandinat wrote:
> Hi all,
>
> On 08/25/2011 07:51 PM, Ajay Kumar wrote:
> > Just as a note, there are many drivers like mx3fb.c, au1200fb.c and OMAP
> > seem to be doing window/plane positioning in their driver code.
> > Is it possible to have this window positioning support at a common place?
>
> Good point. Congratulations for figuring out that I like to standardize things.
> But I think your suggestion is far from being enough to be useful for userspace
> (which is our goal so that applications can be reused along drivers and don't
> need to know about individual drivers).
>
> So let me at first summarize how I understand you implemented those things after
> having a brief look at some of the drivers:
> Windows are rectangular screen areas whose pixel data come from other locations.
> The other locations are accessible via other framebuffer devices (e.g. fb1). So
> in this area the data of fb1 is shown and not the data of fb0 that would be
> normally shown.
Here's what we have on OMAP:
We have a bunch of hardware overlays, each of which can go to one
output. When using fbdev, the pixel data for overlays comes from the
framebuffers. One fb can be used as a pixel source for multiple
overlays.
So, for example, the "connections" can be like these:
Initial configuration
.-----. .------. .------.
| fb0 |-------->| ovl0 |-----.---->| LCD |
'-----' '------' | '------'
.-----. .------. |
| fb1 |-------->| ovl1 |-----|
'-----' '------' |
.-----. .------. | .------.
| fb2 |-------->| ovl2 |-----' | TV |
'-----' '------' '------'
Video on fb1, shown on LCD and TV
.-----. .------. .------.
| fb0 |-------->| ovl0 |-----.---->| LCD |
'-----' '------' | '------'
.-----. .------. |
| fb1 |----.--->| ovl1 |-----'
'-----' | '------'
.-----. | .------. .------.
| fb2 | '--->| ovl2 |---------->| TV |
'-----' '------' '------'
And how the actual image is composited on the display, we have first the
fb side (no news there):
.---------------------fb-vxres---------------------.
| |
| |
| (xoffset,yoffset) |
| .-------fb-xres------------. |
f | | |
b f | |
| b | |
v | | |
y y | |
r r | |
e e | |
s s | |
| | | |
| '--------------------------' |
| |
| |
'--------------------------------------------------'
The area marked by x/yoffset and x/yres is used as the pixel source for
the overlay.
On the display we have something like this:
.----------------disp-xres----------------.
| |
| (xpos,ypos) |
| .-----ovl-xres-----. |
d | | |
i o | |
s v | |
p l | |
| | | |
y y | |
r r | |
e e | |
s s | |
| | | |
| '------------------' |
| |
'-----------------------------------------'
The x/ypos of the overlay does not have any relation to the x/yoffset of
the framebuffer. The overlay's x/yres is the same as the fb's x/yres in
case the overlay doesn't support scaling (OMAP's case this is true for
overlay0). Otherwise the overlay's x/yres is only limited by the HW's
scaling capabilities.
The overlays have a priority order, and where there's no overlay, a
background color is used. So like this:
.-----------------------------------------.
| background color |
| .-----------------------. |
| | ovl0 | |
| | | |
| | .--------------. | |
| | | ovl1 | | |
| | | | | |
| | | .-------------------. |
| | | | ovl2 | |
| | '--------| | |
| '----------| | |
| | | |
| | | |
| | | |
| '-------------------' |
'-----------------------------------------'
As you may have noticed, this raises in annoying problem: the
framebuffer does not represent the display, and thus the display related
information in the fb, like video timings, are difficult or impossible
to use. I haven't found any solution to this.
> So in addition to your proposed positioning I think we should also have the
> following to give userspace a useful set of functionality:
>
> - a way to discover how the screen is composited (how many windows are there,
> how they are stacked and how to access those)
>
> - a way to enable/disable windows (make them (in)visible)
>
> - reporting and selecting how the window content can be mixed with the root
> screen (overwrite, source or destination color keying)
Right, and there may not be a root screen at all, like on OMAP. (if the
background color doesn't count as a root screen).
> - things like window size and color format could be handled by the usual fb API
> used on the window. However there might be restrictions which cause them to be
> not 100% API compatible (for example when changing the color format if the
> windows are required to have the same format as the root screen)
>
> - do we need to worry about hardware (up/down) scaling of the window content?
>
>
> So is that what you need for a standardized window implementation?
> Any additional things that were useful/needed in this context?
> Would you consider adding support for this API in your drivers? (as
> standardizing wouldn't be useful if nobody would implement it)
We already have the current implementation in omapfb, so while a
standard API would be nice, we still need to keep the old custom API for
compatibility.
Another thing is that I'm not sure how much features we should try to
add to fbdev. The fbdev model is a bit limited (for example, the timings
problem above, or double/triple buffering), and at least we are
currently looking at using DRM. There will still be a framebuffer, but
that's mostly for fbconsole and such, and doesn't need to support
overlays, scaling, and other more complex features.
Tomi
^ permalink raw reply
* [PATCH 0/6] fbdev: au1200fb updates
From: Manuel Lauss @ 2011-09-02 14:40 UTC (permalink / raw)
To: linux-fbdev
This is a resend of 5 patches Paul Mundt previously added to the fbdev tree,
plus a new one to silence excessive dmesg output. Please consider for 3.2.
Among other things, patch #2 fixes an annoying build failure with CONFIG_PM=y
which has been bugging Ralf Baechle (MIPS maintainer) for quite some time.
Run-tested on DB1200 and DB1300 evaluation boards.
Manuel Lauss (6):
au1200fb: use framebuffer_alloc()
au1200fb: fixup PM
au1200fb: make number of windows configurable at load time.
au1200fb: switch to FB_SYS helpers
au1200fb: fix hardcoded IRQ
fbdev: au1200fb: silence debug output
drivers/video/Kconfig | 7 +-
drivers/video/au1200fb.c | 299 ++++++++++++++++++++--------------------------
2 files changed, 136 insertions(+), 170 deletions(-)
--
1.7.6
^ permalink raw reply
* [PATCH 1/6] au1200fb: use framebuffer_alloc()
From: Manuel Lauss @ 2011-09-02 14:40 UTC (permalink / raw)
To: linux-fbdev
Convert to use framebuffer_alloc() instead of using fb_info embedded
into device context (which broke the driver in the past due to un-
initialized elements).
Signed-off-by: Manuel Lauss <manuel.lauss@googlemail.com>
---
drivers/video/au1200fb.c | 80 ++++++++++++++++++++++++++-------------------
1 files changed, 46 insertions(+), 34 deletions(-)
diff --git a/drivers/video/au1200fb.c b/drivers/video/au1200fb.c
index 5dff32a..442d005 100644
--- a/drivers/video/au1200fb.c
+++ b/drivers/video/au1200fb.c
@@ -150,7 +150,7 @@ struct au1200_lcd_iodata_t {
/* Private, per-framebuffer management information (independent of the panel itself) */
struct au1200fb_device {
- struct fb_info fb_info; /* FB driver info record */
+ struct fb_info *fb_info; /* FB driver info record */
int plane;
unsigned char* fb_mem; /* FrameBuffer memory map */
@@ -158,7 +158,7 @@ struct au1200fb_device {
dma_addr_t fb_phys;
};
-static struct au1200fb_device _au1200fb_devices[CONFIG_FB_AU1200_DEVS];
+static struct fb_info *_au1200fb_infos[CONFIG_FB_AU1200_DEVS];
/********************************************************************/
/* LCD controller restrictions */
@@ -713,7 +713,7 @@ static int fbinfo2index (struct fb_info *fb_info)
int i;
for (i = 0; i < CONFIG_FB_AU1200_DEVS; ++i) {
- if (fb_info = (struct fb_info *)(&_au1200fb_devices[i].fb_info))
+ if (fb_info = _au1200fb_infos[i])
return i;
}
printk("au1200fb: ERROR: fbinfo2index failed!\n");
@@ -962,7 +962,7 @@ static void au1200_setmode(struct au1200fb_device *fbdev)
lcd->window[plane].winctrl2 = ( 0
| LCD_WINCTRL2_CKMODE_00
| LCD_WINCTRL2_DBM
- | LCD_WINCTRL2_BX_N( fbdev->fb_info.fix.line_length)
+ | LCD_WINCTRL2_BX_N(fbdev->fb_info->fix.line_length)
| LCD_WINCTRL2_SCX_1
| LCD_WINCTRL2_SCY_1
) ;
@@ -1050,7 +1050,7 @@ static void au1200fb_update_fbinfo(struct fb_info *fbi)
static int au1200fb_fb_check_var(struct fb_var_screeninfo *var,
struct fb_info *fbi)
{
- struct au1200fb_device *fbdev = (struct au1200fb_device *)fbi;
+ struct au1200fb_device *fbdev = fbi->par;
u32 pixclock;
int screen_size, plane;
@@ -1142,7 +1142,7 @@ static int au1200fb_fb_check_var(struct fb_var_screeninfo *var,
*/
static int au1200fb_fb_set_par(struct fb_info *fbi)
{
- struct au1200fb_device *fbdev = (struct au1200fb_device *)fbi;
+ struct au1200fb_device *fbdev = fbi->par;
au1200fb_update_fbinfo(fbi);
au1200_setmode(fbdev);
@@ -1246,7 +1246,7 @@ static int au1200fb_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
{
unsigned int len;
unsigned long start=0, off;
- struct au1200fb_device *fbdev = (struct au1200fb_device *) info;
+ struct au1200fb_device *fbdev = info->par;
#ifdef CONFIG_PM
au1xxx_pm_access(LCD_pm_dev);
@@ -1561,10 +1561,9 @@ static irqreturn_t au1200fb_handle_irq(int irq, void* dev_id)
static int au1200fb_init_fbinfo(struct au1200fb_device *fbdev)
{
- struct fb_info *fbi = &fbdev->fb_info;
+ struct fb_info *fbi = fbdev->fb_info;
int bpp;
- memset(fbi, 0, sizeof(struct fb_info));
fbi->fbops = &au1200fb_fb_ops;
bpp = winbpp(win->w[fbdev->plane].mode_winctrl1);
@@ -1626,11 +1625,13 @@ static int au1200fb_init_fbinfo(struct au1200fb_device *fbdev)
static int au1200fb_drv_probe(struct platform_device *dev)
{
struct au1200fb_device *fbdev;
+ struct fb_info *fbi = NULL;
unsigned long page;
int bpp, plane, ret;
- if (!dev)
- return -EINVAL;
+ /* shut gcc up */
+ ret = 0;
+ fbdev = NULL;
for (plane = 0; plane < CONFIG_FB_AU1200_DEVS; ++plane) {
bpp = winbpp(win->w[plane].mode_winctrl1);
@@ -1639,8 +1640,15 @@ static int au1200fb_drv_probe(struct platform_device *dev)
if (win->w[plane].yres = 0)
win->w[plane].yres = panel->Yres;
- fbdev = &_au1200fb_devices[plane];
- memset(fbdev, 0, sizeof(struct au1200fb_device));
+ fbi = framebuffer_alloc(sizeof(struct au1200fb_device),
+ &dev->dev);
+ if (!fbi)
+ goto failed;
+
+ _au1200fb_infos[plane] = fbi;
+ fbdev = fbi->par;
+ fbdev->fb_info = fbi;
+
fbdev->plane = plane;
/* Allocate the framebuffer to the maximum screen size */
@@ -1673,21 +1681,20 @@ static int au1200fb_drv_probe(struct platform_device *dev)
goto failed;
/* Register new framebuffer */
- if ((ret = register_framebuffer(&fbdev->fb_info)) < 0) {
+ ret = register_framebuffer(fbi);
+ if (ret < 0) {
print_err("cannot register new framebuffer");
goto failed;
}
- au1200fb_fb_set_par(&fbdev->fb_info);
+ au1200fb_fb_set_par(fbi);
#if !defined(CONFIG_FRAMEBUFFER_CONSOLE) && defined(CONFIG_LOGO)
if (plane = 0)
- if (fb_prepare_logo(&fbdev->fb_info, FB_ROTATE_UR)) {
+ if (fb_prepare_logo(fbi, FB_ROTATE_UR)) {
/* Start display and show logo on boot */
- fb_set_cmap(&fbdev->fb_info.cmap,
- &fbdev->fb_info);
-
- fb_show_logo(&fbdev->fb_info, FB_ROTATE_UR);
+ fb_set_cmap(&fbi->cmap, fbi);
+ fb_show_logo(fbi, FB_ROTATE_UR);
}
#endif
}
@@ -1705,12 +1712,13 @@ static int au1200fb_drv_probe(struct platform_device *dev)
failed:
/* NOTE: This only does the current plane/window that failed; others are still active */
if (fbdev->fb_mem)
- dma_free_noncoherent(dev, PAGE_ALIGN(fbdev->fb_len),
+ dma_free_noncoherent(&dev->dev, PAGE_ALIGN(fbdev->fb_len),
fbdev->fb_mem, fbdev->fb_phys);
- if (fbdev->fb_info.cmap.len != 0)
- fb_dealloc_cmap(&fbdev->fb_info.cmap);
- if (fbdev->fb_info.pseudo_palette)
- kfree(fbdev->fb_info.pseudo_palette);
+ if (fbi) {
+ if (fbi->cmap.len != 0)
+ fb_dealloc_cmap(&fbi->cmap);
+ kfree(fbi->pseudo_palette);
+ }
if (plane = 0)
free_irq(AU1200_LCD_INT, (void*)dev);
return ret;
@@ -1719,6 +1727,7 @@ failed:
static int au1200fb_drv_remove(struct platform_device *dev)
{
struct au1200fb_device *fbdev;
+ struct fb_info *fbi;
int plane;
if (!dev)
@@ -1727,20 +1736,22 @@ static int au1200fb_drv_remove(struct platform_device *dev)
/* Turn off the panel */
au1200_setpanel(NULL);
- for (plane = 0; plane < CONFIG_FB_AU1200_DEVS; ++plane)
- {
- fbdev = &_au1200fb_devices[plane];
+ for (plane = 0; plane < CONFIG_FB_AU1200_DEVS; ++plane) {
+ fbi = _au1200fb_infos[plane];
+ fbdev = fbi->par;
/* Clean up all probe data */
- unregister_framebuffer(&fbdev->fb_info);
+ unregister_framebuffer(fbi);
if (fbdev->fb_mem)
dma_free_noncoherent(&dev->dev,
PAGE_ALIGN(fbdev->fb_len),
fbdev->fb_mem, fbdev->fb_phys);
- if (fbdev->fb_info.cmap.len != 0)
- fb_dealloc_cmap(&fbdev->fb_info.cmap);
- if (fbdev->fb_info.pseudo_palette)
- kfree(fbdev->fb_info.pseudo_palette);
+ if (fbi->cmap.len != 0)
+ fb_dealloc_cmap(&fbi->cmap);
+ kfree(fbi->pseudo_palette);
+
+ framebuffer_release(fbi);
+ _au1200fb_infos[plane] = NULL;
}
free_irq(AU1200_LCD_INT, (void *)dev);
@@ -1749,7 +1760,8 @@ static int au1200fb_drv_remove(struct platform_device *dev)
}
#ifdef CONFIG_PM
-static int au1200fb_drv_suspend(struct platform_device *dev, u32 state)
+static int au1200fb_drv_suspend(struct platform_device *dev,
+ pm_message_t state)
{
/* TODO */
return 0;
--
1.7.6
^ permalink raw reply related
* [PATCH 2/6] au1200fb: fixup PM
From: Manuel Lauss @ 2011-09-02 14:40 UTC (permalink / raw)
To: linux-fbdev
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1252", Size: 7074 bytes --]
Remove last traces of the unused custom Alchemy PM code, implement
suspend/resume callbacks.
Signed-off-by: Manuel Lauss <manuel.lauss@googlemail.com>
---
drivers/video/au1200fb.c | 154 ++++++++++++----------------------------------
1 files changed, 39 insertions(+), 115 deletions(-)
diff --git a/drivers/video/au1200fb.c b/drivers/video/au1200fb.c
index 442d005..480ecb1 100644
--- a/drivers/video/au1200fb.c
+++ b/drivers/video/au1200fb.c
@@ -46,10 +46,6 @@
#include <asm/mach-au1x00/au1000.h>
#include "au1200fb.h"
-#ifdef CONFIG_PM
-#include <asm/mach-au1x00/au1xxx_pm.h>
-#endif
-
#ifndef CONFIG_FB_AU1200_DEVS
#define CONFIG_FB_AU1200_DEVS 4
#endif
@@ -205,12 +201,6 @@ struct window_settings {
extern int board_au1200fb_panel_init (void);
extern int board_au1200fb_panel_shutdown (void);
-#ifdef CONFIG_PM
-int au1200fb_pm_callback(au1xxx_power_dev_t *dev,
- au1xxx_request_t request, void *data);
-au1xxx_power_dev_t *LCD_pm_dev;
-#endif
-
/*
* Default window configurations
*/
@@ -652,25 +642,6 @@ static struct panel_settings known_lcd_panels[]
/********************************************************************/
-#ifdef CONFIG_PM
-static int set_brightness(unsigned int brightness)
-{
- unsigned int hi1, divider;
-
- /* limit brightness pwm duty to >= 30/1600 */
- if (brightness < 30) {
- brightness = 30;
- }
- divider = (lcd->pwmdiv & 0x3FFFF) + 1;
- hi1 = (lcd->pwmhi >> 16) + 1;
- hi1 = (((brightness & 0xFF) + 1) * divider >> 8);
- lcd->pwmhi &= 0xFFFF;
- lcd->pwmhi |= (hi1 << 16);
-
- return brightness;
-}
-#endif /* CONFIG_PM */
-
static int winbpp (unsigned int winctrl1)
{
int bits = 0;
@@ -1248,10 +1219,6 @@ static int au1200fb_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
unsigned long start=0, off;
struct au1200fb_device *fbdev = info->par;
-#ifdef CONFIG_PM
- au1xxx_pm_access(LCD_pm_dev);
-#endif
-
if (vma->vm_pgoff > (~0UL >> PAGE_SHIFT)) {
return -EINVAL;
}
@@ -1461,10 +1428,6 @@ static int au1200fb_ioctl(struct fb_info *info, unsigned int cmd,
int plane;
int val;
-#ifdef CONFIG_PM
- au1xxx_pm_access(LCD_pm_dev);
-#endif
-
plane = fbinfo2index(info);
print_dbg("au1200fb: ioctl %d on plane %d\n", cmd, plane);
@@ -1622,7 +1585,7 @@ static int au1200fb_init_fbinfo(struct au1200fb_device *fbdev)
/* AU1200 LCD controller device driver */
-static int au1200fb_drv_probe(struct platform_device *dev)
+static int __devinit au1200fb_drv_probe(struct platform_device *dev)
{
struct au1200fb_device *fbdev;
struct fb_info *fbi = NULL;
@@ -1633,6 +1596,9 @@ static int au1200fb_drv_probe(struct platform_device *dev)
ret = 0;
fbdev = NULL;
+ /* Kickstart the panel */
+ au1200_setpanel(panel);
+
for (plane = 0; plane < CONFIG_FB_AU1200_DEVS; ++plane) {
bpp = winbpp(win->w[plane].mode_winctrl1);
if (win->w[plane].xres = 0)
@@ -1724,15 +1690,12 @@ failed:
return ret;
}
-static int au1200fb_drv_remove(struct platform_device *dev)
+static int __devexit au1200fb_drv_remove(struct platform_device *dev)
{
struct au1200fb_device *fbdev;
struct fb_info *fbi;
int plane;
- if (!dev)
- return -ENODEV;
-
/* Turn off the panel */
au1200_setpanel(NULL);
@@ -1760,31 +1723,53 @@ static int au1200fb_drv_remove(struct platform_device *dev)
}
#ifdef CONFIG_PM
-static int au1200fb_drv_suspend(struct platform_device *dev,
- pm_message_t state)
+static int au1200fb_drv_suspend(struct device *dev)
{
- /* TODO */
+ au1200_setpanel(NULL);
+
+ lcd->outmask = 0;
+ au_sync();
+
return 0;
}
-static int au1200fb_drv_resume(struct platform_device *dev)
+static int au1200fb_drv_resume(struct device *dev)
{
- /* TODO */
+ struct fb_info *fbi;
+ int i;
+
+ /* Kickstart the panel */
+ au1200_setpanel(panel);
+
+ for (i = 0; i < CONFIG_FB_AU1200_DEVS; i++) {
+ fbi = _au1200fb_infos[i];
+ au1200fb_fb_set_par(fbi);
+ }
+
return 0;
}
+
+static const struct dev_pm_ops au1200fb_pmops = {
+ .suspend = au1200fb_drv_suspend,
+ .resume = au1200fb_drv_resume,
+ .freeze = au1200fb_drv_suspend,
+ .thaw = au1200fb_drv_resume,
+};
+
+#define AU1200FB_PMOPS (&au1200fb_pmops)
+
+#else
+#define AU1200FB_PMOPS NULL
#endif /* CONFIG_PM */
static struct platform_driver au1200fb_driver = {
.driver = {
- .name = "au1200-lcd",
- .owner = THIS_MODULE,
+ .name = "au1200-lcd",
+ .owner = THIS_MODULE,
+ .pm = AU1200FB_PMOPS,
},
.probe = au1200fb_drv_probe,
- .remove = au1200fb_drv_remove,
-#ifdef CONFIG_PM
- .suspend = au1200fb_drv_suspend,
- .resume = au1200fb_drv_resume,
-#endif
+ .remove = __devexit_p(au1200fb_drv_remove),
};
/*-------------------------------------------------------------------------*/
@@ -1847,56 +1832,6 @@ static void au1200fb_setup(void)
}
}
-#ifdef CONFIG_PM
-static int au1200fb_pm_callback(au1xxx_power_dev_t *dev,
- au1xxx_request_t request, void *data) {
- int retval = -1;
- unsigned int d = 0;
- unsigned int brightness = 0;
-
- if (request = AU1XXX_PM_SLEEP) {
- board_au1200fb_panel_shutdown();
- }
- else if (request = AU1XXX_PM_WAKEUP) {
- if(dev->prev_state = SLEEP_STATE)
- {
- int plane;
- au1200_setpanel(panel);
- for (plane = 0; plane < CONFIG_FB_AU1200_DEVS; ++plane) {
- struct au1200fb_device *fbdev;
- fbdev = &_au1200fb_devices[plane];
- au1200fb_fb_set_par(&fbdev->fb_info);
- }
- }
-
- d = *((unsigned int*)data);
- if(d <\x10) brightness = 26;
- else if(d< ) brightness = 51;
- else if(d<0) brightness = 77;
- else if(d<@) brightness = 102;
- else if(d<P) brightness = 128;
- else if(d<`) brightness = 153;
- else if(d<p) brightness = 179;
- else if(d<€) brightness = 204;
- else if(d<) brightness = 230;
- else brightness = 255;
- set_brightness(brightness);
- } else if (request = AU1XXX_PM_GETSTATUS) {
- return dev->cur_state;
- } else if (request = AU1XXX_PM_ACCESS) {
- if (dev->cur_state != SLEEP_STATE)
- return retval;
- else {
- au1200_setpanel(panel);
- }
- } else if (request = AU1XXX_PM_IDLE) {
- } else if (request = AU1XXX_PM_CLEANUP) {
- }
-
- return retval;
-}
-#endif
-
static int __init au1200fb_init(void)
{
print_info("" DRIVER_DESC "");
@@ -1911,17 +1846,6 @@ static int __init au1200fb_init(void)
printk(DRIVER_NAME ": Panel %d %s\n", panel_index, panel->name);
printk(DRIVER_NAME ": Win %d %s\n", window_index, win->name);
- /* Kickstart the panel, the framebuffers/windows come soon enough */
- au1200_setpanel(panel);
-
- #ifdef CONFIG_PM
- LCD_pm_dev = new_au1xxx_power_device("LCD", &au1200fb_pm_callback, NULL);
- if ( LCD_pm_dev = NULL)
- printk(KERN_INFO "Unable to create a power management device entry for the au1200fb.\n");
- else
- printk(KERN_INFO "Power management device entry for the au1200fb loaded.\n");
- #endif
-
return platform_driver_register(&au1200fb_driver);
}
--
1.7.6
^ permalink raw reply related
* [PATCH 3/6] au1200fb: make number of windows configurable at load time.
From: Manuel Lauss @ 2011-09-02 14:40 UTC (permalink / raw)
To: linux-fbdev
Make the number of framebuffer windows and the window configuration
selectable at the kernel commandline instead of hardcoding it
in the kernel config.
Signed-off-by: Manuel Lauss <manuel.lauss@googlemail.com>
---
drivers/video/au1200fb.c | 53 +++++++++++++++++++++++++++++++++------------
1 files changed, 39 insertions(+), 14 deletions(-)
diff --git a/drivers/video/au1200fb.c b/drivers/video/au1200fb.c
index 480ecb1..4b58f7b 100644
--- a/drivers/video/au1200fb.c
+++ b/drivers/video/au1200fb.c
@@ -46,10 +46,6 @@
#include <asm/mach-au1x00/au1000.h>
#include "au1200fb.h"
-#ifndef CONFIG_FB_AU1200_DEVS
-#define CONFIG_FB_AU1200_DEVS 4
-#endif
-
#define DRIVER_NAME "au1200fb"
#define DRIVER_DESC "LCD controller driver for AU1200 processors"
@@ -154,7 +150,6 @@ struct au1200fb_device {
dma_addr_t fb_phys;
};
-static struct fb_info *_au1200fb_infos[CONFIG_FB_AU1200_DEVS];
/********************************************************************/
/* LCD controller restrictions */
@@ -167,10 +162,18 @@ static struct fb_info *_au1200fb_infos[CONFIG_FB_AU1200_DEVS];
/* Default number of visible screen buffer to allocate */
#define AU1200FB_NBR_VIDEO_BUFFERS 1
+/* Default maximum number of fb devices to create */
+#define MAX_DEVICE_COUNT 4
+
+/* Default window configuration entry to use (see windows[]) */
+#define DEFAULT_WINDOW_INDEX 2
+
/********************************************************************/
+static struct fb_info *_au1200fb_infos[MAX_DEVICE_COUNT];
static struct au1200_lcd *lcd = (struct au1200_lcd *) AU1200_LCD_ADDR;
-static int window_index = 2; /* default is zero */
+static int device_count = MAX_DEVICE_COUNT;
+static int window_index = DEFAULT_WINDOW_INDEX; /* default is zero */
static int panel_index = 2; /* default is zero */
static struct window_settings *win;
static struct panel_settings *panel;
@@ -683,7 +686,7 @@ static int fbinfo2index (struct fb_info *fb_info)
{
int i;
- for (i = 0; i < CONFIG_FB_AU1200_DEVS; ++i) {
+ for (i = 0; i < device_count; ++i) {
if (fb_info = _au1200fb_infos[i])
return i;
}
@@ -1599,7 +1602,7 @@ static int __devinit au1200fb_drv_probe(struct platform_device *dev)
/* Kickstart the panel */
au1200_setpanel(panel);
- for (plane = 0; plane < CONFIG_FB_AU1200_DEVS; ++plane) {
+ for (plane = 0; plane < device_count; ++plane) {
bpp = winbpp(win->w[plane].mode_winctrl1);
if (win->w[plane].xres = 0)
win->w[plane].xres = panel->Xres;
@@ -1699,7 +1702,7 @@ static int __devexit au1200fb_drv_remove(struct platform_device *dev)
/* Turn off the panel */
au1200_setpanel(NULL);
- for (plane = 0; plane < CONFIG_FB_AU1200_DEVS; ++plane) {
+ for (plane = 0; plane < device_count; ++plane) {
fbi = _au1200fb_infos[plane];
fbdev = fbi->par;
@@ -1741,7 +1744,7 @@ static int au1200fb_drv_resume(struct device *dev)
/* Kickstart the panel */
au1200_setpanel(panel);
- for (i = 0; i < CONFIG_FB_AU1200_DEVS; i++) {
+ for (i = 0; i < device_count; i++) {
fbi = _au1200fb_infos[i];
au1200fb_fb_set_par(fbi);
}
@@ -1776,10 +1779,10 @@ static struct platform_driver au1200fb_driver = {
/* Kernel driver */
-static void au1200fb_setup(void)
+static int au1200fb_setup(void)
{
- char* options = NULL;
- char* this_opt;
+ char *options = NULL;
+ char *this_opt, *endptr;
int num_panels = ARRAY_SIZE(known_lcd_panels);
int panel_idx = -1;
@@ -1824,12 +1827,33 @@ static void au1200fb_setup(void)
nohwcursor = 1;
}
+ else if (strncmp(this_opt, "devices:", 8) = 0) {
+ this_opt += 8;
+ device_count = simple_strtol(this_opt,
+ &endptr, 0);
+ if ((device_count < 0) ||
+ (device_count > MAX_DEVICE_COUNT))
+ device_count = MAX_DEVICE_COUNT;
+ }
+
+ else if (strncmp(this_opt, "wincfg:", 7) = 0) {
+ this_opt += 7;
+ window_index = simple_strtol(this_opt,
+ &endptr, 0);
+ if ((window_index < 0) ||
+ (window_index >= ARRAY_SIZE(windows)))
+ window_index = DEFAULT_WINDOW_INDEX;
+ }
+
+ else if (strncmp(this_opt, "off", 3) = 0)
+ return 1;
/* Unsupported option */
else {
print_warn("Unsupported option \"%s\"", this_opt);
}
}
}
+ return 0;
}
static int __init au1200fb_init(void)
@@ -1837,7 +1861,8 @@ static int __init au1200fb_init(void)
print_info("" DRIVER_DESC "");
/* Setup driver with options */
- au1200fb_setup();
+ if (au1200fb_setup())
+ return -ENODEV;
/* Point to the panel selected */
panel = &known_lcd_panels[panel_index];
--
1.7.6
^ permalink raw reply related
* [PATCH 4/6] au1200fb: switch to FB_SYS helpers
From: Manuel Lauss @ 2011-09-02 14:40 UTC (permalink / raw)
To: linux-fbdev
Since all video memory is in system ram, use FB_SYS helpers.
Signed-off-by: Manuel Lauss <manuel.lauss@googlemail.com>
---
drivers/video/Kconfig | 7 ++++---
drivers/video/au1200fb.c | 8 +++++---
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 549b960..5e19de9 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -1756,9 +1756,10 @@ config FB_AU1100
config FB_AU1200
bool "Au1200 LCD Driver"
depends on (FB = y) && MIPS && SOC_AU1200
- select FB_CFB_FILLRECT
- select FB_CFB_COPYAREA
- select FB_CFB_IMAGEBLIT
+ select FB_SYS_FILLRECT
+ select FB_SYS_COPYAREA
+ select FB_SYS_IMAGEBLIT
+ select FB_SYS_FOPS
help
This is the framebuffer driver for the AMD Au1200 SOC. It can drive
various panels and CRTs by passing in kernel cmd line option
diff --git a/drivers/video/au1200fb.c b/drivers/video/au1200fb.c
index 4b58f7b..b1b16d9 100644
--- a/drivers/video/au1200fb.c
+++ b/drivers/video/au1200fb.c
@@ -1502,9 +1502,11 @@ static struct fb_ops au1200fb_fb_ops = {
.fb_set_par = au1200fb_fb_set_par,
.fb_setcolreg = au1200fb_fb_setcolreg,
.fb_blank = au1200fb_fb_blank,
- .fb_fillrect = cfb_fillrect,
- .fb_copyarea = cfb_copyarea,
- .fb_imageblit = cfb_imageblit,
+ .fb_fillrect = sys_fillrect,
+ .fb_copyarea = sys_copyarea,
+ .fb_imageblit = sys_imageblit,
+ .fb_read = fb_sys_read,
+ .fb_write = fb_sys_write,
.fb_sync = NULL,
.fb_ioctl = au1200fb_ioctl,
.fb_mmap = au1200fb_fb_mmap,
--
1.7.6
^ permalink raw reply related
* [PATCH 5/6] au1200fb: fix hardcoded IRQ
From: Manuel Lauss @ 2011-09-02 14:40 UTC (permalink / raw)
To: linux-fbdev
Use the IRQ provided by platform resource information.
Required for Au1300 support.
Signed-off-by: Manuel Lauss <manuel.lauss@googlemail.com>
---
drivers/video/au1200fb.c | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/video/au1200fb.c b/drivers/video/au1200fb.c
index b1b16d9..ed5dcdb2 100644
--- a/drivers/video/au1200fb.c
+++ b/drivers/video/au1200fb.c
@@ -1595,7 +1595,7 @@ static int __devinit au1200fb_drv_probe(struct platform_device *dev)
struct au1200fb_device *fbdev;
struct fb_info *fbi = NULL;
unsigned long page;
- int bpp, plane, ret;
+ int bpp, plane, ret, irq;
/* shut gcc up */
ret = 0;
@@ -1671,10 +1671,12 @@ static int __devinit au1200fb_drv_probe(struct platform_device *dev)
}
/* Now hook interrupt too */
- if ((ret = request_irq(AU1200_LCD_INT, au1200fb_handle_irq,
- IRQF_DISABLED | IRQF_SHARED, "lcd", (void *)dev)) < 0) {
+ irq = platform_get_irq(dev, 0);
+ ret = request_irq(irq, au1200fb_handle_irq,
+ IRQF_DISABLED | IRQF_SHARED, "lcd", (void *)dev);
+ if (ret) {
print_err("fail to request interrupt line %d (err: %d)",
- AU1200_LCD_INT, ret);
+ irq, ret);
goto failed;
}
@@ -1722,7 +1724,7 @@ static int __devexit au1200fb_drv_remove(struct platform_device *dev)
_au1200fb_infos[plane] = NULL;
}
- free_irq(AU1200_LCD_INT, (void *)dev);
+ free_irq(platform_get_irq(dev, 0), (void *)dev);
return 0;
}
--
1.7.6
^ permalink raw reply related
* [PATCH 6/6] fbdev: au1200fb: silence debug output
From: Manuel Lauss @ 2011-09-02 14:40 UTC (permalink / raw)
To: linux-fbdev
it's annoying and takes up way too much space in dmesg.
Signed-off-by: Manuel Lauss <manuel.lauss@googlemail.com>
---
drivers/video/au1200fb.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/video/au1200fb.c b/drivers/video/au1200fb.c
index ed5dcdb2..a19a40e 100644
--- a/drivers/video/au1200fb.c
+++ b/drivers/video/au1200fb.c
@@ -49,7 +49,7 @@
#define DRIVER_NAME "au1200fb"
#define DRIVER_DESC "LCD controller driver for AU1200 processors"
-#define DEBUG 1
+#define DEBUG 0
#define print_err(f, arg...) printk(KERN_ERR DRIVER_NAME ": " f "\n", ## arg)
#define print_warn(f, arg...) printk(KERN_WARNING DRIVER_NAME ": " f "\n", ## arg)
--
1.7.6
^ permalink raw reply related
* Re: [PATCH] fb: avoid possible deadlock caused by fb_set_suspend
From: Guennadi Liakhovetski @ 2011-09-02 16:06 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: Bruno Prémont, lethal, linux-fbdev, francis.moro, torvalds,
linux-kernel, Herton Ronaldo Krzesinski, stable
In-Reply-To: <4E5FA7F9.9030802@gmx.de>
Hi Florian
On Thu, 1 Sep 2011, Florian Tobias Schandinat wrote:
> ping
>
> Guennadi, I really want this issue fixed. Please have a look at Bruno's patch
> otherwise your driver might remain or get even more broken...
>
> I am scheduling Herton's patch for the next merge window.
So, the patch looks simple and correct, when applied on top of
http://marc.info/?l=linux-kernel&m\x130833638508657&w=2
But it doesn't apply anymore and after fixing a trivial merge conflict, it
fails to build, which is also trivially fixed. Please, add my
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
to the set and use this updated version:
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
index 7d54e2c..647ba98 100644
--- a/drivers/video/sh_mobile_hdmi.c
+++ b/drivers/video/sh_mobile_hdmi.c
@@ -1111,6 +1111,7 @@ static long sh_hdmi_clk_configure(struct sh_hdmi *hdmi, unsigned long hdmi_rate,
static void sh_hdmi_edid_work_fn(struct work_struct *work)
{
struct sh_hdmi *hdmi = container_of(work, struct sh_hdmi, edid_work.work);
+ struct fb_info *info;
struct sh_mobile_hdmi_info *pdata = hdmi->dev->platform_data;
struct sh_mobile_lcdc_chan *ch;
int ret;
@@ -1123,8 +1124,9 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
mutex_lock(&hdmi->mutex);
+ info = hdmi->info;
+
if (hdmi->hp_state = HDMI_HOTPLUG_CONNECTED) {
- struct fb_info *info = hdmi->info;
unsigned long parent_rate = 0, hdmi_rate;
ret = sh_hdmi_read_edid(hdmi, &hdmi_rate, &parent_rate);
@@ -1148,42 +1150,45 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
ch = info->par;
- console_lock();
+ if (lock_fb_info(info)) {
+ console_lock();
- /* HDMI plug in */
- if (!sh_hdmi_must_reconfigure(hdmi) &&
- info->state = FBINFO_STATE_RUNNING) {
- /*
- * First activation with the default monitor - just turn
- * on, if we run a resume here, the logo disappears
- */
- if (lock_fb_info(info)) {
+ /* HDMI plug in */
+ if (!sh_hdmi_must_reconfigure(hdmi) &&
+ info->state = FBINFO_STATE_RUNNING) {
+ /*
+ * First activation with the default monitor - just turn
+ * on, if we run a resume here, the logo disappears
+ */
info->var.width = hdmi->var.width;
info->var.height = hdmi->var.height;
sh_hdmi_display_on(hdmi, info);
- unlock_fb_info(info);
+ } else {
+ /* New monitor or have to wake up */
+ fb_set_suspend(info, 0);
}
- } else {
- /* New monitor or have to wake up */
- fb_set_suspend(info, 0);
- }
- console_unlock();
+ console_unlock();
+ unlock_fb_info(info);
+ }
} else {
ret = 0;
- if (!hdmi->info)
+ if (!info)
goto out;
hdmi->monspec.modedb_len = 0;
fb_destroy_modedb(hdmi->monspec.modedb);
hdmi->monspec.modedb = NULL;
- console_lock();
+ if (lock_fb_info(info)) {
+ console_lock();
- /* HDMI disconnect */
- fb_set_suspend(hdmi->info, 1);
+ /* HDMI disconnect */
+ fb_set_suspend(info, 1);
- console_unlock();
+ console_unlock();
+ unlock_fb_info(info);
+ }
}
out:
Thanks
Guennadi
>
>
> Regards,
>
> Florian Tobias Schandinat
>
>
> On 06/18/2011 09:19 AM, Bruno Prémont wrote:
> > Guennadi, could you have a look at (completely untested) patch which avoids
> > possible deadlock due to inverted lock taking order on hotplug as well
> > as "readding" lock_fb_info() for fb_set_suspend() call after Herton's
> > patch to fb_set_suspend().
> >
> > Thanks,
> > Bruno
> >
> >
> > On Sat, 18 June 2011 Bruno Prémont <bonbons@linux-vserver.org> wrote:
> >> On Fri, 17 June 2011 Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:
> >>> From: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
> >>>
> >>> A lock ordering issue can cause deadlocks: in framebuffer/console code,
> >>> all needed struct fb_info locks are taken before acquire_console_sem(),
> >>> in places which need to take console semaphore.
> >>>
> >>> But fb_set_suspend is always called with console semaphore held, and
> >>> inside it we call lock_fb_info which gets the fb_info lock, inverse
> >>> locking order of what the rest of the code does. This causes a real
> >>> deadlock issue, when we write to state fb sysfs attribute (which calls
> >>> fb_set_suspend) while a framebuffer is being unregistered by
> >>> remove_conflicting_framebuffers, as can be shown by following show
> >>> blocked state trace on a test program which loads i915 and runs another
> >>> forked processes writing to state attribute:
> >>>
> >>> Test process with semaphore held and trying to get fb_info lock:
> >>
> >> ...
> >>
> >>> fb-test2 which reproduces above is available on kernel.org bug #26232.
> >>> To solve this issue, avoid calling lock_fb_info inside fb_set_suspend,
> >>> and move it out to where needed (callers of fb_set_suspend must call
> >>> lock_fb_info before if needed). So far, the only place which needs to
> >>> call lock_fb_info is store_fbstate, all other places which calls
> >>> fb_set_suspend are suspend/resume hooks that should not need the lock as
> >>> they should be run only when processes are already frozen in
> >>> suspend/resume.
> >>
> >> From a quick look through FB drivers in 2.6.39 I've found one that would need
> >> more work:
> >> - drivers/video/sh_mobile_hdmi.c: sh_hdmi_edid_work_fn()
> >> Should get changed to
> >> a) right locking order in case (hdmi->hp_state = HDMI_HOTPLUG_CONNECTED)
> >> b) lock fb_info in the other case
> >> For this one fb_set_suspend() does get call in a hotplug worker,
> >> thus independently on suspend/resume process.
> >>
> >> The rest does match the suspend/resume hook pattern mentioned.
> >>
> >> Bruno
> >>
> >>
> >>> References: https://bugzilla.kernel.org/show_bug.cgi?id&232
> >>> Signed-off-by: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
> >>> Signed-off-by: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> >>> Cc: stable@kernel.org
> >>> ---
> >>> drivers/video/fbmem.c | 3 ---
> >>> drivers/video/fbsysfs.c | 3 +++
> >>> 2 files changed, 3 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> >>> index 5aac00e..ad93629 100644
> >>> --- a/drivers/video/fbmem.c
> >>> +++ b/drivers/video/fbmem.c
> >>> @@ -1738,8 +1738,6 @@ void fb_set_suspend(struct fb_info *info, int state)
> >>> {
> >>> struct fb_event event;
> >>>
> >>> - if (!lock_fb_info(info))
> >>> - return;
> >>> event.info = info;
> >>> if (state) {
> >>> fb_notifier_call_chain(FB_EVENT_SUSPEND, &event);
> >>> @@ -1748,7 +1746,6 @@ void fb_set_suspend(struct fb_info *info, int state)
> >>> info->state = FBINFO_STATE_RUNNING;
> >>> fb_notifier_call_chain(FB_EVENT_RESUME, &event);
> >>> }
> >>> - unlock_fb_info(info);
> >>> }
> >>>
> >>> /**
> >>> diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
> >>> index 04251ce..67afa9c 100644
> >>> --- a/drivers/video/fbsysfs.c
> >>> +++ b/drivers/video/fbsysfs.c
> >>> @@ -399,9 +399,12 @@ static ssize_t store_fbstate(struct device *device,
> >>>
> >>> state = simple_strtoul(buf, &last, 0);
> >>>
> >>> + if (!lock_fb_info(fb_info))
> >>> + return -ENODEV;
> >>> console_lock();
> >>> fb_set_suspend(fb_info, (int)state);
> >>> console_unlock();
> >>> + unlock_fb_info(fb_info);
> >>>
> >>> return count;
> >>> }
> >
> >
> > diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
> > index 2b9e56a..b1a13ab 100644
> > --- a/drivers/video/sh_mobile_hdmi.c
> > +++ b/drivers/video/sh_mobile_hdmi.c
> > @@ -1151,27 +1151,27 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
> >
> > ch = info->par;
> >
> > - console_lock();
> > + if (lock_fb_info(info)) {
> > + console_lock();
> >
> > - /* HDMI plug in */
> > - if (!sh_hdmi_must_reconfigure(hdmi) &&
> > - info->state = FBINFO_STATE_RUNNING) {
> > - /*
> > - * First activation with the default monitor - just turn
> > - * on, if we run a resume here, the logo disappears
> > - */
> > - if (lock_fb_info(info)) {
> > + /* HDMI plug in */
> > + if (!sh_hdmi_must_reconfigure(hdmi) &&
> > + info->state = FBINFO_STATE_RUNNING) {
> > + /*
> > + * First activation with the default monitor - just turn
> > + * on, if we run a resume here, the logo disappears
> > + */
> > info->var.width = hdmi->var.width;
> > info->var.height = hdmi->var.height;
> > sh_hdmi_display_on(hdmi, info);
> > - unlock_fb_info(info);
> > + } else {
> > + /* New monitor or have to wake up */
> > + fb_set_suspend(info, 0);
> > }
> > - } else {
> > - /* New monitor or have to wake up */
> > - fb_set_suspend(info, 0);
> > - }
> >
> > - console_unlock();
> > + console_unlock();
> > + unlock_fb_info(info);
> > + }
> > } else {
> > ret = 0;
> > if (!hdmi->info)
> > @@ -1181,12 +1181,15 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
> > fb_destroy_modedb(hdmi->monspec.modedb);
> > hdmi->monspec.modedb = NULL;
> >
> > - console_lock();
> > + if (lock_fb_info(info)) {
> > + console_lock();
> >
> > - /* HDMI disconnect */
> > - fb_set_suspend(hdmi->info, 1);
> > + /* HDMI disconnect */
> > + fb_set_suspend(hdmi->info, 1);
> >
> > - console_unlock();
> > + console_unlock();
> > + unlock_fb_info(info);
> > + }
> > pm_runtime_put(hdmi->dev);
> > }
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply related
* Re: [PATCH] fb: avoid possible deadlock caused by fb_set_suspend
From: Florian Tobias Schandinat @ 2011-09-02 16:46 UTC (permalink / raw)
To: Bruno Prémont
Cc: Guennadi Liakhovetski, lethal, linux-fbdev, francis.moro,
torvalds, linux-kernel, Herton Ronaldo Krzesinski, stable
In-Reply-To: <Pine.LNX.4.64.1109021802560.4731@axis700.grange>
Hi Bruno, Guennadi,
On 09/02/2011 04:06 PM, Guennadi Liakhovetski wrote:
> Hi Florian
>
> On Thu, 1 Sep 2011, Florian Tobias Schandinat wrote:
>
>> ping
>>
>> Guennadi, I really want this issue fixed. Please have a look at Bruno's patch
>> otherwise your driver might remain or get even more broken...
>>
>> I am scheduling Herton's patch for the next merge window.
>
> So, the patch looks simple and correct, when applied on top of
> http://marc.info/?l=linux-kernel&m\x130833638508657&w=2
> But it doesn't apply anymore and after fixing a trivial merge conflict, it
> fails to build, which is also trivially fixed. Please, add my
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
So I guess we can finally solve this issue, thanks.
On the other hand I just noticed the original patch didn't have Bruno's
Signed-off and a commit message was missing. So may I ask you, Bruno, to add
your Signed-off or preferably resend the version tested by Guennadi in the
appropriate patch format.
Thanks,
Florian Tobias Schandinat
>
> to the set and use this updated version:
>
> diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
> index 7d54e2c..647ba98 100644
> --- a/drivers/video/sh_mobile_hdmi.c
> +++ b/drivers/video/sh_mobile_hdmi.c
> @@ -1111,6 +1111,7 @@ static long sh_hdmi_clk_configure(struct sh_hdmi *hdmi, unsigned long hdmi_rate,
> static void sh_hdmi_edid_work_fn(struct work_struct *work)
> {
> struct sh_hdmi *hdmi = container_of(work, struct sh_hdmi, edid_work.work);
> + struct fb_info *info;
> struct sh_mobile_hdmi_info *pdata = hdmi->dev->platform_data;
> struct sh_mobile_lcdc_chan *ch;
> int ret;
> @@ -1123,8 +1124,9 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
>
> mutex_lock(&hdmi->mutex);
>
> + info = hdmi->info;
> +
> if (hdmi->hp_state = HDMI_HOTPLUG_CONNECTED) {
> - struct fb_info *info = hdmi->info;
> unsigned long parent_rate = 0, hdmi_rate;
>
> ret = sh_hdmi_read_edid(hdmi, &hdmi_rate, &parent_rate);
> @@ -1148,42 +1150,45 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
>
> ch = info->par;
>
> - console_lock();
> + if (lock_fb_info(info)) {
> + console_lock();
>
> - /* HDMI plug in */
> - if (!sh_hdmi_must_reconfigure(hdmi) &&
> - info->state = FBINFO_STATE_RUNNING) {
> - /*
> - * First activation with the default monitor - just turn
> - * on, if we run a resume here, the logo disappears
> - */
> - if (lock_fb_info(info)) {
> + /* HDMI plug in */
> + if (!sh_hdmi_must_reconfigure(hdmi) &&
> + info->state = FBINFO_STATE_RUNNING) {
> + /*
> + * First activation with the default monitor - just turn
> + * on, if we run a resume here, the logo disappears
> + */
> info->var.width = hdmi->var.width;
> info->var.height = hdmi->var.height;
> sh_hdmi_display_on(hdmi, info);
> - unlock_fb_info(info);
> + } else {
> + /* New monitor or have to wake up */
> + fb_set_suspend(info, 0);
> }
> - } else {
> - /* New monitor or have to wake up */
> - fb_set_suspend(info, 0);
> - }
>
> - console_unlock();
> + console_unlock();
> + unlock_fb_info(info);
> + }
> } else {
> ret = 0;
> - if (!hdmi->info)
> + if (!info)
> goto out;
>
> hdmi->monspec.modedb_len = 0;
> fb_destroy_modedb(hdmi->monspec.modedb);
> hdmi->monspec.modedb = NULL;
>
> - console_lock();
> + if (lock_fb_info(info)) {
> + console_lock();
>
> - /* HDMI disconnect */
> - fb_set_suspend(hdmi->info, 1);
> + /* HDMI disconnect */
> + fb_set_suspend(info, 1);
>
> - console_unlock();
> + console_unlock();
> + unlock_fb_info(info);
> + }
> }
>
> out:
>
> Thanks
> Guennadi
>
>>
>>
>> Regards,
>>
>> Florian Tobias Schandinat
>>
>>
>> On 06/18/2011 09:19 AM, Bruno Prémont wrote:
>>> Guennadi, could you have a look at (completely untested) patch which avoids
>>> possible deadlock due to inverted lock taking order on hotplug as well
>>> as "readding" lock_fb_info() for fb_set_suspend() call after Herton's
>>> patch to fb_set_suspend().
>>>
>>> Thanks,
>>> Bruno
>>>
>>>
>>> On Sat, 18 June 2011 Bruno Prémont <bonbons@linux-vserver.org> wrote:
>>>> On Fri, 17 June 2011 Florian Tobias Schandinat <FlorianSchandinat@gmx.de> wrote:
>>>>> From: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
>>>>>
>>>>> A lock ordering issue can cause deadlocks: in framebuffer/console code,
>>>>> all needed struct fb_info locks are taken before acquire_console_sem(),
>>>>> in places which need to take console semaphore.
>>>>>
>>>>> But fb_set_suspend is always called with console semaphore held, and
>>>>> inside it we call lock_fb_info which gets the fb_info lock, inverse
>>>>> locking order of what the rest of the code does. This causes a real
>>>>> deadlock issue, when we write to state fb sysfs attribute (which calls
>>>>> fb_set_suspend) while a framebuffer is being unregistered by
>>>>> remove_conflicting_framebuffers, as can be shown by following show
>>>>> blocked state trace on a test program which loads i915 and runs another
>>>>> forked processes writing to state attribute:
>>>>>
>>>>> Test process with semaphore held and trying to get fb_info lock:
>>>>
>>>> ...
>>>>
>>>>> fb-test2 which reproduces above is available on kernel.org bug #26232.
>>>>> To solve this issue, avoid calling lock_fb_info inside fb_set_suspend,
>>>>> and move it out to where needed (callers of fb_set_suspend must call
>>>>> lock_fb_info before if needed). So far, the only place which needs to
>>>>> call lock_fb_info is store_fbstate, all other places which calls
>>>>> fb_set_suspend are suspend/resume hooks that should not need the lock as
>>>>> they should be run only when processes are already frozen in
>>>>> suspend/resume.
>>>>
>>>> From a quick look through FB drivers in 2.6.39 I've found one that would need
>>>> more work:
>>>> - drivers/video/sh_mobile_hdmi.c: sh_hdmi_edid_work_fn()
>>>> Should get changed to
>>>> a) right locking order in case (hdmi->hp_state = HDMI_HOTPLUG_CONNECTED)
>>>> b) lock fb_info in the other case
>>>> For this one fb_set_suspend() does get call in a hotplug worker,
>>>> thus independently on suspend/resume process.
>>>>
>>>> The rest does match the suspend/resume hook pattern mentioned.
>>>>
>>>> Bruno
>>>>
>>>>
>>>>> References: https://bugzilla.kernel.org/show_bug.cgi?id&232
>>>>> Signed-off-by: Herton Ronaldo Krzesinski <herton@mandriva.com.br>
>>>>> Signed-off-by: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
>>>>> Cc: stable@kernel.org
>>>>> ---
>>>>> drivers/video/fbmem.c | 3 ---
>>>>> drivers/video/fbsysfs.c | 3 +++
>>>>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
>>>>> index 5aac00e..ad93629 100644
>>>>> --- a/drivers/video/fbmem.c
>>>>> +++ b/drivers/video/fbmem.c
>>>>> @@ -1738,8 +1738,6 @@ void fb_set_suspend(struct fb_info *info, int state)
>>>>> {
>>>>> struct fb_event event;
>>>>>
>>>>> - if (!lock_fb_info(info))
>>>>> - return;
>>>>> event.info = info;
>>>>> if (state) {
>>>>> fb_notifier_call_chain(FB_EVENT_SUSPEND, &event);
>>>>> @@ -1748,7 +1746,6 @@ void fb_set_suspend(struct fb_info *info, int state)
>>>>> info->state = FBINFO_STATE_RUNNING;
>>>>> fb_notifier_call_chain(FB_EVENT_RESUME, &event);
>>>>> }
>>>>> - unlock_fb_info(info);
>>>>> }
>>>>>
>>>>> /**
>>>>> diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
>>>>> index 04251ce..67afa9c 100644
>>>>> --- a/drivers/video/fbsysfs.c
>>>>> +++ b/drivers/video/fbsysfs.c
>>>>> @@ -399,9 +399,12 @@ static ssize_t store_fbstate(struct device *device,
>>>>>
>>>>> state = simple_strtoul(buf, &last, 0);
>>>>>
>>>>> + if (!lock_fb_info(fb_info))
>>>>> + return -ENODEV;
>>>>> console_lock();
>>>>> fb_set_suspend(fb_info, (int)state);
>>>>> console_unlock();
>>>>> + unlock_fb_info(fb_info);
>>>>>
>>>>> return count;
>>>>> }
>>>
>>>
>>> diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
>>> index 2b9e56a..b1a13ab 100644
>>> --- a/drivers/video/sh_mobile_hdmi.c
>>> +++ b/drivers/video/sh_mobile_hdmi.c
>>> @@ -1151,27 +1151,27 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
>>>
>>> ch = info->par;
>>>
>>> - console_lock();
>>> + if (lock_fb_info(info)) {
>>> + console_lock();
>>>
>>> - /* HDMI plug in */
>>> - if (!sh_hdmi_must_reconfigure(hdmi) &&
>>> - info->state = FBINFO_STATE_RUNNING) {
>>> - /*
>>> - * First activation with the default monitor - just turn
>>> - * on, if we run a resume here, the logo disappears
>>> - */
>>> - if (lock_fb_info(info)) {
>>> + /* HDMI plug in */
>>> + if (!sh_hdmi_must_reconfigure(hdmi) &&
>>> + info->state = FBINFO_STATE_RUNNING) {
>>> + /*
>>> + * First activation with the default monitor - just turn
>>> + * on, if we run a resume here, the logo disappears
>>> + */
>>> info->var.width = hdmi->var.width;
>>> info->var.height = hdmi->var.height;
>>> sh_hdmi_display_on(hdmi, info);
>>> - unlock_fb_info(info);
>>> + } else {
>>> + /* New monitor or have to wake up */
>>> + fb_set_suspend(info, 0);
>>> }
>>> - } else {
>>> - /* New monitor or have to wake up */
>>> - fb_set_suspend(info, 0);
>>> - }
>>>
>>> - console_unlock();
>>> + console_unlock();
>>> + unlock_fb_info(info);
>>> + }
>>> } else {
>>> ret = 0;
>>> if (!hdmi->info)
>>> @@ -1181,12 +1181,15 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
>>> fb_destroy_modedb(hdmi->monspec.modedb);
>>> hdmi->monspec.modedb = NULL;
>>>
>>> - console_lock();
>>> + if (lock_fb_info(info)) {
>>> + console_lock();
>>>
>>> - /* HDMI disconnect */
>>> - fb_set_suspend(hdmi->info, 1);
>>> + /* HDMI disconnect */
>>> + fb_set_suspend(hdmi->info, 1);
>>>
>>> - console_unlock();
>>> + console_unlock();
>>> + unlock_fb_info(info);
>>> + }
>>> pm_runtime_put(hdmi->dev);
>>> }
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
>
^ permalink raw reply
* [PATCH] fb: sh-mobile: Fix deadlock risk between lock_fb_info() and
From: Bruno Prémont @ 2011-09-02 17:24 UTC (permalink / raw)
To: Florian Tobias Schandinat
Cc: Guennadi Liakhovetski, lethal, linux-fbdev, francis.moro,
torvalds, linux-kernel, Herton Ronaldo Krzesinski, stable
In-Reply-To: <4E61086C.3050102@gmx.de>
Following on Herton's patch "fb: avoid possible deadlock caused by
fb_set_suspend" which moves lock_fb_info() out of fb_set_suspend()
to its callers, correct sh-mobile's locking around call to
fb_set_suspend() and the same sort of deaklocks with console_lock()
due to order of taking the lock.
console_lock() must be taken while fb_info is already locked and fb_info
must be locked while calling fb_set_suspend().
Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
Hi Florian,
> On the other hand I just noticed the original patch didn't have
> Bruno's Signed-off and a commit message was missing. So may I ask you,
> Bruno, to add your Signed-off or preferably resend the version tested
> by Guennadi in the appropriate patch format.
Here it is with commit message and the Signed-off-by's.
Thanks,
Bruno
drivers/video/sh_mobile_hdmi.c | 47 ++++++++++++++++++++++-----------------
1 files changed, 26 insertions(+), 21 deletions(-)
diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
index 7d54e2c..647ba98 100644
--- a/drivers/video/sh_mobile_hdmi.c
+++ b/drivers/video/sh_mobile_hdmi.c
@@ -1111,6 +1111,7 @@ static long sh_hdmi_clk_configure(struct sh_hdmi *hdmi, unsigned long hdmi_rate,
static void sh_hdmi_edid_work_fn(struct work_struct *work)
{
struct sh_hdmi *hdmi = container_of(work, struct sh_hdmi, edid_work.work);
+ struct fb_info *info;
struct sh_mobile_hdmi_info *pdata = hdmi->dev->platform_data;
struct sh_mobile_lcdc_chan *ch;
int ret;
@@ -1123,8 +1124,9 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
mutex_lock(&hdmi->mutex);
+ info = hdmi->info;
+
if (hdmi->hp_state = HDMI_HOTPLUG_CONNECTED) {
- struct fb_info *info = hdmi->info;
unsigned long parent_rate = 0, hdmi_rate;
ret = sh_hdmi_read_edid(hdmi, &hdmi_rate, &parent_rate);
@@ -1148,42 +1150,45 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
ch = info->par;
- console_lock();
+ if (lock_fb_info(info)) {
+ console_lock();
- /* HDMI plug in */
- if (!sh_hdmi_must_reconfigure(hdmi) &&
- info->state = FBINFO_STATE_RUNNING) {
- /*
- * First activation with the default monitor - just turn
- * on, if we run a resume here, the logo disappears
- */
- if (lock_fb_info(info)) {
+ /* HDMI plug in */
+ if (!sh_hdmi_must_reconfigure(hdmi) &&
+ info->state = FBINFO_STATE_RUNNING) {
+ /*
+ * First activation with the default monitor - just turn
+ * on, if we run a resume here, the logo disappears
+ */
info->var.width = hdmi->var.width;
info->var.height = hdmi->var.height;
sh_hdmi_display_on(hdmi, info);
- unlock_fb_info(info);
+ } else {
+ /* New monitor or have to wake up */
+ fb_set_suspend(info, 0);
}
- } else {
- /* New monitor or have to wake up */
- fb_set_suspend(info, 0);
- }
- console_unlock();
+ console_unlock();
+ unlock_fb_info(info);
+ }
} else {
ret = 0;
- if (!hdmi->info)
+ if (!info)
goto out;
hdmi->monspec.modedb_len = 0;
fb_destroy_modedb(hdmi->monspec.modedb);
hdmi->monspec.modedb = NULL;
- console_lock();
+ if (lock_fb_info(info)) {
+ console_lock();
- /* HDMI disconnect */
- fb_set_suspend(hdmi->info, 1);
+ /* HDMI disconnect */
+ fb_set_suspend(info, 1);
- console_unlock();
+ console_unlock();
+ unlock_fb_info(info);
+ }
}
out:
^ permalink raw reply related
* Re: [PATCH 6/6] fbdev: au1200fb: silence debug output
From: Florian Tobias Schandinat @ 2011-09-02 20:51 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1314974451-8535-7-git-send-email-manuel.lauss@googlemail.com>
On 09/02/2011 02:40 PM, Manuel Lauss wrote:
> it's annoying and takes up way too much space in dmesg.
...and is no longer needed, I guess.
I applied this patch.
As I merged Paul's old branch when I replaced it in linux-next I already had the
5 other patches.
Thanks,
Florian Tobias Schandinat
>
> Signed-off-by: Manuel Lauss <manuel.lauss@googlemail.com>
> ---
> drivers/video/au1200fb.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/au1200fb.c b/drivers/video/au1200fb.c
> index ed5dcdb2..a19a40e 100644
> --- a/drivers/video/au1200fb.c
> +++ b/drivers/video/au1200fb.c
> @@ -49,7 +49,7 @@
> #define DRIVER_NAME "au1200fb"
> #define DRIVER_DESC "LCD controller driver for AU1200 processors"
>
> -#define DEBUG 1
> +#define DEBUG 0
>
> #define print_err(f, arg...) printk(KERN_ERR DRIVER_NAME ": " f "\n", ## arg)
> #define print_warn(f, arg...) printk(KERN_WARNING DRIVER_NAME ": " f "\n", ## arg)
^ permalink raw reply
* Re: [PATCH] fb: sh-mobile: Fix deadlock risk between lock_fb_info()
From: Florian Tobias Schandinat @ 2011-09-02 20:54 UTC (permalink / raw)
To: Bruno Prémont
Cc: Guennadi Liakhovetski, lethal, linux-fbdev, francis.moro,
torvalds, linux-kernel, Herton Ronaldo Krzesinski, stable
In-Reply-To: <20110902192403.10f9cff7@neptune.home>
On 09/02/2011 05:24 PM, Bruno Prémont wrote:
> Following on Herton's patch "fb: avoid possible deadlock caused by
> fb_set_suspend" which moves lock_fb_info() out of fb_set_suspend()
> to its callers, correct sh-mobile's locking around call to
> fb_set_suspend() and the same sort of deaklocks with console_lock()
> due to order of taking the lock.
>
> console_lock() must be taken while fb_info is already locked and fb_info
> must be locked while calling fb_set_suspend().
>
> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>
> Hi Florian,
>
>> On the other hand I just noticed the original patch didn't have
>> Bruno's Signed-off and a commit message was missing. So may I ask you,
>> Bruno, to add your Signed-off or preferably resend the version tested
>> by Guennadi in the appropriate patch format.
>
> Here it is with commit message and the Signed-off-by's.
Applied and added stable as Cc as I think this should be applied as well
whenever Herton's patch is applied.
Thanks,
Florian Tobias Schandinat
>
> Thanks,
> Bruno
>
>
>
>
> drivers/video/sh_mobile_hdmi.c | 47 ++++++++++++++++++++++-----------------
> 1 files changed, 26 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
> index 7d54e2c..647ba98 100644
> --- a/drivers/video/sh_mobile_hdmi.c
> +++ b/drivers/video/sh_mobile_hdmi.c
> @@ -1111,6 +1111,7 @@ static long sh_hdmi_clk_configure(struct sh_hdmi *hdmi, unsigned long hdmi_rate,
> static void sh_hdmi_edid_work_fn(struct work_struct *work)
> {
> struct sh_hdmi *hdmi = container_of(work, struct sh_hdmi, edid_work.work);
> + struct fb_info *info;
> struct sh_mobile_hdmi_info *pdata = hdmi->dev->platform_data;
> struct sh_mobile_lcdc_chan *ch;
> int ret;
> @@ -1123,8 +1124,9 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
>
> mutex_lock(&hdmi->mutex);
>
> + info = hdmi->info;
> +
> if (hdmi->hp_state = HDMI_HOTPLUG_CONNECTED) {
> - struct fb_info *info = hdmi->info;
> unsigned long parent_rate = 0, hdmi_rate;
>
> ret = sh_hdmi_read_edid(hdmi, &hdmi_rate, &parent_rate);
> @@ -1148,42 +1150,45 @@ static void sh_hdmi_edid_work_fn(struct work_struct *work)
>
> ch = info->par;
>
> - console_lock();
> + if (lock_fb_info(info)) {
> + console_lock();
>
> - /* HDMI plug in */
> - if (!sh_hdmi_must_reconfigure(hdmi) &&
> - info->state = FBINFO_STATE_RUNNING) {
> - /*
> - * First activation with the default monitor - just turn
> - * on, if we run a resume here, the logo disappears
> - */
> - if (lock_fb_info(info)) {
> + /* HDMI plug in */
> + if (!sh_hdmi_must_reconfigure(hdmi) &&
> + info->state = FBINFO_STATE_RUNNING) {
> + /*
> + * First activation with the default monitor - just turn
> + * on, if we run a resume here, the logo disappears
> + */
> info->var.width = hdmi->var.width;
> info->var.height = hdmi->var.height;
> sh_hdmi_display_on(hdmi, info);
> - unlock_fb_info(info);
> + } else {
> + /* New monitor or have to wake up */
> + fb_set_suspend(info, 0);
> }
> - } else {
> - /* New monitor or have to wake up */
> - fb_set_suspend(info, 0);
> - }
>
> - console_unlock();
> + console_unlock();
> + unlock_fb_info(info);
> + }
> } else {
> ret = 0;
> - if (!hdmi->info)
> + if (!info)
> goto out;
>
> hdmi->monspec.modedb_len = 0;
> fb_destroy_modedb(hdmi->monspec.modedb);
> hdmi->monspec.modedb = NULL;
>
> - console_lock();
> + if (lock_fb_info(info)) {
> + console_lock();
>
> - /* HDMI disconnect */
> - fb_set_suspend(hdmi->info, 1);
> + /* HDMI disconnect */
> + fb_set_suspend(info, 1);
>
> - console_unlock();
> + console_unlock();
> + unlock_fb_info(info);
> + }
> }
>
> out:
>
^ permalink raw reply
* Re: [PATCH] video: mb862xx-i2c: fix for reliable decoder register
From: Florian Tobias Schandinat @ 2011-09-02 20:57 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1314892421-13471-1-git-send-email-agust@denx.de>
On 09/01/2011 03:53 PM, Anatolij Gustschin wrote:
> Increase delay when polling for tx status. This fixes
> the unreliable video decoder i2c register access.
>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
Looks good to me, applied.
Thanks,
Florian Tobias Schandinat
> ---
> drivers/video/mb862xx/mb862xx-i2c.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/mb862xx/mb862xx-i2c.c b/drivers/video/mb862xx/mb862xx-i2c.c
> index b953099..934081d 100644
> --- a/drivers/video/mb862xx/mb862xx-i2c.c
> +++ b/drivers/video/mb862xx/mb862xx-i2c.c
> @@ -23,7 +23,7 @@ static int mb862xx_i2c_wait_event(struct i2c_adapter *adap)
> u32 reg;
>
> do {
> - udelay(1);
> + udelay(10);
> reg = inreg(i2c, GC_I2C_BCR);
> if (reg & (I2C_INT | I2C_BER))
> break;
^ permalink raw reply
* [RFC v2] allow multiple concurrent visible displays
From: Florian Tobias Schandinat @ 2011-09-03 1:29 UTC (permalink / raw)
To: linux-kernel
Cc: linux-fbdev, Greg Kroah-Hartman, Dave Airlie, Arnd Bergmann,
Bernie Thompson, Martin Decky, Florian Tobias Schandinat
Hi all,
this is the second version of my multi-display support.
Compared to the first version:
The bug reported in the first patch that prevented booting when
"fbcon=cmap:01" was given is fixed (annoying NULL pointer in vc)
and a dummy vc_display_fg management was added to prevent
modifying vc structures we do not "own".
The second patch is new and a hack that allows graphic and console
applications to work concurrently.
Background:
At the moment only one display (in multi-display setups) will be
updated when
(1) multiple console applications run on different displays
(2) a graphic application runs in the active vt on one display and a
console application on another display
(3) multiple graphic (framebuffer) applications run on different
displays (usually)
The first patch fixes (1) and I'm pretty happy with it. Works well
for me except rarely cursor visibility glitches but that might be a
bug somewhere else.
(2) requires some sort of policy change. At the moment we forbid
any console from working just because on one vt a graphic
application started. Obviously that's not what I want and also not
what most applications require. Therefore changing it to only allow
the application to use the graphic resource associated with the vt
it switches to KD_GRAPHIC seems reasonable to me. If there are
concerns about compatiblity we could make this change an option or
introduce a new KD_PGRAPHICS which behaves as I want (though the
last one would be ugly and require changing all userspace to work).
My second patch gives this functionality but I do not expect it to
be acceptable as is. At first I wanted to revert f700d6e5 but for
some reason the result did not even boot so I hacked my way around
it. Basically my question here aside the policy change is why do we
blank when entering graphics mode and why do we not allow updates
to other consoles when one is blanked?
(3) is a completly different beast which will probably require
changes in userspace applications. Most framebuffer applications at
the moment just access the framebuffer if they are in the current
vt and stop on vt change. To make this working concurrently the
applications would need a way to get a notification when its
visibility changes. As this one requires much more work and thought
than the (1) and (2) I'll at first concentrate on solving those.
But if anyone has any suggestions on this one I'd be verry happy.
Best regards,
Florian Tobias Schandinat
Florian Tobias Schandinat (2):
fbdev: allow multiple concurrent visible consoles
vt: dirty hack
drivers/tty/vt/vt.c | 4 +---
drivers/video/console/fbcon.c | 34 ++++++++++++++++++++++++++++++----
drivers/video/console/fbcon.h | 1 +
3 files changed, 32 insertions(+), 7 deletions(-)
^ permalink raw reply
* [PATCH 1/2] fbdev: allow multiple concurrent visible consoles
From: Florian Tobias Schandinat @ 2011-09-03 1:29 UTC (permalink / raw)
To: linux-kernel
Cc: linux-fbdev, Greg Kroah-Hartman, Dave Airlie, Arnd Bergmann,
Bernie Thompson, Martin Decky, Florian Tobias Schandinat
In-Reply-To: <1315013361-3191-1-git-send-email-FlorianSchandinat@gmx.de>
This patch allows having multiple visible consoles that receive
display updates. For example one can have running "top" to monitor
the system on fb0 and at the same time work on a shell on fb1.
At the moment that works only with consoles, not with console and
graphical application nor with two graphical applications.
Signed-off-by: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
---
drivers/video/console/fbcon.c | 34 ++++++++++++++++++++++++++++++----
drivers/video/console/fbcon.h | 1 +
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 8745637..ce9a686 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -94,6 +94,7 @@ enum {
};
static struct display fb_display[MAX_NR_CONSOLES];
+static struct vc_data *dummy_fg;
static signed char con2fb_map[MAX_NR_CONSOLES];
static signed char con2fb_map_boot[MAX_NR_CONSOLES];
@@ -816,7 +817,8 @@ static int set_con2fb_map(int unit, int newidx, int user)
int oldidx = con2fb_map[unit];
struct fb_info *info = registered_fb[newidx];
struct fb_info *oldinfo = NULL;
- int found, err = 0;
+ struct fbcon_ops *ops;
+ int found, err = 0, visible = vc && CON_IS_VISIBLE(vc);
if (oldidx = newidx)
return 0;
@@ -839,6 +841,15 @@ static int set_con2fb_map(int unit, int newidx, int user)
if (!err && !found)
err = con2fb_acquire_newinfo(vc, info, unit, oldidx);
+ if (!err && vc && oldinfo && oldinfo->fbcon_par) {
+ ops = oldinfo->fbcon_par;
+ if (vc->vc_display_fg = &ops->vc_fg) {
+ if (CON_IS_VISIBLE(vc))
+ ops->vc_fg = NULL;
+
+ vc->vc_display_fg = &dummy_fg;
+ }
+ }
/*
* If old fb is not mapped to any of the consoles,
@@ -852,6 +863,13 @@ static int set_con2fb_map(int unit, int newidx, int user)
int show_logo = (fg_console = 0 && !user &&
logo_shown != FBCON_LOGO_DONTSHOW);
+ ops = info->fbcon_par;
+ if (!ops->vc_fg || visible)
+ ops->vc_fg = vc;
+
+ if (vc && vc->vc_display_fg = &dummy_fg)
+ vc->vc_display_fg = &ops->vc_fg;
+
if (!found)
fbcon_add_cursor_timer(info);
con2fb_map_boot[unit] = newidx;
@@ -1023,8 +1041,10 @@ static void fbcon_init(struct vc_data *vc, int init)
int logo = 1, new_rows, new_cols, rows, cols, charcnt = 256;
int cap, ret;
- if (info_idx = -1 || info = NULL)
- return;
+ if (info_idx = -1 || info = NULL) {
+ vc->vc_display_fg = &dummy_fg;
+ return;
+ }
cap = info->flags;
@@ -1089,6 +1109,10 @@ static void fbcon_init(struct vc_data *vc, int init)
con_copy_unimap(vc, svc);
ops = info->fbcon_par;
+ if (!ops->vc_fg)
+ ops->vc_fg = vc;
+
+ vc->vc_display_fg = &ops->vc_fg;
p->con_rotate = initial_rotation;
set_blitting_type(vc, info);
@@ -1184,8 +1208,10 @@ static void fbcon_deinit(struct vc_data *vc)
if (!ops)
goto finished;
- if (CON_IS_VISIBLE(vc))
+ if (CON_IS_VISIBLE(vc)) {
fbcon_del_cursor_timer(info);
+ ops->vc_fg = NULL;
+ }
ops->flags &= ~FBCON_FLAGS_INIT;
finished:
diff --git a/drivers/video/console/fbcon.h b/drivers/video/console/fbcon.h
index 6bd2e0c..adc7316 100644
--- a/drivers/video/console/fbcon.h
+++ b/drivers/video/console/fbcon.h
@@ -69,6 +69,7 @@ struct fbcon_ops {
struct timer_list cursor_timer; /* Cursor timer */
struct fb_cursor cursor_state;
struct display *p;
+ struct vc_data *vc_fg;
int currcon; /* Current VC. */
int cursor_flash;
int cursor_reset;
--
1.6.3.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox