* Re: Should I use FBINFO_VIRTFB?
From: Konrad Rzeszutek Wilk @ 2011-12-15 21:13 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <4EEA5DA7.3010406@freescale.com>
On Thu, Dec 15, 2011 at 02:50:47PM -0600, Timur Tabi wrote:
> I'm cleaning up my fbdev driver, and I just noticed FBINFO_VIRTFB:
>
> #define FBINFO_VIRTFB 0x0004 /* FB is System RAM, not device. */
>
> I am currently not setting this flag, but I am allocating my framebuffer in system ram via dma_alloc_coherent(). I don't see any good documentation for this flag, but I suspect I should be enabling it. What exactly does this flag do?
It basically inhibits from using VM_IO which should not be set on
System RAM. In your case you are using System RAM, so please do set it.
>
> I'd also like some explanation for these two macros, which appear to be related:
>
> #define FBINFO_PARTIAL_PAN_OK 0x0040 /* otw use pan only for double-buffering */
> #define FBINFO_READS_FAST 0x0080 /* soft-copy faster than rendering */
Not really. They have a different function, but you are better of looking in the code
to see how they are used.
>
> --
> Timur Tabi
> Linux kernel developer at Freescale
>
> --
> 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
^ permalink raw reply
* Should I use FBINFO_VIRTFB?
From: Timur Tabi @ 2011-12-15 20:50 UTC (permalink / raw)
To: linux-fbdev
I'm cleaning up my fbdev driver, and I just noticed FBINFO_VIRTFB:
#define FBINFO_VIRTFB 0x0004 /* FB is System RAM, not device. */
I am currently not setting this flag, but I am allocating my framebuffer in system ram via dma_alloc_coherent(). I don't see any good documentation for this flag, but I suspect I should be enabling it. What exactly does this flag do?
I'd also like some explanation for these two macros, which appear to be related:
#define FBINFO_PARTIAL_PAN_OK 0x0040 /* otw use pan only for double-buffering */
#define FBINFO_READS_FAST 0x0080 /* soft-copy faster than rendering */
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH 27/57] fbdev: sh_mobile_lcdc: Pass a video mode to the
From: Guennadi Liakhovetski @ 2011-12-15 19:01 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-28-git-send-email-laurent.pinchart@ideasonboard.com>
On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> Pass pointers to struct fb_videomode and struct fb_monspecs instead of
> struct fb_var_screeninfo to the notify callback.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/video/sh_mobile_hdmi.c | 59 +++++++++++++++++--------------------
> drivers/video/sh_mobile_lcdcfb.c | 40 ++++++++++++++-----------
> drivers/video/sh_mobile_lcdcfb.h | 3 +-
> 3 files changed, 51 insertions(+), 51 deletions(-)
[snip]
> diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
> index 21e5f10..4ec216e 100644
> --- a/drivers/video/sh_mobile_lcdcfb.c
> +++ b/drivers/video/sh_mobile_lcdcfb.c
[ditto]
> @@ -433,12 +432,17 @@ static int sh_mobile_lcdc_display_notify(struct sh_mobile_lcdc_chan *ch,
> }
> break;
>
> - case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE:
> + case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE: {
> + struct fb_var_screeninfo var;
> +
> /* Validate a proposed new mode */
> - var->bits_per_pixel = info->var.bits_per_pixel;
> - ret = info->fbops->fb_check_var(var, info);
> + fb_videomode_to_var(&var, mode);
> + var.bits_per_pixel = info->var.bits_per_pixel;
> + var.grayscale = info->var.grayscale;
> + ret = info->fbops->fb_check_var(&var, info);
> break;
> }
> + }
nitpick - please, realign:-)
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH v4 3/5] powerpc/mpc5121: shared DIU framebuffer support
From: Tabi Timur-B04825 @ 2011-12-15 17:27 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1279893639-24333-4-git-send-email-agust@denx.de>
On Fri, Jul 23, 2010 at 9:00 AM, Anatolij Gustschin <agust@denx.de> wrote:
> @@ -1471,7 +1476,9 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev,
> goto error2;
> }
>
> - out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU anyway*/
> + diu_mode = in_be32(&dr.diu_reg->diu_mode);
> + if (diu_mode != MFB_MODE1)
> + out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU */
Anatolij,
I know this patch is old, but I just noticed something odd about it
that I need your help with.
In the above snippet, you test for != MFB_MODE1. My understanding is
that U-boot only supports modes 0 and 1, never modes 2 or 3. So
diu_mode can only ever be 0 or 1. That means that that the above code
is equivalent to:
diu_mode = in_be32(&dr.diu_reg->diu_mode);
if (diu_mode = 0)
out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU */
which is silly, because now we're writing 0 to diu_mode only if it's already 0.
Am I missing something?
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH 24/57] fbdev: sh_mobile_lcdc: Return display connection
From: Guennadi Liakhovetski @ 2011-12-15 17:11 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-25-git-send-email-laurent.pinchart@ideasonboard.com>
On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> Return true if the display is connected and false otherwise. Set the fb
> info state to FBINFO_STATE_SUSPENDED in the sh_mobile_lcdc driver when
> the display is not connected.
Hmm... I'm not sure I like functions, that return either a negative error,
or _several_ non-negative success values. How about returning -ENODEV?
Thanks
Guennadi
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/video/sh_mipi_dsi.c | 2 +-
> drivers/video/sh_mobile_hdmi.c | 9 +++++----
> drivers/video/sh_mobile_lcdcfb.c | 3 +++
> drivers/video/sh_mobile_lcdcfb.h | 3 +++
> 4 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/video/sh_mipi_dsi.c b/drivers/video/sh_mipi_dsi.c
> index 1ede247..5ff3742 100644
> --- a/drivers/video/sh_mipi_dsi.c
> +++ b/drivers/video/sh_mipi_dsi.c
> @@ -412,7 +412,7 @@ static int mipi_display_on(struct sh_mobile_lcdc_entity *entity)
>
> sh_mipi_dsi_enable(mipi, true);
>
> - return 0;
> + return SH_MOBILE_LCDC_DISPLAY_CONNECTED;
>
> mipi_display_on_fail1:
> pm_runtime_put_sync(&mipi->pdev->dev);
> diff --git a/drivers/video/sh_mobile_hdmi.c b/drivers/video/sh_mobile_hdmi.c
> index c22e123..1464abf 100644
> --- a/drivers/video/sh_mobile_hdmi.c
> +++ b/drivers/video/sh_mobile_hdmi.c
> @@ -1004,9 +1004,9 @@ static int sh_hdmi_display_on(struct sh_mobile_lcdc_entity *entity)
> {
> struct sh_hdmi *hdmi = entity_to_sh_hdmi(entity);
> struct sh_mobile_lcdc_chan *ch = entity->lcdc;
> - struct fb_info *info = ch->info;
>
> - dev_dbg(hdmi->dev, "%s(%p): state %x\n", __func__, hdmi, info->state);
> + dev_dbg(hdmi->dev, "%s(%p): state %x\n", __func__, hdmi,
> + hdmi->hp_state);
>
> /*
> * hp_state can be set to
> @@ -1021,12 +1021,13 @@ static int sh_hdmi_display_on(struct sh_mobile_lcdc_entity *entity)
> dev_dbg(hdmi->dev, "HDMI running\n");
> break;
> case HDMI_HOTPLUG_DISCONNECTED:
> - info->state = FBINFO_STATE_SUSPENDED;
> default:
> hdmi->var = ch->display_var;
> }
>
> - return 0;
> + return hdmi->hp_state = HDMI_HOTPLUG_DISCONNECTED
> + ? SH_MOBILE_LCDC_DISPLAY_DISCONNECTED
> + : SH_MOBILE_LCDC_DISPLAY_CONNECTED;
> }
>
> static void sh_hdmi_display_off(struct sh_mobile_lcdc_entity *entity)
> diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
> index 47108aa..4b03aa5 100644
> --- a/drivers/video/sh_mobile_lcdcfb.c
> +++ b/drivers/video/sh_mobile_lcdcfb.c
> @@ -342,6 +342,9 @@ static void sh_mobile_lcdc_display_on(struct sh_mobile_lcdc_chan *ch)
> ret = ch->tx_dev->ops->display_on(ch->tx_dev);
> if (ret < 0)
> return;
> +
> + if (ret = SH_MOBILE_LCDC_DISPLAY_DISCONNECTED)
> + ch->info->state = FBINFO_STATE_SUSPENDED;
> }
>
> /* HDMI must be enabled before LCDC configuration */
> diff --git a/drivers/video/sh_mobile_lcdcfb.h b/drivers/video/sh_mobile_lcdcfb.h
> index b2cb8e6..6fb956c 100644
> --- a/drivers/video/sh_mobile_lcdcfb.h
> +++ b/drivers/video/sh_mobile_lcdcfb.h
> @@ -21,6 +21,9 @@ struct sh_mobile_lcdc_entity;
> struct sh_mobile_lcdc_priv;
> struct sh_mobile_lcdc_chan;
>
> +#define SH_MOBILE_LCDC_DISPLAY_DISCONNECTED 0
> +#define SH_MOBILE_LCDC_DISPLAY_CONNECTED 1
> +
> struct sh_mobile_lcdc_entity_ops {
> /* Display */
> int (*display_on)(struct sh_mobile_lcdc_entity *entity);
> --
> 1.7.3.4
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 21/57] sh_mobile_lcdc: Add an lcdc channel pointer to
From: Guennadi Liakhovetski @ 2011-12-15 16:16 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-22-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Laurent
On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> The field will be used by the transmitter drivers to access
> sh_mobile_lcdc_chan fields such as fb_info.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/video/sh_mobile_lcdcfb.c | 5 ++++-
> drivers/video/sh_mobile_lcdcfb.h | 2 ++
> 2 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
> index cb5ea3c..2dccfde 100644
> --- a/drivers/video/sh_mobile_lcdcfb.c
> +++ b/drivers/video/sh_mobile_lcdcfb.c
> @@ -1497,8 +1497,10 @@ static int sh_mobile_lcdc_remove(struct platform_device *pdev)
> if (!info || !info->device)
> continue;
>
> - if (ch->tx_dev)
> + if (ch->tx_dev) {
> + ch->tx_dev->lcdc = NULL;
> module_put(ch->cfg.tx_dev->dev.driver->owner);
> + }
>
> if (ch->sglist)
> vfree(ch->sglist);
> @@ -1608,6 +1610,7 @@ sh_mobile_lcdc_channel_init(struct sh_mobile_lcdc_priv *priv,
> return -EINVAL;
> }
> ch->tx_dev = platform_get_drvdata(cfg->tx_dev);
> + ch->tx_dev->lcdc = ch;
I do not have a kernel, patched with your patches up to 20/57;-) so, I
cannot verify - can ch->tx_dev at this point not be NULL?
> }
>
> /* Iterate through the modes to validate them and find the highest
> diff --git a/drivers/video/sh_mobile_lcdcfb.h b/drivers/video/sh_mobile_lcdcfb.h
> index 9601b92..36cd564 100644
> --- a/drivers/video/sh_mobile_lcdcfb.h
> +++ b/drivers/video/sh_mobile_lcdcfb.h
> @@ -19,6 +19,7 @@ struct fb_info;
> struct module;
> struct sh_mobile_lcdc_entity;
> struct sh_mobile_lcdc_priv;
> +struct sh_mobile_lcdc_chan;
>
> struct sh_mobile_lcdc_entity_ops {
> /* Display */
> @@ -30,6 +31,7 @@ struct sh_mobile_lcdc_entity_ops {
> struct sh_mobile_lcdc_entity {
> struct module *owner;
> const struct sh_mobile_lcdc_entity_ops *ops;
> + struct sh_mobile_lcdc_chan *lcdc;
> };
>
> /*
> --
> 1.7.3.4
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 12/57] arm: mach-shmobile: Add LCDC tx_dev field to
From: Guennadi Liakhovetski @ 2011-12-15 14:01 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-13-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Laurent
On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> Make sure the transmitter devices get registered before the associated
> LCDC devices.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> arch/arm/mach-shmobile/board-ap4evb.c | 272 ++++++++++++++++---------------
> arch/arm/mach-shmobile/board-mackerel.c | 66 ++++----
> 2 files changed, 176 insertions(+), 162 deletions(-)
>
> diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
> index dca4860..e0a6b3d 100644
> --- a/arch/arm/mach-shmobile/board-ap4evb.c
> +++ b/arch/arm/mach-shmobile/board-ap4evb.c
> @@ -445,82 +445,6 @@ static struct platform_device usb1_host_device = {
> .resource = usb1_host_resources,
> };
>
> -static const struct fb_videomode ap4evb_lcdc_modes[] = {
> - {
> -#ifdef CONFIG_AP4EVB_QHD
> - .name = "R63302(QHD)",
> - .xres = 544,
> - .yres = 961,
> - .left_margin = 72,
> - .right_margin = 600,
> - .hsync_len = 16,
> - .upper_margin = 8,
> - .lower_margin = 8,
> - .vsync_len = 2,
> - .sync = FB_SYNC_VERT_HIGH_ACT | FB_SYNC_HOR_HIGH_ACT,
> -#else
> - .name = "WVGA Panel",
> - .xres = 800,
> - .yres = 480,
> - .left_margin = 220,
> - .right_margin = 110,
> - .hsync_len = 70,
> - .upper_margin = 20,
> - .lower_margin = 5,
> - .vsync_len = 5,
> - .sync = 0,
> -#endif
> - },
> -};
> -static struct sh_mobile_meram_cfg lcd_meram_cfg = {
> - .icb[0] = {
> - .marker_icb = 28,
> - .cache_icb = 24,
> - .meram_offset = 0x0,
> - .meram_size = 0x40,
> - },
> - .icb[1] = {
> - .marker_icb = 29,
> - .cache_icb = 25,
> - .meram_offset = 0x40,
> - .meram_size = 0x40,
> - },
> -};
> -
> -static struct sh_mobile_lcdc_info lcdc_info = {
> - .meram_dev = &meram_info,
> - .ch[0] = {
> - .chan = LCDC_CHAN_MAINLCD,
> - .fourcc = V4L2_PIX_FMT_RGB565,
> - .lcd_cfg = ap4evb_lcdc_modes,
> - .num_cfg = ARRAY_SIZE(ap4evb_lcdc_modes),
> - .meram_cfg = &lcd_meram_cfg,
> - }
> -};
> -
> -static struct resource lcdc_resources[] = {
> - [0] = {
> - .name = "LCDC",
> - .start = 0xfe940000, /* P4-only space */
> - .end = 0xfe943fff,
> - .flags = IORESOURCE_MEM,
> - },
> - [1] = {
> - .start = intcs_evt2irq(0x580),
> - .flags = IORESOURCE_IRQ,
> - },
> -};
> -
> -static struct platform_device lcdc_device = {
> - .name = "sh_mobile_lcdc_fb",
> - .num_resources = ARRAY_SIZE(lcdc_resources),
> - .resource = lcdc_resources,
> - .dev = {
> - .platform_data = &lcdc_info,
> - .coherent_dma_mask = ~0,
> - },
> -};
> -
> /*
> * QHD display
> */
> @@ -601,6 +525,8 @@ static struct resource mipidsi0_resources[] = {
> },
> };
>
> +static struct sh_mobile_lcdc_info lcdc_info;
> +
> static struct sh_mipi_dsi_info mipidsi0_info = {
> .data_format = MIPI_RGB888,
> .lcd_chan = &lcdc_info.ch[0],
> @@ -627,6 +553,86 @@ static struct platform_device *qhd_devices[] __initdata = {
> };
> #endif /* CONFIG_AP4EVB_QHD */
>
> +/* LCDC0 */
> +static const struct fb_videomode ap4evb_lcdc_modes[] = {
Hm, I must be missing something. I thought you were moving all the structs
around to make them available for reference _without_ forward
declarations. But you _do_ end up using a forward declaration anyway. Here
and for HDMI below too. So, what's the point? Why not just forward declare
that one struct, that's needed and leave the rest where it currently is
in the file?
> + {
> +#ifdef CONFIG_AP4EVB_QHD
> + .name = "R63302(QHD)",
> + .xres = 544,
> + .yres = 961,
> + .left_margin = 72,
> + .right_margin = 600,
> + .hsync_len = 16,
> + .upper_margin = 8,
> + .lower_margin = 8,
> + .vsync_len = 2,
> + .sync = FB_SYNC_VERT_HIGH_ACT | FB_SYNC_HOR_HIGH_ACT,
> +#else
> + .name = "WVGA Panel",
> + .xres = 800,
> + .yres = 480,
> + .left_margin = 220,
> + .right_margin = 110,
> + .hsync_len = 70,
> + .upper_margin = 20,
> + .lower_margin = 5,
> + .vsync_len = 5,
> + .sync = 0,
> +#endif
> + },
> +};
> +static struct sh_mobile_meram_cfg lcd_meram_cfg = {
> + .icb[0] = {
> + .marker_icb = 28,
> + .cache_icb = 24,
> + .meram_offset = 0x0,
> + .meram_size = 0x40,
> + },
> + .icb[1] = {
> + .marker_icb = 29,
> + .cache_icb = 25,
> + .meram_offset = 0x40,
> + .meram_size = 0x40,
> + },
> +};
> +
> +static struct sh_mobile_lcdc_info lcdc_info = {
> + .meram_dev = &meram_info,
> + .ch[0] = {
> + .chan = LCDC_CHAN_MAINLCD,
> + .fourcc = V4L2_PIX_FMT_RGB565,
> + .lcd_cfg = ap4evb_lcdc_modes,
> + .num_cfg = ARRAY_SIZE(ap4evb_lcdc_modes),
> + .meram_cfg = &lcd_meram_cfg,
> +#ifdef CONFIG_AP4EVB_QHD
> + .tx_dev = &mipidsi0_device,
> +#endif
> + }
> +};
> +
> +static struct resource lcdc_resources[] = {
> + [0] = {
> + .name = "LCDC",
> + .start = 0xfe940000, /* P4-only space */
> + .end = 0xfe943fff,
> + .flags = IORESOURCE_MEM,
> + },
> + [1] = {
> + .start = intcs_evt2irq(0x580),
> + .flags = IORESOURCE_IRQ,
> + },
> +};
> +
> +static struct platform_device lcdc_device = {
> + .name = "sh_mobile_lcdc_fb",
> + .num_resources = ARRAY_SIZE(lcdc_resources),
> + .resource = lcdc_resources,
> + .dev = {
> + .platform_data = &lcdc_info,
> + .coherent_dma_mask = ~0,
> + },
> +};
> +
> /* FSI */
> #define IRQ_FSI evt2irq(0x1840)
> static int __fsi_set_rate(struct clk *clk, long rate, int enable)
> @@ -793,6 +799,62 @@ static struct platform_device fsi_device = {
> static struct platform_device fsi_ak4643_device = {
> .name = "sh_fsi2_a_ak4643",
> };
> +
> +/* LCDC1 */
> +static long ap4evb_clk_optimize(unsigned long target, unsigned long *best_freq,
> + unsigned long *parent_freq);
> +
> +static struct sh_mobile_lcdc_info sh_mobile_lcdc1_info;
> +
> +static struct sh_mobile_hdmi_info hdmi_info = {
> + .lcd_chan = &sh_mobile_lcdc1_info.ch[0],
> + .flags = HDMI_SND_SRC_SPDIF,
> + .clk_optimize_parent = ap4evb_clk_optimize,
> +};
> +
> +static struct resource hdmi_resources[] = {
> + [0] = {
> + .name = "HDMI",
> + .start = 0xe6be0000,
> + .end = 0xe6be00ff,
> + .flags = IORESOURCE_MEM,
> + },
> + [1] = {
> + /* There's also an HDMI interrupt on INTCS @ 0x18e0 */
> + .start = evt2irq(0x17e0),
> + .flags = IORESOURCE_IRQ,
> + },
> +};
> +
> +static struct platform_device hdmi_device = {
> + .name = "sh-mobile-hdmi",
> + .num_resources = ARRAY_SIZE(hdmi_resources),
> + .resource = hdmi_resources,
> + .id = -1,
> + .dev = {
> + .platform_data = &hdmi_info,
> + },
> +};
> +
> +static long ap4evb_clk_optimize(unsigned long target, unsigned long *best_freq,
> + unsigned long *parent_freq)
> +{
> + struct clk *hdmi_ick = clk_get(&hdmi_device.dev, "ick");
> + long error;
> +
> + if (IS_ERR(hdmi_ick)) {
> + int ret = PTR_ERR(hdmi_ick);
> + pr_err("Cannot get HDMI ICK: %d\n", ret);
> + return ret;
> + }
> +
> + error = clk_round_parent(hdmi_ick, target, best_freq, parent_freq, 1, 64);
> +
> + clk_put(hdmi_ick);
> +
> + return error;
> +}
> +
> static struct sh_mobile_meram_cfg hdmi_meram_cfg = {
> .icb[0] = {
> .marker_icb = 30,
> @@ -818,6 +880,7 @@ static struct sh_mobile_lcdc_info sh_mobile_lcdc1_info = {
> .clock_divider = 1,
> .flags = LCDC_FLAGS_DWPOL,
> .meram_cfg = &hdmi_meram_cfg,
> + .tx_dev = &hdmi_device,
> }
> };
>
> @@ -845,63 +908,10 @@ static struct platform_device lcdc1_device = {
> },
> };
>
> -static long ap4evb_clk_optimize(unsigned long target, unsigned long *best_freq,
> - unsigned long *parent_freq);
> -
> -
> -static struct sh_mobile_hdmi_info hdmi_info = {
> - .lcd_chan = &sh_mobile_lcdc1_info.ch[0],
> - .flags = HDMI_SND_SRC_SPDIF,
> - .clk_optimize_parent = ap4evb_clk_optimize,
> -};
> -
> -static struct resource hdmi_resources[] = {
> - [0] = {
> - .name = "HDMI",
> - .start = 0xe6be0000,
> - .end = 0xe6be00ff,
> - .flags = IORESOURCE_MEM,
> - },
> - [1] = {
> - /* There's also an HDMI interrupt on INTCS @ 0x18e0 */
> - .start = evt2irq(0x17e0),
> - .flags = IORESOURCE_IRQ,
> - },
> -};
> -
> -static struct platform_device hdmi_device = {
> - .name = "sh-mobile-hdmi",
> - .num_resources = ARRAY_SIZE(hdmi_resources),
> - .resource = hdmi_resources,
> - .id = -1,
> - .dev = {
> - .platform_data = &hdmi_info,
> - },
> -};
> -
> static struct platform_device fsi_hdmi_device = {
> .name = "sh_fsi2_b_hdmi",
> };
>
> -static long ap4evb_clk_optimize(unsigned long target, unsigned long *best_freq,
> - unsigned long *parent_freq)
> -{
> - struct clk *hdmi_ick = clk_get(&hdmi_device.dev, "ick");
> - long error;
> -
> - if (IS_ERR(hdmi_ick)) {
> - int ret = PTR_ERR(hdmi_ick);
> - pr_err("Cannot get HDMI ICK: %d\n", ret);
> - return ret;
> - }
> -
> - error = clk_round_parent(hdmi_ick, target, best_freq, parent_freq, 1, 64);
> -
> - clk_put(hdmi_ick);
> -
> - return error;
> -}
> -
> static struct gpio_led ap4evb_leds[] = {
> {
> .name = "led4",
> @@ -1036,9 +1046,9 @@ static struct platform_device *ap4evb_devices[] __initdata = {
> &fsi_ak4643_device,
> &fsi_hdmi_device,
> &sh_mmcif_device,
> - &lcdc1_device,
> - &lcdc_device,
> &hdmi_device,
> + &lcdc_device,
> + &lcdc1_device,
> &ceu_device,
> &ap4evb_camera,
> &meram_device,
> diff --git a/arch/arm/mach-shmobile/board-mackerel.c b/arch/arm/mach-shmobile/board-mackerel.c
> index 4ed0138..ccdd9fc 100644
> --- a/arch/arm/mach-shmobile/board-mackerel.c
> +++ b/arch/arm/mach-shmobile/board-mackerel.c
> @@ -431,6 +431,38 @@ static struct platform_device lcdc_device = {
> },
> };
>
> +/* HDMI */
> +static struct sh_mobile_lcdc_info hdmi_lcdc_info;
> +
> +static struct sh_mobile_hdmi_info hdmi_info = {
> + .lcd_chan = &hdmi_lcdc_info.ch[0],
> + .flags = HDMI_SND_SRC_SPDIF,
> +};
> +
> +static struct resource hdmi_resources[] = {
> + [0] = {
> + .name = "HDMI",
> + .start = 0xe6be0000,
> + .end = 0xe6be00ff,
> + .flags = IORESOURCE_MEM,
> + },
> + [1] = {
> + /* There's also an HDMI interrupt on INTCS @ 0x18e0 */
> + .start = evt2irq(0x17e0),
> + .flags = IORESOURCE_IRQ,
> + },
> +};
> +
> +static struct platform_device hdmi_device = {
> + .name = "sh-mobile-hdmi",
> + .num_resources = ARRAY_SIZE(hdmi_resources),
> + .resource = hdmi_resources,
> + .id = -1,
> + .dev = {
> + .platform_data = &hdmi_info,
> + },
> +};
> +
> static struct sh_mobile_meram_cfg hdmi_meram_cfg = {
> .icb[0] = {
> .marker_icb = 30,
> @@ -445,7 +477,7 @@ static struct sh_mobile_meram_cfg hdmi_meram_cfg = {
> .meram_size = 0x100,
> },
> };
> -/* HDMI */
> +
> static struct sh_mobile_lcdc_info hdmi_lcdc_info = {
> .meram_dev = &mackerel_meram_info,
> .clock_source = LCDC_CLK_EXTERNAL,
> @@ -456,6 +488,7 @@ static struct sh_mobile_lcdc_info hdmi_lcdc_info = {
> .clock_divider = 1,
> .flags = LCDC_FLAGS_DWPOL,
> .meram_cfg = &hdmi_meram_cfg,
> + .tx_dev = &hdmi_device,
> }
> };
>
> @@ -483,35 +516,6 @@ static struct platform_device hdmi_lcdc_device = {
> },
> };
>
> -static struct sh_mobile_hdmi_info hdmi_info = {
> - .lcd_chan = &hdmi_lcdc_info.ch[0],
> - .flags = HDMI_SND_SRC_SPDIF,
> -};
> -
> -static struct resource hdmi_resources[] = {
> - [0] = {
> - .name = "HDMI",
> - .start = 0xe6be0000,
> - .end = 0xe6be00ff,
> - .flags = IORESOURCE_MEM,
> - },
> - [1] = {
> - /* There's also an HDMI interrupt on INTCS @ 0x18e0 */
> - .start = evt2irq(0x17e0),
> - .flags = IORESOURCE_IRQ,
> - },
> -};
> -
> -static struct platform_device hdmi_device = {
> - .name = "sh-mobile-hdmi",
> - .num_resources = ARRAY_SIZE(hdmi_resources),
> - .resource = hdmi_resources,
> - .id = -1,
> - .dev = {
> - .platform_data = &hdmi_info,
> - },
> -};
> -
> static struct platform_device fsi_hdmi_device = {
> .name = "sh_fsi2_b_hdmi",
> };
> @@ -1313,8 +1317,8 @@ static struct platform_device *mackerel_devices[] __initdata = {
> &sh_mmcif_device,
> &ceu_device,
> &mackerel_camera,
> - &hdmi_lcdc_device,
> &hdmi_device,
> + &hdmi_lcdc_device,
> &meram_device,
> };
>
Sorry, could you elaborate, why this is needed? If this is really
important, then I'd prefer to test this before committing. If OTOH this is
unimportant, and my assumption, that normally it's the platform driver
registration order, that's important, not device registration order, is
correct, then you don't have to swap the order?
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 48/57] fbdev: sh_mobile_meram: Allocate ICBs automatically
From: Laurent Pinchart @ 2011-12-15 11:39 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-49-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Damian,
On Thursday 15 December 2011 03:58:12 Damian Hobson-Garcia wrote:
> Hi Laurent,
>
> On 2011/12/13 23:02, Laurent Pinchart wrote:
> > Instead of manually specifying the ICBs to use in platform data,
> > allocate them automatically at runtime.
> >
> > The MERAM registration function now returns a pointer to an opaque MERAM
> > object, which is passed to the update and unregistration functions.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
>
> So one concern that I have about this is in regards to sharing ICBs with
> user-space drivers. Since there are user space drivers (via UIO) for blocks
> like the VEU, that may want to have access to MERAM, we need a way to
> communicate which ICBs are free to user space. With the hard-coded platform
> data we could easily assume that kernel drivers would use, for example, the
> upper 16 ICBs and so user-space drivers were free to use the lower 16.
You're right, I forgot about that :-/
> One simple temporary workaround might be to provide a range of useable ICBs
> in the platform data. The MERAM memory allocation range can always easily be
> tweaked by using the meram_resources structure.
That sounds good to me. I'll implement that.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 47/57] fbdev: sh_mobile_meram: Use genalloc to manage MERAM allocation
From: Laurent Pinchart @ 2011-12-15 11:37 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-48-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Damian,
Thanks for the review.
On Thursday 15 December 2011 03:43:06 Damian Hobson-Garcia wrote:
> On 2011/12/13 23:02, Laurent Pinchart wrote:
> > @@ -195,43 +196,40 @@ static int meram_check_overlap(struct
> > sh_mobile_meram_priv *priv,
> >
> > test_bit(new->cache_icb, &priv->used_icb))
> >
> > return 1;
> >
> > - for (i = 0; i < MERAM_ICB_NUM; i++) {
> > - if (!test_bit(i, &priv->used_icb))
> > - continue;
> > -
> > - used_start = MERAM_CACHE_START(priv->icbs[i].region);
> > - used_end = MERAM_CACHE_END(priv->icbs[i].region);
> > - meram_start = new->meram_offset;
> > - meram_end = new->meram_offset + new->meram_size;
> > -
> > - if ((meram_start >= used_start && meram_start < used_end) ||
> > - (meram_end > used_start && meram_end < used_end))
> > - return 1;
> > - }
> > -
> >
> > return 0;
> >
> > }
> >
> > -/* Mark the specified ICB as used. */
> > -static void meram_mark(struct sh_mobile_meram_priv *priv,
> > +/* Allocate memory for the ICBs and mark them as used. */
> > +static int meram_alloc(struct sh_mobile_meram_priv *priv,
> >
> > const struct sh_mobile_meram_icb_cfg *new,
> > int pixelformat)
> >
> > {
> >
> > + struct sh_mobile_meram_icb *marker = &priv->icbs[new->marker_icb];
> > + unsigned long mem;
> > +
> > + mem = gen_pool_alloc(priv->pool, new->meram_size * 1024);
> > + if (mem = 0)
> > + return -ENOMEM;
> > +
> >
> > __set_bit(new->marker_icb, &priv->used_icb);
> > __set_bit(new->cache_icb, &priv->used_icb);
> >
> > - priv->icbs[new->marker_icb].region = MERAM_CACHE_SET(new->meram_offset,
> > - new->meram_size);
> > - priv->icbs[new->cache_icb].region = MERAM_CACHE_SET(new->meram_offset,
> > - new->meram_size);
> > - priv->icbs[new->marker_icb].current_reg = 1;
> > - priv->icbs[new->marker_icb].pixelformat = pixelformat;
> > + marker->offset = mem - priv->meram;
>
> I might have missed this somewhere, but it doesn't look like priv->meram
> is assigned anywhere.
Oops. That got lost in a rebase operation :-/ I'll fix it.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 1/2] OMAPDSS: DISPC: Update Fir Coefficients
From: Tomi Valkeinen @ 2011-12-15 8:55 UTC (permalink / raw)
To: Chandrabhanu Mahapatra; +Cc: linux-omap, linux-fbdev
In-Reply-To: <1323838310-4439-1-git-send-email-cmahapatra@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1746 bytes --]
On Wed, 2011-12-14 at 10:21 +0530, Chandrabhanu Mahapatra wrote:
> +const struct dispc_coef *dispc_ovl_get_scale_coef(int inc, int five_taps)
> +{
> + int i;
> + static const struct {
> + int Mmin;
> + int Mmax;
> + const struct dispc_coef *coef_3;
> + const struct dispc_coef *coef_5;
> + } coefs[] = {
> + { 26, 32, coef3_M32, coef5_M32 },
> + { 22, 26, coef3_M26, coef5_M26 },
> + { 19, 22, coef3_M22, coef5_M22 },
> + { 16, 19, coef3_M19, coef5_M19 },
> + { 14, 16, coef3_M16, coef5_M16 },
> + { 13, 14, coef3_M14, coef5_M14 },
> + { 12, 13, coef3_M13, coef5_M13 },
> + { 11, 12, coef3_M12, coef5_M12 },
> + { 10, 11, coef3_M11, coef5_M11 },
> + { 9, 10, coef3_M10, coef5_M10 },
> + { 8, 9, coef3_M9, coef5_M9 },
> + { 3, 8, coef3_M8, coef5_M8 },
> + /*
> + * When upscaling more than two times, blockiness and outlines
> + * around the image are observed when M8 tables are used. M11,
> + * M16 and M19 tables are used to prevent this.
> + */
> + { 2, 3, coef3_M11, coef5_M11 },
> + { 1, 2, coef3_M16, coef5_M16 },
> + };
> +
> + inc /= 128;
> + for (i = 0; i < ARRAY_LEN(coefs); ++i)
> + if (inc > coefs[i].Mmin && inc <= coefs[i].Mmax)
> + return five_taps ? coefs[i].coef_5 : coefs[i].coef_3;
> + if (inc == 1)
> + return five_taps ? coef3_M19 : coef5_M19;
> + return NULL;
> +}
Why don't you handle the inc == 1 case the same as others? Just have an
entry in the table for Mmin=0, Mmax = 1.
Also, I think it's a bit confusing that Mmin is exclusive and Mmax is
inclusive in the comparison. It makes the table a bit hard to read, when
looking at which entry is used for which inc. I'd recommend using
inclusive comparison for both.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] offb: Add palette hack for qemu "standard vga"
From: Benjamin Herrenschmidt @ 2011-12-15 8:09 UTC (permalink / raw)
To: Andreas Färber; +Cc: linux-fbdev, linuxppc-dev, kvm-ppc
In-Reply-To: <4EE9A6A5.2050600@suse.de>
On Thu, 2011-12-15 at 08:49 +0100, Andreas Färber wrote:
> Am 15.12.2011 00:58, schrieb Benjamin Herrenschmidt:
> > We rename the mach64 hack to "simple" since that's also applicable
> > to anything using VGA-style DAC IO ports (set to 8-bit DAC) and we
> > use it for qemu vga.
> >
> > Note that this is keyed on a device-tree "compatible" property that
> > is currently only set by an upcoming version of SLOF when using the
> > qemu "pseries" platform. This is on purpose as other qemu ppc platforms
> > using OpenBIOS aren't properly setting the DAC to 8-bit at the time of
> > the writing of this patch.
> >
> > We can fix OpenBIOS later to do that and add the required property, in
> > which case it will be matched by this change.
>
> Just let me know what's needed for OpenBIOS.
> Is this just for -vga std as opposed to the default cirrus?
Yes. Cirrus isn't the default on mac99 and on pseries (tho I will
eventually add a SLOF driver for it as well).
For OpenBIOS I was thinking about just sending you a patch :-) But if
you have more time than I do, what is needed is:
- Set the 8-bit DAC bit in the VBE enable register when initializing
the card (0x20 off the top of my mind but dbl check). Remove your >> 2
in your palette setting.
- Implement color! so prom_init can set the initial palette (but that's
not strictly necessary).
- I assume that the VGA device already has a device_type of "display",
can be open()'ed from the client interface and will have the necessary
properties to be used by offb (width, height, linebytes, depth, and
address if fits in 32-bit (if not, ignore it, offb will pick the largest
BAR)).
- Stick "qemu,std-vga" into the compatible property of the vga PCI
device.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 2/2] offb: Add palette hack for qemu "standard vga" framebuffer
From: Andreas Färber @ 2011-12-15 7:49 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linux-fbdev, linuxppc-dev, kvm-ppc
In-Reply-To: <1323907109.21839.28.camel@pasglop>
Am 15.12.2011 00:58, schrieb Benjamin Herrenschmidt:
> We rename the mach64 hack to "simple" since that's also applicable
> to anything using VGA-style DAC IO ports (set to 8-bit DAC) and we
> use it for qemu vga.
>
> Note that this is keyed on a device-tree "compatible" property that
> is currently only set by an upcoming version of SLOF when using the
> qemu "pseries" platform. This is on purpose as other qemu ppc platforms
> using OpenBIOS aren't properly setting the DAC to 8-bit at the time of
> the writing of this patch.
>
> We can fix OpenBIOS later to do that and add the required property, in
> which case it will be matched by this change.
Just let me know what's needed for OpenBIOS.
Is this just for -vga std as opposed to the default cirrus?
Cheers,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply
* [PATCH 3/15] i810: fix module_param bool abuse.
From: Rusty Russell @ 2011-12-15 3:15 UTC (permalink / raw)
To: lkml - Kernel Mailing List; +Cc: Antonino Daplas
The driver says "module_param(ddc3, bool, 0);". But it's not a used
as a bool, it's used as a count.
Make it a bool.
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: linux-fbdev@vger.kernel.org
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
drivers/video/i810/i810_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/i810/i810_main.c b/drivers/video/i810/i810_main.c
--- a/drivers/video/i810/i810_main.c
+++ b/drivers/video/i810/i810_main.c
@@ -147,7 +147,7 @@ static int vyres __devinitdata;
static int sync __devinitdata;
static int extvga __devinitdata;
static int dcolor __devinitdata;
-static int ddc3 __devinitdata = 2;
+static bool ddc3 __devinitdata;
/*------------------------------------------------------------*/
@@ -1776,7 +1776,7 @@ static void __devinit i810_init_defaults
if (sync)
par->dev_flags |= ALWAYS_SYNC;
- par->ddc_num = ddc3;
+ par->ddc_num = (ddc3 ? 3 : 2);
if (bpp < 8)
bpp = 8;
@@ -1999,7 +1999,7 @@ static int __devinit i810fb_setup(char *
else if (!strncmp(this_opt, "dcolor", 6))
dcolor = 1;
else if (!strncmp(this_opt, "ddc3", 4))
- ddc3 = 3;
+ ddc3 = true;
else
mode_option = this_opt;
}
^ permalink raw reply
* Re: [PATCH 48/57] fbdev: sh_mobile_meram: Allocate ICBs automatically
From: Damian Hobson-Garcia @ 2011-12-15 2:58 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-49-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Laurent,
On 2011/12/13 23:02, Laurent Pinchart wrote:
> Instead of manually specifying the ICBs to use in platform data,
> allocate them automatically at runtime.
>
> The MERAM registration function now returns a pointer to an opaque MERAM
> object, which is passed to the update and unregistration functions.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
So one concern that I have about this is in regards to sharing ICBs with
user-space drivers. Since there are user space drivers (via UIO) for
blocks like the VEU, that may want to have access to MERAM, we need
a way to communicate which ICBs are free to user space. With the
hard-coded platform data we could easily assume that kernel drivers
would use, for example, the upper 16 ICBs and so user-space drivers were
free to use the lower 16.
One simple temporary workaround might be to provide a range of useable
ICBs in the platform data. The MERAM memory allocation range can always
easily be tweaked by using the meram_resources structure.
Thanks,
Damian
^ permalink raw reply
* Re: [PATCH 47/57] fbdev: sh_mobile_meram: Use genalloc to manage
From: Damian Hobson-Garcia @ 2011-12-15 2:43 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-48-git-send-email-laurent.pinchart@ideasonboard.com>
On 2011/12/13 23:02, Laurent Pinchart wrote:
> @@ -195,43 +196,40 @@ static int meram_check_overlap(struct sh_mobile_meram_priv *priv,
> test_bit(new->cache_icb, &priv->used_icb))
> return 1;
>
> - for (i = 0; i < MERAM_ICB_NUM; i++) {
> - if (!test_bit(i, &priv->used_icb))
> - continue;
> -
> - used_start = MERAM_CACHE_START(priv->icbs[i].region);
> - used_end = MERAM_CACHE_END(priv->icbs[i].region);
> - meram_start = new->meram_offset;
> - meram_end = new->meram_offset + new->meram_size;
> -
> - if ((meram_start >= used_start && meram_start < used_end) ||
> - (meram_end > used_start && meram_end < used_end))
> - return 1;
> - }
> -
> return 0;
> }
>
> -/* Mark the specified ICB as used. */
> -static void meram_mark(struct sh_mobile_meram_priv *priv,
> +/* Allocate memory for the ICBs and mark them as used. */
> +static int meram_alloc(struct sh_mobile_meram_priv *priv,
> const struct sh_mobile_meram_icb_cfg *new,
> int pixelformat)
> {
> + struct sh_mobile_meram_icb *marker = &priv->icbs[new->marker_icb];
> + unsigned long mem;
> +
> + mem = gen_pool_alloc(priv->pool, new->meram_size * 1024);
> + if (mem = 0)
> + return -ENOMEM;
> +
> __set_bit(new->marker_icb, &priv->used_icb);
> __set_bit(new->cache_icb, &priv->used_icb);
>
> - priv->icbs[new->marker_icb].region = MERAM_CACHE_SET(new->meram_offset,
> - new->meram_size);
> - priv->icbs[new->cache_icb].region = MERAM_CACHE_SET(new->meram_offset,
> - new->meram_size);
> - priv->icbs[new->marker_icb].current_reg = 1;
> - priv->icbs[new->marker_icb].pixelformat = pixelformat;
> + marker->offset = mem - priv->meram;
I might have missed this somewhere, but it doesn't look like priv->meram
is assigned anywhere.
Damian
--
Damian Hobson-Garcia
IGEL Co.,Ltd
http://www.igel.co.jp
^ permalink raw reply
* [PATCH 2/2] offb: Add palette hack for qemu "standard vga"
From: Benjamin Herrenschmidt @ 2011-12-14 23:58 UTC (permalink / raw)
To: linux-fbdev; +Cc: linuxppc-dev, kvm-ppc
We rename the mach64 hack to "simple" since that's also applicable
to anything using VGA-style DAC IO ports (set to 8-bit DAC) and we
use it for qemu vga.
Note that this is keyed on a device-tree "compatible" property that
is currently only set by an upcoming version of SLOF when using the
qemu "pseries" platform. This is on purpose as other qemu ppc platforms
using OpenBIOS aren't properly setting the DAC to 8-bit at the time of
the writing of this patch.
We can fix OpenBIOS later to do that and add the required property, in
which case it will be matched by this change.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/video/offb.c | 19 +++++++++++++++----
1 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/video/offb.c b/drivers/video/offb.c
index 915acae..da7cf79 100644
--- a/drivers/video/offb.c
+++ b/drivers/video/offb.c
@@ -41,13 +41,14 @@
/* Supported palette hacks */
enum {
cmap_unknown,
- cmap_m64, /* ATI Mach64 */
+ cmap_simple, /* ATI Mach64 */
cmap_r128, /* ATI Rage128 */
cmap_M3A, /* ATI Rage Mobility M3 Head A */
cmap_M3B, /* ATI Rage Mobility M3 Head B */
cmap_radeon, /* ATI Radeon */
cmap_gxt2000, /* IBM GXT2000 */
cmap_avivo, /* ATI R5xx */
+ cmap_qemu, /* qemu vga */
};
struct offb_par {
@@ -138,7 +139,7 @@ static int offb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,
return 0;
switch (par->cmap_type) {
- case cmap_m64:
+ case cmap_simple:
writeb(regno, par->cmap_adr);
writeb(red, par->cmap_data);
writeb(green, par->cmap_data);
@@ -208,7 +209,7 @@ static int offb_blank(int blank, struct fb_info *info)
if (blank)
for (i = 0; i < 256; i++) {
switch (par->cmap_type) {
- case cmap_m64:
+ case cmap_simple:
writeb(i, par->cmap_adr);
for (j = 0; j < 3; j++)
writeb(0, par->cmap_data);
@@ -350,7 +351,7 @@ static void offb_init_palette_hacks(struct fb_info *info, struct device_node *dp
par->cmap_adr ioremap(base + 0x7ff000, 0x1000) + 0xcc0;
par->cmap_data = par->cmap_adr + 1;
- par->cmap_type = cmap_m64;
+ par->cmap_type = cmap_simple;
} else if (dp && (of_device_is_compatible(dp, "pci1014,b7") ||
of_device_is_compatible(dp, "pci1014,21c"))) {
par->cmap_adr = offb_map_reg(dp, 0, 0x6000, 0x1000);
@@ -371,6 +372,16 @@ static void offb_init_palette_hacks(struct fb_info *info, struct device_node *dp
par->cmap_type = cmap_avivo;
}
of_node_put(pciparent);
+ } else if (dp && of_device_is_compatible(dp, "qemu,std-vga")) {
+ const u32 io_of_addr[3] = { 0x01000000, 0x0, 0x0 };
+ u64 io_addr = of_translate_address(dp, io_of_addr);
+ if (io_addr != OF_BAD_ADDR) {
+ par->cmap_adr = ioremap(io_addr + 0x3c8, 2);
+ if (par->cmap_adr) {
+ par->cmap_type = cmap_simple;
+ par->cmap_data = par->cmap_adr + 1;
+ }
+ }
}
info->fix.visual = (par->cmap_type != cmap_unknown) ?
FB_VISUAL_PSEUDOCOLOR : FB_VISUAL_STATIC_PSEUDOCOLOR;
^ permalink raw reply related
* [PATCH 1/2] offb: Fix bug in calculating requested vram size
From: Benjamin Herrenschmidt @ 2011-12-14 23:58 UTC (permalink / raw)
To: linux-fbdev; +Cc: linuxppc-dev, kvm-ppc
From 448820776363da565f221c020f4ccb3c610faec3 Mon Sep 17 00:00:00 2001
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Wed, 14 Dec 2011 16:52:02 +1100
Subject:
We used to try to request 8 times more vram than needed, which would
fail if the card has a too small BAR (observed with qemu).
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
drivers/video/offb.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
(I'm happy to carry that in the powerpc tree)
diff --git a/drivers/video/offb.c b/drivers/video/offb.c
index cb163a5..915acae 100644
--- a/drivers/video/offb.c
+++ b/drivers/video/offb.c
@@ -381,7 +381,7 @@ static void __init offb_init_fb(const char *name, const char *full_name,
int pitch, unsigned long address,
int foreign_endian, struct device_node *dp)
{
- unsigned long res_size = pitch * height * (depth + 7) / 8;
+ unsigned long res_size = pitch * height;
struct offb_par *par = &default_par;
unsigned long res_start = address;
struct fb_fix_screeninfo *fix;
^ permalink raw reply related
* Re: [PATCH 02/57] fbdev: sh_mobile_lcdc: Mark init-only symbols with __devinit(const)
From: Laurent Pinchart @ 2011-12-14 12:30 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-3-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Guennadi,
On Wednesday 14 December 2011 12:07:26 Guennadi Liakhovetski wrote:
> On Wed, 14 Dec 2011, Laurent Pinchart wrote:
> > On Tuesday 13 December 2011 23:23:32 Guennadi Liakhovetski wrote:
> > > On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > > > default_720p and sh_mobile_lcdc_check_interface are used at device
> > > > initialization time only. Mark them as __devinitconst and __devinit
> > > > respectively.
> > > >
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > ---
> > > >
> > > > drivers/video/sh_mobile_lcdcfb.c | 5 +++--
> > > > 1 files changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/video/sh_mobile_lcdcfb.c
> > > > b/drivers/video/sh_mobile_lcdcfb.c index 8b18360..a6bf4fb 100644
> > > > --- a/drivers/video/sh_mobile_lcdcfb.c
> > > > +++ b/drivers/video/sh_mobile_lcdcfb.c
> > > > @@ -1459,7 +1459,7 @@ static int sh_mobile_lcdc_notify(struct
> > > > notifier_block *nb,
> > > >
> > > > * Probe/remove and driver init/exit
> > > > */
> > > >
> > > > -static const struct fb_videomode default_720p = {
> > > > +static const struct fb_videomode default_720p __devinitconst = {
> > > >
> > > > .name = "HDMI 720p",
> > > > .xres = 1280,
> > > > .yres = 720,
> > > >
> > > > @@ -1528,7 +1528,8 @@ static int sh_mobile_lcdc_remove(struct
> > > > platform_device *pdev)
> > > >
> > > > return 0;
> > > >
> > > > }
> > > >
> > > > -static int sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan
> > > > *ch) +static int __devinit
> > > > +sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan *ch)
> > >
> > > Personally, I don't like this type of line splitting very much, I
> > > prefer the grep output to show the function type and all attributes,
> > > but this, certainly, would not be the reason to change this specific
> > > hunk:-) But this might be: the whole file so far has all function
> > > definitions at least up to and including the first parameter on one
> > > line, so, I find, this change would introduce an inconsistency in the
> > > file style. The line would __only__ be 83 characters long if you put
> > > it all on one line. I think, it would look better then:-)
> >
> > It's a matter of limits, as usual... I find 80 to be a nice limit,
> > although in some cases I'm fine with exceeding it (one example is a long
> > list of #defines with a small comment describing each macro, as in the
> > list of FOURCCs in linux/videodev2.h for instance). For code I try to
> > respect the 80 characters limit. Another reason is that it produces
> > checkpatch.pl warnings, which force me to manually look at all the
> > warnings to check which ones are acceptable.
> >
> > Regarding consistency, other patches in this series use the same style,
> > so that function won't feel alone anymore ;-)
> >
> > I can change this (and other instances of the same coding style) if you
> > insist. BTW, a possible improvement would be to shorten the
> > sh_mobile_lcdc_ prefix. I find it too long, and I was thinking about
> > reducing it to shm_lcdc_ (although shm reffers to shared memory, so that
> > might not be the best idea).
>
> How about sh_lcdc?
Sounds good to me. Does anyone have an objection ?
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 33/57] fbdev: sh_mobile_lcdc: Add sh_mobile_format_info() function
From: Laurent Pinchart @ 2011-12-14 11:30 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-34-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Damian,
On Wednesday 14 December 2011 04:04:52 Damian Hobson-Garcia wrote:
> On 2011/12/13 23:02, Laurent Pinchart wrote:
> > --- a/drivers/video/sh_mobile_lcdcfb.c
> > +++ b/drivers/video/sh_mobile_lcdcfb.c
> > @@ -447,6 +447,75 @@ static int sh_mobile_lcdc_display_notify(struct
> > sh_mobile_lcdc_chan *ch,
> >
> > * Format helpers
> > */
> >
> > +struct sh_mobile_lcdc_format_info {
> > + u32 fourcc;
> > + unsigned int bpp;
> > + bool yuv;
> > + u32 lddfr;
> > +};
> > +
> > +static const struct sh_mobile_lcdc_format_info sh_mobile_format_infos[]
> > = { + {
> > + .fourcc = V4L2_PIX_FMT_RGB565,
> > + .bpp = 12,
>
> I think that this should be 16 instead of 12.
Oops, you're right. Thanks. I'll fix that.
> > @@ -665,37 +726,20 @@ static void __sh_mobile_lcdc_start(struct
> > sh_mobile_lcdc_priv *priv)
> >
> > /* Setup geometry, format, frame buffer memory and operation mode. */
> > for (k = 0; k < ARRAY_SIZE(priv->ch); k++) {
> >
> > + const struct sh_mobile_lcdc_format_info *format;
> > + u32 fourcc;
> > +
> >
> > ch = &priv->ch[k];
> > if (!ch->enabled)
> >
> > continue;
> >
> > sh_mobile_lcdc_geometry(ch);
> >
> > - switch (sh_mobile_format_fourcc(&ch->info->var)) {
> > - case V4L2_PIX_FMT_RGB565:
> > - tmp = LDDFR_PKF_RGB16;
> > - break;
> > - case V4L2_PIX_FMT_BGR24:
> > - tmp = LDDFR_PKF_RGB24;
> > - break;
> > - case V4L2_PIX_FMT_BGR32:
> > - tmp = LDDFR_PKF_ARGB32;
> > - break;
> > - case V4L2_PIX_FMT_NV12:
> > - case V4L2_PIX_FMT_NV21:
> > - tmp = LDDFR_CC | LDDFR_YF_420;
> > - break;
> > - case V4L2_PIX_FMT_NV16:
> > - case V4L2_PIX_FMT_NV61:
> > - tmp = LDDFR_CC | LDDFR_YF_422;
> > - break;
> > - case V4L2_PIX_FMT_NV24:
> > - case V4L2_PIX_FMT_NV42:
> > - tmp = LDDFR_CC | LDDFR_YF_444;
> > - break;
> > - }
> > + fourcc = sh_mobile_format_fourcc(&ch->info->var);
> > + format = sh_mobile_format_info(fourcc);
> > + tmp = format->lddfr;
>
> Do you need to check if format is NULL here?
The fourcc is validated in both sh_mobile_lcdc_channel_init() (for the value
passed to the driver through platform data) and sh_mobile_check_var() (for the
value requested by userspace). format should thus never be NULL here.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 11/57] fbdev: sh_mobile_lcdc: Handle HDMI/MIPI transmitter device directly
From: Laurent Pinchart @ 2011-12-14 11:10 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-12-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Guennadi,
On Wednesday 14 December 2011 01:03:06 Guennadi Liakhovetski wrote:
> On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > Pass a pointer to the transmitter device through platform data, retrieve
> > the corresponding sh_mobile_lcdc_entity structure in the probe method
> > and call the transmitter display_on/off methods directly.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >
> > drivers/video/sh_mobile_lcdcfb.c | 33
> > ++++++++++++++++++++++++++++----- drivers/video/sh_mobile_lcdcfb.h |
> > 2 ++
> > include/video/sh_mobile_lcdc.h | 2 ++
> > 3 files changed, 32 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/video/sh_mobile_lcdcfb.c
> > b/drivers/video/sh_mobile_lcdcfb.c index afa1fac..f1bbae6 100644
> > --- a/drivers/video/sh_mobile_lcdcfb.c
> > +++ b/drivers/video/sh_mobile_lcdcfb.c
> > @@ -338,6 +338,13 @@ static void sh_mobile_lcdc_deferred_io_touch(struct
> > fb_info *info)
> >
> > static void sh_mobile_lcdc_display_on(struct sh_mobile_lcdc_chan *ch)
> > {
> >
> > struct sh_mobile_lcdc_board_cfg *board_cfg = &ch->cfg.board_cfg;
> >
> > + int ret;
> > +
> > + if (ch->tx_dev) {
> > + ret = ch->tx_dev->ops->display_on(ch->tx_dev, ch->info);
>
> ->ops or ->display_o{n,ff}() cannot be NULL?
They must be provided, at least with the transmitters we currently have. This
code will likely get reworked to support things like HDMI-on-DSI, so I don't
think adding a pre
> also
>
> + int ret = ch->tx_dev->ops->display_on(ch->tx_dev, ch->info);
>
> would suffice;-)
Or even if (ch->tx_dev->ops->display_on(ch->tx_dev, ch->info) < 0)
I'll fix that.
> > + if (ret < 0)
> > + return;
> > + }
> >
> > /* HDMI must be enabled before LCDC configuration */
> > if (board_cfg->display_on && try_module_get(board_cfg->owner)) {
> >
> > @@ -354,6 +361,9 @@ static void sh_mobile_lcdc_display_off(struct
> > sh_mobile_lcdc_chan *ch)
> >
> > board_cfg->display_off(board_cfg->board_data);
> > module_put(board_cfg->owner);
> >
> > }
> >
> > +
> > + if (ch->tx_dev)
> > + ch->tx_dev->ops->display_off(ch->tx_dev);
> >
> > }
> >
> > /*
> > -----------------------------------------------------------------------
> > ------
> >
> > @@ -1490,18 +1500,21 @@ static int sh_mobile_lcdc_remove(struct
> > platform_device *pdev)
> >
> > sh_mobile_lcdc_stop(priv);
> >
> > for (i = 0; i < ARRAY_SIZE(priv->ch); i++) {
> >
> > - info = priv->ch[i].info;
> > + struct sh_mobile_lcdc_chan *ch = &priv->ch[i];
> >
> > + info = ch->info;
> >
> > if (!info || !info->device)
> >
> > continue;
> >
> > - if (priv->ch[i].sglist)
> > - vfree(priv->ch[i].sglist);
> > + if (ch->tx_dev)
> > + module_put(ch->cfg.tx_dev->dev.driver->owner);
>
> This is now the same ->owner, as the one taken in your
> sh_mobile_lcdc_display_o{n,ff}() functions? IIUC, you're now adding this
> new ->owner field to later (17/57) remove the original one?
It's the same owner, yes. The idea is to get a reference to the transmitter
device module at probe() time and release it at remove() time instead of doing
so in display_on() and display_off(), and to remove the owner field hack from
platform data.
> > +
> > + if (ch->sglist)
> > + vfree(ch->sglist);
> >
> > if (info->screen_base)
> >
> > dma_free_coherent(&pdev->dev, info->fix.smem_len,
> >
> > - info->screen_base,
> > - priv->ch[i].dma_handle);
> > + info->screen_base, ch->dma_handle);
> >
> > fb_dealloc_cmap(&info->cmap);
> > framebuffer_release(info);
> >
> > }
> >
> > @@ -1596,6 +1609,16 @@ sh_mobile_lcdc_channel_init(struct
> > sh_mobile_lcdc_priv *priv,
> >
> > info->pseudo_palette = &ch->pseudo_palette;
> > info->flags = FBINFO_FLAG_DEFAULT;
> >
> > + if (cfg->tx_dev) {
> > + if (!cfg->tx_dev->dev.driver ||
> > + !try_module_get(cfg->tx_dev->dev.driver->owner)) {
> > + dev_warn(priv->dev, "unable to get transmitter "
> > + "device\n");
>
> Grrr... Pleeeease, don't split strings. If you really have to program on
> vt220;-) at least do
>
> + dev_warn(priv->dev,
> + "unable to get transmitter device\n");
Sorry about that. Will fix.
> > + return -EINVAL;
> > + }
> > + ch->tx_dev = platform_get_drvdata(cfg->tx_dev);
> > + }
> > +
> >
> > /* Iterate through the modes to validate them and find the highest
> >
> > * resolution.
> > */
> >
> > diff --git a/drivers/video/sh_mobile_lcdcfb.h
> > b/drivers/video/sh_mobile_lcdcfb.h index d79e5aa..9601b92 100644
> > --- a/drivers/video/sh_mobile_lcdcfb.h
> > +++ b/drivers/video/sh_mobile_lcdcfb.h
> > @@ -41,6 +41,8 @@ struct sh_mobile_lcdc_entity {
> >
> > */
> >
> > struct sh_mobile_lcdc_chan {
> >
> > struct sh_mobile_lcdc_priv *lcdc;
> >
> > + struct sh_mobile_lcdc_entity *tx_dev;
> > +
> >
> > unsigned long *reg_offs;
> > unsigned long ldmt1r_value;
> > unsigned long enabled; /* ME and SE in LDCNT2R */
> >
> > diff --git a/include/video/sh_mobile_lcdc.h
> > b/include/video/sh_mobile_lcdc.h index fe30b75..0ec59e1 100644
> > --- a/include/video/sh_mobile_lcdc.h
> > +++ b/include/video/sh_mobile_lcdc.h
> > @@ -186,6 +186,8 @@ struct sh_mobile_lcdc_chan_cfg {
> >
> > struct sh_mobile_lcdc_bl_info bl_info;
> > struct sh_mobile_lcdc_sys_bus_cfg sys_bus_cfg; /* only for SYSn I/F */
> > struct sh_mobile_meram_cfg *meram_cfg;
> >
> > +
> > + struct platform_device *tx_dev; /* HDMI/MIPI transmitter device */
>
> "MIPI" is too generic, IMHO
Will HDMI/DSI do ?
> Hm, could we, maybe, have different names for sh_mobile_lcdc_chan::tx_dev
> and sh_mobile_lcdc_chan_cfg::tx_dev?;-)
Sure. Do you have any suggestion ? :-)
> > };
> >
> > struct sh_mobile_lcdc_info {
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 02/57] fbdev: sh_mobile_lcdc: Mark init-only symbols with
From: Guennadi Liakhovetski @ 2011-12-14 11:07 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-3-git-send-email-laurent.pinchart@ideasonboard.com>
On Wed, 14 Dec 2011, Laurent Pinchart wrote:
> Hi Guennadi,
>
> On Tuesday 13 December 2011 23:23:32 Guennadi Liakhovetski wrote:
> > On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > > default_720p and sh_mobile_lcdc_check_interface are used at device
> > > initialization time only. Mark them as __devinitconst and __devinit
> > > respectively.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > >
> > > drivers/video/sh_mobile_lcdcfb.c | 5 +++--
> > > 1 files changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/video/sh_mobile_lcdcfb.c
> > > b/drivers/video/sh_mobile_lcdcfb.c index 8b18360..a6bf4fb 100644
> > > --- a/drivers/video/sh_mobile_lcdcfb.c
> > > +++ b/drivers/video/sh_mobile_lcdcfb.c
> > > @@ -1459,7 +1459,7 @@ static int sh_mobile_lcdc_notify(struct
> > > notifier_block *nb,
> > >
> > > * Probe/remove and driver init/exit
> > > */
> > >
> > > -static const struct fb_videomode default_720p = {
> > > +static const struct fb_videomode default_720p __devinitconst = {
> > >
> > > .name = "HDMI 720p",
> > > .xres = 1280,
> > > .yres = 720,
> > >
> > > @@ -1528,7 +1528,8 @@ static int sh_mobile_lcdc_remove(struct
> > > platform_device *pdev)
> > >
> > > return 0;
> > >
> > > }
> > >
> > > -static int sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan
> > > *ch) +static int __devinit
> > > +sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan *ch)
> >
> > Personally, I don't like this type of line splitting very much, I prefer
> > the grep output to show the function type and all attributes, but this,
> > certainly, would not be the reason to change this specific hunk:-) But
> > this might be: the whole file so far has all function definitions at least
> > up to and including the first parameter on one line, so, I find, this
> > change would introduce an inconsistency in the file style. The line would
> > __only__ be 83 characters long if you put it all on one line. I think, it
> > would look better then:-)
>
> It's a matter of limits, as usual... I find 80 to be a nice limit, although in
> some cases I'm fine with exceeding it (one example is a long list of #defines
> with a small comment describing each macro, as in the list of FOURCCs in
> linux/videodev2.h for instance). For code I try to respect the 80 characters
> limit. Another reason is that it produces checkpatch.pl warnings, which force
> me to manually look at all the warnings to check which ones are acceptable.
>
> Regarding consistency, other patches in this series use the same style, so
> that function won't feel alone anymore ;-)
>
> I can change this (and other instances of the same coding style) if you
> insist. BTW, a possible improvement would be to shorten the sh_mobile_lcdc_
> prefix. I find it too long, and I was thinking about reducing it to shm_lcdc_
> (although shm reffers to shared memory, so that might not be the best idea).
How about sh_lcdc?
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: [PATCH 06/57] fbdev: sh_mobile_hdmi: Don't access LCDC channel in notifier callback
From: Laurent Pinchart @ 2011-12-14 10:47 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-7-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Guennadi,
On Wednesday 14 December 2011 00:02:11 Guennadi Liakhovetski wrote:
> On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > Instead of relying on info->par being a pointer to an LCDC channel, cast
> > the notifier block pointer to an sh_hdmi pointer.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >
> > drivers/video/sh_mobile_hdmi.c | 6 +++---
> > 1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/video/sh_mobile_hdmi.c
> > b/drivers/video/sh_mobile_hdmi.c index 647ba98..b1e90f5 100644
> > --- a/drivers/video/sh_mobile_hdmi.c
> > +++ b/drivers/video/sh_mobile_hdmi.c
> > @@ -225,6 +225,8 @@ struct sh_hdmi {
> >
> > struct notifier_block notifier;
> >
> > };
> >
> > +#define notifier_to_hdmi(n) container_of(n, struct sh_hdmi, notifier)
> > +
> >
> > static void hdmi_write(struct sh_hdmi *hdmi, u8 data, u8 reg)
> > {
> >
> > iowrite8(data, hdmi->base + reg);
> >
> > @@ -1204,9 +1206,7 @@ static int sh_hdmi_notify(struct notifier_block
> > *nb,
> >
> > {
> >
> > struct fb_event *event = data;
> > struct fb_info *info = event->info;
> >
> > - struct sh_mobile_lcdc_chan *ch = info->par;
> > - struct sh_mobile_lcdc_board_cfg *board_cfg = &ch->cfg.board_cfg;
> > - struct sh_hdmi *hdmi = board_cfg->board_data;
> > + struct sh_hdmi *hdmi = notifier_to_hdmi(nb);
> >
> > if (!hdmi || nb != &hdmi->notifier || hdmi->info != info)
>
> Then you also can drop the first two of the three checks above. If I'm not
> mistaken, in a HDMI / LCD set up, if this notifier is called for the LCD
> panel, hdmi will wtill point to the HDMI-related sh_hdmi object, whose
> ->info will then mismatch the info pointer, derived from the notifier
> "data" pointer.
That's correct. The first two checks don't hurt, but will always be false.
I'll remove them.
> > return NOTIFY_DONE;
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 06/57] fbdev: sh_mobile_hdmi: Don't access LCDC channel in notifier callback
From: Laurent Pinchart @ 2011-12-14 10:41 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-7-git-send-email-laurent.pinchart@ideasonboard.com>
On Wednesday 14 December 2011 00:02:11 Guennadi Liakhovetski wrote:
> On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > Instead of relying on info->par being a pointer to an LCDC channel, cast
> > the notifier block pointer to an sh_hdmi pointer.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >
> > drivers/video/sh_mobile_hdmi.c | 6 +++---
> > 1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/video/sh_mobile_hdmi.c
> > b/drivers/video/sh_mobile_hdmi.c index 647ba98..b1e90f5 100644
> > --- a/drivers/video/sh_mobile_hdmi.c
> > +++ b/drivers/video/sh_mobile_hdmi.c
> > @@ -225,6 +225,8 @@ struct sh_hdmi {
> >
> > struct notifier_block notifier;
> >
> > };
> >
> > +#define notifier_to_hdmi(n) container_of(n, struct sh_hdmi, notifier)
> > +
> >
> > static void hdmi_write(struct sh_hdmi *hdmi, u8 data, u8 reg)
> > {
> >
> > iowrite8(data, hdmi->base + reg);
> >
> > @@ -1204,9 +1206,7 @@ static int sh_hdmi_notify(struct notifier_block
> > *nb,
> >
> > {
> >
> > struct fb_event *event = data;
> > struct fb_info *info = event->info;
> >
> > - struct sh_mobile_lcdc_chan *ch = info->par;
> > - struct sh_mobile_lcdc_board_cfg *board_cfg = &ch->cfg.board_cfg;
> > - struct sh_hdmi *hdmi = board_cfg->board_data;
> > + struct sh_hdmi *hdmi = notifier_to_hdmi(nb);
> >
> > if (!hdmi || nb != &hdmi->notifier || hdmi->info != info)
>
> Then you also can drop the first two of the three checks above. If I'm not
> mistaken, in a HDMI / LCD set up, if this notifier is called for the LCD
> panel, hdmi will wtill point to the HDMI-related sh_hdmi object, whose
> ->info will then mismatch the info pointer, derived from the notifier
> "data" pointer.
>
> > return NOTIFY_DONE;
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 02/57] fbdev: sh_mobile_lcdc: Mark init-only symbols with __devinit(const)
From: Laurent Pinchart @ 2011-12-14 10:40 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1323784972-24205-3-git-send-email-laurent.pinchart@ideasonboard.com>
Hi Guennadi,
On Tuesday 13 December 2011 23:23:32 Guennadi Liakhovetski wrote:
> On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > default_720p and sh_mobile_lcdc_check_interface are used at device
> > initialization time only. Mark them as __devinitconst and __devinit
> > respectively.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >
> > drivers/video/sh_mobile_lcdcfb.c | 5 +++--
> > 1 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/video/sh_mobile_lcdcfb.c
> > b/drivers/video/sh_mobile_lcdcfb.c index 8b18360..a6bf4fb 100644
> > --- a/drivers/video/sh_mobile_lcdcfb.c
> > +++ b/drivers/video/sh_mobile_lcdcfb.c
> > @@ -1459,7 +1459,7 @@ static int sh_mobile_lcdc_notify(struct
> > notifier_block *nb,
> >
> > * Probe/remove and driver init/exit
> > */
> >
> > -static const struct fb_videomode default_720p = {
> > +static const struct fb_videomode default_720p __devinitconst = {
> >
> > .name = "HDMI 720p",
> > .xres = 1280,
> > .yres = 720,
> >
> > @@ -1528,7 +1528,8 @@ static int sh_mobile_lcdc_remove(struct
> > platform_device *pdev)
> >
> > return 0;
> >
> > }
> >
> > -static int sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan
> > *ch) +static int __devinit
> > +sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan *ch)
>
> Personally, I don't like this type of line splitting very much, I prefer
> the grep output to show the function type and all attributes, but this,
> certainly, would not be the reason to change this specific hunk:-) But
> this might be: the whole file so far has all function definitions at least
> up to and including the first parameter on one line, so, I find, this
> change would introduce an inconsistency in the file style. The line would
> __only__ be 83 characters long if you put it all on one line. I think, it
> would look better then:-)
It's a matter of limits, as usual... I find 80 to be a nice limit, although in
some cases I'm fine with exceeding it (one example is a long list of #defines
with a small comment describing each macro, as in the list of FOURCCs in
linux/videodev2.h for instance). For code I try to respect the 80 characters
limit. Another reason is that it produces checkpatch.pl warnings, which force
me to manually look at all the warnings to check which ones are acceptable.
Regarding consistency, other patches in this series use the same style, so
that function won't feel alone anymore ;-)
I can change this (and other instances of the same coding style) if you
insist. BTW, a possible improvement would be to shorten the sh_mobile_lcdc_
prefix. I find it too long, and I was thinking about reducing it to shm_lcdc_
(although shm reffers to shared memory, so that might not be the best idea).
> > {
> >
> > int interface_type = ch->cfg.interface_type;
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH 2/2] OMAPDSS: DISPC: Update Scaling Clock Logic
From: Chandrabhanu Mahapatra @ 2011-12-14 4:53 UTC (permalink / raw)
To: =tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
Clock requirements for scaling in OMAP2, OMAP3 and OMAP4 are different. In
OMAP2 and OMAP3 the required clock rate is a function of pixel clock, vertical
downscale ratio and horizontal downscale ratio whereas in OMAP4 it is a
function of pixel clock and horizontal downscale ratio only. Selection of 3-tap
vs 5-tap coefficients depends on clock rate line buffer width in OMAP3 whereas
in OMAP4 it is independent of clock rate and line buffer width. In OMAP2 3-tap
for vertical and 5-tap for horizontal scaling is used. In OMAP4 5-tap is used
both for horizontal and vertical scaling for better performance. Also, the
number and width of line buffers differs in OMAP3 and OMAP4.
So, clock functions have been fined tuned for OMAP3 and support has been added
added for OMAP4. This code has been tested on OMAP2, OMAP3 and OMAP4, and
scaling issues due to clock errors have been resolved.
Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
drivers/video/omap2/dss/dispc.c | 66 ++++++++++++++++++++++----------
drivers/video/omap2/dss/dss_features.c | 7 +++
drivers/video/omap2/dss/dss_features.h | 1 +
3 files changed, 54 insertions(+), 20 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index b981983..6c3feb4 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -1554,6 +1554,9 @@ static unsigned long calc_fclk_five_taps(enum omap_channel channel, u16 width,
u32 fclk = 0;
u64 tmp, pclk = dispc_mgr_pclk_rate(channel);
+ if (height <= out_height && width <= out_width)
+ return (unsigned long) pclk;
+
if (height > out_height) {
struct omap_dss_device *dssdev = dispc_mgr_get_device(channel);
unsigned int ppl = dssdev->panel.timings.x_res;
@@ -1608,7 +1611,16 @@ static unsigned long calc_fclk(enum omap_channel channel, u16 width,
else
vf = 1;
- return dispc_mgr_pclk_rate(channel) * vf * hf;
+ if (cpu_is_omap24xx()) {
+ if (vf > 1 && hf > 1)
+ return dispc_mgr_pclk_rate(channel) * 4;
+ else
+ return dispc_mgr_pclk_rate(channel) * 2;
+ } else if (cpu_is_omap34xx()) {
+ return dispc_mgr_pclk_rate(channel) * vf * hf;
+ } else {
+ return dispc_mgr_pclk_rate(channel) * hf;
+ }
}
static int dispc_ovl_calc_scaling(enum omap_plane plane,
@@ -1618,6 +1630,8 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
{
struct omap_overlay *ovl = omap_dss_get_overlay(plane);
const int maxdownscale = dss_feat_get_param_max(FEAT_PARAM_DOWNSCALE);
+ const int maxsinglelinewidth + dss_feat_get_param_max(FEAT_PARAM_LINEWIDTH);
unsigned long fclk = 0;
if ((ovl->caps & OMAP_DSS_OVL_CAP_SCALE) = 0) {
@@ -1635,28 +1649,40 @@ static int dispc_ovl_calc_scaling(enum omap_plane plane,
out_height > height * 8)
return -EINVAL;
- /* Must use 5-tap filter? */
- *five_taps = height > out_height * 2;
-
- if (!*five_taps) {
+ if (cpu_is_omap24xx()) {
+ if (width > maxsinglelinewidth)
+ DSSERR("Cannot scale max input width exceeded");
+ *five_taps = false;
+ fclk = calc_fclk(channel, width, height, out_width,
+ out_height);
+ } else if (cpu_is_omap34xx()) {
+ if (width > (maxsinglelinewidth * 2)) {
+ DSSERR("Cannot setup scaling");
+ DSSERR("width exceeds maximum width possible");
+ return -EINVAL;
+ }
+ fclk = calc_fclk_five_taps(channel, width, height, out_width,
+ out_height, color_mode);
+ if (width > maxsinglelinewidth) {
+ if (height > out_height && height < out_height * 2)
+ *five_taps = false;
+ else {
+ DSSERR("cannot setup scaling with five taps");
+ return -EINVAL;
+ }
+ }
+ if (!*five_taps)
+ fclk = calc_fclk(channel, width, height, out_width,
+ out_height);
+ } else {
+ if (width > maxsinglelinewidth) {
+ DSSERR("Cannot scale width exceeds max line width");
+ return -EINVAL;
+ }
fclk = calc_fclk(channel, width, height, out_width,
out_height);
-
- /* Try 5-tap filter if 3-tap fclk is too high */
- if (cpu_is_omap34xx() && height > out_height &&
- fclk > dispc_fclk_rate())
- *five_taps = true;
}
- if (width > (2048 >> *five_taps)) {
- DSSERR("failed to set up scaling, fclk too low\n");
- return -EINVAL;
- }
-
- if (*five_taps)
- fclk = calc_fclk_five_taps(channel, width, height,
- out_width, out_height, color_mode);
-
DSSDBG("required fclk rate = %lu Hz\n", fclk);
DSSDBG("current fclk rate = %lu Hz\n", dispc_fclk_rate());
@@ -1676,7 +1702,7 @@ int dispc_ovl_setup(enum omap_plane plane, struct omap_overlay_info *oi,
u32 fifo_low, u32 fifo_high)
{
struct omap_overlay *ovl = omap_dss_get_overlay(plane);
- bool five_taps = false;
+ bool five_taps = true;
bool fieldmode = 0;
int r, cconv = 0;
unsigned offset0, offset1;
diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
index b402699..5e4b829 100644
--- a/drivers/video/omap2/dss/dss_features.c
+++ b/drivers/video/omap2/dss/dss_features.c
@@ -304,6 +304,11 @@ static const struct dss_param_range omap2_dss_param_range[] = {
[FEAT_PARAM_DSIPLL_FINT] = { 0, 0 },
[FEAT_PARAM_DSIPLL_LPDIV] = { 0, 0 },
[FEAT_PARAM_DOWNSCALE] = { 1, 2 },
+ /*
+ * Assuming the line width buffer to be 768 pixels as OMAP2 DISPC
+ * scaler cannot scale a image with width more than 768.
+ */
+ [FEAT_PARAM_LINEWIDTH] = { 1, 768 },
};
static const struct dss_param_range omap3_dss_param_range[] = {
@@ -316,6 +321,7 @@ static const struct dss_param_range omap3_dss_param_range[] = {
[FEAT_PARAM_DSIPLL_FINT] = { 750000, 2100000 },
[FEAT_PARAM_DSIPLL_LPDIV] = { 1, (1 << 13) - 1},
[FEAT_PARAM_DOWNSCALE] = { 1, 4 },
+ [FEAT_PARAM_LINEWIDTH] = { 1, 1024 },
};
static const struct dss_param_range omap4_dss_param_range[] = {
@@ -328,6 +334,7 @@ static const struct dss_param_range omap4_dss_param_range[] = {
[FEAT_PARAM_DSIPLL_FINT] = { 500000, 2500000 },
[FEAT_PARAM_DSIPLL_LPDIV] = { 0, (1 << 13) - 1 },
[FEAT_PARAM_DOWNSCALE] = { 1, 4 },
+ [FEAT_PARAM_LINEWIDTH] = { 1, 2048 },
};
/* OMAP2 DSS Features */
diff --git a/drivers/video/omap2/dss/dss_features.h b/drivers/video/omap2/dss/dss_features.h
index 6a6c05d..cd833bb 100644
--- a/drivers/video/omap2/dss/dss_features.h
+++ b/drivers/video/omap2/dss/dss_features.h
@@ -86,6 +86,7 @@ enum dss_range_param {
FEAT_PARAM_DSIPLL_FINT,
FEAT_PARAM_DSIPLL_LPDIV,
FEAT_PARAM_DOWNSCALE,
+ FEAT_PARAM_LINEWIDTH,
};
/* DSS Feature Functions */
--
1.7.1
^ 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