* [PATCH v2 2/3] video: xilinxfb: Do not use out_be32 IO function
From: Michal Simek @ 2013-05-30 9:41 UTC (permalink / raw)
To: linux-kernel
Cc: Michal Simek, Michal Simek, Florian Tobias Schandinat,
linux-fbdev
In-Reply-To: <27e53d5633766ab0dc1ac492023d5a0d0199e3aa.1369906849.git.michal.simek@xilinx.com>
[-- Attachment #1: Type: text/plain, Size: 3215 bytes --]
out_be32 IO function is not supported by ARM.
It is only available for PPC and Microblaze.
Remove all out_be32 references and start to use __raw_writel
function.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes in v2: None
drivers/video/xilinxfb.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index aecd15d..000185a 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -57,7 +57,7 @@
* In case of direct PLB access the second control register will be at
* an offset of 4 as compared to the DCR access where the offset is 1
* i.e. REG_CTRL. So this is taken care in the function
- * xilinx_fb_out_be32 where it left shifts the offset 2 times in case of
+ * xilinx_fb_out32 where it left shifts the offset 2 times in case of
* direct PLB access.
*/
#define NUM_REGS 2
@@ -150,11 +150,11 @@ struct xilinxfb_drvdata {
* To perform the read/write on the registers we need to check on
* which bus its connected and call the appropriate write API.
*/
-static void xilinx_fb_out_be32(struct xilinxfb_drvdata *drvdata, u32 offset,
+static void xilinx_fb_out32(struct xilinxfb_drvdata *drvdata, u32 offset,
u32 val)
{
if (drvdata->flags & PLB_ACCESS_FLAG)
- out_be32(drvdata->regs + (offset << 2), val);
+ __raw_writel(val, drvdata->regs + (offset << 2));
#ifdef CONFIG_PPC_DCR
else
dcr_write(drvdata->dcr_host, offset, val);
@@ -197,7 +197,7 @@ xilinx_fb_blank(int blank_mode, struct fb_info *fbi)
switch (blank_mode) {
case FB_BLANK_UNBLANK:
/* turn on panel */
- xilinx_fb_out_be32(drvdata, REG_CTRL, drvdata->reg_ctrl_default);
+ xilinx_fb_out32(drvdata, REG_CTRL, drvdata->reg_ctrl_default);
break;
case FB_BLANK_NORMAL:
@@ -205,7 +205,7 @@ xilinx_fb_blank(int blank_mode, struct fb_info *fbi)
case FB_BLANK_HSYNC_SUSPEND:
case FB_BLANK_POWERDOWN:
/* turn off panel */
- xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+ xilinx_fb_out32(drvdata, REG_CTRL, 0);
default:
break;
@@ -280,13 +280,13 @@ static int xilinxfb_assign(struct device *dev,
memset_io((void __iomem *)drvdata->fb_virt, 0, fbsize);
/* Tell the hardware where the frame buffer is */
- xilinx_fb_out_be32(drvdata, REG_FB_ADDR, drvdata->fb_phys);
+ xilinx_fb_out32(drvdata, REG_FB_ADDR, drvdata->fb_phys);
/* Turn on the display */
drvdata->reg_ctrl_default = REG_CTRL_ENABLE;
if (pdata->rotate_screen)
drvdata->reg_ctrl_default |= REG_CTRL_ROTATE;
- xilinx_fb_out_be32(drvdata, REG_CTRL,
+ xilinx_fb_out32(drvdata, REG_CTRL,
drvdata->reg_ctrl_default);
/* Fill struct fb_info */
@@ -345,7 +345,7 @@ err_cmap:
iounmap(drvdata->fb_virt);
/* Turn off the display */
- xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+ xilinx_fb_out32(drvdata, REG_CTRL, 0);
err_fbmem:
if (drvdata->flags & PLB_ACCESS_FLAG)
@@ -381,7 +381,7 @@ static int xilinxfb_release(struct device *dev)
iounmap(drvdata->fb_virt);
/* Turn off the display */
- xilinx_fb_out_be32(drvdata, REG_CTRL, 0);
+ xilinx_fb_out32(drvdata, REG_CTRL, 0);
/* Release the resources, as allocated based on interface */
if (drvdata->flags & PLB_ACCESS_FLAG) {
--
1.8.2.3
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply related
* [PATCH v2 3/3] video: xilinxfb: Use driver for Xilinx ARM Zynq
From: Michal Simek @ 2013-05-30 9:41 UTC (permalink / raw)
To: linux-kernel
Cc: Michal Simek, Michal Simek, Florian Tobias Schandinat,
linux-fbdev
In-Reply-To: <27e53d5633766ab0dc1ac492023d5a0d0199e3aa.1369906849.git.michal.simek@xilinx.com>
[-- Attachment #1: Type: text/plain, Size: 697 bytes --]
From: Michal Simek <monstr@monstr.eu>
Enable this driver for all Xilinx platforms.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
Changes in v2: None
drivers/video/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 2e937bd..2c301f8 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -2188,7 +2188,7 @@ config FB_PS3_DEFAULT_SIZE_M
config FB_XILINX
tristate "Xilinx frame buffer support"
- depends on FB && (XILINX_VIRTEX || MICROBLAZE)
+ depends on FB && (XILINX_VIRTEX || MICROBLAZE || ARCH_ZYNQ)
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
--
1.8.2.3
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply related
* Re: [PATCH] fbdev: bfin-lq035q1-fb: Use dev_pm_ops
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-30 10:45 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Jingoo Han, 'Michael Hennerich',
'Rafael J. Wysocki', 'Tomi Valkeinen',
linux-fbdev, linux-pm
In-Reply-To: <51A70532.3070502@metafoo.de>
On 09:52 Thu 30 May , Lars-Peter Clausen wrote:
> On 05/30/2013 09:14 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 09:32 Thu 30 May , Jingoo Han wrote:
> >> On Thursday, May 30, 2013 4:20 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>> On 16:24 Wed 29 May , Michael Hennerich wrote:
> >>>> On 05/29/2013 02:17 PM, Lars-Peter Clausen wrote:
> >>>>> Use dev_pm_ops instead of the legacy suspend/resume callbacks.
> >>>>>
> >>>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> >>>> Acked-by: Michael Hennerich <michael.hennerich@analog.com>
> >>>>> ---
> >>>>> drivers/video/bfin-lq035q1-fb.c | 22 ++++++++++++++--------
> >>>>> 1 file changed, 14 insertions(+), 8 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/video/bfin-lq035q1-fb.c b/drivers/video/bfin-lq035q1-fb.c
> >>>>> index 29d8c04..6084c17 100644
> >>>>> --- a/drivers/video/bfin-lq035q1-fb.c
> >>>>> +++ b/drivers/video/bfin-lq035q1-fb.c
> >>>>> @@ -170,16 +170,19 @@ static int lq035q1_spidev_remove(struct spi_device *spi)
> >>>>> return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_SHUT);
> >>>>> }
> >>>>> -#ifdef CONFIG_PM
> >>>>> -static int lq035q1_spidev_suspend(struct spi_device *spi, pm_message_t state)
> >>>>> +#ifdef CONFIG_PM_SLEEP
> >>>>> +static int lq035q1_spidev_suspend(struct device *dev)
> >>>>> {
> >>>>> + struct spi_device *spi = to_spi_device(dev);
> >>>>> +
> >>>>> return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_SHUT);
> >>>>> }
> >>>>> -static int lq035q1_spidev_resume(struct spi_device *spi)
> >>>>> +static int lq035q1_spidev_resume(struct device *dev)
> >>>>> {
> >>>>> - int ret;
> >>>>> + struct spi_device *spi = to_spi_device(dev);
> >>>>> struct spi_control *ctl = spi_get_drvdata(spi);
> >>>>> + int ret;
> >>>>> ret = lq035q1_control(spi, LQ035_DRIVER_OUTPUT_CTL, ctl->mode);
> >>>>> if (ret)
> >>>>> @@ -187,9 +190,13 @@ static int lq035q1_spidev_resume(struct spi_device *spi)
> >>>>> return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_ON);
> >>>>> }
> >>>>> +
> >>>>> +static SIMPLE_DEV_PM_OPS(lq035q1_spidev_pm_ops, lq035q1_spidev_suspend,
> >>>>> + lq035q1_spidev_resume);
> >>>>> +#define LQ035Q1_SPIDEV_PM_OPS (&lq035q1_spidev_pm_ops)
> >>>>> +
> >>>>> #else
> >>>>> -# define lq035q1_spidev_suspend NULL
> >>>>> -# define lq035q1_spidev_resume NULL
> >>>>> +#define LQ035Q1_SPIDEV_PM_OPS NULL
> >>>>> #endif
> >>> we really need to ahve a macro like for DT of_match_ptr to drop the #else
> >>
> >> Hi Jean-Christophe PLAGNIOL-VILLARD,
> >>
> >> I submitted the following patch. :)
> >> (https://patchwork.kernel.org/patch/2502971/)
> >>
> >> --- a/include/linux/pm.h
> >> +++ b/include/linux/pm.h
> >> @@ -55,8 +55,10 @@ struct device;
> >>
> >> #ifdef CONFIG_PM
> >> extern const char power_group_name[]; /* = "power" */
> >> +#define pm_ops_ptr(_ptr) (_ptr)
> >> #else
> >> #define power_group_name NULL
> >> +#define pm_ops_ptr(_ptr) NULL
> >> #endif
> >>
> >>
> >> This patch was accepted by Rafael Wysocki, and will be merged to v3.11-rc1.
> >>
> > Lars-Peter please update with and
>
> Since the code depends on CONFIG_PM_SLEEP and not CONFIG_PM I don't think
> the macro will work.
se please ad a similar macros too
Best Regards,
J.
>
> - Lars
>
^ permalink raw reply
* Re: [PATCH] backlight: Turn backlight on/off when necessary
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-30 11:03 UTC (permalink / raw)
To: Liu Ying; +Cc: FlorianSchandinat, linux-fbdev, linux-kernel, liu.y.victor
In-Reply-To: <1369901633-31561-1-git-send-email-Ying.Liu@freescale.com>
On 16:13 Thu 30 May , Liu Ying wrote:
> We don't have to turn backlight on/off everytime a blanking
> or unblanking event comes because the backlight status may have
> already been what we want. Another thought is that one backlight
> device may be shared by multiple framebuffers. We don't hope that
> blanking one of the framebuffers would turn the backlight off for
> all the other framebuffers, since they are likely active to show
> display content. This patch adds logic to record each framebuffer's
> backlight status to determine the backlight device use count and
> whether the backlight should be turned on or off.
>
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> ---
> drivers/video/backlight/backlight.c | 23 +++++++++++++++++------
> include/linux/backlight.h | 6 ++++++
> 2 files changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index c74e7aa..97ea2b8 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -31,13 +31,14 @@ static const char *const backlight_types[] = {
> defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
> /* This callback gets called when something important happens inside a
> * framebuffer driver. We're looking if that important event is blanking,
> - * and if it is, we're switching backlight power as well ...
> + * and if it is and necessary, we're switching backlight power as well ...
> */
> static int fb_notifier_callback(struct notifier_block *self,
> unsigned long event, void *data)
> {
> struct backlight_device *bd;
> struct fb_event *evdata = data;
> + int node = evdata->info->node;
>
> /* If we aren't interested in this event, skip it immediately ... */
> if (event != FB_EVENT_BLANK && event != FB_EVENT_CONBLANK)
> @@ -49,11 +50,21 @@ static int fb_notifier_callback(struct notifier_block *self,
> if (!bd->ops->check_fb ||
> bd->ops->check_fb(bd, evdata->info)) {
> bd->props.fb_blank = *(int *)evdata->data;
> - if (bd->props.fb_blank = FB_BLANK_UNBLANK)
> - bd->props.state &= ~BL_CORE_FBBLANK;
> - else
> - bd->props.state |= BL_CORE_FBBLANK;
> - backlight_update_status(bd);
> + if (bd->props.fb_blank = FB_BLANK_UNBLANK &&
> + !bd->fb_bl_on[node]) {
> + bd->fb_bl_on[node] = true;
> + if (!bd->use_count++) {
> + bd->props.state &= ~BL_CORE_FBBLANK;
> + backlight_update_status(bd);
> + }
> + } else if (bd->props.fb_blank != FB_BLANK_UNBLANK &&
> + bd->fb_bl_on[node]) {
> + bd->fb_bl_on[node] = false;
> + if (!(--bd->use_count)) {
> + bd->props.state |= BL_CORE_FBBLANK;
> + backlight_update_status(bd);
> + }
> + }
> }
> mutex_unlock(&bd->ops_lock);
> return 0;
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index da9a082..5de71a0 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -9,6 +9,7 @@
> #define _LINUX_BACKLIGHT_H
>
> #include <linux/device.h>
> +#include <linux/fb.h>
> #include <linux/mutex.h>
> #include <linux/notifier.h>
>
> @@ -101,6 +102,11 @@ struct backlight_device {
> struct notifier_block fb_notif;
>
> struct device dev;
> +
> + /* Multiple framebuffers may share one backlight device */
> + bool fb_bl_on[FB_MAX];
I don't like such array at all
I understand the fact you will have only on hw backlight for x fb or overlay
but have a static on no
if you want to track all user create a strcut and register it or do more
simple just as a int to count the number of user and shut down it if 0 and
enable it otherwise
Best Regards,
J.
> +
> + int use_count;
> };
>
> static inline void backlight_update_status(struct backlight_device *bd)
> --
> 1.7.1
>
>
> --
> 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
* Re: [PATCH 03/32] OMAPDSS: add omap_dss_find_output()
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-30 11:07 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1369906493-27538-4-git-send-email-tomi.valkeinen@ti.com>
On 12:34 Thu 30 May , Tomi Valkeinen wrote:
> Add a support function to find a DSS output by given name. This is used
> in later patches to link the panels to DSS outputs.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/output.c | 13 +++++++++++++
> include/video/omapdss.h | 1 +
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/video/omap2/dss/output.c b/drivers/video/omap2/dss/output.c
> index 5214df6..3274628 100644
> --- a/drivers/video/omap2/dss/output.c
> +++ b/drivers/video/omap2/dss/output.c
> @@ -115,6 +115,19 @@ struct omap_dss_output *omap_dss_get_output(enum omap_dss_output_id id)
> }
> EXPORT_SYMBOL(omap_dss_get_output);
GPL please
>
> +struct omap_dss_output *omap_dss_find_output(const char *name)
> +{
> + struct omap_dss_output *out;
> +
> + list_for_each_entry(out, &output_list, list) {
> + if (strcmp(out->name, name) = 0)
> + return out;
> + }
> +
> + return NULL;
> +}
I this in so many drivers could we have a macro to generate such function?
> +EXPORT_SYMBOL(omap_dss_find_output);
ditto GPL
Best Regards,
J.
> +
> static const struct dss_mgr_ops *dss_mgr_ops;
>
> int dss_install_mgr_ops(const struct dss_mgr_ops *mgr_ops)
> diff --git a/include/video/omapdss.h b/include/video/omapdss.h
> index d5f1fff..aad47a2 100644
> --- a/include/video/omapdss.h
> +++ b/include/video/omapdss.h
> @@ -779,6 +779,7 @@ int omap_dss_get_num_overlays(void);
> struct omap_overlay *omap_dss_get_overlay(int num);
>
> struct omap_dss_output *omap_dss_get_output(enum omap_dss_output_id id);
> +struct omap_dss_output *omap_dss_find_output(const char *name);
> int omapdss_output_set_device(struct omap_dss_output *out,
> struct omap_dss_device *dssdev);
> int omapdss_output_unset_device(struct omap_dss_output *out);
> --
> 1.8.1.2
>
> --
> 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
* Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-30 11:09 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1369906493-27538-6-git-send-email-tomi.valkeinen@ti.com>
On 12:34 Thu 30 May , Tomi Valkeinen wrote:
> When using DT, dss device does not have platform data. However,
> dss_get_ctx_loss_count() uses dss device's platform data to find the
> get_ctx_loss_count function pointer.
>
> To fix this, dss_get_ctx_loss_count() needs to be changed to get the
> platform data from the omapdss device, which is a "virtual" device and
> always has platform data.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/dss.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
> index 94f66f9..bd01608 100644
> --- a/drivers/video/omap2/dss/dss.c
> +++ b/drivers/video/omap2/dss/dss.c
> @@ -157,7 +157,8 @@ static void dss_restore_context(void)
>
> int dss_get_ctx_loss_count(void)
> {
> - struct omap_dss_board_info *board_data = dss.pdev->dev.platform_data;
> + struct platform_device *core_pdev = dss_get_core_pdev();
> + struct omap_dss_board_info *board_data = core_pdev->dev.platform_data;
how about store the pdata in the drivers internal struct and if !dt
you ust do this
dss_dev->pdata = *pdev->dev.platform_data;
to copy it and we do not alloc it for dt
Best Regards,
J.
> int cnt;
>
> if (!board_data->get_context_loss_count)
> --
> 1.8.1.2
>
> --
> 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
* Re: [PATCH 06/32] OMAPDSS: DPI: fix regulators for DT
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-30 11:12 UTC (permalink / raw)
To: Tomi Valkeinen, Grant Likely, Arnd Bergmann, Linus Walleij,
gregkh
Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1369906493-27538-7-git-send-email-tomi.valkeinen@ti.com>
On 12:34 Thu 30 May , Tomi Valkeinen wrote:
> On some platforms DPI requires a regulator to be enabled to power up the
> output pins. This regulator is, for some reason, currently attached to
> the virtual omapdss device, instead of the DPI device. This does not
> work for DT, as the regulator mappings need to be described in the DT
> data, and the virtual omapdss device is not present there.
>
> Fix the issue by acquiring the regulator in the DPI device. To retain
> compatibility with the current board files, the old method of getting
> the regulator is kept. The old method can be removed when the board
> files have been changed to pass the regulator to DPI.
as discuss with Arnd we should handle regular enable and disable at device
probe for every device as we do for pinctrl
Best Regards,
J.
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/dpi.c | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
> index ef8fca2..43f6b26 100644
> --- a/drivers/video/omap2/dss/dpi.c
> +++ b/drivers/video/omap2/dss/dpi.c
> @@ -37,7 +37,10 @@
> #include "dss_features.h"
>
> static struct {
> + struct platform_device *pdev;
> +
> struct regulator *vdds_dsi_reg;
> + bool vdds_dsi_from_core;
> struct platform_device *dsidev;
>
> struct mutex lock;
> @@ -583,10 +586,15 @@ static int dpi_init_display(struct omap_dss_device *dssdev)
> struct regulator *vdds_dsi;
>
> vdds_dsi = dss_get_vdds_dsi();
> -
> if (IS_ERR(vdds_dsi)) {
> - DSSERR("can't get VDDS_DSI regulator\n");
> - return PTR_ERR(vdds_dsi);
> + vdds_dsi = regulator_get(&dpi.pdev->dev, "vdds_dsi");
> + if (IS_ERR(vdds_dsi)) {
> + DSSERR("can't get VDDS_DSI regulator\n");
> + return PTR_ERR(vdds_dsi);
> + }
> + dpi.vdds_dsi_from_core = false;
> + } else {
> + dpi.vdds_dsi_from_core = true;
> }
>
> dpi.vdds_dsi_reg = vdds_dsi;
> @@ -702,6 +710,8 @@ static int omap_dpi_probe(struct platform_device *pdev)
> {
> int r;
>
> + dpi.pdev = pdev;
> +
> mutex_init(&dpi.lock);
>
> dpi_init_output(pdev);
> @@ -725,6 +735,11 @@ static int __exit omap_dpi_remove(struct platform_device *pdev)
>
> dpi_uninit_output(pdev);
>
> + if (dpi.vdds_dsi_reg != NULL && dpi.vdds_dsi_from_core = false) {
> + regulator_put(dpi.vdds_dsi_reg);
> + dpi.vdds_dsi_reg = NULL;
> + }
> +
> return 0;
> }
>
> --
> 1.8.1.2
>
> --
> 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
* Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT
From: Tomi Valkeinen @ 2013-05-30 11:28 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <20130530110945.GI19468@game.jcrosoft.org>
[-- Attachment #1: Type: text/plain, Size: 2260 bytes --]
On 30/05/13 14:09, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 12:34 Thu 30 May , Tomi Valkeinen wrote:
>> When using DT, dss device does not have platform data. However,
>> dss_get_ctx_loss_count() uses dss device's platform data to find the
>> get_ctx_loss_count function pointer.
>>
>> To fix this, dss_get_ctx_loss_count() needs to be changed to get the
>> platform data from the omapdss device, which is a "virtual" device and
>> always has platform data.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>> drivers/video/omap2/dss/dss.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
>> index 94f66f9..bd01608 100644
>> --- a/drivers/video/omap2/dss/dss.c
>> +++ b/drivers/video/omap2/dss/dss.c
>> @@ -157,7 +157,8 @@ static void dss_restore_context(void)
>>
>> int dss_get_ctx_loss_count(void)
>> {
>> - struct omap_dss_board_info *board_data = dss.pdev->dev.platform_data;
>> + struct platform_device *core_pdev = dss_get_core_pdev();
>> + struct omap_dss_board_info *board_data = core_pdev->dev.platform_data;
>
> how about store the pdata in the drivers internal struct and if !dt
> you ust do this
>
> dss_dev->pdata = *pdev->dev.platform_data;
>
> to copy it and we do not alloc it for dt
It's not quite that simple. We need some OMAP arch functions (like
get_ctx_loss_count here) that are not currently exposed via any other
method to drivers except passing a function pointer with platform data.
We need that also when booting with DT.
We have a bunch of devices for the display subsystem hardware blocks,
like the "dss" here. When booting with DT, these blocks are represented
in the DT data, and do not have platform data.
We also have a "virtual" device, "omapdss", which doesn't match any hw
block. It's created in the arch setup stage. It's really a legacy thing,
but with DT it can be used conveniently to pass the platform data.
The problem this patch fixes is that we used to pass the arch functions
for each of those HW block drivers. But with DT, we need to get the arch
functions from the "omapdss" device, gotten with dss_get_core_pdev().
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH 06/32] OMAPDSS: DPI: fix regulators for DT
From: Tomi Valkeinen @ 2013-05-30 11:35 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD
Cc: Grant Likely, Arnd Bergmann, Linus Walleij, gregkh, linux-fbdev,
linux-omap, Archit Taneja
In-Reply-To: <20130530111253.GJ19468@game.jcrosoft.org>
[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]
On 30/05/13 14:12, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 12:34 Thu 30 May , Tomi Valkeinen wrote:
>> On some platforms DPI requires a regulator to be enabled to power up the
>> output pins. This regulator is, for some reason, currently attached to
>> the virtual omapdss device, instead of the DPI device. This does not
>> work for DT, as the regulator mappings need to be described in the DT
>> data, and the virtual omapdss device is not present there.
>>
>> Fix the issue by acquiring the regulator in the DPI device. To retain
>> compatibility with the current board files, the old method of getting
>> the regulator is kept. The old method can be removed when the board
>> files have been changed to pass the regulator to DPI.
>
> as discuss with Arnd we should handle regular enable and disable at device
> probe for every device as we do for pinctrl
I'm not sure what you mean. Enable of what? The regulator? Why would we
enable it in the device's probe, as the device may never even be used?
Or do you mean regulator_get()? Doing that in DPI driver's probe is
problematic, as not all boards have any panels using DPI, and thus they
may not have the regulator set up, but all boards have the DPI device
added automatically. So for those boards the DPI probe would fail,
always, if we tried to get the regulator.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH 03/32] OMAPDSS: add omap_dss_find_output()
From: Tomi Valkeinen @ 2013-05-30 11:40 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <20130530110707.GH19468@game.jcrosoft.org>
[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]
On 30/05/13 14:07, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 12:34 Thu 30 May , Tomi Valkeinen wrote:
>> Add a support function to find a DSS output by given name. This is used
>> in later patches to link the panels to DSS outputs.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>> drivers/video/omap2/dss/output.c | 13 +++++++++++++
>> include/video/omapdss.h | 1 +
>> 2 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/video/omap2/dss/output.c b/drivers/video/omap2/dss/output.c
>> index 5214df6..3274628 100644
>> --- a/drivers/video/omap2/dss/output.c
>> +++ b/drivers/video/omap2/dss/output.c
>> @@ -115,6 +115,19 @@ struct omap_dss_output *omap_dss_get_output(enum omap_dss_output_id id)
>> }
>> EXPORT_SYMBOL(omap_dss_get_output);
> GPL please
The omapdss driver uses EXPORT_SYMBOL. I don't want to start mixing both
EXPORT_SYMBOLs and EXPORT_SYMBOL_GPLs.
>> +struct omap_dss_output *omap_dss_find_output(const char *name)
>> +{
>> + struct omap_dss_output *out;
>> +
>> + list_for_each_entry(out, &output_list, list) {
>> + if (strcmp(out->name, name) == 0)
>> + return out;
>> + }
>> +
>> + return NULL;
>> +}
> I this in so many drivers could we have a macro to generate such function?
What would that help? Wouldn't it just increase the code size of the kernel?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH 06/32] OMAPDSS: DPI: fix regulators for DT
From: Arnd Bergmann @ 2013-05-30 13:05 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Jean-Christophe PLAGNIOL-VILLARD, Grant Likely, Linus Walleij,
gregkh, linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <51A73990.5040709@ti.com>
On Thursday 30 May 2013, Tomi Valkeinen wrote:
> On 30/05/13 14:12, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 12:34 Thu 30 May , Tomi Valkeinen wrote:
> >> On some platforms DPI requires a regulator to be enabled to power up the
> >> output pins. This regulator is, for some reason, currently attached to
> >> the virtual omapdss device, instead of the DPI device. This does not
> >> work for DT, as the regulator mappings need to be described in the DT
> >> data, and the virtual omapdss device is not present there.
> >>
> >> Fix the issue by acquiring the regulator in the DPI device. To retain
> >> compatibility with the current board files, the old method of getting
> >> the regulator is kept. The old method can be removed when the board
> >> files have been changed to pass the regulator to DPI.
> >
> > as discuss with Arnd we should handle regular enable and disable at device
> > probe for every device as we do for pinctrl
>
> I'm not sure what you mean. Enable of what? The regulator? Why would we
> enable it in the device's probe, as the device may never even be used?
It's an idea I had a while ago, but not yet discussed in the open.
Jean-Christophe just posted patches to move the mapping of interrupt numbers
into platform_drv_probe(), just before calling the driver ->probe() callback,
and we already have similar code to set up the default pinctrl state of
a device before calling probe().
This can be extended to further subsystems, but that has to be done
carefully to avoid regressions. Ideally we would move a lot of boilerplate
code out of the driver specific ->probe() function into common code.
Possible candidates for this include:
* calling devm_request_mem_region for the "reg" property
* calling devm_ioremap on the "reg" property"
* calling devm_gpio_request for all gpio lines
* calling devm_regulator_get on all regulators
* calling devm_reset_control_get on all reset lines
* calling devm_dma_request_slave_channel on all dma channels
* calling devm_of_pwm_get for all pwm channels
* ...
For most of these (maybe all), I think we need some form of opt-in
model on the driver side because there are cases where aquiring some
of these resources is not mandatory, and it only works if the driver
is using DT probing.
IF we want to do this, it also needs a lot of thought, and we shouldn't
do it carelessly. We might also need some extra infrastructure in revres
to simplify access to the resources we got from the OF node.
The irq resources are particularly trivial because we already claim
them in of_platform_populate, so moving that to platform_drv_probe()
is straightforward and solves existing bugs without creating a huge
regression potential, but it's harder for the others.
Arnd
^ permalink raw reply
* Re: [PATCH 1/8] video: atmel_lcdfb: fix platform data struct
From: Hans-Christian Egtvedt @ 2013-05-30 14:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130530072328.GE19468@game.jcrosoft.org>
Around Thu 30 May 2013 09:23:28 +0200 or thereabout, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 08:39 Thu 30 May , Richard Genoud wrote:
>> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
>> > On 19:44 Wed 29 May , Richard Genoud wrote:
>> >> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
>> >> > On 16:36 Wed 29 May , Richard Genoud wrote:
>> >> >> 2013/4/11 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
<snipp diff>
>> >> >> > + bool lcdcon_pol_negative;
>> >> >> I think lcdcon_pol_negative should be part of pdata, because it really
>> >> >> depends on how the PWM is wired on the board.
>> >> >>
>> >> >
>> >> > maybe but no one mainline use it on any pdata for non-dt boars
>> >> > so I did not want to expose it
>> >> Well, at least, I'm using it :)
>> >> (and I guess that Andreas is using it also, otherwise he wouldn't have
>> >> introduce it !)
>> >
>> > yes but pdata is for non-dt boards, for dt you can keep it in struct
>> > atmel_lcdfb_info and add a property
>> >
>> > if non-dt boards want it my answer is I do not care switch to DT
>>
>> ok (I use a full DT board based on sam9g35)
>>
>> so I'll add something like
>> sinfo->lcdcon_pol_negative = of_property_read_bool(display_np,
>> "atmel,lcdcon-pwm-pulse-low");
>> in /atmel_lcdfb.c
>>
>> But I thought the goal of this patch was to separate driver data from
>> platform specific data, and IMHO, lcdcon_pol_negative is a specificity
>> of the platform.
>
> You are right but as non one mainline use it as pdata I choose to drop it
> and only keep it on the driver as we can still use it for DT
>
> It's a way to force peopoe to switch to DT
FYI AVR32 architecture doesn't support device tree, nor is it likely to do
so. However there are no users of the PWM output from the LCDC block for the
boards in the current kernel, and I doubt new boards will pop.
--
Hans-Christian Egtvedt
^ permalink raw reply
* Re: [PATCH 1/8] video: atmel_lcdfb: fix platform data struct
From: Andreas Bießmann @ 2013-05-30 14:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20130530072328.GE19468@game.jcrosoft.org>
On 30.05.13 09:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 08:39 Thu 30 May , Richard Genoud wrote:
>> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
>>> On 19:44 Wed 29 May , Richard Genoud wrote:
>>>> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
>>>>> On 16:36 Wed 29 May , Richard Genoud wrote:
>>>>>> 2013/4/11 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
>>>>>>> Today we mix pdata and drivers data in the struct atmel_lcdfb_info
>>>>>>> Fix it and introduce a new struct atmel_lcdfb_pdata for platform data only
>>>>>>>
>>>>>>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>>>>>>> Cc: linux-fbdev@vger.kernel.org
>>>>>>> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>>>>>> Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>
>>>>>>> ---
>>>>>>> arch/arm/mach-at91/at91sam9261_devices.c | 6 +-
>>>>>>> arch/arm/mach-at91/at91sam9263_devices.c | 6 +-
>>>>>>> arch/arm/mach-at91/at91sam9g45_devices.c | 6 +-
>>>>>>> arch/arm/mach-at91/at91sam9rl_devices.c | 6 +-
>>>>>>> arch/arm/mach-at91/board-sam9261ek.c | 6 +-
>>>>>>> arch/arm/mach-at91/board-sam9263ek.c | 4 +-
>>>>>>> arch/arm/mach-at91/board-sam9m10g45ek.c | 4 +-
>>>>>>> arch/arm/mach-at91/board-sam9rlek.c | 4 +-
>>>>>>> arch/arm/mach-at91/board.h | 4 +-
>>>>>>> arch/avr32/boards/atngw100/evklcd10x.c | 6 +-
>>>>>>> arch/avr32/boards/atngw100/mrmt.c | 4 +-
>>>>>>> arch/avr32/boards/atstk1000/atstk1000.h | 2 +-
>>>>>>> arch/avr32/boards/atstk1000/setup.c | 2 +-
>>>>>>> arch/avr32/boards/favr-32/setup.c | 2 +-
>>>>>>> arch/avr32/boards/hammerhead/setup.c | 2 +-
>>>>>>> arch/avr32/boards/merisc/display.c | 2 +-
>>>>>>> arch/avr32/boards/mimc200/setup.c | 4 +-
>>>>>>> arch/avr32/mach-at32ap/at32ap700x.c | 8 +--
>>>>>>> arch/avr32/mach-at32ap/include/mach/board.h | 4 +-
>>>>>>> drivers/video/atmel_lcdfb.c | 104 +++++++++++++++++----------
>>>>>>> include/video/atmel_lcdc.h | 24 +------
>>>>>>> 21 files changed, 109 insertions(+), 101 deletions(-)
>>>>>>>
>>>>>> [snip]
>>>>>>> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
>>>>>>> index c1a2914..98733cd4 100644
>>>>>>> --- a/drivers/video/atmel_lcdfb.c
>>>>>>> +++ b/drivers/video/atmel_lcdfb.c
>>>>>>> @@ -20,12 +20,45 @@
>>>>>>> #include <linux/gfp.h>
>>>>>>> #include <linux/module.h>
>>>>>>> #include <linux/platform_data/atmel.h>
>>>>>>> +#include <video/of_display_timing.h>
>>>>>>>
>>>>>>> #include <mach/cpu.h>
>>>>>>> #include <asm/gpio.h>
>>>>>>>
>>>>>>> #include <video/atmel_lcdc.h>
>>>>>>>
>>>>>>> +struct atmel_lcdfb_config {
>>>>>>> + bool have_alt_pixclock;
>>>>>>> + bool have_hozval;
>>>>>>> + bool have_intensity_bit;
>>>>>>> +};
>>>>>>> +
>>>>>>> + /* LCD Controller info data structure, stored in device platform_data */
>>>>>>> +struct atmel_lcdfb_info {
>>>>>>> + spinlock_t lock;
>>>>>>> + struct fb_info *info;
>>>>>>> + void __iomem *mmio;
>>>>>>> + int irq_base;
>>>>>>> + struct work_struct task;
>>>>>>> +
>>>>>>> + unsigned int smem_len;
>>>>>>> + struct platform_device *pdev;
>>>>>>> + struct clk *bus_clk;
>>>>>>> + struct clk *lcdc_clk;
>>>>>>> +
>>>>>>> + struct backlight_device *backlight;
>>>>>>> + u8 bl_power;
>>>>>>> + bool lcdcon_pol_negative;
>>>>>> I think lcdcon_pol_negative should be part of pdata, because it really
>>>>>> depends on how the PWM is wired on the board.
>>>>>>
>>>>>
>>>>> maybe but no one mainline use it on any pdata for non-dt boars
>>>>> so I did not want to expose it
>>>> Well, at least, I'm using it :)
>>>> (and I guess that Andreas is using it also, otherwise he wouldn't have
>>>> introduce it !)
>>>
>>> yes but pdata is for non-dt boards, for dt you can keep it in struct
>>> atmel_lcdfb_info and add a property
>>>
>>> if non-dt boards want it my answer is I do not care switch to DT
>>
>> ok (I use a full DT board based on sam9g35)
>>
>> so I'll add something like
>> sinfo->lcdcon_pol_negative = of_property_read_bool(display_np,
>> "atmel,lcdcon-pwm-pulse-low");
>> in /atmel_lcdfb.c
>>
>> But I thought the goal of this patch was to separate driver data from
>> platform specific data, and IMHO, lcdcon_pol_negative is a specificity
>> of the platform.
>
> You are right but as non one mainline use it as pdata I choose to drop it
> and only keep it on the driver as we can still use it for DT
>
> It's a way to force peopoe to switch to DT
well, we use it on an avr32 board (not mainlined; though no kernel
update planned currently) ... but no way to use DT currently.
Best regards
Andreas Bießmann
^ permalink raw reply
* Re: [PATCH 1/8] video: atmel_lcdfb: fix platform data struct
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-30 14:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <51A761DA.6020109@corscience.de>
On 16:27 Thu 30 May , Andreas Bießmann wrote:
> On 30.05.13 09:23, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 08:39 Thu 30 May , Richard Genoud wrote:
> >> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> >>> On 19:44 Wed 29 May , Richard Genoud wrote:
> >>>> 2013/5/29 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> >>>>> On 16:36 Wed 29 May , Richard Genoud wrote:
> >>>>>> 2013/4/11 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> >>>>>>> Today we mix pdata and drivers data in the struct atmel_lcdfb_info
> >>>>>>> Fix it and introduce a new struct atmel_lcdfb_pdata for platform data only
> >>>>>>>
> >>>>>>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> >>>>>>> Cc: linux-fbdev@vger.kernel.org
> >>>>>>> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> >>>>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
> >>>>>>> Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>
> >>>>>>> ---
> >>>>>>> arch/arm/mach-at91/at91sam9261_devices.c | 6 +-
> >>>>>>> arch/arm/mach-at91/at91sam9263_devices.c | 6 +-
> >>>>>>> arch/arm/mach-at91/at91sam9g45_devices.c | 6 +-
> >>>>>>> arch/arm/mach-at91/at91sam9rl_devices.c | 6 +-
> >>>>>>> arch/arm/mach-at91/board-sam9261ek.c | 6 +-
> >>>>>>> arch/arm/mach-at91/board-sam9263ek.c | 4 +-
> >>>>>>> arch/arm/mach-at91/board-sam9m10g45ek.c | 4 +-
> >>>>>>> arch/arm/mach-at91/board-sam9rlek.c | 4 +-
> >>>>>>> arch/arm/mach-at91/board.h | 4 +-
> >>>>>>> arch/avr32/boards/atngw100/evklcd10x.c | 6 +-
> >>>>>>> arch/avr32/boards/atngw100/mrmt.c | 4 +-
> >>>>>>> arch/avr32/boards/atstk1000/atstk1000.h | 2 +-
> >>>>>>> arch/avr32/boards/atstk1000/setup.c | 2 +-
> >>>>>>> arch/avr32/boards/favr-32/setup.c | 2 +-
> >>>>>>> arch/avr32/boards/hammerhead/setup.c | 2 +-
> >>>>>>> arch/avr32/boards/merisc/display.c | 2 +-
> >>>>>>> arch/avr32/boards/mimc200/setup.c | 4 +-
> >>>>>>> arch/avr32/mach-at32ap/at32ap700x.c | 8 +--
> >>>>>>> arch/avr32/mach-at32ap/include/mach/board.h | 4 +-
> >>>>>>> drivers/video/atmel_lcdfb.c | 104 +++++++++++++++++----------
> >>>>>>> include/video/atmel_lcdc.h | 24 +------
> >>>>>>> 21 files changed, 109 insertions(+), 101 deletions(-)
> >>>>>>>
> >>>>>> [snip]
> >>>>>>> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> >>>>>>> index c1a2914..98733cd4 100644
> >>>>>>> --- a/drivers/video/atmel_lcdfb.c
> >>>>>>> +++ b/drivers/video/atmel_lcdfb.c
> >>>>>>> @@ -20,12 +20,45 @@
> >>>>>>> #include <linux/gfp.h>
> >>>>>>> #include <linux/module.h>
> >>>>>>> #include <linux/platform_data/atmel.h>
> >>>>>>> +#include <video/of_display_timing.h>
> >>>>>>>
> >>>>>>> #include <mach/cpu.h>
> >>>>>>> #include <asm/gpio.h>
> >>>>>>>
> >>>>>>> #include <video/atmel_lcdc.h>
> >>>>>>>
> >>>>>>> +struct atmel_lcdfb_config {
> >>>>>>> + bool have_alt_pixclock;
> >>>>>>> + bool have_hozval;
> >>>>>>> + bool have_intensity_bit;
> >>>>>>> +};
> >>>>>>> +
> >>>>>>> + /* LCD Controller info data structure, stored in device platform_data */
> >>>>>>> +struct atmel_lcdfb_info {
> >>>>>>> + spinlock_t lock;
> >>>>>>> + struct fb_info *info;
> >>>>>>> + void __iomem *mmio;
> >>>>>>> + int irq_base;
> >>>>>>> + struct work_struct task;
> >>>>>>> +
> >>>>>>> + unsigned int smem_len;
> >>>>>>> + struct platform_device *pdev;
> >>>>>>> + struct clk *bus_clk;
> >>>>>>> + struct clk *lcdc_clk;
> >>>>>>> +
> >>>>>>> + struct backlight_device *backlight;
> >>>>>>> + u8 bl_power;
> >>>>>>> + bool lcdcon_pol_negative;
> >>>>>> I think lcdcon_pol_negative should be part of pdata, because it really
> >>>>>> depends on how the PWM is wired on the board.
> >>>>>>
> >>>>>
> >>>>> maybe but no one mainline use it on any pdata for non-dt boars
> >>>>> so I did not want to expose it
> >>>> Well, at least, I'm using it :)
> >>>> (and I guess that Andreas is using it also, otherwise he wouldn't have
> >>>> introduce it !)
> >>>
> >>> yes but pdata is for non-dt boards, for dt you can keep it in struct
> >>> atmel_lcdfb_info and add a property
> >>>
> >>> if non-dt boards want it my answer is I do not care switch to DT
> >>
> >> ok (I use a full DT board based on sam9g35)
> >>
> >> so I'll add something like
> >> sinfo->lcdcon_pol_negative = of_property_read_bool(display_np,
> >> "atmel,lcdcon-pwm-pulse-low");
> >> in /atmel_lcdfb.c
> >>
> >> But I thought the goal of this patch was to separate driver data from
> >> platform specific data, and IMHO, lcdcon_pol_negative is a specificity
> >> of the platform.
> >
> > You are right but as non one mainline use it as pdata I choose to drop it
> > and only keep it on the driver as we can still use it for DT
> >
> > It's a way to force peopoe to switch to DT
>
> well, we use it on an avr32 board (not mainlined; though no kernel
> update planned currently) ... but no way to use DT currently.
If there is 1 ARV32 user it's an other story
Best Regards,
J.
>
> Best regards
>
> Andreas Bießmann
^ permalink raw reply
* Re: [PATCH 03/32] OMAPDSS: add omap_dss_find_output()
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-30 15:40 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <51A73A9A.8010801@ti.com>
On 14:40 Thu 30 May , Tomi Valkeinen wrote:
> On 30/05/13 14:07, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 12:34 Thu 30 May , Tomi Valkeinen wrote:
> >> Add a support function to find a DSS output by given name. This is used
> >> in later patches to link the panels to DSS outputs.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >> ---
> >> drivers/video/omap2/dss/output.c | 13 +++++++++++++
> >> include/video/omapdss.h | 1 +
> >> 2 files changed, 14 insertions(+)
> >>
> >> diff --git a/drivers/video/omap2/dss/output.c b/drivers/video/omap2/dss/output.c
> >> index 5214df6..3274628 100644
> >> --- a/drivers/video/omap2/dss/output.c
> >> +++ b/drivers/video/omap2/dss/output.c
> >> @@ -115,6 +115,19 @@ struct omap_dss_output *omap_dss_get_output(enum omap_dss_output_id id)
> >> }
> >> EXPORT_SYMBOL(omap_dss_get_output);
> > GPL please
>
> The omapdss driver uses EXPORT_SYMBOL. I don't want to start mixing both
> EXPORT_SYMBOLs and EXPORT_SYMBOL_GPLs.
I do not like EXPORT_SYMBOL at all
I stringly prefer to switch all of them to _GPL
but will not refuse the patch for this in this case
>
> >> +struct omap_dss_output *omap_dss_find_output(const char *name)
> >> +{
> >> + struct omap_dss_output *out;
> >> +
> >> + list_for_each_entry(out, &output_list, list) {
> >> + if (strcmp(out->name, name) = 0)
> >> + return out;
> >> + }
> >> +
> >> + return NULL;
> >> +}
> > I this in so many drivers could we have a macro to generate such function?
>
> What would that help? Wouldn't it just increase the code size of the kernel?
increase no as it's not an inline function but a macro to generate the
function
after help yes but people may not like so as you wish
Best Regards,
J.
>
> Tomi
>
>
^ permalink raw reply
* Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-30 15:43 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <51A737DC.1010900@ti.com>
On 14:28 Thu 30 May , Tomi Valkeinen wrote:
> On 30/05/13 14:09, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 12:34 Thu 30 May , Tomi Valkeinen wrote:
> >> When using DT, dss device does not have platform data. However,
> >> dss_get_ctx_loss_count() uses dss device's platform data to find the
> >> get_ctx_loss_count function pointer.
> >>
> >> To fix this, dss_get_ctx_loss_count() needs to be changed to get the
> >> platform data from the omapdss device, which is a "virtual" device and
> >> always has platform data.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >> ---
> >> drivers/video/omap2/dss/dss.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
> >> index 94f66f9..bd01608 100644
> >> --- a/drivers/video/omap2/dss/dss.c
> >> +++ b/drivers/video/omap2/dss/dss.c
> >> @@ -157,7 +157,8 @@ static void dss_restore_context(void)
> >>
> >> int dss_get_ctx_loss_count(void)
> >> {
> >> - struct omap_dss_board_info *board_data = dss.pdev->dev.platform_data;
> >> + struct platform_device *core_pdev = dss_get_core_pdev();
> >> + struct omap_dss_board_info *board_data = core_pdev->dev.platform_data;
> >
> > how about store the pdata in the drivers internal struct and if !dt
> > you ust do this
> >
> > dss_dev->pdata = *pdev->dev.platform_data;
> >
> > to copy it and we do not alloc it for dt
>
> It's not quite that simple. We need some OMAP arch functions (like
> get_ctx_loss_count here) that are not currently exposed via any other
> method to drivers except passing a function pointer with platform data.
> We need that also when booting with DT.
>
> We have a bunch of devices for the display subsystem hardware blocks,
> like the "dss" here. When booting with DT, these blocks are represented
> in the DT data, and do not have platform data.
>
> We also have a "virtual" device, "omapdss", which doesn't match any hw
> block. It's created in the arch setup stage. It's really a legacy thing,
> but with DT it can be used conveniently to pass the platform data.
>
> The problem this patch fixes is that we used to pass the arch functions
> for each of those HW block drivers. But with DT, we need to get the arch
> functions from the "omapdss" device, gotten with dss_get_core_pdev().
ok
do not take it bad is it worth the effort those 54 patches?
is not better to work on DRM?
just an open question
Best Regards,
J.
>
> Tomi
>
>
^ permalink raw reply
* Re: [PATCH 06/32] OMAPDSS: DPI: fix regulators for DT
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-30 15:54 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Tomi Valkeinen, Grant Likely, Linus Walleij, gregkh, linux-fbdev,
linux-omap, Archit Taneja
In-Reply-To: <201305301505.00529.arnd@arndb.de>
On 15:05 Thu 30 May , Arnd Bergmann wrote:
> On Thursday 30 May 2013, Tomi Valkeinen wrote:
> > On 30/05/13 14:12, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > On 12:34 Thu 30 May , Tomi Valkeinen wrote:
> > >> On some platforms DPI requires a regulator to be enabled to power up the
> > >> output pins. This regulator is, for some reason, currently attached to
> > >> the virtual omapdss device, instead of the DPI device. This does not
> > >> work for DT, as the regulator mappings need to be described in the DT
> > >> data, and the virtual omapdss device is not present there.
> > >>
> > >> Fix the issue by acquiring the regulator in the DPI device. To retain
> > >> compatibility with the current board files, the old method of getting
> > >> the regulator is kept. The old method can be removed when the board
> > >> files have been changed to pass the regulator to DPI.
> > >
> > > as discuss with Arnd we should handle regular enable and disable at device
> > > probe for every device as we do for pinctrl
> >
> > I'm not sure what you mean. Enable of what? The regulator? Why would we
> > enable it in the device's probe, as the device may never even be used?
>
> It's an idea I had a while ago, but not yet discussed in the open.
>
> Jean-Christophe just posted patches to move the mapping of interrupt numbers
> into platform_drv_probe(), just before calling the driver ->probe() callback,
> and we already have similar code to set up the default pinctrl state of
> a device before calling probe().
>
> This can be extended to further subsystems, but that has to be done
> carefully to avoid regressions. Ideally we would move a lot of boilerplate
> code out of the driver specific ->probe() function into common code.
> Possible candidates for this include:
>
> * calling devm_request_mem_region for the "reg" property
> * calling devm_ioremap on the "reg" property"
> * calling devm_gpio_request for all gpio lines
> * calling devm_regulator_get on all regulators
> * calling devm_reset_control_get on all reset lines
> * calling devm_dma_request_slave_channel on all dma channels
> * calling devm_of_pwm_get for all pwm channels
> * ...
>
> For most of these (maybe all), I think we need some form of opt-in
> model on the driver side because there are cases where aquiring some
> of these resources is not mandatory, and it only works if the driver
> is using DT probing.
>
> IF we want to do this, it also needs a lot of thought, and we shouldn't
> do it carelessly. We might also need some extra infrastructure in revres
> to simplify access to the resources we got from the OF node.
>
> The irq resources are particularly trivial because we already claim
> them in of_platform_populate, so moving that to platform_drv_probe()
> is straightforward and solves existing bugs without creating a huge
> regression potential, but it's harder for the others.
Yeah I agree with Arnd
we need to start to move this way but for DT only first and carefully
Best Regards,
J.
>
> Arnd
> --
> 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
* Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT
From: Tomi Valkeinen @ 2013-05-30 16:14 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <20130530154355.GM19468@game.jcrosoft.org>
[-- Attachment #1: Type: text/plain, Size: 3270 bytes --]
On 30/05/13 18:43, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 14:28 Thu 30 May , Tomi Valkeinen wrote:
>> On 30/05/13 14:09, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 12:34 Thu 30 May , Tomi Valkeinen wrote:
>>>> When using DT, dss device does not have platform data. However,
>>>> dss_get_ctx_loss_count() uses dss device's platform data to find the
>>>> get_ctx_loss_count function pointer.
>>>>
>>>> To fix this, dss_get_ctx_loss_count() needs to be changed to get the
>>>> platform data from the omapdss device, which is a "virtual" device and
>>>> always has platform data.
>>>>
>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>>> ---
>>>> drivers/video/omap2/dss/dss.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
>>>> index 94f66f9..bd01608 100644
>>>> --- a/drivers/video/omap2/dss/dss.c
>>>> +++ b/drivers/video/omap2/dss/dss.c
>>>> @@ -157,7 +157,8 @@ static void dss_restore_context(void)
>>>>
>>>> int dss_get_ctx_loss_count(void)
>>>> {
>>>> - struct omap_dss_board_info *board_data = dss.pdev->dev.platform_data;
>>>> + struct platform_device *core_pdev = dss_get_core_pdev();
>>>> + struct omap_dss_board_info *board_data = core_pdev->dev.platform_data;
>>>
>>> how about store the pdata in the drivers internal struct and if !dt
>>> you ust do this
>>>
>>> dss_dev->pdata = *pdev->dev.platform_data;
>>>
>>> to copy it and we do not alloc it for dt
>>
>> It's not quite that simple. We need some OMAP arch functions (like
>> get_ctx_loss_count here) that are not currently exposed via any other
>> method to drivers except passing a function pointer with platform data.
>> We need that also when booting with DT.
>>
>> We have a bunch of devices for the display subsystem hardware blocks,
>> like the "dss" here. When booting with DT, these blocks are represented
>> in the DT data, and do not have platform data.
>>
>> We also have a "virtual" device, "omapdss", which doesn't match any hw
>> block. It's created in the arch setup stage. It's really a legacy thing,
>> but with DT it can be used conveniently to pass the platform data.
>>
>> The problem this patch fixes is that we used to pass the arch functions
>> for each of those HW block drivers. But with DT, we need to get the arch
>> functions from the "omapdss" device, gotten with dss_get_core_pdev().
>
> ok
>
> do not take it bad is it worth the effort those 54 patches?
>
> is not better to work on DRM?
> just an open question
Both omapfb and omapdrm use omapdss. omapdss provides the HW level
support, and also panel support. At some point in the future we'll
probably deprecate omapfb, and move wholly to omapdrm. At that point
omapdss and omapdrm can be merged together, simplifying the design.
And regarding the amount of the patches, there has been some bad design
decisions in the history of omapdss, and as it's a big driver (plus the
panel drivers), it takes quite a bit to fix these. There will be more
coming to convert the rest of the panel drivers, and also to implement
DT support. But those all also work for omapdrm, so it's not fbdev-only
stuff.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH] fbdev: bfin-lq035q1-fb: Use dev_pm_ops
From: Lars-Peter Clausen @ 2013-05-30 16:32 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD
Cc: Jingoo Han, 'Michael Hennerich',
'Rafael J. Wysocki', 'Tomi Valkeinen',
linux-fbdev, linux-pm
In-Reply-To: <20130530104535.GF19468@game.jcrosoft.org>
On 05/30/2013 12:45 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 09:52 Thu 30 May , Lars-Peter Clausen wrote:
>> On 05/30/2013 09:14 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 09:32 Thu 30 May , Jingoo Han wrote:
>>>> On Thursday, May 30, 2013 4:20 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>>>> On 16:24 Wed 29 May , Michael Hennerich wrote:
>>>>>> On 05/29/2013 02:17 PM, Lars-Peter Clausen wrote:
>>>>>>> Use dev_pm_ops instead of the legacy suspend/resume callbacks.
>>>>>>>
>>>>>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>>>>>> Acked-by: Michael Hennerich <michael.hennerich@analog.com>
>>>>>>> ---
>>>>>>> drivers/video/bfin-lq035q1-fb.c | 22 ++++++++++++++--------
>>>>>>> 1 file changed, 14 insertions(+), 8 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/video/bfin-lq035q1-fb.c b/drivers/video/bfin-lq035q1-fb.c
>>>>>>> index 29d8c04..6084c17 100644
>>>>>>> --- a/drivers/video/bfin-lq035q1-fb.c
>>>>>>> +++ b/drivers/video/bfin-lq035q1-fb.c
>>>>>>> @@ -170,16 +170,19 @@ static int lq035q1_spidev_remove(struct spi_device *spi)
>>>>>>> return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_SHUT);
>>>>>>> }
>>>>>>> -#ifdef CONFIG_PM
>>>>>>> -static int lq035q1_spidev_suspend(struct spi_device *spi, pm_message_t state)
>>>>>>> +#ifdef CONFIG_PM_SLEEP
>>>>>>> +static int lq035q1_spidev_suspend(struct device *dev)
>>>>>>> {
>>>>>>> + struct spi_device *spi = to_spi_device(dev);
>>>>>>> +
>>>>>>> return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_SHUT);
>>>>>>> }
>>>>>>> -static int lq035q1_spidev_resume(struct spi_device *spi)
>>>>>>> +static int lq035q1_spidev_resume(struct device *dev)
>>>>>>> {
>>>>>>> - int ret;
>>>>>>> + struct spi_device *spi = to_spi_device(dev);
>>>>>>> struct spi_control *ctl = spi_get_drvdata(spi);
>>>>>>> + int ret;
>>>>>>> ret = lq035q1_control(spi, LQ035_DRIVER_OUTPUT_CTL, ctl->mode);
>>>>>>> if (ret)
>>>>>>> @@ -187,9 +190,13 @@ static int lq035q1_spidev_resume(struct spi_device *spi)
>>>>>>> return lq035q1_control(spi, LQ035_SHUT_CTL, LQ035_ON);
>>>>>>> }
>>>>>>> +
>>>>>>> +static SIMPLE_DEV_PM_OPS(lq035q1_spidev_pm_ops, lq035q1_spidev_suspend,
>>>>>>> + lq035q1_spidev_resume);
>>>>>>> +#define LQ035Q1_SPIDEV_PM_OPS (&lq035q1_spidev_pm_ops)
>>>>>>> +
>>>>>>> #else
>>>>>>> -# define lq035q1_spidev_suspend NULL
>>>>>>> -# define lq035q1_spidev_resume NULL
>>>>>>> +#define LQ035Q1_SPIDEV_PM_OPS NULL
>>>>>>> #endif
>>>>> we really need to ahve a macro like for DT of_match_ptr to drop the #else
>>>>
>>>> Hi Jean-Christophe PLAGNIOL-VILLARD,
>>>>
>>>> I submitted the following patch. :)
>>>> (https://patchwork.kernel.org/patch/2502971/)
>>>>
>>>> --- a/include/linux/pm.h
>>>> +++ b/include/linux/pm.h
>>>> @@ -55,8 +55,10 @@ struct device;
>>>>
>>>> #ifdef CONFIG_PM
>>>> extern const char power_group_name[]; /* = "power" */
>>>> +#define pm_ops_ptr(_ptr) (_ptr)
>>>> #else
>>>> #define power_group_name NULL
>>>> +#define pm_ops_ptr(_ptr) NULL
>>>> #endif
>>>>
>>>>
>>>> This patch was accepted by Rafael Wysocki, and will be merged to v3.11-rc1.
>>>>
>>> Lars-Peter please update with and
>>
>> Since the code depends on CONFIG_PM_SLEEP and not CONFIG_PM I don't think
>> the macro will work.
>
> se please ad a similar macros too
I'm not really convinced that this *_ops_ptr thing is the right approach.
Besides that the scope of this patch is to convert the driver from legacy
suspend callbacks to dev_pm_ops not to remove the #ifdef's.
- Lars
^ permalink raw reply
* Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT
From: Kevin Hilman @ 2013-05-30 16:36 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <1369906493-27538-6-git-send-email-tomi.valkeinen@ti.com>
Tomi Valkeinen <tomi.valkeinen@ti.com> writes:
> When using DT, dss device does not have platform data. However,
> dss_get_ctx_loss_count() uses dss device's platform data to find the
> get_ctx_loss_count function pointer.
>
> To fix this, dss_get_ctx_loss_count() needs to be changed to get the
> platform data from the omapdss device, which is a "virtual" device and
> always has platform data.
Dumb question (since the DSS device model has always been beyond my
grasp): how/why does the virtual device still have platform_data on a DT
boot?
Also, this context_loss_count and DT boot is starting to get cumbersome,
and there's currently no (good) way of handling the context loss in a
generic way without pdata function pointers.
I'm starting to think we should move towards dropping this OMAP-specific
context loss counting, and just assume context is always lost. If there
are performance problems, runtime PM autosuspend timeouts can be used to
avoid the transitions.
Or, on some devices, I suspect comparing a few registers against their
power-on reset values might be a quicker way of detecting lost context
and would avoid having to call into platform code all together.
Kevin
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
> drivers/video/omap2/dss/dss.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
> index 94f66f9..bd01608 100644
> --- a/drivers/video/omap2/dss/dss.c
> +++ b/drivers/video/omap2/dss/dss.c
> @@ -157,7 +157,8 @@ static void dss_restore_context(void)
>
> int dss_get_ctx_loss_count(void)
> {
> - struct omap_dss_board_info *board_data = dss.pdev->dev.platform_data;
> + struct platform_device *core_pdev = dss_get_core_pdev();
> + struct omap_dss_board_info *board_data = core_pdev->dev.platform_data;
> int cnt;
>
> if (!board_data->get_context_loss_count)
^ permalink raw reply
* Re: [PATCH 03/32] OMAPDSS: add omap_dss_find_output()
From: Tomi Valkeinen @ 2013-05-30 16:54 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <20130530154055.GL19468@game.jcrosoft.org>
[-- Attachment #1: Type: text/plain, Size: 2266 bytes --]
On 30/05/13 18:40, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 14:40 Thu 30 May , Tomi Valkeinen wrote:
>> On 30/05/13 14:07, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 12:34 Thu 30 May , Tomi Valkeinen wrote:
>>>> Add a support function to find a DSS output by given name. This is used
>>>> in later patches to link the panels to DSS outputs.
>>>>
>>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>>> ---
>>>> drivers/video/omap2/dss/output.c | 13 +++++++++++++
>>>> include/video/omapdss.h | 1 +
>>>> 2 files changed, 14 insertions(+)
>>>>
>>>> diff --git a/drivers/video/omap2/dss/output.c b/drivers/video/omap2/dss/output.c
>>>> index 5214df6..3274628 100644
>>>> --- a/drivers/video/omap2/dss/output.c
>>>> +++ b/drivers/video/omap2/dss/output.c
>>>> @@ -115,6 +115,19 @@ struct omap_dss_output *omap_dss_get_output(enum omap_dss_output_id id)
>>>> }
>>>> EXPORT_SYMBOL(omap_dss_get_output);
>>> GPL please
>>
>> The omapdss driver uses EXPORT_SYMBOL. I don't want to start mixing both
>> EXPORT_SYMBOLs and EXPORT_SYMBOL_GPLs.
>
> I do not like EXPORT_SYMBOL at all
> I stringly prefer to switch all of them to _GPL
>
> but will not refuse the patch for this in this case
I have nothing against changing omapdss to use _GPL only. I've never
heard anyone using non-GPL panel drivers with omap.
Following these patch sets I can remove lots of the old code, which
contains the majority of the EXPORT_SYMBOLs in omapdss. I'll change
omapdss to use _GPL after that removal.
>>>> +struct omap_dss_output *omap_dss_find_output(const char *name)
>>>> +{
>>>> + struct omap_dss_output *out;
>>>> +
>>>> + list_for_each_entry(out, &output_list, list) {
>>>> + if (strcmp(out->name, name) == 0)
>>>> + return out;
>>>> + }
>>>> +
>>>> + return NULL;
>>>> +}
>>> I this in so many drivers could we have a macro to generate such function?
>>
>> What would that help? Wouldn't it just increase the code size of the kernel?
>
> increase no as it's not an inline function but a macro to generate the
> function
> after help yes but people may not like so as you wish
I don't think I understand what you mean then. What kind of macro are
you talking about?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* Re: [PATCH 05/32] OMAPDSS: fix dss_get_ctx_loss_count for DT
From: Tomi Valkeinen @ 2013-05-30 17:21 UTC (permalink / raw)
To: Kevin Hilman; +Cc: linux-fbdev, linux-omap, Archit Taneja
In-Reply-To: <874ndk5sse.fsf@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 2534 bytes --]
On 30/05/13 19:36, Kevin Hilman wrote:
> Tomi Valkeinen <tomi.valkeinen@ti.com> writes:
>
>> When using DT, dss device does not have platform data. However,
>> dss_get_ctx_loss_count() uses dss device's platform data to find the
>> get_ctx_loss_count function pointer.
>>
>> To fix this, dss_get_ctx_loss_count() needs to be changed to get the
>> platform data from the omapdss device, which is a "virtual" device and
>> always has platform data.
>
> Dumb question (since the DSS device model has always been beyond my
> grasp): how/why does the virtual device still have platform_data on a DT
> boot?
The virtual device will be created in generic omap arch code for DT boot
also.
The omapdss virtual device is an old relic. Originally we only had the
omapdss device, not the sub-devices for HW block like we have now. After
adding the sub-devices, omapdss device was still left, as it provided
useful functionality. It wasn't strictly needed anymore, but I've just
never had time to start refactoring code to remove it.
Now, with DT, it was just an easy way around to pass the func pointers.
Note that we also pass set_min_bus_tput, and a version identifier for
the DSS HW. The PM funcs could perhaps be handled some other way, but
I'm not sure how I could pass the DSS HW version.
> Also, this context_loss_count and DT boot is starting to get cumbersome,
> and there's currently no (good) way of handling the context loss in a
> generic way without pdata function pointers.
>
> I'm starting to think we should move towards dropping this OMAP-specific
> context loss counting, and just assume context is always lost. If there
> are performance problems, runtime PM autosuspend timeouts can be used to
> avoid the transitions.
>
> Or, on some devices, I suspect comparing a few registers against their
> power-on reset values might be a quicker way of detecting lost context
> and would avoid having to call into platform code all together.
Hmm, true. I'll look at this. Maybe I won't need get_ctx_loss in omapdss
at all.
How about omap_pm_set_min_bus_tput(), can that be handled in some
generic manner? Although, we currently use that in a bit hacky case.
What we're doing is not really setting min bus tput, but we're just
preventing OPP50.
The DSS clocks have different maximums on OPP50 than on OPP100, and we
need to keep the core in OPP100 to keep DSS working. So what I'd rather
use is "prevent_opp50()" than setting arbitrarily high min bus tput.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply
* [RESEND PATCH 1/1] MAINTAINERS: Framebuffer Layer maintainers update
From: Jean-Christophe PLAGNIOL-VILLARD @ 2013-05-30 20:23 UTC (permalink / raw)
To: linux-kernel
Cc: Jean-Christophe PLAGNIOL-VILLARD, Tomi Valkeinen, Olof Johansson,
Andrew Morton, Linus Torvalds, Arnd Bergmann,
Florian Tobias Schandinat, linux-fbdev
Tomi and I will now take care of the Framebuffer Layer
The git tree is now on kernel.org
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Olof Johansson <olof@lixom.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc: linux-fbdev@vger.kernel.org
---
Hi,
resend as git send-email drop my name in the From
Best Regards,
J.
MAINTAINERS | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index fd3a495..7714c3c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3322,11 +3322,12 @@ F: drivers/net/wan/dlci.c
F: drivers/net/wan/sdla.c
FRAMEBUFFER LAYER
-M: Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
+M: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
+M: Tomi Valkeinen <tomi.valkeinen@ti.com>
L: linux-fbdev@vger.kernel.org
W: http://linux-fbdev.sourceforge.net/
Q: http://patchwork.kernel.org/project/linux-fbdev/list/
-T: git git://github.com/schandinat/linux-2.6.git fbdev-next
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/plagnioj/linux-fbdev.git
S: Maintained
F: Documentation/fb/
F: Documentation/devicetree/bindings/fb/
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v8, part3 06/14] mm, acornfb: use free_reserved_area() to simplify code
From: Andrew Morton @ 2013-05-30 21:58 UTC (permalink / raw)
To: Jiang Liu
Cc: Jiang Liu, David Rientjes, Wen Congyang, Mel Gorman, Minchan Kim,
KAMEZAWA Hiroyuki, Michal Hocko, James Bottomley, Sergei Shtylyov,
David Howells, Mark Salter, Jianguo Wu, linux-mm, linux-arch,
linux-kernel, Florian Tobias Schandinat, linux-fbdev
In-Reply-To: <1369575522-26405-7-git-send-email-jiang.liu@huawei.com>
On Sun, 26 May 2013 21:38:34 +0800 Jiang Liu <liuj97@gmail.com> wrote:
> Use common help function free_reserved_area() to simplify code.
http://ozlabs.org/~akpm/mmots/broken-out/drivers-video-acornfbc-remove-dead-code.patch
removes all the code which your patch alters.
^ permalink raw reply
* Re: [PATCH v2 2/3] video: xilinxfb: Do not use out_be32 IO function
From: Arnd Bergmann @ 2013-05-30 22:04 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, Michal Simek, Florian Tobias Schandinat,
linux-fbdev
In-Reply-To: <a15de155a3f32fc1416833df1b281db05d347541.1369906849.git.michal.simek@xilinx.com>
On Thursday 30 May 2013 11:41:01 Michal Simek wrote:
> * To perform the read/write on the registers we need to check on
> * which bus its connected and call the appropriate write API.
> */
> -static void xilinx_fb_out_be32(struct xilinxfb_drvdata *drvdata, u32 offset,
> +static void xilinx_fb_out32(struct xilinxfb_drvdata *drvdata, u32 offset,
> u32 val)
> {
> if (drvdata->flags & PLB_ACCESS_FLAG)
> - out_be32(drvdata->regs + (offset << 2), val);
> + __raw_writel(val, drvdata->regs + (offset << 2));
> #ifdef CONFIG_PPC_DCR
> else
> dcr_write(drvdata->dcr_host, offset, val);
>
This is probably missing barriers, and is wrong on systems on which
the endianess of the device is different from the CPU.
You already have an indirection in there, so I guess it won't hurt
to create a third case for little-endian registers and add
another bit in drvdata->flags, or make it depend on the architecture,
if the endianess of the device registers is known at compile time.
Arnd
^ 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