* Re: [GIT PULL] ARM: amba: Enable module alias autogeneration for
From: Dave Martin @ 2011-11-22 12:07 UTC (permalink / raw)
To: Alessandro Rubini
Cc: linux, patches, linux-kernel, linux-arm-kernel, alan, a.zummo,
alsa-devel, cjb, dan.j.williams, dmitry.torokhov, grant.likely,
perex, jassisinghbrar, julia, linus.walleij, linux-fbdev,
linux-input, linux-mmc, linux-serial, linux-watchdog, lethal,
Pawel.Moll, rtc-linux, spi-devel-general, tiwai, vinod.koul, wim
In-Reply-To: <20111122115632.GA5165@mail.gnudd.com>
On Tue, Nov 22, 2011 at 12:56:32PM +0100, Alessandro Rubini wrote:
> > I have Acks on some of the driver patches and no comments on the
> > rest. I've been circulating these for some time, so if you're
> > happy to pull those driver patches via your tree, please go ahead.
>
> Sure I have no problem with the nomadik-rng.c, (so please have my
> Acked-by: -- sorry for not noting it earlier) but I think this
thanks
> file2alias thing needs a serious rework to split each bus in a
> different file, using ELF sections like we do elsewhere.
>
> I posted a patch last week but I got no comment at all. Maybe this
> is the time to ask for comments? I can rebase and repost if needed.
>
> lkml.org/lkml/2011/11/4/125 (0/2: description of the work)
> lkml.org/lkml/2011/11/4/126 (1/2: turn the if plethora to table lookup)
> lkml.org/lkml/2011/11/4/127 (2/2: partial split to type-specific files).
Splitting this up doesn't look like a silly idea -- I thought it was
unfortunate to have to patch a core file just to add a bus type.
I guess the question is how many new buses we get over time. If they
only get added gradually, people might not be too concerned. Of course,
this is one of those maintenance issues which can only get worse as time
goes on.
Cheers
---Dave
^ permalink raw reply
* Re: [PATCH 08/65] OMAPDSS: remove partial update from panel-taal
From: Tomi Valkeinen @ 2011-11-22 12:32 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <4ECB8D39.4000005@ti.com>
[-- Attachment #1: Type: text/plain, Size: 803 bytes --]
On Tue, 2011-11-22 at 17:23 +0530, Archit Taneja wrote:
> > - r = taal_set_update_window(td, x, y, w, h);
> > + /* XXX no need to send this every frame, but dsi break if not
> done */
> > + r = taal_set_update_window(td, 0, 0,
> > + td->panel_config->timings.x_res,
> > + td->panel_config->timings.y_res);
>
> How about sending a null short packet, and a BTA after that. This
> will
> keep automatic TE mode in place, and we'll need to send 1 short
> packet
> instead of 2 long packets every frame.
To be honest, I didn't spend any time with this. True, sending a null
packet and BTA instead of the 2 long packets is possible. But probably
even better would be to track the TE status, and send a BTA only when
needed.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: QVGA on a hardware which supports only >= VGA
From: Christian Gmeiner @ 2011-11-22 13:52 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <CAH9NwWdUZHfhqpNwcfxK_EnsMLeEazB50rQ=m65U3+tjhpwGJQ@mail.gmail.com>
ping
--
Christian Gmeiner, MSc
2011/11/18 Christian Gmeiner <christian.gmeiner@gmail.com>:
> Hi all,
>
> I am currently in the process to get some local patches into mainline
> and/or fix my
> problems the right way. At the moment I am stuck with this problem:
>
> I am on a device (x86), which is Geode lx800 based and there is a
> panel connected to
> it. The problem is that two different screen resolutions are used. QVGA and VGA.
> Should not sound like a problem, but the lx800 can only do VGA. So get
> it working,
> I need to pass the correct resolution via kernel cmd to the lxfb
> driver and I am using
> this patch (based on 2.6.36.4 kernel):
>
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -1050,6 +1050,7 @@ static long do_fb_ioctl(struct fb_info *info,
> unsigned int cmd,
> ret = copy_to_user(argp, &var, sizeof(var)) ? -EFAULT : 0;
> break;
> case FBIOPUT_VSCREENINFO:
> +#if 0
> if (copy_from_user(&var, argp, sizeof(var)))
> return -EFAULT;
> if (!lock_fb_info(info))
> @@ -1062,6 +1063,8 @@ static long do_fb_ioctl(struct fb_info *info,
> unsigned int cmd,
> unlock_fb_info(info);
> if (!ret && copy_to_user(argp, &var, sizeof(var)))
> ret = -EFAULT;
> +#endif
> + ret = 0;
> break;
> case FBIOGET_FSCREENINFO:
> if (!lock_fb_info(info))
> @@ -1086,6 +1089,7 @@ static long do_fb_ioctl(struct fb_info *info,
> unsigned int cmd,
> ret = fb_cmap_to_user(&cmap_from, &cmap);
> break;
> case FBIOPAN_DISPLAY:
> +#if 0
> if (copy_from_user(&var, argp, sizeof(var)))
> return -EFAULT;
> if (!lock_fb_info(info))
> @@ -1096,6 +1100,8 @@ static long do_fb_ioctl(struct fb_info *info,
> unsigned int cmd,
> unlock_fb_info(info);
> if (ret = 0 && copy_to_user(argp, &var, sizeof(var)))
> return -EFAULT;
> +#endif
> + ret = 0;
> break;
> case FBIO_CURSOR:
> ret = -EINVAL;
>
> I need to mention that I am using a custom BIOS and Linux based
> bootloader (kexec) in the
> boot process. At the moment I am looking into coreboot, but I wanted
> to know how to
> solve this "problem" the correct way.
>
> To sum it up: I want to use a QVGA panel with a hardware that only
> supports >= VGA.
>
> Thanks
> --
> Christian Gmeiner, MSc
>
^ permalink raw reply
* Re: [GIT PULL] ARM: amba: Enable module alias autogeneration for
From: Alessandro Rubini @ 2011-11-22 15:19 UTC (permalink / raw)
To: dave.martin-QSEj5FYQhm4dnm+yROfE0A
Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Pawel.Moll-5wv7dgnIgG8,
tiwai-l3A5Bk7waGM, linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA, perex-/Fr2/VpizcU,
wim-IQzOog9fTRqzQB+pC5nmwQ, julia-dAYI7NvHqcQ,
linux-lFZ/pmaqli7XmaaqVzeoHQ, vinod.koul-ral2JQCrhuEAvxtiuMwx3w,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, cjb-2X9k7bc8m7Mdnm+yROfE0A,
linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw, patches-QSEj5FYQhm4dnm+yROfE0A,
dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
alan-VuQAYsv1563Yd54FQh9/CA, a.zummo-BfzFCNDTiLLj+vYz1yj4TQ,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-mmc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
lethal-M7jkjyW5wf5g9hUCZPvPmw,
spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <20111122120756.GF2066-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
>> lkml.org/lkml/2011/11/4/125 (0/2: description of the work)
>> lkml.org/lkml/2011/11/4/126 (1/2: turn the if plethora to table lookup)
>> lkml.org/lkml/2011/11/4/127 (2/2: partial split to type-specific files).
>
> Splitting this up doesn't look like a silly idea -- I thought it was
> unfortunate to have to patch a core file just to add a bus type.
>
> I guess the question is how many new buses we get over time.
Well, CERN will have 2 in a short time. And I'm sure the bus concept
is so good that a number of them exists as external patches. Being
forced to patch a core file is not good, with conflicts that accumulate
over time.
I'd better be able to drop in my own file and add a kconfig-aware line
in Makefile. I think the implementation is straightforward. Who should
I send it to?
thanks
/alessandro
^ permalink raw reply
* Re: [GIT PULL] ARM: amba: Enable module alias autogeneration for
From: Greg KH @ 2011-11-22 19:20 UTC (permalink / raw)
To: Alessandro Rubini
Cc: dave.martin, Pawel.Moll, tiwai, linus.walleij, alsa-devel,
grant.likely, wim, julia, linux, vinod.koul, jassisinghbrar,
linux-serial, linux-input, cjb, linux-watchdog, rtc-linux,
patches, linux-fbdev, dan.j.williams, linux-arm-kernel, alan,
a.zummo, dmitry.torokhov, linux-mmc, linux-kernel, lethal,
spi-devel-general
In-Reply-To: <20111122115632.GA5165@mail.gnudd.com>
On Tue, Nov 22, 2011 at 12:56:32PM +0100, Alessandro Rubini wrote:
> > I have Acks on some of the driver patches and no comments on the
> > rest. I've been circulating these for some time, so if you're
> > happy to pull those driver patches via your tree, please go ahead.
>
> Sure I have no problem with the nomadik-rng.c, (so please have my
> Acked-by: -- sorry for not noting it earlier) but I think this
> file2alias thing needs a serious rework to split each bus in a
> different file, using ELF sections like we do elsewhere.
>
> I posted a patch last week but I got no comment at all. Maybe this
> is the time to ask for comments? I can rebase and repost if needed.
>
> lkml.org/lkml/2011/11/4/125 (0/2: description of the work)
> lkml.org/lkml/2011/11/4/126 (1/2: turn the if plethora to table lookup)
> lkml.org/lkml/2011/11/4/127 (2/2: partial split to type-specific files).
Ah, I missed that, yes, this would be a good thing to have, we've been
wanting it for _years_.
greg k-h
^ permalink raw reply
* Re: [PATCH 08/65] OMAPDSS: remove partial update from panel-taal
From: Archit Taneja @ 2011-11-23 5:51 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <1321965130.1887.46.camel@deskari>
On Tuesday 22 November 2011 06:02 PM, Tomi Valkeinen wrote:
> On Tue, 2011-11-22 at 17:23 +0530, Archit Taneja wrote:
>>> - r = taal_set_update_window(td, x, y, w, h);
>>> + /* XXX no need to send this every frame, but dsi break if not
>> done */
>>> + r = taal_set_update_window(td, 0, 0,
>>> + td->panel_config->timings.x_res,
>>> + td->panel_config->timings.y_res);
>>
>> How about sending a null short packet, and a BTA after that. This
>> will
>> keep automatic TE mode in place, and we'll need to send 1 short
>> packet
>> instead of 2 long packets every frame.
>
> To be honest, I didn't spend any time with this. True, sending a null
> packet and BTA instead of the 2 long packets is possible. But probably
> even better would be to track the TE status, and send a BTA only when
> needed.
Right, if we maintain the TE status, we may not need to send a packet at
all, so I guess we could stick with this for now, and remove it once we
start maintaining the number of BTAs we have sent to the panel.
Archit
>
> Tomi
>
^ permalink raw reply
* Re: [PATCH 09/65] OMAPDSS: pass ovl manager to dss_start_update
From: Archit Taneja @ 2011-11-23 5:55 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <1321953724-6350-10-git-send-email-tomi.valkeinen@ti.com>
On Tuesday 22 November 2011 02:51 PM, Tomi Valkeinen wrote:
> dss_start_update() takes currently the dss device as a parameter. Change
> the parameter to ovl manager, as that is what the dss_start_update()
> actually needs.
Minor comment: We could rename dss_start_update() to
dss_mgr_start_update() to stick to the new way of telling if this
function is meant for an overlay or a manager.
Archit
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/dsi.c | 2 +-
> drivers/video/omap2/dss/dss.h | 2 +-
> drivers/video/omap2/dss/manager.c | 7 ++-----
> 3 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index 9ef04ff..e5a2dcc 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -4063,7 +4063,7 @@ static void dsi_update_screen_dispc(struct omap_dss_device *dssdev,
> msecs_to_jiffies(250));
> BUG_ON(r = 0);
>
> - dss_start_update(dssdev);
> + dss_start_update(dssdev->manager);
>
> if (dsi->te_enabled) {
> /* disable LP_RX_TO, so that we can receive TE. Time to wait
> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> index 7f6a612..0937bd8 100644
> --- a/drivers/video/omap2/dss/dss.h
> +++ b/drivers/video/omap2/dss/dss.h
> @@ -182,7 +182,7 @@ void default_get_overlay_fifo_thresholds(enum omap_plane plane,
> int dss_init_overlay_managers(struct platform_device *pdev);
> void dss_uninit_overlay_managers(struct platform_device *pdev);
> int dss_mgr_wait_for_go_ovl(struct omap_overlay *ovl);
> -void dss_start_update(struct omap_dss_device *dssdev);
> +void dss_start_update(struct omap_overlay_manager *mgr);
>
> /* overlay */
> void dss_init_overlays(struct platform_device *pdev);
> diff --git a/drivers/video/omap2/dss/manager.c b/drivers/video/omap2/dss/manager.c
> index c616f85..bc28bfa 100644
> --- a/drivers/video/omap2/dss/manager.c
> +++ b/drivers/video/omap2/dss/manager.c
> @@ -896,17 +896,14 @@ static int configure_dispc(void)
> return r;
> }
>
> -void dss_start_update(struct omap_dss_device *dssdev)
> +void dss_start_update(struct omap_overlay_manager *mgr)
> {
> struct manager_cache_data *mc;
> struct overlay_cache_data *oc;
> const int num_ovls = dss_feat_get_num_ovls();
> const int num_mgrs = dss_feat_get_num_mgrs();
> - struct omap_overlay_manager *mgr;
> int i;
>
> - mgr = dssdev->manager;
> -
> mc =&dss_cache.manager_cache[mgr->id];
>
> mc->do_manual_update = true;
> @@ -929,7 +926,7 @@ void dss_start_update(struct omap_dss_device *dssdev)
> mc->shadow_dirty = false;
> }
>
> - dssdev->manager->enable(dssdev->manager);
> + mgr->enable(mgr);
> }
>
> static void dss_apply_irq_handler(void *data, u32 mask)
^ permalink raw reply
* Re: [PATCH] video:da8xx-fb: Disable and reset sequence on version2
From: Florian Tobias Schandinat @ 2011-11-23 6:51 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1321358543-23124-1-git-send-email-prakash.pm@ti.com>
On 11/15/2011 12:02 PM, Manjunathappa, Prakash wrote:
> Patch follows the disable and software reset sequence specified
> in version2 to LCDC functional specification.
> Without this flicker is observed on re-enabling the LCDC.
>
> Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
Applied.
Thanks,
Florian Tobias Schandinat
> ---
> drivers/video/da8xx-fb.c | 15 ++++++++++++++-
> 1 files changed, 14 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index e111971..6b27751 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -118,6 +118,7 @@
> /* Clock registers available only on Version 2 */
> #define LCD_CLK_ENABLE_REG 0x6c
> #define LCD_CLK_RESET_REG 0x70
> +#define LCD_CLK_MAIN_RESET BIT(3)
>
> #define LCD_NUM_BUFFERS 2
>
> @@ -246,6 +247,10 @@ static inline void lcd_enable_raster(void)
> {
> u32 reg;
>
> + /* Bring LCDC out of reset */
> + if (lcd_revision = LCD_VERSION_2)
> + lcdc_write(0, LCD_CLK_RESET_REG);
> +
> reg = lcdc_read(LCD_RASTER_CTRL_REG);
> if (!(reg & LCD_RASTER_ENABLE))
> lcdc_write(reg | LCD_RASTER_ENABLE, LCD_RASTER_CTRL_REG);
> @@ -259,6 +264,10 @@ static inline void lcd_disable_raster(void)
> reg = lcdc_read(LCD_RASTER_CTRL_REG);
> if (reg & LCD_RASTER_ENABLE)
> lcdc_write(reg & ~LCD_RASTER_ENABLE, LCD_RASTER_CTRL_REG);
> +
> + if (lcd_revision = LCD_VERSION_2)
> + /* Write 1 to reset LCDC */
> + lcdc_write(LCD_CLK_MAIN_RESET, LCD_CLK_RESET_REG);
> }
>
> static void lcd_blit(int load_mode, struct da8xx_fb_par *par)
> @@ -618,8 +627,12 @@ static void lcd_reset(struct da8xx_fb_par *par)
> lcdc_write(0, LCD_DMA_CTRL_REG);
> lcdc_write(0, LCD_RASTER_CTRL_REG);
>
> - if (lcd_revision = LCD_VERSION_2)
> + if (lcd_revision = LCD_VERSION_2) {
> lcdc_write(0, LCD_INT_ENABLE_SET_REG);
> + /* Write 1 to reset */
> + lcdc_write(LCD_CLK_MAIN_RESET, LCD_CLK_RESET_REG);
> + lcdc_write(0, LCD_CLK_RESET_REG);
> + }
> }
>
> static void lcd_calc_clk_divider(struct da8xx_fb_par *par)
^ permalink raw reply
* Re: [PATCH 3/3] Drivers: video: controlfb: fixed a brace coding style
From: Florian Tobias Schandinat @ 2011-11-23 6:52 UTC (permalink / raw)
To: Zac Storer; +Cc: linux-fbdev, linux-kernel
In-Reply-To: <1321590896-8249-1-git-send-email-zac.3.14159@gmail.com>
On 11/18/2011 04:34 AM, Zac Storer wrote:
> Fixed a brace coding style issue.
>
> Signed-off-by: Zac Storer <zac.3.14159@gmail.com>
Applied.
Thanks,
Florian Tobias Schandinat
> ---
> drivers/video/controlfb.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/controlfb.c b/drivers/video/controlfb.c
> index 7b2c40a..0c189b3 100644
> --- a/drivers/video/controlfb.c
> +++ b/drivers/video/controlfb.c
> @@ -420,7 +420,7 @@ static int __init init_control(struct fb_info_control *p)
>
> /* Try to pick a video mode out of NVRAM if we have one. */
> #ifdef CONFIG_NVRAM
> - if (default_cmode = CMODE_NVRAM){
> + if (default_cmode = CMODE_NVRAM) {
> cmode = nvram_read_byte(NV_CMODE);
> if(cmode < CMODE_8 || cmode > CMODE_32)
> cmode = CMODE_8;
^ permalink raw reply
* Re: [PATCH 4/4] Drivers: video: sbuslib: fixed a brace coding style
From: Florian Tobias Schandinat @ 2011-11-23 6:52 UTC (permalink / raw)
To: Zac Storer; +Cc: linux-fbdev, linux-kernel
In-Reply-To: <1321591138-8334-1-git-send-email-zac.3.14159@gmail.com>
On 11/18/2011 04:38 AM, Zac Storer wrote:
> Fixed a brace coding style issue.
>
> Signed-off-by: Zac Storer <zac.3.14159@gmail.com>
Applied.
Thanks,
Florian Tobias Schandinat
> ---
> drivers/video/sbuslib.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/sbuslib.c b/drivers/video/sbuslib.c
> index 37d764a..3c1de98 100644
> --- a/drivers/video/sbuslib.c
> +++ b/drivers/video/sbuslib.c
> @@ -76,7 +76,7 @@ int sbusfb_mmap_helper(struct sbus_mmap_map *map,
> map_offset = (physbase + map[i].poff) & POFF_MASK;
> break;
> }
> - if (!map_size){
> + if (!map_size) {
> page += PAGE_SIZE;
> continue;
> }
^ permalink raw reply
* Re: [PATCH] viafb: correct sync polarity for OLPC DCON
From: Florian Tobias Schandinat @ 2011-11-23 6:53 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <20111121150557.9EB9D9D401E@zog.reactivated.net>
On 11/21/2011 03:05 PM, Daniel Drake wrote:
> While the OLPC display appears to be able to handle either positive
> or negative sync, the Display Controller only recognises positive sync.
>
> This brings viafb (for XO-1.5) in line with lxfb (for XO-1) and
> fixes a recent regression where the XO-1.5 DCON could no longer be
> frozen. Thanks to Florian Tobias Schandinat for helping identify
> the fix.
>
> Test case: from a vt,
> echo 1 > /sys/devices/platform/dcon/freeze
> should cause the current screen contents to freeze, rather than garbage being
> displayed.
>
> Signed-off-by: Daniel Drake <dsd@laptop.org>
Applied.
Thanks,
Florian Tobias Schandinat
> ---
> drivers/video/via/share.h | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/via/share.h b/drivers/video/via/share.h
> index 69d882c..c01c1c1 100644
> --- a/drivers/video/via/share.h
> +++ b/drivers/video/via/share.h
> @@ -559,8 +559,8 @@
> #define M1200X720_R60_VSP POSITIVE
>
> /* 1200x900@60 Sync Polarity (DCON) */
> -#define M1200X900_R60_HSP NEGATIVE
> -#define M1200X900_R60_VSP NEGATIVE
> +#define M1200X900_R60_HSP POSITIVE
> +#define M1200X900_R60_VSP POSITIVE
>
> /* 1280x600@60 Sync Polarity (GTF Mode) */
> #define M1280x600_R60_HSP NEGATIVE
^ permalink raw reply
* Re: [PATCH 0/15] fbdev: sh_mipi_dsi: care un-explained register settings
From: Florian Tobias Schandinat @ 2011-11-23 6:59 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <874nydc3vz.wl%kuninori.morimoto.gx@renesas.com>
Hi Magnus, Kuninori,
On 11/21/2011 10:34 AM, Magnus Damm wrote:
> Hi Florian,
>
> On Sat, Nov 12, 2011 at 12:37 AM, Florian Tobias Schandinat
> <FlorianSchandinat@gmx.de> wrote:
>> Hi Guennadi, Magnus,
>>
>> On 11/09/2011 04:32 AM, kuninori.morimoto.gx@renesas.com wrote:
>>> Dear Florian, Paul
>>>
>>> These are sh_mipi_dsi update patches.
>>>
>>> Kuninori Morimoto (15):
>>> fbdev: sh_mobile_lcdcfb: fixup LDHAJR :: HSYNPAJ needs mask
>>> fbdev: sh_mipi_dsi: tidyup dsip_clk
>>> fbdev: sh_mipi_dsi: typo fix of SH_MIPI_DSI_HBPBM
>>> fbdev: sh_mipi_dsi: tidyup VMCTR2 parameter expression
>>> fbdev: sh_mipi_dsi: add SH_MIPI_DSI_HFPBM flag
>>> fbdev: sh_mipi_dsi: add SH_MIPI_DSI_BL2E flag
>>> fbdev: sh_mipi_dsi: add lane control support
>>> fbdev: sh_mipi_dsi: add sync_pulses/sync_events/burst mode
>>> fbdev: sh_mipi_dsi: add VMLEN1/VMLEN2 calculation
>>> fbdev: sh_mipi_dsi: add set_dot_clock() for each platform
>>> fbdev: sh_mipi_dsi: add HSxxCLK support
>>> fbdev: sh_mipi_dsi: sh_mipi has pdata instead of dev
>>> fbdev: sh_mipi_dsi: fixup setup timing of sh_mipi_setup()
>>> fbdev: sh_mipi_dsi: fixup setup timing of SYSCONF
>>> fbdev: sh_mipi_dsi: fixup setup timing DSICTRL
>>
>> can you have a look at these patches?
>> After a quick glance they look okay to me, but as this is all about hardware
>> (which I don't know) I'd be happier if someone more qualified could have a look
>> at it.
>
> They all look fine to me. Please add:
>
> Acked-by: Magnus Damm <damm@opensource.se>
Thanks for having a look. I've added it to all patches and applied them to my tree.
Thanks,
Florian Tobias Schandinat
^ permalink raw reply
* Re: [GIT PULL] OMAP DSS fixes for 3.2-rc
From: Florian Tobias Schandinat @ 2011-11-23 7:15 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap mailing list
In-Reply-To: <1321604174.1814.41.camel@deskari>
Hi Tomi,
On 11/18/2011 08:16 AM, Tomi Valkeinen wrote:
> Hi Florian,
>
> Here are a few OMAP display subsystem fixes for 3.2-rc.
>
> One for the old omapfb, which was missing include module.h, and two for
> the omapdss fixing issues related to HDMI.
Pulled, but...
> The following changes since commit cfcfc9eca2bcbd26a8e206baeb005b055dbf8e37:
>
> Linux 3.2-rc2 (2011-11-15 15:02:59 -0200)
...could you please try to base on something that is already in my tree, perhaps
just add patches on top of the last pull request you sent me. If you need
something more recent, that is okay, but don't force me to needlessly update my
tree.
Thanks,
Florian Tobias Schandinat
>
> are available in the git repository at:
> git://gitorious.org/linux-omap-dss2/linux.git for-3.2-rc
>
> Tomi Valkeinen (3):
> OMAPDSS: HDMI: fix returned HDMI pixel clock
> OMAPFB: fix compilation warnings due to missing include
> OMAPDSS: DISPC: skip scaling calculations when not scaling
>
> drivers/video/omap/dispc.c | 1 +
> drivers/video/omap2/dss/dispc.c | 11 +++++------
> drivers/video/omap2/dss/hdmi.c | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
^ permalink raw reply
* Re: [GIT PULL] OMAP DSS fixes for 3.2-rc
From: Tomi Valkeinen @ 2011-11-23 7:31 UTC (permalink / raw)
To: Florian Tobias Schandinat; +Cc: linux-fbdev, linux-omap mailing list
In-Reply-To: <4ECC9DAF.3000005@gmx.de>
[-- Attachment #1: Type: text/plain, Size: 887 bytes --]
On Wed, 2011-11-23 at 07:15 +0000, Florian Tobias Schandinat wrote:
> Hi Tomi,
>
> On 11/18/2011 08:16 AM, Tomi Valkeinen wrote:
> > Hi Florian,
> >
> > Here are a few OMAP display subsystem fixes for 3.2-rc.
> >
> > One for the old omapfb, which was missing include module.h, and two for
> > the omapdss fixing issues related to HDMI.
>
> Pulled, but...
Thanks!
> > The following changes since commit cfcfc9eca2bcbd26a8e206baeb005b055dbf8e37:
> >
> > Linux 3.2-rc2 (2011-11-15 15:02:59 -0200)
>
> ...could you please try to base on something that is already in my tree, perhaps
> just add patches on top of the last pull request you sent me. If you need
> something more recent, that is okay, but don't force me to needlessly update my
> tree.
Ah, of course. I didn't think of that. I'll base the patches on top of
your tree from now on.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 09/65] OMAPDSS: pass ovl manager to dss_start_update
From: Tomi Valkeinen @ 2011-11-23 7:32 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <4ECC8A6D.5050000@ti.com>
[-- Attachment #1: Type: text/plain, Size: 522 bytes --]
On Wed, 2011-11-23 at 11:23 +0530, Archit Taneja wrote:
> On Tuesday 22 November 2011 02:51 PM, Tomi Valkeinen wrote:
> > dss_start_update() takes currently the dss device as a parameter. Change
> > the parameter to ovl manager, as that is what the dss_start_update()
> > actually needs.
>
> Minor comment: We could rename dss_start_update() to
> dss_mgr_start_update() to stick to the new way of telling if this
> function is meant for an overlay or a manager.
Good point, I'll make the change.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 35/65] OMAPDSS: APPLY: move spinlock outside the struct
From: Archit Taneja @ 2011-11-23 9:37 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <1321953724-6350-36-git-send-email-tomi.valkeinen@ti.com>
On Tuesday 22 November 2011 02:51 PM, Tomi Valkeinen wrote:
> dss_cache struct contains a spinlock used to protect the struct. A more
> logical place for the spinlock is outside the struct that it is
> protecting. So move it there.
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/apply.c | 22 ++++++++++++----------
> 1 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
> index 23c723a..17639c0 100644
> --- a/drivers/video/omap2/dss/apply.c
> +++ b/drivers/video/omap2/dss/apply.c
> @@ -89,13 +89,15 @@ struct mgr_priv_data {
> };
>
> static struct {
> - spinlock_t lock;
> struct ovl_priv_data ovl_priv_data_array[MAX_DSS_OVERLAYS];
> struct mgr_priv_data mgr_priv_data_array[MAX_DSS_MANAGERS];
>
> bool irq_enabled;
> } dss_cache;
>
> +/* protects dss_cache */
> +static spinlock_t data_lock;
Minor comment: The name 'data_lock' doesn't tell much that its
protecting the dss_cache struct. Probably 'cache_lock' or
'priv_data_lock' or something like that may be more informative.
Archit
> +
> static struct ovl_priv_data *get_ovl_priv(struct omap_overlay *ovl)
> {
> return&dss_cache.ovl_priv_data_array[ovl->id];
> @@ -108,7 +110,7 @@ static struct mgr_priv_data *get_mgr_priv(struct omap_overlay_manager *mgr)
>
> void dss_apply_init(void)
> {
> - spin_lock_init(&dss_cache.lock);
> + spin_lock_init(&data_lock);
> }
>
> static bool ovl_manual_update(struct omap_overlay *ovl)
> @@ -149,10 +151,10 @@ int dss_mgr_wait_for_go(struct omap_overlay_manager *mgr)
> unsigned long flags;
> bool shadow_dirty, dirty;
>
> - spin_lock_irqsave(&dss_cache.lock, flags);
> + spin_lock_irqsave(&data_lock, flags);
> dirty = mp->dirty;
> shadow_dirty = mp->shadow_dirty;
> - spin_unlock_irqrestore(&dss_cache.lock, flags);
> + spin_unlock_irqrestore(&data_lock, flags);
>
> if (!dirty&& !shadow_dirty) {
> r = 0;
> @@ -212,10 +214,10 @@ int dss_mgr_wait_for_go_ovl(struct omap_overlay *ovl)
> unsigned long flags;
> bool shadow_dirty, dirty;
>
> - spin_lock_irqsave(&dss_cache.lock, flags);
> + spin_lock_irqsave(&data_lock, flags);
> dirty = op->dirty;
> shadow_dirty = op->shadow_dirty;
> - spin_unlock_irqrestore(&dss_cache.lock, flags);
> + spin_unlock_irqrestore(&data_lock, flags);
>
> if (!dirty&& !shadow_dirty) {
> r = 0;
> @@ -464,7 +466,7 @@ static void dss_apply_irq_handler(void *data, u32 mask)
> for (i = 0; i< num_mgrs; i++)
> mgr_busy[i] = dispc_mgr_go_busy(i);
>
> - spin_lock(&dss_cache.lock);
> + spin_lock(&data_lock);
>
> for (i = 0; i< num_ovls; ++i) {
> ovl = omap_dss_get_overlay(i);
> @@ -498,7 +500,7 @@ static void dss_apply_irq_handler(void *data, u32 mask)
> dss_unregister_vsync_isr();
>
> end:
> - spin_unlock(&dss_cache.lock);
> + spin_unlock(&data_lock);
> }
>
> static int omap_dss_mgr_apply_ovl(struct omap_overlay *ovl)
> @@ -620,7 +622,7 @@ int omap_dss_mgr_apply(struct omap_overlay_manager *mgr)
> if (r)
> return r;
>
> - spin_lock_irqsave(&dss_cache.lock, flags);
> + spin_lock_irqsave(&data_lock, flags);
>
> /* Configure overlays */
> list_for_each_entry(ovl,&mgr->overlays, list)
> @@ -641,7 +643,7 @@ int omap_dss_mgr_apply(struct omap_overlay_manager *mgr)
> dss_write_regs();
> }
>
> - spin_unlock_irqrestore(&dss_cache.lock, flags);
> + spin_unlock_irqrestore(&data_lock, flags);
>
> dispc_runtime_put();
>
^ permalink raw reply
* Re: [PATCH 35/65] OMAPDSS: APPLY: move spinlock outside the struct
From: Archit Taneja @ 2011-11-23 9:39 UTC (permalink / raw)
To: Archit Taneja; +Cc: Tomi Valkeinen, linux-fbdev, linux-omap, archit
In-Reply-To: <4ECCBBFC.10002@ti.com>
On Wednesday 23 November 2011 02:55 PM, Archit Taneja wrote:
<snip>
>
> Minor comment: The name 'data_lock' doesn't tell much that its
> protecting the dss_cache struct. Probably 'cache_lock' or
> 'priv_data_lock' or something like that may be more informative.
>
> Archit
Ah, just saw the next patch, you renamed dss_cache to dss_data, so
'data_lock' seems to make more sense now.
Archit
>
>> +
>> static struct ovl_priv_data *get_ovl_priv(struct omap_overlay *ovl)
>> {
>> return&dss_cache.ovl_priv_data_array[ovl->id];
>> @@ -108,7 +110,7 @@ static struct mgr_priv_data *get_mgr_priv(struct
>> omap_overlay_manager *mgr)
>>
>> void dss_apply_init(void)
>> {
>> - spin_lock_init(&dss_cache.lock);
>> + spin_lock_init(&data_lock);
>> }
>>
>> static bool ovl_manual_update(struct omap_overlay *ovl)
>> @@ -149,10 +151,10 @@ int dss_mgr_wait_for_go(struct
>> omap_overlay_manager *mgr)
>> unsigned long flags;
>> bool shadow_dirty, dirty;
>>
>> - spin_lock_irqsave(&dss_cache.lock, flags);
>> + spin_lock_irqsave(&data_lock, flags);
>> dirty = mp->dirty;
>> shadow_dirty = mp->shadow_dirty;
>> - spin_unlock_irqrestore(&dss_cache.lock, flags);
>> + spin_unlock_irqrestore(&data_lock, flags);
>>
>> if (!dirty&& !shadow_dirty) {
>> r = 0;
>> @@ -212,10 +214,10 @@ int dss_mgr_wait_for_go_ovl(struct omap_overlay
>> *ovl)
>> unsigned long flags;
>> bool shadow_dirty, dirty;
>>
>> - spin_lock_irqsave(&dss_cache.lock, flags);
>> + spin_lock_irqsave(&data_lock, flags);
>> dirty = op->dirty;
>> shadow_dirty = op->shadow_dirty;
>> - spin_unlock_irqrestore(&dss_cache.lock, flags);
>> + spin_unlock_irqrestore(&data_lock, flags);
>>
>> if (!dirty&& !shadow_dirty) {
>> r = 0;
>> @@ -464,7 +466,7 @@ static void dss_apply_irq_handler(void *data, u32
>> mask)
>> for (i = 0; i< num_mgrs; i++)
>> mgr_busy[i] = dispc_mgr_go_busy(i);
>>
>> - spin_lock(&dss_cache.lock);
>> + spin_lock(&data_lock);
>>
>> for (i = 0; i< num_ovls; ++i) {
>> ovl = omap_dss_get_overlay(i);
>> @@ -498,7 +500,7 @@ static void dss_apply_irq_handler(void *data, u32
>> mask)
>> dss_unregister_vsync_isr();
>>
>> end:
>> - spin_unlock(&dss_cache.lock);
>> + spin_unlock(&data_lock);
>> }
>>
>> static int omap_dss_mgr_apply_ovl(struct omap_overlay *ovl)
>> @@ -620,7 +622,7 @@ int omap_dss_mgr_apply(struct omap_overlay_manager
>> *mgr)
>> if (r)
>> return r;
>>
>> - spin_lock_irqsave(&dss_cache.lock, flags);
>> + spin_lock_irqsave(&data_lock, flags);
>>
>> /* Configure overlays */
>> list_for_each_entry(ovl,&mgr->overlays, list)
>> @@ -641,7 +643,7 @@ int omap_dss_mgr_apply(struct omap_overlay_manager
>> *mgr)
>> dss_write_regs();
>> }
>>
>> - spin_unlock_irqrestore(&dss_cache.lock, flags);
>> + spin_unlock_irqrestore(&data_lock, flags);
>>
>> dispc_runtime_put();
>>
>
>
^ permalink raw reply
* Re: [PATCH 40/65] OMAPDSS: APPLY: add mutex
From: Archit Taneja @ 2011-11-23 9:49 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <1321953724-6350-41-git-send-email-tomi.valkeinen@ti.com>
Hi,
On Tuesday 22 November 2011 02:51 PM, Tomi Valkeinen wrote:
> The functions in apply.c, called mostly via function pointers in overlay
> and overlay_manager structs, will be divided into two groups. The other
> group will not sleep and can be called from interrupts, and the other
> group may sleep.
Small sentence issue above, both groups are called the 'other group'.
>
> The idea is that the non-sleeping functions may only change certain
> settings in overlays and managers, and those settings may only affect
> the particular overlay/manager. For example, set the base address of the
> overlay.
>
> The blocking functions, however, will handle more complex configuration
> changes. For example, when an overlay is enabled and fifo-merge feature
> is used, we need to do the enable in multiple steps, waiting in between,
> and the change affects multiple overlays and managers.
>
> This patch adds the mutex which is used in the blocking functions to
> have exclusive access to overlays and overlay managers.
Previously, when we changed the links between 'overlay->managers' and
'manager->devices', it wasn't protected by a lock. Why is it needed now?
As an example, suppose we are changing a manager's device to some other
display. Is this lock preventing someone else to get the older
'mgr->device' rather than the new one?
Archit
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/apply.c | 71 ++++++++++++++++++++++++++++++++++-----
> 1 files changed, 62 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
> index b935264..fb6d3c2 100644
> --- a/drivers/video/omap2/dss/apply.c
> +++ b/drivers/video/omap2/dss/apply.c
> @@ -97,6 +97,8 @@ static struct {
>
> /* protects dss_data */
> static spinlock_t data_lock;
> +/* lock for blocking functions */
> +static DEFINE_MUTEX(apply_lock);
>
> static struct ovl_priv_data *get_ovl_priv(struct omap_overlay *ovl)
> {
> @@ -639,14 +641,22 @@ int omap_dss_mgr_apply(struct omap_overlay_manager *mgr)
>
> void dss_mgr_enable(struct omap_overlay_manager *mgr)
> {
> + mutex_lock(&apply_lock);
> +
> dispc_mgr_enable(mgr->id, true);
> mgr->enabled = true;
> +
> + mutex_unlock(&apply_lock);
> }
>
> void dss_mgr_disable(struct omap_overlay_manager *mgr)
> {
> + mutex_lock(&apply_lock);
> +
> dispc_mgr_enable(mgr->id, false);
> mgr->enabled = false;
> +
> + mutex_unlock(&apply_lock);
> }
>
> int dss_mgr_set_info(struct omap_overlay_manager *mgr,
> @@ -669,44 +679,65 @@ int dss_mgr_set_device(struct omap_overlay_manager *mgr,
> {
> int r;
>
> + mutex_lock(&apply_lock);
> +
> if (dssdev->manager) {
> DSSERR("display '%s' already has a manager '%s'\n",
> dssdev->name, dssdev->manager->name);
> - return -EINVAL;
> + r = -EINVAL;
> + goto err;
> }
>
> if ((mgr->supported_displays& dssdev->type) = 0) {
> DSSERR("display '%s' does not support manager '%s'\n",
> dssdev->name, mgr->name);
> - return -EINVAL;
> + r = -EINVAL;
> + goto err;
> }
>
> dssdev->manager = mgr;
> mgr->device = dssdev;
> mgr->device_changed = true;
>
> + mutex_unlock(&apply_lock);
> +
> return 0;
> +err:
> + mutex_unlock(&apply_lock);
> + return r;
> }
>
> int dss_mgr_unset_device(struct omap_overlay_manager *mgr)
> {
> + int r;
> +
> + mutex_lock(&apply_lock);
> +
> if (!mgr->device) {
> DSSERR("failed to unset display, display not set.\n");
> - return -EINVAL;
> + r = -EINVAL;
> + goto err;
> }
>
> /*
> * Don't allow currently enabled displays to have the overlay manager
> * pulled out from underneath them
> */
> - if (mgr->device->state != OMAP_DSS_DISPLAY_DISABLED)
> - return -EINVAL;
> + if (mgr->device->state != OMAP_DSS_DISPLAY_DISABLED) {
> + r = -EINVAL;
> + goto err;
> + }
>
> mgr->device->manager = NULL;
> mgr->device = NULL;
> mgr->device_changed = true;
>
> + mutex_unlock(&apply_lock);
> +
> return 0;
> +err:
> + mutex_unlock(&apply_lock);
> + return r;
> }
>
>
> @@ -729,18 +760,24 @@ void dss_ovl_get_info(struct omap_overlay *ovl,
> int dss_ovl_set_manager(struct omap_overlay *ovl,
> struct omap_overlay_manager *mgr)
> {
> + int r;
> +
> if (!mgr)
> return -EINVAL;
>
> + mutex_lock(&apply_lock);
> +
> if (ovl->manager) {
> DSSERR("overlay '%s' already has a manager '%s'\n",
> ovl->name, ovl->manager->name);
> - return -EINVAL;
> + r = -EINVAL;
> + goto err;
> }
>
> if (ovl->info.enabled) {
> DSSERR("overlay has to be disabled to change the manager\n");
> - return -EINVAL;
> + r = -EINVAL;
> + goto err;
> }
>
> ovl->manager = mgr;
> @@ -760,25 +797,41 @@ int dss_ovl_set_manager(struct omap_overlay *ovl,
> * the overlay, but before moving the overlay to TV.
> */
>
> + mutex_unlock(&apply_lock);
> +
> return 0;
> +err:
> + mutex_unlock(&apply_lock);
> + return r;
> }
>
> int dss_ovl_unset_manager(struct omap_overlay *ovl)
> {
> + int r;
> +
> + mutex_lock(&apply_lock);
> +
> if (!ovl->manager) {
> DSSERR("failed to detach overlay: manager not set\n");
> - return -EINVAL;
> + r = -EINVAL;
> + goto err;
> }
>
> if (ovl->info.enabled) {
> DSSERR("overlay has to be disabled to unset the manager\n");
> - return -EINVAL;
> + r = -EINVAL;
> + goto err;
> }
>
> ovl->manager = NULL;
> list_del(&ovl->list);
> ovl->manager_changed = true;
>
> + mutex_unlock(&apply_lock);
> +
> return 0;
> +err:
> + mutex_unlock(&apply_lock);
> + return r;
> }
>
^ permalink raw reply
* Re: [PATCH 41/65] OMAPDSS: APPLY: add missing uses of spinlock
From: Archit Taneja @ 2011-11-23 9:57 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <1321953724-6350-42-git-send-email-tomi.valkeinen@ti.com>
On Tuesday 22 November 2011 02:51 PM, Tomi Valkeinen wrote:
> The functions in apply.c, called mostly via function pointers in overlay
> and overlay_manager structs, will be divided into two groups. The other
> group will not sleep and can be called from interrupts, and the other
> group may sleep.
>
> The idea is that the non-sleeping functions may only change certain
> settings in overlays and managers, and those settings may only affect
> the particular overlay/manager. For example, set the base address of the
> overlay.
>
> The blocking functions, however, will handle more complex configuration
> changes. For example, when an overlay is enabled and fifo-merge feature
> is used, we need to do the enable in multiple steps, waiting in between,
> and the change affects multiple overlays and managers.
>
> apply.c already contains a spinlock, which has been used to protect
> (badly) the dss_data. This patch adds locks/unlocks of the spinlock to
> the missing places, and the lock should now properly protect dss_data.
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/apply.c | 29 +++++++++++++++++++++++++++++
> 1 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
> index fb6d3c2..9ad2a36 100644
> --- a/drivers/video/omap2/dss/apply.c
> +++ b/drivers/video/omap2/dss/apply.c
> @@ -405,6 +405,9 @@ void dss_start_update(struct omap_overlay_manager *mgr)
> struct mgr_priv_data *mp = get_mgr_priv(mgr);
> struct ovl_priv_data *op;
> struct omap_overlay *ovl;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&data_lock, flags);
>
> mp->do_manual_update = true;
> dss_write_regs();
> @@ -418,6 +421,8 @@ void dss_start_update(struct omap_overlay_manager *mgr)
> mp->shadow_dirty = false;
>
> dispc_mgr_enable(mgr->id, true);
> +
> + spin_unlock_irqrestore(&data_lock, flags);
> }
>
> static void dss_apply_irq_handler(void *data, u32 mask);
> @@ -662,16 +667,28 @@ void dss_mgr_disable(struct omap_overlay_manager *mgr)
> int dss_mgr_set_info(struct omap_overlay_manager *mgr,
> struct omap_overlay_manager_info *info)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&data_lock, flags);
> +
> mgr->info = *info;
> mgr->info_dirty = true;
>
> + spin_unlock_irqrestore(&data_lock, flags);
> +
> return 0;
> }
>
> void dss_mgr_get_info(struct omap_overlay_manager *mgr,
> struct omap_overlay_manager_info *info)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&data_lock, flags);
> +
> *info = mgr->info;
> +
> + spin_unlock_irqrestore(&data_lock, flags);
> }
>
> int dss_mgr_set_device(struct omap_overlay_manager *mgr,
> @@ -745,16 +762,28 @@ err:
> int dss_ovl_set_info(struct omap_overlay *ovl,
> struct omap_overlay_info *info)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&data_lock, flags);
> +
> ovl->info = *info;
> ovl->info_dirty = true;
>
> + spin_unlock_irqrestore(&data_lock, flags);
> +
> return 0;
> }
>
> void dss_ovl_get_info(struct omap_overlay *ovl,
> struct omap_overlay_info *info)
> {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&data_lock, flags);
> +
> *info = ovl->info;
> +
> + spin_unlock_irqrestore(&data_lock, flags);
> }
The get/set info functions for overlays and managers only modify the
omap_overlay_info or manager_info structs, these aren't really a part of
'dss_data', they only become a part of dss_data only when we call
mgr->apply().
So, are we protecting these functions so that 2 users of the same
overlay don't see incorrect info values?
Archit
>
> int dss_ovl_set_manager(struct omap_overlay *ovl,
^ permalink raw reply
* Re: [PATCH 41/65] OMAPDSS: APPLY: add missing uses of spinlock
From: Tomi Valkeinen @ 2011-11-23 10:12 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <4ECCC332.4050608@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1663 bytes --]
On Wed, 2011-11-23 at 15:26 +0530, Archit Taneja wrote:
> On Tuesday 22 November 2011 02:51 PM, Tomi Valkeinen wrote:
> > int dss_mgr_set_device(struct omap_overlay_manager *mgr,
> > @@ -745,16 +762,28 @@ err:
> > int dss_ovl_set_info(struct omap_overlay *ovl,
> > struct omap_overlay_info *info)
> > {
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&data_lock, flags);
> > +
> > ovl->info = *info;
> > ovl->info_dirty = true;
> >
> > + spin_unlock_irqrestore(&data_lock, flags);
> > +
> > return 0;
> > }
> >
> > void dss_ovl_get_info(struct omap_overlay *ovl,
> > struct omap_overlay_info *info)
> > {
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&data_lock, flags);
> > +
> > *info = ovl->info;
> > +
> > + spin_unlock_irqrestore(&data_lock, flags);
> > }
>
> The get/set info functions for overlays and managers only modify the
> omap_overlay_info or manager_info structs, these aren't really a part of
> 'dss_data', they only become a part of dss_data only when we call
> mgr->apply().
>
> So, are we protecting these functions so that 2 users of the same
> overlay don't see incorrect info values?
True, at this point the data_lock is a bit vague, and is protecting also
the info fields in omap_overlay and omap_overlay_manager.
A lock is needed, though, as otherwise the info struct may be only
partial. E.g. somebody calls set_info, which is half way copying the
values, and somebody else calls apply or get_info.
In the next patches the infos will be moved into the dss_data, and then
using dss_lock spin lock makes more sense.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 40/65] OMAPDSS: APPLY: add mutex
From: Tomi Valkeinen @ 2011-11-23 10:17 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <4ECCC158.80902@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2410 bytes --]
On Wed, 2011-11-23 at 15:18 +0530, Archit Taneja wrote:
> Hi,
>
> On Tuesday 22 November 2011 02:51 PM, Tomi Valkeinen wrote:
> > The functions in apply.c, called mostly via function pointers in overlay
> > and overlay_manager structs, will be divided into two groups. The other
> > group will not sleep and can be called from interrupts, and the other
> > group may sleep.
>
> Small sentence issue above, both groups are called the 'other group'.
Thanks, fixed.
> >
> > The idea is that the non-sleeping functions may only change certain
> > settings in overlays and managers, and those settings may only affect
> > the particular overlay/manager. For example, set the base address of the
> > overlay.
> >
> > The blocking functions, however, will handle more complex configuration
> > changes. For example, when an overlay is enabled and fifo-merge feature
> > is used, we need to do the enable in multiple steps, waiting in between,
> > and the change affects multiple overlays and managers.
> >
> > This patch adds the mutex which is used in the blocking functions to
> > have exclusive access to overlays and overlay managers.
>
> Previously, when we changed the links between 'overlay->managers' and
> 'manager->devices', it wasn't protected by a lock. Why is it needed now?
Previously many places were missing a lock =).
> As an example, suppose we are changing a manager's device to some other
> display. Is this lock preventing someone else to get the older
> 'mgr->device' rather than the new one?
Hmm. We need some lock there, that's for sure, as set/unset manager are
changing the manager's list of overlays. However, it is also protected
by the spinlock, so in that sense mutex is not necessary.
I have to say I'm not sure if mutex is needed at this point. However,
consider the end result when fifo-merge is used:
dss_ovl_enable() will take the mutex, then it does the configuration in
multiple steps, doing multiple spin_lock & spin_unlocks, waiting in
between.
If we had only a spinlock in set/unset_manager, the manager could be
changed while dss_ovl_enable is doing the process of enabling the
overlay.
So I may have added mutexes or spinlocks a bit early in the series to
some places, but I don't see any harm in that. It'd be rather difficult
to try to find the exact spots where a lock becomes a requirement.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 42/65] OMAPDSS: DSI: call mgr_enable/disable for cmd mode
From: Archit Taneja @ 2011-11-23 10:22 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <1321953724-6350-43-git-send-email-tomi.valkeinen@ti.com>
On Tuesday 22 November 2011 02:51 PM, Tomi Valkeinen wrote:
> The current code uses dsi_video_mode_enable/disable functions to
> enable/disable DISPC output for video mode displays. For command mode
> displays we have no notion in the DISPC side of whether the panel is
> enabled, except when a dss_start_update() call is made.
>
> However, to properly maintain the DISPC state in apply.c, we need to
> know if a manager used for a manual update display is currently in use.
>
> This patch achieves that by changing dsi_video_mode_enable/disable to
> dsi_enable/disable_video_output, which is called by both video and
> command mode displays. For video mode displays it starts the actual
> pixel stream, as it did before. For command mode displays it doesn't do
> anything else than mark that the manager is currently in use.
dsi_video_mode_enable() doesn't only enable the DISPC output, it also
sends the long packet header to start video mode transfer.
I think it would be better if we had 2 separate functions, one which
starts/stops DSI video mode, and the other which enables/disables the
DISPC video port.
This way, a manual update panel would need to call only
dsi_enable/disable_video_output(which just enables or disables the
manager), whereas a video mode panel will need to call both.
This is just a suggestion though. It's probably okay to have both in the
same function too. We might have to separate them out later if we were
planning to standardise mipi dsi across SoCs.
Archit
>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/displays/panel-taal.c | 6 ++
> drivers/video/omap2/dss/apply.c | 6 ++-
> drivers/video/omap2/dss/dsi.c | 73 +++++++++++++++-------------
> include/video/omapdss.h | 4 +-
> 4 files changed, 51 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/video/omap2/displays/panel-taal.c b/drivers/video/omap2/displays/panel-taal.c
> index dd64bd1..00c5c61 100644
> --- a/drivers/video/omap2/displays/panel-taal.c
> +++ b/drivers/video/omap2/displays/panel-taal.c
> @@ -1182,6 +1182,10 @@ static int taal_power_on(struct omap_dss_device *dssdev)
> if (r)
> goto err;
>
> + r = dsi_enable_video_output(dssdev, td->channel);
> + if (r)
> + goto err;
> +
> td->enabled = 1;
>
> if (!td->intro_printed) {
> @@ -1211,6 +1215,8 @@ static void taal_power_off(struct omap_dss_device *dssdev)
> struct taal_data *td = dev_get_drvdata(&dssdev->dev);
> int r;
>
> + dsi_disable_video_output(dssdev, td->channel);
> +
> r = taal_dcs_write_0(td, MIPI_DCS_SET_DISPLAY_OFF);
> if (!r)
> r = taal_sleep_in(td);
> diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
> index 9ad2a36..66f4c56 100644
> --- a/drivers/video/omap2/dss/apply.c
> +++ b/drivers/video/omap2/dss/apply.c
> @@ -648,7 +648,8 @@ void dss_mgr_enable(struct omap_overlay_manager *mgr)
> {
> mutex_lock(&apply_lock);
>
> - dispc_mgr_enable(mgr->id, true);
> + if (!mgr_manual_update(mgr))
> + dispc_mgr_enable(mgr->id, true);
> mgr->enabled = true;
>
> mutex_unlock(&apply_lock);
> @@ -658,7 +659,8 @@ void dss_mgr_disable(struct omap_overlay_manager *mgr)
> {
> mutex_lock(&apply_lock);
>
> - dispc_mgr_enable(mgr->id, false);
> + if (!mgr_manual_update(mgr))
> + dispc_mgr_enable(mgr->id, false);
> mgr->enabled = false;
>
> mutex_unlock(&apply_lock);
> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
> index 08d3de90..a35f3fb 100644
> --- a/drivers/video/omap2/dss/dsi.c
> +++ b/drivers/video/omap2/dss/dsi.c
> @@ -3939,65 +3939,70 @@ static void dsi_proto_timings(struct omap_dss_device *dssdev)
> }
> }
>
> -int dsi_video_mode_enable(struct omap_dss_device *dssdev, int channel)
> +int dsi_enable_video_output(struct omap_dss_device *dssdev, int channel)
> {
> struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
> int bpp = dsi_get_pixel_size(dssdev->panel.dsi_pix_fmt);
> u8 data_type;
> u16 word_count;
>
> - switch (dssdev->panel.dsi_pix_fmt) {
> - case OMAP_DSS_DSI_FMT_RGB888:
> - data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24;
> - break;
> - case OMAP_DSS_DSI_FMT_RGB666:
> - data_type = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
> - break;
> - case OMAP_DSS_DSI_FMT_RGB666_PACKED:
> - data_type = MIPI_DSI_PACKED_PIXEL_STREAM_18;
> - break;
> - case OMAP_DSS_DSI_FMT_RGB565:
> - data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16;
> - break;
> - default:
> - BUG();
> - };
> + if (dssdev->panel.dsi_mode = OMAP_DSS_DSI_VIDEO_MODE) {
> + switch (dssdev->panel.dsi_pix_fmt) {
> + case OMAP_DSS_DSI_FMT_RGB888:
> + data_type = MIPI_DSI_PACKED_PIXEL_STREAM_24;
> + break;
> + case OMAP_DSS_DSI_FMT_RGB666:
> + data_type = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
> + break;
> + case OMAP_DSS_DSI_FMT_RGB666_PACKED:
> + data_type = MIPI_DSI_PACKED_PIXEL_STREAM_18;
> + break;
> + case OMAP_DSS_DSI_FMT_RGB565:
> + data_type = MIPI_DSI_PACKED_PIXEL_STREAM_16;
> + break;
> + default:
> + BUG();
> + };
>
> - dsi_if_enable(dsidev, false);
> - dsi_vc_enable(dsidev, channel, false);
> + dsi_if_enable(dsidev, false);
> + dsi_vc_enable(dsidev, channel, false);
>
> - /* MODE, 1 = video mode */
> - REG_FLD_MOD(dsidev, DSI_VC_CTRL(channel), 1, 4, 4);
> + /* MODE, 1 = video mode */
> + REG_FLD_MOD(dsidev, DSI_VC_CTRL(channel), 1, 4, 4);
>
> - word_count = DIV_ROUND_UP(dssdev->panel.timings.x_res * bpp, 8);
> + word_count = DIV_ROUND_UP(dssdev->panel.timings.x_res * bpp, 8);
>
> - dsi_vc_write_long_header(dsidev, channel, data_type, word_count, 0);
> + dsi_vc_write_long_header(dsidev, channel, data_type,
> + word_count, 0);
>
> - dsi_vc_enable(dsidev, channel, true);
> - dsi_if_enable(dsidev, true);
> + dsi_vc_enable(dsidev, channel, true);
> + dsi_if_enable(dsidev, true);
> + }
>
> dss_mgr_enable(dssdev->manager);
>
> return 0;
> }
> -EXPORT_SYMBOL(dsi_video_mode_enable);
> +EXPORT_SYMBOL(dsi_enable_video_output);
>
> -void dsi_video_mode_disable(struct omap_dss_device *dssdev, int channel)
> +void dsi_disable_video_output(struct omap_dss_device *dssdev, int channel)
> {
> struct platform_device *dsidev = dsi_get_dsidev_from_dssdev(dssdev);
>
> - dsi_if_enable(dsidev, false);
> - dsi_vc_enable(dsidev, channel, false);
> + if (dssdev->panel.dsi_mode = OMAP_DSS_DSI_VIDEO_MODE) {
> + dsi_if_enable(dsidev, false);
> + dsi_vc_enable(dsidev, channel, false);
>
> - /* MODE, 0 = command mode */
> - REG_FLD_MOD(dsidev, DSI_VC_CTRL(channel), 0, 4, 4);
> + /* MODE, 0 = command mode */
> + REG_FLD_MOD(dsidev, DSI_VC_CTRL(channel), 0, 4, 4);
>
> - dsi_vc_enable(dsidev, channel, true);
> - dsi_if_enable(dsidev, true);
> + dsi_vc_enable(dsidev, channel, true);
> + dsi_if_enable(dsidev, true);
> + }
>
> dss_mgr_disable(dssdev->manager);
> }
> -EXPORT_SYMBOL(dsi_video_mode_disable);
> +EXPORT_SYMBOL(dsi_disable_video_output);
>
> static void dsi_update_screen_dispc(struct omap_dss_device *dssdev,
> u16 w, u16 h)
> diff --git a/include/video/omapdss.h b/include/video/omapdss.h
> index eaeca89..25ef771 100644
> --- a/include/video/omapdss.h
> +++ b/include/video/omapdss.h
> @@ -294,8 +294,8 @@ int dsi_vc_set_max_rx_packet_size(struct omap_dss_device *dssdev, int channel,
> u16 len);
> int dsi_vc_send_null(struct omap_dss_device *dssdev, int channel);
> int dsi_vc_send_bta_sync(struct omap_dss_device *dssdev, int channel);
> -int dsi_video_mode_enable(struct omap_dss_device *dssdev, int channel);
> -void dsi_video_mode_disable(struct omap_dss_device *dssdev, int channel);
> +int dsi_enable_video_output(struct omap_dss_device *dssdev, int channel);
> +void dsi_disable_video_output(struct omap_dss_device *dssdev, int channel);
>
> /* Board specific data */
> struct omap_dss_board_info {
^ permalink raw reply
* Re: [PATCH 35/65] OMAPDSS: APPLY: move spinlock outside the struct
From: Sergey Kibrik @ 2011-11-23 10:29 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <1321953724-6350-36-git-send-email-tomi.valkeinen@ti.com>
On 11/22/2011 11:21 AM, Tomi Valkeinen wrote:
> dss_cache struct contains a spinlock used to protect the struct. A more
> logical place for the spinlock is outside the struct that it is
> protecting. So move it there.
a small question: isn't it clearer to keep lock inside struct, so it would be easier to read code? Say, if we meet
> spin_lock_irqsave(&dss_cache.lock, flags);
in code we already aware of what struct being actually protected, and in case of external lock it's not that obvious
--
regards,
Sergey
^ permalink raw reply
* Re: [PATCH 42/65] OMAPDSS: DSI: call mgr_enable/disable for cmd
From: Tomi Valkeinen @ 2011-11-23 10:42 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <4ECCC68C.7070106@ti.com>
[-- Attachment #1: Type: text/plain, Size: 3031 bytes --]
On Wed, 2011-11-23 at 15:40 +0530, Archit Taneja wrote:
> On Tuesday 22 November 2011 02:51 PM, Tomi Valkeinen wrote:
> > The current code uses dsi_video_mode_enable/disable functions to
> > enable/disable DISPC output for video mode displays. For command mode
> > displays we have no notion in the DISPC side of whether the panel is
> > enabled, except when a dss_start_update() call is made.
> >
> > However, to properly maintain the DISPC state in apply.c, we need to
> > know if a manager used for a manual update display is currently in use.
> >
> > This patch achieves that by changing dsi_video_mode_enable/disable to
> > dsi_enable/disable_video_output, which is called by both video and
> > command mode displays. For video mode displays it starts the actual
> > pixel stream, as it did before. For command mode displays it doesn't do
> > anything else than mark that the manager is currently in use.
>
> dsi_video_mode_enable() doesn't only enable the DISPC output, it also
> sends the long packet header to start video mode transfer.
>
> I think it would be better if we had 2 separate functions, one which
> starts/stops DSI video mode, and the other which enables/disables the
> DISPC video port.
>
> This way, a manual update panel would need to call only
> dsi_enable/disable_video_output(which just enables or disables the
> manager), whereas a video mode panel will need to call both.
>
> This is just a suggestion though. It's probably okay to have both in the
> same function too. We might have to separate them out later if we were
> planning to standardise mipi dsi across SoCs.
If you think from the panel driver's point of view, it doesn't know
about DISPC. It just wants to enable the video stream (on video mode
displays).
If we had two functions, could only the first be used? I.e. is it
possible to just enable the video mode transfer, without enabling DISPC?
If not, I'm not sure what would be the use for two separate functions.
And even if it can, I'm not sure what use it would be to enable only the
video mode output without the actual pixel data from DISPC.
It is true that the function in thsi patch is not the best one. For
command mode display it's more about reserving the ovl manager for use
than actually enabling it. Then again, how I thought the function's
purpose was that it enables the DSI video output, but because for
command mode there's need to trigger the actual frame transfer later,
the function doesn't start the pixel feed for command mode displays. It
just "prepares" the output.
But even if we had the functions separated, the function called by video
mode and command mode displays would be different, as for the former one
it enables the pixel stream, and for latter one it just reserves the
output.
So, I'm fine with changing the function, but the reasoning for what
functions we have and what they do should come from the panel driver's
perspective, not because of OMAP DSS's HW details.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 35/65] OMAPDSS: APPLY: move spinlock outside the struct
From: Tomi Valkeinen @ 2011-11-23 10:47 UTC (permalink / raw)
To: Sergey Kibrik; +Cc: linux-fbdev, linux-omap, archit
In-Reply-To: <4ECCCB07.10702@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1276 bytes --]
On Wed, 2011-11-23 at 12:29 +0200, Sergey Kibrik wrote:
> On 11/22/2011 11:21 AM, Tomi Valkeinen wrote:
> > dss_cache struct contains a spinlock used to protect the struct. A more
> > logical place for the spinlock is outside the struct that it is
> > protecting. So move it there.
>
> a small question: isn't it clearer to keep lock inside struct, so it would be easier to read code? Say, if we meet
>
> > spin_lock_irqsave(&dss_cache.lock, flags);
>
> in code we already aware of what struct being actually protected, and in case of external lock it's not that obvious
But if you meet code like:
op = get_ovl_priv(ovl);
You don't see that the data is inside the struct protected with the
spinlock. So you still need to understand what it protects, and what the
above function returns.
But I see your point. I'm not sure which way is better. I thought it
like this: the lock protects a struct, but if the lock is inside the
struct, the lock would protect also itself. Which it doesn't.
That said, I'm fine with both ways. It doesn't matter much. I didn't
really look for any established patterns for this in the kernel code,
but if everybody else have their locks inside the structs they protect,
then obviously we should also.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
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