Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH] fb: only enable console lock in fb for VGA console
From: Jun Nie @ 2012-09-29 10:46 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <CAGA24M+PmObK3ZJWvtNhBzxO4WkGk_xyveQiZNtZe7xGdHrbFw@mail.gmail.com>

2012/9/29 Benjamin Herrenschmidt <benh@kernel.crashing.org>:
> On Sat, 2012-09-29 at 14:03 +0800, Jun Nie wrote:
>
>> Hi Ben,
>>     Yes. If framebuffer console is disabled, I do not see any need to
>> hold console lock in FB. Please help indicate the risk if any. Thanks!
>
> Because there is no other lock left in the entire fbdev layer.. a lot of
> fbdev drivers have no locking of their own and rely on the console
> semaphore in more than one way.
>
> Ben.
>
How about to add another fb_mutex? FB driver can only hold it when
framebuffer console is disabled, while hold both when fb console is
enabled.


>> Jun
>> >> Signed-off-by: Jun Nie <njun@marvell.com>
>> >> ---
>> >>  drivers/video/arkfb.c                  |   11 +++++------
>> >>  drivers/video/aty/aty128fb.c           |   11 +++++------
>> >>  drivers/video/aty/atyfb_base.c         |   11 +++++------
>> >>  drivers/video/aty/radeon_pm.c          |    9 ++++-----
>> >>  drivers/video/chipsfb.c                |    8 ++++----
>> >>  drivers/video/da8xx-fb.c               |    9 ++++-----
>> >>  drivers/video/fb-puv3.c                |    9 ++++-----
>> >>  drivers/video/fbmem.c                  |   28 ++++++++++++++++++++++------
>> >>  drivers/video/fbsysfs.c                |   21 ++++++++++-----------
>> >>  drivers/video/geode/gxfb_core.c        |    9 ++++-----
>> >>  drivers/video/geode/lxfb_core.c        |    8 ++++----
>> >>  drivers/video/i740fb.c                 |   11 +++++------
>> >>  drivers/video/i810/i810_main.c         |    9 ++++-----
>> >>  drivers/video/jz4740_fb.c              |    9 ++++-----
>> >>  drivers/video/mx3fb.c                  |    9 ++++-----
>> >>  drivers/video/nvidia/nvidia.c          |    9 ++++-----
>> >>  drivers/video/ps3fb.c                  |   15 +++++++--------
>> >>  drivers/video/pxafb.c                  |    5 ++---
>> >>  drivers/video/s3fb.c                   |   15 +++++++--------
>> >>  drivers/video/savage/savagefb_driver.c |    9 ++++-----
>> >>  drivers/video/sh_mobile_lcdcfb.c       |   13 ++++++-------
>> >>  drivers/video/sm501fb.c                |    9 ++++-----
>> >>  drivers/video/tmiofb.c                 |   11 +++++------
>> >>  drivers/video/via/viafbdev.c           |    8 ++++----
>> >>  drivers/video/vt8623fb.c               |   11 +++++------
>> >>  drivers/video/xen-fbfront.c            |    5 ++---
>> >>  include/linux/fb.h                     |    2 ++
>> >>  27 files changed, 140 insertions(+), 144 deletions(-)
>> >>
>> >> diff --git a/drivers/video/arkfb.c b/drivers/video/arkfb.c
>> >> index 555dd4c..774d902 100644
>> >> --- a/drivers/video/arkfb.c
>> >> +++ b/drivers/video/arkfb.c
>> >> @@ -23,7 +23,6 @@
>> >>  #include <linux/svga.h>
>> >>  #include <linux/init.h>
>> >>  #include <linux/pci.h>
>> >> -#include <linux/console.h> /* Why should fb driver call console
>> >> functions? because console_lock() */
>> >>  #include <video/vga.h>
>> >>
>> >>  #ifdef CONFIG_MTRR
>> >> @@ -1124,12 +1123,12 @@ static int ark_pci_suspend (struct pci_dev*
>> >> dev, pm_message_t state)
>> >>
>> >>       dev_info(info->device, "suspend\n");
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       mutex_lock(&(par->open_lock));
>> >>
>> >>       if ((state.event = PM_EVENT_FREEZE) || (par->ref_count = 0)) {
>> >>               mutex_unlock(&(par->open_lock));
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>               return 0;
>> >>       }
>> >>
>> >> @@ -1140,7 +1139,7 @@ static int ark_pci_suspend (struct pci_dev* dev,
>> >> pm_message_t state)
>> >>       pci_set_power_state(dev, pci_choose_state(dev, state));
>> >>
>> >>       mutex_unlock(&(par->open_lock));
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >> @@ -1155,7 +1154,7 @@ static int ark_pci_resume (struct pci_dev* dev)
>> >>
>> >>       dev_info(info->device, "resume\n");
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       mutex_lock(&(par->open_lock));
>> >>
>> >>       if (par->ref_count = 0)
>> >> @@ -1174,7 +1173,7 @@ static int ark_pci_resume (struct pci_dev* dev)
>> >>
>> >>  fail:
>> >>       mutex_unlock(&(par->open_lock));
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>       return 0;
>> >>  }
>> >>  #else
>> >> diff --git a/drivers/video/aty/aty128fb.c b/drivers/video/aty/aty128fb.c
>> >> index 747442d..1c6b80b 100644
>> >> --- a/drivers/video/aty/aty128fb.c
>> >> +++ b/drivers/video/aty/aty128fb.c
>> >> @@ -60,7 +60,6 @@
>> >>  #include <linux/init.h>
>> >>  #include <linux/pci.h>
>> >>  #include <linux/ioport.h>
>> >> -#include <linux/console.h>
>> >>  #include <linux/backlight.h>
>> >>  #include <asm/io.h>
>> >>
>> >> @@ -1884,7 +1883,7 @@ static void aty128_early_resume(void *data)
>> >>               return;
>> >>       pci_restore_state(par->pdev);
>> >>       aty128_do_resume(par->pdev);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>  }
>> >>  #endif /* CONFIG_PPC_PMAC */
>> >>
>> >> @@ -2461,7 +2460,7 @@ static int aty128_pci_suspend(struct pci_dev
>> >> *pdev, pm_message_t state)
>> >>
>> >>       printk(KERN_DEBUG "aty128fb: suspending...\n");
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>
>> >>       fb_set_suspend(info, 1);
>> >>
>> >> @@ -2493,7 +2492,7 @@ static int aty128_pci_suspend(struct pci_dev
>> >> *pdev, pm_message_t state)
>> >>       if (state.event != PM_EVENT_ON)
>> >>               aty128_set_suspend(par, 1);
>> >>
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       pdev->dev.power.power_state = state;
>> >>
>> >> @@ -2550,9 +2549,9 @@ static int aty128_pci_resume(struct pci_dev *pdev)
>> >>  {
>> >>       int rc;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       rc = aty128_do_resume(pdev);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return rc;
>> >>  }
>> >> diff --git a/drivers/video/aty/atyfb_base.c b/drivers/video/aty/atyfb_base.c
>> >> index 3f2e8c1..9bb15ab 100644
>> >> --- a/drivers/video/aty/atyfb_base.c
>> >> +++ b/drivers/video/aty/atyfb_base.c
>> >> @@ -58,7 +58,6 @@
>> >>  #include <linux/slab.h>
>> >>  #include <linux/vmalloc.h>
>> >>  #include <linux/delay.h>
>> >> -#include <linux/console.h>
>> >>  #include <linux/fb.h>
>> >>  #include <linux/init.h>
>> >>  #include <linux/pci.h>
>> >> @@ -2065,7 +2064,7 @@ static int atyfb_pci_suspend(struct pci_dev
>> >> *pdev, pm_message_t state)
>> >>       if (state.event = pdev->dev.power.power_state.event)
>> >>               return 0;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>
>> >>       fb_set_suspend(info, 1);
>> >>
>> >> @@ -2093,14 +2092,14 @@ static int atyfb_pci_suspend(struct pci_dev
>> >> *pdev, pm_message_t state)
>> >>               par->lock_blank = 0;
>> >>               atyfb_blank(FB_BLANK_UNBLANK, info);
>> >>               fb_set_suspend(info, 0);
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>               return -EIO;
>> >>       }
>> >>  #else
>> >>       pci_set_power_state(pdev, pci_choose_state(pdev, state));
>> >>  #endif
>> >>
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       pdev->dev.power.power_state = state;
>> >>
>> >> @@ -2129,7 +2128,7 @@ static int atyfb_pci_resume(struct pci_dev *pdev)
>> >>       if (pdev->dev.power.power_state.event = PM_EVENT_ON)
>> >>               return 0;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>
>> >>       /*
>> >>        * PCI state will have been restored by the core, so
>> >> @@ -2157,7 +2156,7 @@ static int atyfb_pci_resume(struct pci_dev *pdev)
>> >>       par->lock_blank = 0;
>> >>       atyfb_blank(FB_BLANK_UNBLANK, info);
>> >>
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       pdev->dev.power.power_state = PMSG_ON;
>> >>
>> >> diff --git a/drivers/video/aty/radeon_pm.c b/drivers/video/aty/radeon_pm.c
>> >> index 92bda58..a2f07da 100644
>> >> --- a/drivers/video/aty/radeon_pm.c
>> >> +++ b/drivers/video/aty/radeon_pm.c
>> >> @@ -16,7 +16,6 @@
>> >>
>> >>  #include "radeonfb.h"
>> >>
>> >> -#include <linux/console.h>
>> >>  #include <linux/agp_backend.h>
>> >>
>> >>  #ifdef CONFIG_PPC_PMAC
>> >> @@ -2626,7 +2625,7 @@ int radeonfb_pci_suspend(struct pci_dev *pdev,
>> >> pm_message_t mesg)
>> >>               goto done;
>> >>       }
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>
>> >>       fb_set_suspend(info, 1);
>> >>
>> >> @@ -2690,7 +2689,7 @@ int radeonfb_pci_suspend(struct pci_dev *pdev,
>> >> pm_message_t mesg)
>> >>       if (rinfo->pm_mode & radeon_pm_d2)
>> >>               radeon_set_suspend(rinfo, 1);
>> >>
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>   done:
>> >>       pdev->dev.power.power_state = mesg;
>> >> @@ -2718,7 +2717,7 @@ int radeonfb_pci_resume(struct pci_dev *pdev)
>> >>               if (!console_trylock())
>> >>                       return 0;
>> >>       } else
>> >> -             console_lock();
>> >> +             fb_console_lock();
>> >>
>> >>       printk(KERN_DEBUG "radeonfb (%s): resuming from state: %d...\n",
>> >>              pci_name(pdev), pdev->dev.power.power_state.event);
>> >> @@ -2783,7 +2782,7 @@ int radeonfb_pci_resume(struct pci_dev *pdev)
>> >>       pdev->dev.power.power_state = PMSG_ON;
>> >>
>> >>   bail:
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return rc;
>> >>  }
>> >> diff --git a/drivers/video/chipsfb.c b/drivers/video/chipsfb.c
>> >> index cff742a..d50fa6b 100644
>> >> --- a/drivers/video/chipsfb.c
>> >> +++ b/drivers/video/chipsfb.c
>> >> @@ -460,10 +460,10 @@ static int chipsfb_pci_suspend(struct pci_dev
>> >> *pdev, pm_message_t state)
>> >>       if (!(state.event & PM_EVENT_SLEEP))
>> >>               goto done;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       chipsfb_blank(1, p);
>> >>       fb_set_suspend(p, 1);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>   done:
>> >>       pdev->dev.power.power_state = state;
>> >>       return 0;
>> >> @@ -473,10 +473,10 @@ static int chipsfb_pci_resume(struct pci_dev *pdev)
>> >>  {
>> >>          struct fb_info *p = pci_get_drvdata(pdev);
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       fb_set_suspend(p, 0);
>> >>       chipsfb_blank(0, p);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       pdev->dev.power.power_state = PMSG_ON;
>> >>       return 0;
>> >> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
>> >> index 7ae9d53..9a36bbc 100644
>> >> --- a/drivers/video/da8xx-fb.c
>> >> +++ b/drivers/video/da8xx-fb.c
>> >> @@ -29,7 +29,6 @@
>> >>  #include <linux/interrupt.h>
>> >>  #include <linux/clk.h>
>> >>  #include <linux/cpufreq.h>
>> >> -#include <linux/console.h>
>> >>  #include <linux/spinlock.h>
>> >>  #include <linux/slab.h>
>> >>  #include <linux/delay.h>
>> >> @@ -1406,14 +1405,14 @@ static int fb_suspend(struct platform_device
>> >> *dev, pm_message_t state)
>> >>       struct fb_info *info = platform_get_drvdata(dev);
>> >>       struct da8xx_fb_par *par = info->par;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       if (par->panel_power_ctrl)
>> >>               par->panel_power_ctrl(0);
>> >>
>> >>       fb_set_suspend(info, 1);
>> >>       lcd_disable_raster();
>> >>       clk_disable(par->lcdc_clk);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >> @@ -1422,7 +1421,7 @@ static int fb_resume(struct platform_device *dev)
>> >>       struct fb_info *info = platform_get_drvdata(dev);
>> >>       struct da8xx_fb_par *par = info->par;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       clk_enable(par->lcdc_clk);
>> >>       lcd_enable_raster();
>> >>
>> >> @@ -1430,7 +1429,7 @@ static int fb_resume(struct platform_device *dev)
>> >>               par->panel_power_ctrl(1);
>> >>
>> >>       fb_set_suspend(info, 0);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >> diff --git a/drivers/video/fb-puv3.c b/drivers/video/fb-puv3.c
>> >> index 60a787f..87c8c4e 100644
>> >> --- a/drivers/video/fb-puv3.c
>> >> +++ b/drivers/video/fb-puv3.c
>> >> @@ -17,7 +17,6 @@
>> >>  #include <linux/clk.h>
>> >>  #include <linux/fb.h>
>> >>  #include <linux/init.h>
>> >> -#include <linux/console.h>
>> >>
>> >>  #include <asm/sizes.h>
>> >>  #include <mach/hardware.h>
>> >> @@ -759,7 +758,7 @@ static int unifb_resume(struct platform_device *dev)
>> >>       if (dev->dev.power.power_state.event = PM_EVENT_ON)
>> >>               return 0;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>
>> >>       if (dev->dev.power.power_state.event = PM_EVENT_SUSPEND) {
>> >>               writel(unifb_regs[0], UDE_FSA);
>> >> @@ -775,7 +774,7 @@ static int unifb_resume(struct platform_device *dev)
>> >>       }
>> >>       dev->dev.power.power_state = PMSG_ON;
>> >>
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return rc;
>> >>  }
>> >> @@ -804,11 +803,11 @@ static int unifb_suspend(struct platform_device
>> >> *dev, pm_message_t mesg)
>> >>               goto done;
>> >>       }
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>
>> >>       /* do nothing... */
>> >>
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>  done:
>> >>       dev->dev.power.power_state = mesg;
>> >> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
>> >> index 0dff12a..c2d4627 100644
>> >> --- a/drivers/video/fbmem.c
>> >> +++ b/drivers/video/fbmem.c
>> >> @@ -85,6 +85,22 @@ EXPORT_SYMBOL(lock_fb_info);
>> >>   * Helpers
>> >>   */
>> >>
>> >> +void fb_console_lock(void)
>> >> +{
>> >> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE
>> >> +     console_lock();
>> >> +#endif
>> >> +}
>> >> +EXPORT_SYMBOL(fb_console_lock);
>> >> +
>> >> +void fb_console_unlock(void)
>> >> +{
>> >> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE
>> >> +     console_unlock();
>> >> +#endif
>> >> +}
>> >> +EXPORT_SYMBOL(fb_console_unlock);
>> >> +
>> >>  int fb_get_color_depth(struct fb_var_screeninfo *var,
>> >>                      struct fb_fix_screeninfo *fix)
>> >>  {
>> >> @@ -1101,11 +1117,11 @@ static long do_fb_ioctl(struct fb_info *info,
>> >> unsigned int cmd,
>> >>                       return -EFAULT;
>> >>               if (!lock_fb_info(info))
>> >>                       return -ENODEV;
>> >> -             console_lock();
>> >> +             fb_console_lock();
>> >>               info->flags |= FBINFO_MISC_USEREVENT;
>> >>               ret = fb_set_var(info, &var);
>> >>               info->flags &= ~FBINFO_MISC_USEREVENT;
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>               unlock_fb_info(info);
>> >>               if (!ret && copy_to_user(argp, &var, sizeof(var)))
>> >>                       ret = -EFAULT;
>> >> @@ -1137,9 +1153,9 @@ static long do_fb_ioctl(struct fb_info *info,
>> >> unsigned int cmd,
>> >>                       return -EFAULT;
>> >>               if (!lock_fb_info(info))
>> >>                       return -ENODEV;
>> >> -             console_lock();
>> >> +             fb_console_lock();
>> >>               ret = fb_pan_display(info, &var);
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>               unlock_fb_info(info);
>> >>               if (ret = 0 && copy_to_user(argp, &var, sizeof(var)))
>> >>                       return -EFAULT;
>> >> @@ -1184,11 +1200,11 @@ static long do_fb_ioctl(struct fb_info *info,
>> >> unsigned int cmd,
>> >>       case FBIOBLANK:
>> >>               if (!lock_fb_info(info))
>> >>                       return -ENODEV;
>> >> -             console_lock();
>> >> +             fb_console_lock();
>> >>               info->flags |= FBINFO_MISC_USEREVENT;
>> >>               ret = fb_blank(info, arg);
>> >>               info->flags &= ~FBINFO_MISC_USEREVENT;
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>               unlock_fb_info(info);
>> >>               break;
>> >>       default:
>> >> diff --git a/drivers/video/fbsysfs.c b/drivers/video/fbsysfs.c
>> >> index a55e366..976d8f5 100644
>> >> --- a/drivers/video/fbsysfs.c
>> >> +++ b/drivers/video/fbsysfs.c
>> >> @@ -18,7 +18,6 @@
>> >>  #include <linux/kernel.h>
>> >>  #include <linux/slab.h>
>> >>  #include <linux/fb.h>
>> >> -#include <linux/console.h>
>> >>  #include <linux/module.h>
>> >>
>> >>  #define FB_SYSFS_FLAG_ATTR 1
>> >> @@ -92,11 +91,11 @@ static int activate(struct fb_info *fb_info,
>> >> struct fb_var_screeninfo *var)
>> >>       int err;
>> >>
>> >>       var->activate |= FB_ACTIVATE_FORCE;
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       fb_info->flags |= FBINFO_MISC_USEREVENT;
>> >>       err = fb_set_var(fb_info, var);
>> >>       fb_info->flags &= ~FBINFO_MISC_USEREVENT;
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>       if (err)
>> >>               return err;
>> >>       return 0;
>> >> @@ -177,7 +176,7 @@ static ssize_t store_modes(struct device *device,
>> >>       if (i * sizeof(struct fb_videomode) != count)
>> >>               return -EINVAL;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       list_splice(&fb_info->modelist, &old_list);
>> >>       fb_videomode_to_modelist((const struct fb_videomode *)buf, i,
>> >>                                &fb_info->modelist);
>> >> @@ -187,7 +186,7 @@ static ssize_t store_modes(struct device *device,
>> >>       } else
>> >>               fb_destroy_modelist(&old_list);
>> >>
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >> @@ -303,11 +302,11 @@ static ssize_t store_blank(struct device *device,
>> >>       char *last = NULL;
>> >>       int err;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       fb_info->flags |= FBINFO_MISC_USEREVENT;
>> >>       err = fb_blank(fb_info, simple_strtoul(buf, &last, 0));
>> >>       fb_info->flags &= ~FBINFO_MISC_USEREVENT;
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>       if (err < 0)
>> >>               return err;
>> >>       return count;
>> >> @@ -366,9 +365,9 @@ static ssize_t store_pan(struct device *device,
>> >>               return -EINVAL;
>> >>       var.yoffset = simple_strtoul(last, &last, 0);
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       err = fb_pan_display(fb_info, &var);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       if (err < 0)
>> >>               return err;
>> >> @@ -403,9 +402,9 @@ static ssize_t store_fbstate(struct device *device,
>> >>
>> >>       if (!lock_fb_info(fb_info))
>> >>               return -ENODEV;
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       fb_set_suspend(fb_info, (int)state);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>       unlock_fb_info(fb_info);
>> >>
>> >>       return count;
>> >> diff --git a/drivers/video/geode/gxfb_core.c b/drivers/video/geode/gxfb_core.c
>> >> index b4f19db..ba7dd0a 100644
>> >> --- a/drivers/video/geode/gxfb_core.c
>> >> +++ b/drivers/video/geode/gxfb_core.c
>> >> @@ -27,7 +27,6 @@
>> >>  #include <linux/mm.h>
>> >>  #include <linux/delay.h>
>> >>  #include <linux/fb.h>
>> >> -#include <linux/console.h>
>> >>  #include <linux/suspend.h>
>> >>  #include <linux/init.h>
>> >>  #include <linux/pci.h>
>> >> @@ -344,10 +343,10 @@ static int gxfb_suspend(struct pci_dev *pdev,
>> >> pm_message_t state)
>> >>       struct fb_info *info = pci_get_drvdata(pdev);
>> >>
>> >>       if (state.event = PM_EVENT_SUSPEND) {
>> >> -             console_lock();
>> >> +             fb_console_lock();
>> >>               gx_powerdown(info);
>> >>               fb_set_suspend(info, 1);
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>       }
>> >>
>> >>       /* there's no point in setting PCI states; we emulate PCI, so
>> >> @@ -361,7 +360,7 @@ static int gxfb_resume(struct pci_dev *pdev)
>> >>       struct fb_info *info = pci_get_drvdata(pdev);
>> >>       int ret;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       ret = gx_powerup(info);
>> >>       if (ret) {
>> >>               printk(KERN_ERR "gxfb:  power up failed!\n");
>> >> @@ -369,7 +368,7 @@ static int gxfb_resume(struct pci_dev *pdev)
>> >>       }
>> >>
>> >>       fb_set_suspend(info, 0);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>       return 0;
>> >>  }
>> >>  #endif
>> >> diff --git a/drivers/video/geode/lxfb_core.c b/drivers/video/geode/lxfb_core.c
>> >> index 416851c..5341bf1 100644
>> >> --- a/drivers/video/geode/lxfb_core.c
>> >> +++ b/drivers/video/geode/lxfb_core.c
>> >> @@ -465,10 +465,10 @@ static int lxfb_suspend(struct pci_dev *pdev,
>> >> pm_message_t state)
>> >>       struct fb_info *info = pci_get_drvdata(pdev);
>> >>
>> >>       if (state.event = PM_EVENT_SUSPEND) {
>> >> -             console_lock();
>> >> +             fb_console_lock();
>> >>               lx_powerdown(info);
>> >>               fb_set_suspend(info, 1);
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>       }
>> >>
>> >>       /* there's no point in setting PCI states; we emulate PCI, so
>> >> @@ -482,7 +482,7 @@ static int lxfb_resume(struct pci_dev *pdev)
>> >>       struct fb_info *info = pci_get_drvdata(pdev);
>> >>       int ret;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       ret = lx_powerup(info);
>> >>       if (ret) {
>> >>               printk(KERN_ERR "lxfb:  power up failed!\n");
>> >> @@ -490,7 +490,7 @@ static int lxfb_resume(struct pci_dev *pdev)
>> >>       }
>> >>
>> >>       fb_set_suspend(info, 0);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>       return 0;
>> >>  }
>> >>  #else
>> >> diff --git a/drivers/video/i740fb.c b/drivers/video/i740fb.c
>> >> index ff3f880..b3fe538 100644
>> >> --- a/drivers/video/i740fb.c
>> >> +++ b/drivers/video/i740fb.c
>> >> @@ -24,7 +24,6 @@
>> >>  #include <linux/pci_ids.h>
>> >>  #include <linux/i2c.h>
>> >>  #include <linux/i2c-algo-bit.h>
>> >> -#include <linux/console.h>
>> >>  #include <video/vga.h>
>> >>
>> >>  #ifdef CONFIG_MTRR
>> >> @@ -1210,13 +1209,13 @@ static int i740fb_suspend(struct pci_dev *dev,
>> >> pm_message_t state)
>> >>       if (state.event = PM_EVENT_FREEZE || state.event = PM_EVENT_PRETHAW)
>> >>               return 0;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       mutex_lock(&(par->open_lock));
>> >>
>> >>       /* do nothing if framebuffer is not active */
>> >>       if (par->ref_count = 0) {
>> >>               mutex_unlock(&(par->open_lock));
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>               return 0;
>> >>       }
>> >>
>> >> @@ -1227,7 +1226,7 @@ static int i740fb_suspend(struct pci_dev *dev,
>> >> pm_message_t state)
>> >>       pci_set_power_state(dev, pci_choose_state(dev, state));
>> >>
>> >>       mutex_unlock(&(par->open_lock));
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >> @@ -1237,7 +1236,7 @@ static int i740fb_resume(struct pci_dev *dev)
>> >>       struct fb_info *info = pci_get_drvdata(dev);
>> >>       struct i740fb_par *par = info->par;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       mutex_lock(&(par->open_lock));
>> >>
>> >>       if (par->ref_count = 0)
>> >> @@ -1253,7 +1252,7 @@ static int i740fb_resume(struct pci_dev *dev)
>> >>
>> >>  fail:
>> >>       mutex_unlock(&(par->open_lock));
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>       return 0;
>> >>  }
>> >>  #else
>> >> diff --git a/drivers/video/i810/i810_main.c b/drivers/video/i810/i810_main.c
>> >> index b83f361..84fe1ed 100644
>> >> --- a/drivers/video/i810/i810_main.c
>> >> +++ b/drivers/video/i810/i810_main.c
>> >> @@ -40,7 +40,6 @@
>> >>  #include <linux/pci_ids.h>
>> >>  #include <linux/resource.h>
>> >>  #include <linux/unistd.h>
>> >> -#include <linux/console.h>
>> >>
>> >>  #include <asm/io.h>
>> >>  #include <asm/div64.h>
>> >> @@ -1574,7 +1573,7 @@ static int i810fb_suspend(struct pci_dev *dev,
>> >> pm_message_t mesg)
>> >>               return 0;
>> >>       }
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       fb_set_suspend(info, 1);
>> >>
>> >>       if (info->fbops->fb_sync)
>> >> @@ -1587,7 +1586,7 @@ static int i810fb_suspend(struct pci_dev *dev,
>> >> pm_message_t mesg)
>> >>       pci_save_state(dev);
>> >>       pci_disable_device(dev);
>> >>       pci_set_power_state(dev, pci_choose_state(dev, mesg));
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >> @@ -1605,7 +1604,7 @@ static int i810fb_resume(struct pci_dev *dev)
>> >>               return 0;
>> >>       }
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       pci_set_power_state(dev, PCI_D0);
>> >>       pci_restore_state(dev);
>> >>
>> >> @@ -1621,7 +1620,7 @@ static int i810fb_resume(struct pci_dev *dev)
>> >>       fb_set_suspend (info, 0);
>> >>       info->fbops->fb_blank(VESA_NO_BLANKING, info);
>> >>  fail:
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>       return 0;
>> >>  }
>> >>  /***********************************************************************
>> >> diff --git a/drivers/video/jz4740_fb.c b/drivers/video/jz4740_fb.c
>> >> index de36693..7e9301a 100644
>> >> --- a/drivers/video/jz4740_fb.c
>> >> +++ b/drivers/video/jz4740_fb.c
>> >> @@ -21,7 +21,6 @@
>> >>  #include <linux/clk.h>
>> >>  #include <linux/delay.h>
>> >>
>> >> -#include <linux/console.h>
>> >>  #include <linux/fb.h>
>> >>
>> >>  #include <linux/dma-mapping.h>
>> >> @@ -778,9 +777,9 @@ static int jzfb_suspend(struct device *dev)
>> >>  {
>> >>       struct jzfb *jzfb = dev_get_drvdata(dev);
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       fb_set_suspend(jzfb->fb, 1);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       mutex_lock(&jzfb->lock);
>> >>       if (jzfb->is_enabled)
>> >> @@ -800,9 +799,9 @@ static int jzfb_resume(struct device *dev)
>> >>               jzfb_enable(jzfb);
>> >>       mutex_unlock(&jzfb->lock);
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       fb_set_suspend(jzfb->fb, 0);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
>> >> index c89f8a8..da25fa1 100644
>> >> --- a/drivers/video/mx3fb.c
>> >> +++ b/drivers/video/mx3fb.c
>> >> @@ -23,7 +23,6 @@
>> >>  #include <linux/ioport.h>
>> >>  #include <linux/dma-mapping.h>
>> >>  #include <linux/dmaengine.h>
>> >> -#include <linux/console.h>
>> >>  #include <linux/clk.h>
>> >>  #include <linux/mutex.h>
>> >>
>> >> @@ -1204,9 +1203,9 @@ static int mx3fb_suspend(struct platform_device
>> >> *pdev, pm_message_t state)
>> >>       struct mx3fb_data *mx3fb = platform_get_drvdata(pdev);
>> >>       struct mx3fb_info *mx3_fbi = mx3fb->fbi->par;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       fb_set_suspend(mx3fb->fbi, 1);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       if (mx3_fbi->blank = FB_BLANK_UNBLANK) {
>> >>               sdc_disable_channel(mx3_fbi);
>> >> @@ -1229,9 +1228,9 @@ static int mx3fb_resume(struct platform_device *pdev)
>> >>               sdc_set_brightness(mx3fb, mx3fb->backlight_level);
>> >>       }
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       fb_set_suspend(mx3fb->fbi, 0);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >> diff --git a/drivers/video/nvidia/nvidia.c b/drivers/video/nvidia/nvidia.c
>> >> index fe13ac5..f47eb2a 100644
>> >> --- a/drivers/video/nvidia/nvidia.c
>> >> +++ b/drivers/video/nvidia/nvidia.c
>> >> @@ -19,7 +19,6 @@
>> >>  #include <linux/fb.h>
>> >>  #include <linux/init.h>
>> >>  #include <linux/pci.h>
>> >> -#include <linux/console.h>
>> >>  #include <linux/backlight.h>
>> >>  #ifdef CONFIG_MTRR
>> >>  #include <asm/mtrr.h>
>> >> @@ -1057,7 +1056,7 @@ static int nvidiafb_suspend(struct pci_dev *dev,
>> >> pm_message_t mesg)
>> >>
>> >>       if (mesg.event = PM_EVENT_PRETHAW)
>> >>               mesg.event = PM_EVENT_FREEZE;
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       par->pm_state = mesg.event;
>> >>
>> >>       if (mesg.event & PM_EVENT_SLEEP) {
>> >> @@ -1070,7 +1069,7 @@ static int nvidiafb_suspend(struct pci_dev *dev,
>> >> pm_message_t mesg)
>> >>       }
>> >>       dev->dev.power.power_state = mesg;
>> >>
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>       return 0;
>> >>  }
>> >>
>> >> @@ -1079,7 +1078,7 @@ static int nvidiafb_resume(struct pci_dev *dev)
>> >>       struct fb_info *info = pci_get_drvdata(dev);
>> >>       struct nvidia_par *par = info->par;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       pci_set_power_state(dev, PCI_D0);
>> >>
>> >>       if (par->pm_state != PM_EVENT_FREEZE) {
>> >> @@ -1097,7 +1096,7 @@ static int nvidiafb_resume(struct pci_dev *dev)
>> >>       nvidiafb_blank(FB_BLANK_UNBLANK, info);
>> >>
>> >>  fail:
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>       return 0;
>> >>  }
>> >>  #else
>> >> diff --git a/drivers/video/ps3fb.c b/drivers/video/ps3fb.c
>> >> index 213fbbc..cb2e174 100644
>> >> --- a/drivers/video/ps3fb.c
>> >> +++ b/drivers/video/ps3fb.c
>> >> @@ -23,7 +23,6 @@
>> >>  #include <linux/string.h>
>> >>  #include <linux/mm.h>
>> >>  #include <linux/interrupt.h>
>> >> -#include <linux/console.h>
>> >>  #include <linux/ioctl.h>
>> >>  #include <linux/kthread.h>
>> >>  #include <linux/freezer.h>
>> >> @@ -515,7 +514,7 @@ static int ps3fb_release(struct fb_info *info, int user)
>> >>                       atomic_set(&ps3fb.ext_flip, 0);
>> >>                       if (console_trylock()) {
>> >>                               ps3fb_sync(info, 0);    /* single buffer */
>> >> -                             console_unlock();
>> >> +                             fb_console_unlock();
>> >>                       }
>> >>               }
>> >>       }
>> >> @@ -830,14 +829,14 @@ static int ps3fb_ioctl(struct fb_info *info,
>> >> unsigned int cmd,
>> >>                       if (vmode) {
>> >>                               var = info->var;
>> >>                               fb_videomode_to_var(&var, vmode);
>> >> -                             console_lock();
>> >> +                             fb_console_lock();
>> >>                               info->flags |= FBINFO_MISC_USEREVENT;
>> >>                               /* Force, in case only special bits changed */
>> >>                               var.activate |= FB_ACTIVATE_FORCE;
>> >>                               par->new_mode_id = val;
>> >>                               retval = fb_set_var(info, &var);
>> >>                               info->flags &= ~FBINFO_MISC_USEREVENT;
>> >> -                             console_unlock();
>> >> +                             fb_console_unlock();
>> >>                       }
>> >>                       break;
>> >>               }
>> >> @@ -881,9 +880,9 @@ static int ps3fb_ioctl(struct fb_info *info,
>> >> unsigned int cmd,
>> >>                       break;
>> >>
>> >>               dev_dbg(info->device, "PS3FB_IOCTL_FSEL:%d\n", val);
>> >> -             console_lock();
>> >> +             fb_console_lock();
>> >>               retval = ps3fb_sync(info, val);
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>               break;
>> >>
>> >>       default:
>> >> @@ -903,9 +902,9 @@ static int ps3fbd(void *arg)
>> >>               set_current_state(TASK_INTERRUPTIBLE);
>> >>               if (ps3fb.is_kicked) {
>> >>                       ps3fb.is_kicked = 0;
>> >> -                     console_lock();
>> >> +                     fb_console_lock();
>> >>                       ps3fb_sync(info, 0);    /* single buffer */
>> >> -                     console_unlock();
>> >> +                     fb_console_unlock();
>> >>               }
>> >>               schedule();
>> >>       }
>> >> diff --git a/drivers/video/pxafb.c b/drivers/video/pxafb.c
>> >> index 3f90255..a1af212 100644
>> >> --- a/drivers/video/pxafb.c
>> >> +++ b/drivers/video/pxafb.c
>> >> @@ -54,7 +54,6 @@
>> >>  #include <linux/mutex.h>
>> >>  #include <linux/kthread.h>
>> >>  #include <linux/freezer.h>
>> >> -#include <linux/console.h>
>> >>
>> >>  #include <mach/hardware.h>
>> >>  #include <asm/io.h>
>> >> @@ -733,9 +732,9 @@ static int overlayfb_open(struct fb_info *info, int user)
>> >>
>> >>       if (ofb->usage++ = 0) {
>> >>               /* unblank the base framebuffer */
>> >> -             console_lock();
>> >> +             fb_console_lock();
>> >>               fb_blank(&ofb->fbi->fb, FB_BLANK_UNBLANK);
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>       }
>> >>
>> >>       return 0;
>> >> diff --git a/drivers/video/s3fb.c b/drivers/video/s3fb.c
>> >> index 1d00736..7759c82 100644
>> >> --- a/drivers/video/s3fb.c
>> >> +++ b/drivers/video/s3fb.c
>> >> @@ -22,7 +22,6 @@
>> >>  #include <linux/svga.h>
>> >>  #include <linux/init.h>
>> >>  #include <linux/pci.h>
>> >> -#include <linux/console.h> /* Why should fb driver call console
>> >> functions? because console_lock() */
>> >>  #include <video/vga.h>
>> >>
>> >>  #include <linux/i2c.h>
>> >> @@ -1445,12 +1444,12 @@ static int s3_pci_suspend(struct pci_dev* dev,
>> >> pm_message_t state)
>> >>
>> >>       dev_info(info->device, "suspend\n");
>> >>
>> >> -     console_lock();
>> >> +     fb_fb_console_lock();
>> >>       mutex_lock(&(par->open_lock));
>> >>
>> >>       if ((state.event = PM_EVENT_FREEZE) || (par->ref_count = 0)) {
>> >>               mutex_unlock(&(par->open_lock));
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>               return 0;
>> >>       }
>> >>
>> >> @@ -1461,7 +1460,7 @@ static int s3_pci_suspend(struct pci_dev* dev,
>> >> pm_message_t state)
>> >>       pci_set_power_state(dev, pci_choose_state(dev, state));
>> >>
>> >>       mutex_unlock(&(par->open_lock));
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >> @@ -1477,12 +1476,12 @@ static int s3_pci_resume(struct pci_dev* dev)
>> >>
>> >>       dev_info(info->device, "resume\n");
>> >>
>> >> -     console_lock();
>> >> +     fb_fb_console_lock();
>> >>       mutex_lock(&(par->open_lock));
>> >>
>> >>       if (par->ref_count = 0) {
>> >>               mutex_unlock(&(par->open_lock));
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>               return 0;
>> >>       }
>> >>
>> >> @@ -1491,7 +1490,7 @@ static int s3_pci_resume(struct pci_dev* dev)
>> >>       err = pci_enable_device(dev);
>> >>       if (err) {
>> >>               mutex_unlock(&(par->open_lock));
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>               dev_err(info->device, "error %d enabling device for resume\n", err);
>> >>               return err;
>> >>       }
>> >> @@ -1501,7 +1500,7 @@ static int s3_pci_resume(struct pci_dev* dev)
>> >>       fb_set_suspend(info, 0);
>> >>
>> >>       mutex_unlock(&(par->open_lock));
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >> diff --git a/drivers/video/savage/savagefb_driver.c
>> >> b/drivers/video/savage/savagefb_driver.c
>> >> index 0d0f52c..bf64221 100644
>> >> --- a/drivers/video/savage/savagefb_driver.c
>> >> +++ b/drivers/video/savage/savagefb_driver.c
>> >> @@ -51,7 +51,6 @@
>> >>  #include <linux/fb.h>
>> >>  #include <linux/pci.h>
>> >>  #include <linux/init.h>
>> >> -#include <linux/console.h>
>> >>
>> >>  #include <asm/io.h>
>> >>  #include <asm/irq.h>
>> >> @@ -2391,7 +2390,7 @@ static int savagefb_suspend(struct pci_dev *dev,
>> >> pm_message_t mesg)
>> >>       if (mesg.event = PM_EVENT_FREEZE)
>> >>               return 0;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       fb_set_suspend(info, 1);
>> >>
>> >>       if (info->fbops->fb_sync)
>> >> @@ -2403,7 +2402,7 @@ static int savagefb_suspend(struct pci_dev *dev,
>> >> pm_message_t mesg)
>> >>       pci_save_state(dev);
>> >>       pci_disable_device(dev);
>> >>       pci_set_power_state(dev, pci_choose_state(dev, mesg));
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >> @@ -2427,7 +2426,7 @@ static int savagefb_resume(struct pci_dev* dev)
>> >>               return 0;
>> >>       }
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>
>> >>       pci_set_power_state(dev, PCI_D0);
>> >>       pci_restore_state(dev);
>> >> @@ -2441,7 +2440,7 @@ static int savagefb_resume(struct pci_dev* dev)
>> >>       savagefb_set_par(info);
>> >>       fb_set_suspend(info, 0);
>> >>       savagefb_blank(FB_BLANK_UNBLANK, info);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >> diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
>> >> index 699487c..2bc1119 100644
>> >> --- a/drivers/video/sh_mobile_lcdcfb.c
>> >> +++ b/drivers/video/sh_mobile_lcdcfb.c
>> >> @@ -11,7 +11,6 @@
>> >>  #include <linux/atomic.h>
>> >>  #include <linux/backlight.h>
>> >>  #include <linux/clk.h>
>> >> -#include <linux/console.h>
>> >>  #include <linux/ctype.h>
>> >>  #include <linux/dma-mapping.h>
>> >>  #include <linux/delay.h>
>> >> @@ -575,7 +574,7 @@ static int sh_mobile_lcdc_display_notify(struct
>> >> sh_mobile_lcdc_chan *ch,
>> >>       case SH_MOBILE_LCDC_EVENT_DISPLAY_CONNECT:
>> >>               /* HDMI plug in */
>> >>               if (lock_fb_info(info)) {
>> >> -                     console_lock();
>> >> +                     fb_console_lock();
>> >>
>> >>                       ch->display.width = monspec->max_x * 10;
>> >>                       ch->display.height = monspec->max_y * 10;
>> >> @@ -594,7 +593,7 @@ static int sh_mobile_lcdc_display_notify(struct
>> >> sh_mobile_lcdc_chan *ch,
>> >>                               fb_set_suspend(info, 0);
>> >>                       }
>> >>
>> >> -                     console_unlock();
>> >> +                     fb_console_unlock();
>> >>                       unlock_fb_info(info);
>> >>               }
>> >>               break;
>> >> @@ -602,9 +601,9 @@ static int sh_mobile_lcdc_display_notify(struct
>> >> sh_mobile_lcdc_chan *ch,
>> >>       case SH_MOBILE_LCDC_EVENT_DISPLAY_DISCONNECT:
>> >>               /* HDMI disconnect */
>> >>               if (lock_fb_info(info)) {
>> >> -                     console_lock();
>> >> +                     fb_console_lock();
>> >>                       fb_set_suspend(info, 1);
>> >> -                     console_unlock();
>> >> +                     fb_console_unlock();
>> >>                       unlock_fb_info(info);
>> >>               }
>> >>               break;
>> >> @@ -1934,9 +1933,9 @@ static int sh_mobile_lcdc_release(struct fb_info
>> >> *info, int user)
>> >>
>> >>       /* Nothing to reconfigure, when called from fbcon */
>> >>       if (user) {
>> >> -             console_lock();
>> >> +             fb_console_lock();
>> >>               sh_mobile_fb_reconfig(info);
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>       }
>> >>
>> >>       mutex_unlock(&ch->open_lock);
>> >> diff --git a/drivers/video/sm501fb.c b/drivers/video/sm501fb.c
>> >> index 3690eff..536f922 100644
>> >> --- a/drivers/video/sm501fb.c
>> >> +++ b/drivers/video/sm501fb.c
>> >> @@ -28,7 +28,6 @@
>> >>  #include <linux/wait.h>
>> >>  #include <linux/platform_device.h>
>> >>  #include <linux/clk.h>
>> >> -#include <linux/console.h>
>> >>  #include <linux/io.h>
>> >>
>> >>  #include <asm/uaccess.h>
>> >> @@ -2104,9 +2103,9 @@ static int sm501fb_suspend_fb(struct sm501fb_info *info,
>> >>
>> >>       /* tell console/fb driver we are suspending */
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       fb_set_suspend(fbi, 1);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       /* backup copies in case chip is powered down over suspend */
>> >>
>> >> @@ -2163,9 +2162,9 @@ static void sm501fb_resume_fb(struct sm501fb_info *info,
>> >>               memcpy_toio(par->cursor.k_addr, par->store_cursor,
>> >>                           par->cursor.size);
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       fb_set_suspend(fbi, 0);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       vfree(par->store_fb);
>> >>       vfree(par->store_cursor);
>> >> diff --git a/drivers/video/tmiofb.c b/drivers/video/tmiofb.c
>> >> index 8e4a446..e266b6b 100644
>> >> --- a/drivers/video/tmiofb.c
>> >> +++ b/drivers/video/tmiofb.c
>> >> @@ -25,8 +25,7 @@
>> >>  #include <linux/fb.h>
>> >>  #include <linux/interrupt.h>
>> >>  #include <linux/delay.h>
>> >> -/* Why should fb driver call console functions? because console_lock() */
>> >> -#include <linux/console.h>
>> >> +/* Why should fb driver call console functions? because fb_console_lock() */
>> >>  #include <linux/mfd/core.h>
>> >>  #include <linux/mfd/tmio.h>
>> >>  #include <linux/uaccess.h>
>> >> @@ -938,7 +937,7 @@ static int tmiofb_suspend(struct platform_device
>> >> *dev, pm_message_t state)
>> >>       const struct mfd_cell *cell = mfd_get_cell(dev);
>> >>       int retval = 0;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>
>> >>       fb_set_suspend(info, 1);
>> >>
>> >> @@ -959,7 +958,7 @@ static int tmiofb_suspend(struct platform_device
>> >> *dev, pm_message_t state)
>> >>       if (cell->suspend)
>> >>               retval = cell->suspend(dev);
>> >>
>> >> -     console_unlock();
>> >> +     fb_fb_console_unlock();
>> >>
>> >>       return retval;
>> >>  }
>> >> @@ -970,7 +969,7 @@ static int tmiofb_resume(struct platform_device *dev)
>> >>       const struct mfd_cell *cell = mfd_get_cell(dev);
>> >>       int retval = 0;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>
>> >>       if (cell->resume) {
>> >>               retval = cell->resume(dev);
>> >> @@ -986,7 +985,7 @@ static int tmiofb_resume(struct platform_device *dev)
>> >>
>> >>       fb_set_suspend(info, 0);
>> >>  out:
>> >> -     console_unlock();
>> >> +     fb_fb_console_unlock();
>> >>       return retval;
>> >>  }
>> >>  #else
>> >> diff --git a/drivers/video/via/viafbdev.c b/drivers/video/via/viafbdev.c
>> >> index c80e770..d36b173 100644
>> >> --- a/drivers/video/via/viafbdev.c
>> >> +++ b/drivers/video/via/viafbdev.c
>> >> @@ -1698,17 +1698,17 @@ static int parse_mode(const char *str, u32
>> >> devices, u32 *xres, u32 *yres)
>> >>  #ifdef CONFIG_PM
>> >>  static int viafb_suspend(void *unused)
>> >>  {
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       fb_set_suspend(viafbinfo, 1);
>> >>       viafb_sync(viafbinfo);
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >>
>> >>  static int viafb_resume(void *unused)
>> >>  {
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       if (viaparinfo->shared->vdev->engine_mmio)
>> >>               viafb_reset_engine(viaparinfo);
>> >>       viafb_set_par(viafbinfo);
>> >> @@ -1716,7 +1716,7 @@ static int viafb_resume(void *unused)
>> >>               viafb_set_par(viafbinfo1);
>> >>       fb_set_suspend(viafbinfo, 0);
>> >>
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>       return 0;
>> >>  }
>> >>
>> >> diff --git a/drivers/video/vt8623fb.c b/drivers/video/vt8623fb.c
>> >> index 4e74d26..cb48d4d 100644
>> >> --- a/drivers/video/vt8623fb.c
>> >> +++ b/drivers/video/vt8623fb.c
>> >> @@ -23,7 +23,6 @@
>> >>  #include <linux/svga.h>
>> >>  #include <linux/init.h>
>> >>  #include <linux/pci.h>
>> >> -#include <linux/console.h> /* Why should fb driver call console
>> >> functions? because console_lock() */
>> >>  #include <video/vga.h>
>> >>
>> >>  #ifdef CONFIG_MTRR
>> >> @@ -845,12 +844,12 @@ static int vt8623_pci_suspend(struct pci_dev*
>> >> dev, pm_message_t state)
>> >>
>> >>       dev_info(info->device, "suspend\n");
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       mutex_lock(&(par->open_lock));
>> >>
>> >>       if ((state.event = PM_EVENT_FREEZE) || (par->ref_count = 0)) {
>> >>               mutex_unlock(&(par->open_lock));
>> >> -             console_unlock();
>> >> +             fb_console_unlock();
>> >>               return 0;
>> >>       }
>> >>
>> >> @@ -861,7 +860,7 @@ static int vt8623_pci_suspend(struct pci_dev* dev,
>> >> pm_message_t state)
>> >>       pci_set_power_state(dev, pci_choose_state(dev, state));
>> >>
>> >>       mutex_unlock(&(par->open_lock));
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >> @@ -876,7 +875,7 @@ static int vt8623_pci_resume(struct pci_dev* dev)
>> >>
>> >>       dev_info(info->device, "resume\n");
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       mutex_lock(&(par->open_lock));
>> >>
>> >>       if (par->ref_count = 0)
>> >> @@ -895,7 +894,7 @@ static int vt8623_pci_resume(struct pci_dev* dev)
>> >>
>> >>  fail:
>> >>       mutex_unlock(&(par->open_lock));
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>
>> >>       return 0;
>> >>  }
>> >> diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
>> >> index b7f5173..770397e7 100644
>> >> --- a/drivers/video/xen-fbfront.c
>> >> +++ b/drivers/video/xen-fbfront.c
>> >> @@ -18,7 +18,6 @@
>> >>   * frame buffer.
>> >>   */
>> >>
>> >> -#include <linux/console.h>
>> >>  #include <linux/kernel.h>
>> >>  #include <linux/errno.h>
>> >>  #include <linux/fb.h>
>> >> @@ -495,12 +494,12 @@ xenfb_make_preferred_console(void)
>> >>       if (console_set_on_cmdline)
>> >>               return;
>> >>
>> >> -     console_lock();
>> >> +     fb_console_lock();
>> >>       for_each_console(c) {
>> >>               if (!strcmp(c->name, "tty") && c->index = 0)
>> >>                       break;
>> >>       }
>> >> -     console_unlock();
>> >> +     fb_console_unlock();
>> >>       if (c) {
>> >>               unregister_console(c);
>> >>               c->flags |= CON_CONSDEV;
>> >> diff --git a/include/linux/fb.h b/include/linux/fb.h
>> >> index ac3f1c6..7ae96ad 100644
>> >> --- a/include/linux/fb.h
>> >> +++ b/include/linux/fb.h
>> >> @@ -1021,6 +1021,8 @@ extern int fb_get_color_depth(struct
>> >> fb_var_screeninfo *var,
>> >>                             struct fb_fix_screeninfo *fix);
>> >>  extern int fb_get_options(char *name, char **option);
>> >>  extern int fb_new_modelist(struct fb_info *info);
>> >> +extern void fb_console_unlock(void);
>> >> +extern void fb_console_lock(void);
>> >>
>> >>  extern struct fb_info *registered_fb[FB_MAX];
>> >>  extern int num_registered_fb;
>> >
>> >
>
>

^ permalink raw reply

* [PATCH V4 0/5] OMAPDSS: Enable dynamic debug printing
From: Chandrabhanu Mahapatra @ 2012-09-29 10:51 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1348826106.git.cmahapatra@ti.com>

Hi everyone,
this patch series aims at cleaning up of DSS of printk()'s enabled with
dss_debug and replace them with generic dynamic debug printing.

The 1st patch
 * moved DEBUG flag definition to Makefile
The 2nd patch
 * created two debug config options OMAP2_DSS_DEBUG and OMAP2_DSS_DEBUGFS
The 3rd patch
 * replaces printk() in DSSDBG definition with pr_debug()
 * removes DSSDBGF definition and replaces its instances with DSSDBG() 
The 4th patch
 * cleans up printk()'s in omap_dispc_unregister_isr() and 
   _dsi_print_reset_status() with pr_debug()
The 5th patch
 * removes dss_debug variable

Changes from V1 to V2:
 * added debug messages to DSSDBG calls
 * added patch "OMAPDSS: Remove dss_debug variable"

Changes from V2 to V3
 * added patch "OMAPDSS: Move definition of DEBUG flag to Makefile"

Changes from V3 to V4:
 * added patch "OMAPDSS: Create new debug config options"
 * broke earlier patch "OMAPDSS: Remove dss_debug variable" into two parts as
   "OMAPDSS: Replace multi part debug prints with pr_debug" and
   "OMAPDSS: Remove dss_debug variable"

All your comments and suggestions are welcome.

Refenence Tree:
git://gitorious.org/linux-omap-dss2/chandrabhanus-linux.git dss_cleanup

Regards,
Chandrabhanu

Chandrabhanu Mahapatra (5):
  OMAPDSS: Move definition of DEBUG flag to Makefile
  OMAPDSS: Create new debug config options
  OMAPDSS: Cleanup DSSDBG with dynamic pr_debug function
  OMAPDSS: Replace multi part debug prints with pr_debug
  OMAPDSS: Remove dss_debug variable

 drivers/video/omap2/dss/Kconfig  |   21 +++++++++++-----
 drivers/video/omap2/dss/Makefile |    1 +
 drivers/video/omap2/dss/apply.c  |    8 +++----
 drivers/video/omap2/dss/core.c   |   11 +++------
 drivers/video/omap2/dss/dispc.c  |   40 ++++++++++++-------------------
 drivers/video/omap2/dss/dsi.c    |   49 ++++++++++++++++----------------------
 drivers/video/omap2/dss/dss.c    |    2 +-
 drivers/video/omap2/dss/dss.h    |   40 ++++++-------------------------
 8 files changed, 66 insertions(+), 106 deletions(-)

-- 
1.7.10


^ permalink raw reply

* [PATCH V4 1/5] OMAPDSS: Move definition of DEBUG flag to Makefile
From: Chandrabhanu Mahapatra @ 2012-09-29 10:51 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1348914940.git.cmahapatra@ti.com>

In OMAPDSS the DEBUG flag is set only after the OMAPDSS module is called, for
which the debugging capabilities are available only after its proper
initialization. As a result of which tracking of bugs prior to or during initial
process becomes difficult. So, the definition of DEBUG is being moved to the
corresponding Makefile.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/Makefile |    1 +
 drivers/video/omap2/dss/dss.h    |    4 ----
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/Makefile b/drivers/video/omap2/dss/Makefile
index 4549869..86493e3 100644
--- a/drivers/video/omap2/dss/Makefile
+++ b/drivers/video/omap2/dss/Makefile
@@ -8,3 +8,4 @@ omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
 omapdss-$(CONFIG_OMAP2_DSS_DSI) += dsi.o
 omapdss-$(CONFIG_OMAP4_DSS_HDMI) += hdmi.o \
 				    hdmi_panel.o ti_hdmi_4xxx_ip.o
+ccflags-$(CONFIG_OMAP2_DSS_DEBUG_SUPPORT) += -DDEBUG
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 6728892..ffbba7e 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -23,10 +23,6 @@
 #ifndef __OMAP2_DSS_H
 #define __OMAP2_DSS_H
 
-#ifdef CONFIG_OMAP2_DSS_DEBUG_SUPPORT
-#define DEBUG
-#endif
-
 #ifdef DEBUG
 extern bool dss_debug;
 #ifdef DSS_SUBSYS_NAME
-- 
1.7.10


^ permalink raw reply related

* [PATCH V4 2/5] OMAPDSS: Create new debug config options
From: Chandrabhanu Mahapatra @ 2012-09-29 10:52 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1348914940.git.cmahapatra@ti.com>

The config option CONFIG_OMAP2_DSS_DEBUG_SUPPORT has been removed and replaced
with CONFIG_OMAP2_DSS_DEBUG and CONFIG_OMAP2_DSS_DEBUGFS. CONFIG_OMAP2_DSS_DEBUG
enables DEBUG flag and CONFIG_OMAP2_DSS_DEBUGFS enables creation of debugfs for
OMAPDSS. Both the config options are disabled by default and can be enabled
independently of one another as per convenience.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/Kconfig  |   21 +++++++++++++++------
 drivers/video/omap2/dss/Makefile |    2 +-
 drivers/video/omap2/dss/core.c   |    6 +++---
 drivers/video/omap2/dss/dss.c    |    2 +-
 drivers/video/omap2/dss/dss.h    |    2 +-
 5 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/video/omap2/dss/Kconfig b/drivers/video/omap2/dss/Kconfig
index 80f5390..866a563 100644
--- a/drivers/video/omap2/dss/Kconfig
+++ b/drivers/video/omap2/dss/Kconfig
@@ -18,16 +18,25 @@ config OMAP2_VRAM_SIZE
 	  You can also set this with "vram=<bytes>" kernel argument, or
 	  in the board file.
 
-config OMAP2_DSS_DEBUG_SUPPORT
-        bool "Debug support"
-	default y
+config OMAP2_DSS_DEBUG
+	bool "Debug support"
+	default n
+	help
+	  This enables printing of debug messages. Alternatively, debug messages
+	  can also be enabled by setting CONFIG_DYNAMIC_DEBUG and then setting
+	  appropriate flags in <debugfs>/dynamic_debug/control.
+
+config OMAP2_DSS_DEBUGFS
+	bool "Debugfs filesystem support"
+	default n
 	help
-	  This enables debug messages. You need to enable printing
-	  with 'debug' module parameter.
+	  This enables debugfs for OMAPDSS at <debugfs>/omapdss. This enables
+	  querying about clock configuration and register configuration of dss,
+	  dispc, dsi, hdmi and rfbi.
 
 config OMAP2_DSS_COLLECT_IRQ_STATS
 	bool "Collect DSS IRQ statistics"
-	depends on OMAP2_DSS_DEBUG_SUPPORT
+	depends on OMAP2_DSS_DEBUGFS
 	default n
 	help
 	  Collect DSS IRQ statistics, printable via debugfs.
diff --git a/drivers/video/omap2/dss/Makefile b/drivers/video/omap2/dss/Makefile
index 86493e3..4070191 100644
--- a/drivers/video/omap2/dss/Makefile
+++ b/drivers/video/omap2/dss/Makefile
@@ -8,4 +8,4 @@ omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
 omapdss-$(CONFIG_OMAP2_DSS_DSI) += dsi.o
 omapdss-$(CONFIG_OMAP4_DSS_HDMI) += hdmi.o \
 				    hdmi_panel.o ti_hdmi_4xxx_ip.o
-ccflags-$(CONFIG_OMAP2_DSS_DEBUG_SUPPORT) += -DDEBUG
+ccflags-$(CONFIG_OMAP2_DSS_DEBUG) += -DDEBUG
diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index b2af72d..1e1f50f 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -138,7 +138,7 @@ int dss_set_min_bus_tput(struct device *dev, unsigned long tput)
 		return 0;
 }
 
-#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUG_SUPPORT)
+#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUGFS)
 static int dss_debug_show(struct seq_file *s, void *unused)
 {
 	void (*func)(struct seq_file *) = s->private;
@@ -193,7 +193,7 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
 
 	return 0;
 }
-#else /* CONFIG_DEBUG_FS && CONFIG_OMAP2_DSS_DEBUG_SUPPORT */
+#else /* CONFIG_DEBUG_FS && CONFIG_OMAP2_DSS_DEBUGFS */
 static inline int dss_initialize_debugfs(void)
 {
 	return 0;
@@ -205,7 +205,7 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
 {
 	return 0;
 }
-#endif /* CONFIG_DEBUG_FS && CONFIG_OMAP2_DSS_DEBUG_SUPPORT */
+#endif /* CONFIG_DEBUG_FS && CONFIG_OMAP2_DSS_DEBUGFS */
 
 /* PLATFORM DEVICE */
 static int omap_dss_pm_notif(struct notifier_block *b, unsigned long v, void *d)
diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 2ab1c3e..fe30855 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -746,7 +746,7 @@ static void dss_runtime_put(void)
 }
 
 /* DEBUGFS */
-#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUG_SUPPORT)
+#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUGFS)
 void dss_debug_dump_clocks(struct seq_file *s)
 {
 	dss_dump_clocks(s);
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index ffbba7e..a8a1ab4 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -301,7 +301,7 @@ enum dss_hdmi_venc_clk_source_select dss_get_hdmi_venc_clk_source(void);
 const char *dss_get_generic_clk_source_name(enum omap_dss_clk_source clk_src);
 void dss_dump_clocks(struct seq_file *s);
 
-#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUG_SUPPORT)
+#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUGFS)
 void dss_debug_dump_clocks(struct seq_file *s);
 #endif
 
-- 
1.7.10


^ permalink raw reply related

* [PATCH V4 3/5] OMAPDSS: Cleanup DSSDBG with dynamic pr_debug function
From: Chandrabhanu Mahapatra @ 2012-09-29 10:52 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1348914940.git.cmahapatra@ti.com>

The printk in DSSDBG function definition is replaced with dynamic debug enabled
pr_debug(). The use of dynamic debugging provides more flexibility as each debug
statement can be enabled or disabled dynamically on basis of source filename,
line number, module name etc., by writing to a control file in debugfs
filesystem. For better understanding please refer to
Documentation/dynamic-debug-howto.txt.

The DSSDBGF() differs from DSSDBG() by providing function name. However,
function name, line number, module name and thread ID can be printed through
dynamic debug by setting appropriate flags 'f','l','m' and 't' in the debugfs
control file. So, DSSDBGF instances are replaced with DSSDBG.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/apply.c |    8 ++++----
 drivers/video/omap2/dss/dsi.c   |   12 ++++++------
 drivers/video/omap2/dss/dss.h   |   34 ++++++++--------------------------
 3 files changed, 18 insertions(+), 36 deletions(-)

diff --git a/drivers/video/omap2/dss/apply.c b/drivers/video/omap2/dss/apply.c
index 19d66f4..e923d9f 100644
--- a/drivers/video/omap2/dss/apply.c
+++ b/drivers/video/omap2/dss/apply.c
@@ -573,7 +573,7 @@ static void dss_ovl_write_regs(struct omap_overlay *ovl)
 	struct mgr_priv_data *mp;
 	int r;
 
-	DSSDBGF("%d", ovl->id);
+	DSSDBG("writing ovl %d regs", ovl->id);
 
 	if (!op->enabled || !op->info_dirty)
 		return;
@@ -608,7 +608,7 @@ static void dss_ovl_write_regs_extra(struct omap_overlay *ovl)
 	struct ovl_priv_data *op = get_ovl_priv(ovl);
 	struct mgr_priv_data *mp;
 
-	DSSDBGF("%d", ovl->id);
+	DSSDBG("writing ovl %d regs extra", ovl->id);
 
 	if (!op->extra_info_dirty)
 		return;
@@ -632,7 +632,7 @@ static void dss_mgr_write_regs(struct omap_overlay_manager *mgr)
 	struct mgr_priv_data *mp = get_mgr_priv(mgr);
 	struct omap_overlay *ovl;
 
-	DSSDBGF("%d", mgr->id);
+	DSSDBG("writing mgr %d regs", mgr->id);
 
 	if (!mp->enabled)
 		return;
@@ -658,7 +658,7 @@ static void dss_mgr_write_regs_extra(struct omap_overlay_manager *mgr)
 {
 	struct mgr_priv_data *mp = get_mgr_priv(mgr);
 
-	DSSDBGF("%d", mgr->id);
+	DSSDBG("writing mgr %d regs extra", mgr->id);
 
 	if (!mp->extra_info_dirty)
 		return;
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index e37e6d8..b0345f3 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -1612,7 +1612,7 @@ int dsi_pll_set_clock_div(struct platform_device *dsidev,
 	u8 regn_start, regn_end, regm_start, regm_end;
 	u8 regm_dispc_start, regm_dispc_end, regm_dsi_start, regm_dsi_end;
 
-	DSSDBGF();
+	DSSDBG("DSI PLL clock config starts");
 
 	dsi->current_cinfo.clkin = cinfo->clkin;
 	dsi->current_cinfo.fint = cinfo->fint;
@@ -2431,7 +2431,7 @@ static int dsi_cio_init(struct platform_device *dsidev)
 	int r;
 	u32 l;
 
-	DSSDBGF();
+	DSSDBG("DSI CIO init starts");
 
 	r = dss_dsi_enable_pads(dsi->module_id, dsi_get_lane_mask(dsidev));
 	if (r)
@@ -2782,7 +2782,7 @@ static void dsi_vc_initial_config(struct platform_device *dsidev, int channel)
 {
 	u32 r;
 
-	DSSDBGF("%d", channel);
+	DSSDBG("Initial config of virtual channel %d", channel);
 
 	r = dsi_read_reg(dsidev, DSI_VC_CTRL(channel));
 
@@ -2814,7 +2814,7 @@ static int dsi_vc_config_source(struct platform_device *dsidev, int channel,
 	if (dsi->vc[channel].source = source)
 		return 0;
 
-	DSSDBGF("%d", channel);
+	DSSDBG("Source config of virtual channel %d", channel);
 
 	dsi_sync_vc(dsidev, channel);
 
@@ -3572,7 +3572,7 @@ static int dsi_enter_ulps(struct platform_device *dsidev)
 	int r, i;
 	unsigned mask;
 
-	DSSDBGF();
+	DSSDBG("Entering ULPS");
 
 	WARN_ON(!dsi_bus_is_locked(dsidev));
 
@@ -4276,7 +4276,7 @@ int omapdss_dsi_set_clocks(struct omap_dss_device *dssdev,
 	unsigned long pck;
 	int r;
 
-	DSSDBGF("ddr_clk %lu, lp_clk %lu", ddr_clk, lp_clk);
+	DSSDBG("Setting DSI clocks: ddr_clk %lu, lp_clk %lu", ddr_clk, lp_clk);
 
 	mutex_lock(&dsi->lock);
 
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index a8a1ab4..8bef7ee 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -25,38 +25,20 @@
 
 #ifdef DEBUG
 extern bool dss_debug;
-#ifdef DSS_SUBSYS_NAME
-#define DSSDBG(format, ...) \
-	if (dss_debug) \
-		printk(KERN_DEBUG "omapdss " DSS_SUBSYS_NAME ": " format, \
-		## __VA_ARGS__)
-#else
-#define DSSDBG(format, ...) \
-	if (dss_debug) \
-		printk(KERN_DEBUG "omapdss: " format, ## __VA_ARGS__)
 #endif
 
-#ifdef DSS_SUBSYS_NAME
-#define DSSDBGF(format, ...) \
-	if (dss_debug) \
-		printk(KERN_DEBUG "omapdss " DSS_SUBSYS_NAME \
-				": %s(" format ")\n", \
-				__func__, \
-				## __VA_ARGS__)
-#else
-#define DSSDBGF(format, ...) \
-	if (dss_debug) \
-		printk(KERN_DEBUG "omapdss: " \
-				": %s(" format ")\n", \
-				__func__, \
-				## __VA_ARGS__)
+#ifdef pr_fmt
+#undef pr_fmt
 #endif
 
-#else /* DEBUG */
-#define DSSDBG(format, ...)
-#define DSSDBGF(format, ...)
+#ifdef DSS_SUBSYS_NAME
+#define pr_fmt(fmt) DSS_SUBSYS_NAME ": " fmt
+#else
+#define pr_fmt(fmt) fmt
 #endif
 
+#define DSSDBG(format, ...) \
+	pr_debug(format, ## __VA_ARGS__)
 
 #ifdef DSS_SUBSYS_NAME
 #define DSSERR(format, ...) \
-- 
1.7.10


^ permalink raw reply related

* [PATCH V4 4/5] OMAPDSS: Replace multi part debug prints with pr_debug
From: Chandrabhanu Mahapatra @ 2012-09-29 10:52 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1348914940.git.cmahapatra@ti.com>

The omap_dispc_unregister_isr() and _dsi_print_reset_status() consist of a
number of debug prints which need to be enabled all at once or none at all. So,
these debug prints in corresponding functions are replaced with one dynamic
debug enabled pr_debug() each.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/dispc.c |   32 +++++++++++++-------------------
 drivers/video/omap2/dss/dsi.c   |   30 ++++++++++++++----------------
 2 files changed, 27 insertions(+), 35 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index a173a94..67d9f3b 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -3675,26 +3675,20 @@ static void print_irq_status(u32 status)
 	if ((status & dispc.irq_error_mask) = 0)
 		return;
 
-	printk(KERN_DEBUG "DISPC IRQ: 0x%x: ", status);
-
-#define PIS(x) \
-	if (status & DISPC_IRQ_##x) \
-		printk(#x " ");
-	PIS(GFX_FIFO_UNDERFLOW);
-	PIS(OCP_ERR);
-	PIS(VID1_FIFO_UNDERFLOW);
-	PIS(VID2_FIFO_UNDERFLOW);
-	if (dss_feat_get_num_ovls() > 3)
-		PIS(VID3_FIFO_UNDERFLOW);
-	PIS(SYNC_LOST);
-	PIS(SYNC_LOST_DIGIT);
-	if (dss_has_feature(FEAT_MGR_LCD2))
-		PIS(SYNC_LOST2);
-	if (dss_has_feature(FEAT_MGR_LCD3))
-		PIS(SYNC_LOST3);
+#define PIS(x) (status & DISPC_IRQ_##x) ? (#x " ") : ""
+
+	pr_debug("DISPC IRQ: 0x%x: %s%s%s%s%s%s%s%s%s\n",
+		status,
+		PIS(OCP_ERR),
+		PIS(GFX_FIFO_UNDERFLOW),
+		PIS(VID1_FIFO_UNDERFLOW),
+		PIS(VID2_FIFO_UNDERFLOW),
+		dss_feat_get_num_ovls() > 3 ? PIS(VID3_FIFO_UNDERFLOW) : "",
+		PIS(SYNC_LOST),
+		PIS(SYNC_LOST_DIGIT),
+		dss_has_feature(FEAT_MGR_LCD2) ? PIS(SYNC_LOST2) : "",
+		dss_has_feature(FEAT_MGR_LCD3) ? PIS(SYNC_LOST3) : "");
 #undef PIS
-
-	printk("\n");
 }
 #endif
 
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index b0345f3..6dd073a 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -1121,14 +1121,6 @@ static void _dsi_print_reset_status(struct platform_device *dsidev)
 	 * I/O. */
 	l = dsi_read_reg(dsidev, DSI_DSIPHY_CFG5);
 
-	printk(KERN_DEBUG "DSI resets: ");
-
-	l = dsi_read_reg(dsidev, DSI_PLL_STATUS);
-	printk("PLL (%d) ", FLD_GET(l, 0, 0));
-
-	l = dsi_read_reg(dsidev, DSI_COMPLEXIO_CFG1);
-	printk("CIO (%d) ", FLD_GET(l, 29, 29));
-
 	if (dss_has_feature(FEAT_DSI_REVERSE_TXCLKESC)) {
 		b0 = 28;
 		b1 = 27;
@@ -1139,14 +1131,20 @@ static void _dsi_print_reset_status(struct platform_device *dsidev)
 		b2 = 26;
 	}
 
-	l = dsi_read_reg(dsidev, DSI_DSIPHY_CFG5);
-	printk("PHY (%x%x%x, %d, %d, %d)\n",
-			FLD_GET(l, b0, b0),
-			FLD_GET(l, b1, b1),
-			FLD_GET(l, b2, b2),
-			FLD_GET(l, 29, 29),
-			FLD_GET(l, 30, 30),
-			FLD_GET(l, 31, 31));
+#define DSI_FLD_GET(fld, start, end)\
+	FLD_GET(dsi_read_reg(dsidev, DSI_##fld), start, end)
+
+	pr_debug("DSI resets: PLL (%d) CIO (%d) PHY (%x%x%x, %d, %d, %d)\n",
+		DSI_FLD_GET(PLL_STATUS, 0, 0),
+		DSI_FLD_GET(COMPLEXIO_CFG1, 29, 29),
+		DSI_FLD_GET(DSIPHY_CFG5, b0, b0),
+		DSI_FLD_GET(DSIPHY_CFG5, b1, b1),
+		DSI_FLD_GET(DSIPHY_CFG5, b2, b2),
+		DSI_FLD_GET(DSIPHY_CFG5, 29, 29),
+		DSI_FLD_GET(DSIPHY_CFG5, 30, 30),
+		DSI_FLD_GET(DSIPHY_CFG5, 31, 31));
+
+#undef DSI_FLD_GET
 }
 #else
 #define _dsi_print_reset_status(x)
-- 
1.7.10


^ permalink raw reply related

* [PATCH V4 5/5] OMAPDSS: Remove dss_debug variable
From: Chandrabhanu Mahapatra @ 2012-09-29 10:52 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <cover.1348914940.git.cmahapatra@ti.com>

All the debug prints have been replaced with pr_debug(). Thus, the dependency on
dss_debug variable is replaced with dyndbg in dynamic debugging mode and DEBUG
flag otherwise. So, the dss_debug variable is removed along with checks for
DEBUG flag.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
 drivers/video/omap2/dss/core.c  |    5 -----
 drivers/video/omap2/dss/dispc.c |    8 ++------
 drivers/video/omap2/dss/dsi.c   |    7 -------
 drivers/video/omap2/dss/dss.h   |    4 ----
 4 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index 1e1f50f..9abcf43 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -53,11 +53,6 @@ static char *def_disp_name;
 module_param_named(def_disp, def_disp_name, charp, 0);
 MODULE_PARM_DESC(def_disp, "default display name");
 
-#ifdef DEBUG
-bool dss_debug;
-module_param_named(debug, dss_debug, bool, 0644);
-#endif
-
 const char *dss_get_default_display_name(void)
 {
 	return core.default_display_name;
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 67d9f3b..b5204b4 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -3669,7 +3669,6 @@ int omap_dispc_unregister_isr(omap_dispc_isr_t isr, void *arg, u32 mask)
 }
 EXPORT_SYMBOL(omap_dispc_unregister_isr);
 
-#ifdef DEBUG
 static void print_irq_status(u32 status)
 {
 	if ((status & dispc.irq_error_mask) = 0)
@@ -3690,7 +3689,6 @@ static void print_irq_status(u32 status)
 		dss_has_feature(FEAT_MGR_LCD3) ? PIS(SYNC_LOST3) : "");
 #undef PIS
 }
-#endif
 
 /* Called from dss.c. Note that we don't touch clocks here,
  * but we presume they are on because we got an IRQ. However,
@@ -3723,10 +3721,8 @@ static irqreturn_t omap_dispc_irq_handler(int irq, void *arg)
 	spin_unlock(&dispc.irq_stats_lock);
 #endif
 
-#ifdef DEBUG
-	if (dss_debug)
-		print_irq_status(irqstatus);
-#endif
+	print_irq_status(irqstatus);
+
 	/* Ack the interrupt. Do it here before clocks are possibly turned
 	 * off */
 	dispc_write_reg(DISPC_IRQSTATUS, irqstatus);
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index 6dd073a..4e41c7e 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -1107,15 +1107,11 @@ static inline void dsi_enable_pll_clock(struct platform_device *dsidev,
 	}
 }
 
-#ifdef DEBUG
 static void _dsi_print_reset_status(struct platform_device *dsidev)
 {
 	u32 l;
 	int b0, b1, b2;
 
-	if (!dss_debug)
-		return;
-
 	/* A dummy read using the SCP interface to any DSIPHY register is
 	 * required after DSIPHY reset to complete the reset of the DSI complex
 	 * I/O. */
@@ -1146,9 +1142,6 @@ static void _dsi_print_reset_status(struct platform_device *dsidev)
 
 #undef DSI_FLD_GET
 }
-#else
-#define _dsi_print_reset_status(x)
-#endif
 
 static inline int dsi_if_enable(struct platform_device *dsidev, bool enable)
 {
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index 8bef7ee..739cfeb 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -23,10 +23,6 @@
 #ifndef __OMAP2_DSS_H
 #define __OMAP2_DSS_H
 
-#ifdef DEBUG
-extern bool dss_debug;
-#endif
-
 #ifdef pr_fmt
 #undef pr_fmt
 #endif
-- 
1.7.10


^ permalink raw reply related

* Re: [PATCH V4 2/5] OMAPDSS: Create new debug config options
From: Sumit Semwal @ 2012-09-29 15:27 UTC (permalink / raw)
  To: Chandrabhanu Mahapatra; +Cc: tomi.valkeinen, linux-omap, linux-fbdev
In-Reply-To: <af097b99b4d2eab96850fd6d8a0c260917aa4f98.1348914940.git.cmahapatra@ti.com>

Hi Chandrabhanu,

On Saturday 29 September 2012 04:19 PM, Chandrabhanu Mahapatra wrote:
> The config option CONFIG_OMAP2_DSS_DEBUG_SUPPORT has been removed and replaced
> with CONFIG_OMAP2_DSS_DEBUG and CONFIG_OMAP2_DSS_DEBUGFS. CONFIG_OMAP2_DSS_DEBUG
> enables DEBUG flag and CONFIG_OMAP2_DSS_DEBUGFS enables creation of debugfs for
> OMAPDSS. Both the config options are disabled by default and can be enabled
> independently of one another as per convenience.
>
> Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
> ---
>   drivers/video/omap2/dss/Kconfig  |   21 +++++++++++++++------
>   drivers/video/omap2/dss/Makefile |    2 +-
>   drivers/video/omap2/dss/core.c   |    6 +++---
>   drivers/video/omap2/dss/dss.c    |    2 +-
>   drivers/video/omap2/dss/dss.h    |    2 +-
>   5 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/Kconfig b/drivers/video/omap2/dss/Kconfig
> index 80f5390..866a563 100644
> --- a/drivers/video/omap2/dss/Kconfig
> +++ b/drivers/video/omap2/dss/Kconfig
> @@ -18,16 +18,25 @@ config OMAP2_VRAM_SIZE
>   	  You can also set this with "vram=<bytes>" kernel argument, or
>   	  in the board file.
>
> -config OMAP2_DSS_DEBUG_SUPPORT
> -        bool "Debug support"
> -	default y
> +config OMAP2_DSS_DEBUG
> +	bool "Debug support"
> +	default n
> +	help
> +	  This enables printing of debug messages. Alternatively, debug messages
> +	  can also be enabled by setting CONFIG_DYNAMIC_DEBUG and then setting
> +	  appropriate flags in <debugfs>/dynamic_debug/control.
> +
> +config OMAP2_DSS_DEBUGFS
> +	bool "Debugfs filesystem support"
> +	default n
You can make it as 'depends on CONFIG_DEBUG_FS', so that your check 
below [1] becomes cleaner.
>   	help
> -	  This enables debug messages. You need to enable printing
> -	  with 'debug' module parameter.
> +	  This enables debugfs for OMAPDSS at <debugfs>/omapdss. This enables
> +	  querying about clock configuration and register configuration of dss,
> +	  dispc, dsi, hdmi and rfbi.
>
>   config OMAP2_DSS_COLLECT_IRQ_STATS
>   	bool "Collect DSS IRQ statistics"
> -	depends on OMAP2_DSS_DEBUG_SUPPORT
> +	depends on OMAP2_DSS_DEBUGFS
>   	default n
>   	help
>   	  Collect DSS IRQ statistics, printable via debugfs.
> diff --git a/drivers/video/omap2/dss/Makefile b/drivers/video/omap2/dss/Makefile
> index 86493e3..4070191 100644
> --- a/drivers/video/omap2/dss/Makefile
> +++ b/drivers/video/omap2/dss/Makefile
> @@ -8,4 +8,4 @@ omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
>   omapdss-$(CONFIG_OMAP2_DSS_DSI) += dsi.o
>   omapdss-$(CONFIG_OMAP4_DSS_HDMI) += hdmi.o \
>   				    hdmi_panel.o ti_hdmi_4xxx_ip.o
> -ccflags-$(CONFIG_OMAP2_DSS_DEBUG_SUPPORT) += -DDEBUG
> +ccflags-$(CONFIG_OMAP2_DSS_DEBUG) += -DDEBUG
> diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
> index b2af72d..1e1f50f 100644
> --- a/drivers/video/omap2/dss/core.c
> +++ b/drivers/video/omap2/dss/core.c
> @@ -138,7 +138,7 @@ int dss_set_min_bus_tput(struct device *dev, unsigned long tput)
>   		return 0;
>   }
>
> -#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUG_SUPPORT)
> +#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUGFS)
[1]: here.
>   static int dss_debug_show(struct seq_file *s, void *unused)
>   {
>   	void (*func)(struct seq_file *) = s->private;
> @@ -193,7 +193,7 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
>
>   	return 0;
>   }
> -#else /* CONFIG_DEBUG_FS && CONFIG_OMAP2_DSS_DEBUG_SUPPORT */
> +#else /* CONFIG_DEBUG_FS && CONFIG_OMAP2_DSS_DEBUGFS */
>   static inline int dss_initialize_debugfs(void)
>   {
>   	return 0;
> @@ -205,7 +205,7 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
>   {
>   	return 0;
>   }
> -#endif /* CONFIG_DEBUG_FS && CONFIG_OMAP2_DSS_DEBUG_SUPPORT */
> +#endif /* CONFIG_DEBUG_FS && CONFIG_OMAP2_DSS_DEBUGFS */
>
>   /* PLATFORM DEVICE */
>   static int omap_dss_pm_notif(struct notifier_block *b, unsigned long v, void *d)
> diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
> index 2ab1c3e..fe30855 100644
> --- a/drivers/video/omap2/dss/dss.c
> +++ b/drivers/video/omap2/dss/dss.c
> @@ -746,7 +746,7 @@ static void dss_runtime_put(void)
>   }
>
>   /* DEBUGFS */
> -#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUG_SUPPORT)
> +#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUGFS)
>   void dss_debug_dump_clocks(struct seq_file *s)
>   {
>   	dss_dump_clocks(s);
> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> index ffbba7e..a8a1ab4 100644
> --- a/drivers/video/omap2/dss/dss.h
> +++ b/drivers/video/omap2/dss/dss.h
> @@ -301,7 +301,7 @@ enum dss_hdmi_venc_clk_source_select dss_get_hdmi_venc_clk_source(void);
>   const char *dss_get_generic_clk_source_name(enum omap_dss_clk_source clk_src);
>   void dss_dump_clocks(struct seq_file *s);
>
> -#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUG_SUPPORT)
> +#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUGFS)
>   void dss_debug_dump_clocks(struct seq_file *s);
>   #endif
>
>


^ permalink raw reply

* Re: [PATCH] fb: only enable console lock in fb for VGA console
From: Paul Mundt @ 2012-09-30 12:17 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <CAGA24M+PmObK3ZJWvtNhBzxO4WkGk_xyveQiZNtZe7xGdHrbFw@mail.gmail.com>

On Sat, Sep 29, 2012 at 01:29:17PM +0800, Jun Nie wrote:
> diff --git a/drivers/video/s3fb.c b/drivers/video/s3fb.c
> index 1d00736..7759c82 100644
> --- a/drivers/video/s3fb.c
> +++ b/drivers/video/s3fb.c
> @@ -1445,12 +1444,12 @@ static int s3_pci_suspend(struct pci_dev* dev,
> pm_message_t state)
> 
>  	dev_info(info->device, "suspend\n");
> 
> -	console_lock();
> +	fb_fb_console_lock();
>  	mutex_lock(&(par->open_lock));
> 
Compiling is overrated anyways.

> diff --git a/drivers/video/tmiofb.c b/drivers/video/tmiofb.c
> index 8e4a446..e266b6b 100644
> --- a/drivers/video/tmiofb.c
> +++ b/drivers/video/tmiofb.c
> @@ -959,7 +958,7 @@ static int tmiofb_suspend(struct platform_device
> *dev, pm_message_t state)
>  	if (cell->suspend)
>  		retval = cell->suspend(dev);
> 
> -	console_unlock();
> +	fb_fb_console_unlock();
> 
>  	return retval;
>  }
> @@ -986,7 +985,7 @@ static int tmiofb_resume(struct platform_device *dev)
> 
>  	fb_set_suspend(info, 0);
>  out:
> -	console_unlock();
> +	fb_fb_console_unlock();
>  	return retval;
>  }
>  #else

Here too.

Turning locks in to no-ops without auditing every single driver's use of
said lock and converting to something to protect the work being done
under lock suggests that this conversion is more mechanical than thought
out beyond your specific use case.

While you've obviously identified a problem that's worth pursuing, it's
going to take a bit more work than mechanical conversion, and it's going
to have to be something that's moved towards incrementally. If you
haven't even compile tested the impacted drivers, it suggests you haven't
spent a great deal of time thinking about what they are doing under said
lock, either..

^ permalink raw reply

* Contact Email: dhldeliveryservice31@fengv.com
From: Piacentini Valeria @ 2012-09-30 20:10 UTC (permalink / raw)
  To: linux-fbdev




This is to re-notify you of the 21,870.56 us dollar  cheque that was deposited here in the DHL office in your  name. You should contact Dispatch Officer Collins Ulrich you are therefore advice to contact the delivery officer
Contact Email: dhldeliveryservice31@fengv.com
Contact Person: Collins Ulrich
Full Name:............ Address:.............
City:........Country:.......... Phone:.........
Regards,
Eric Morrow
United Nations donation coordinator

^ permalink raw reply

* Re: [PATCH V3] video: exynos_dp: Add device tree support to DP driver
From: Ajay kumar @ 2012-10-01  5:52 UTC (permalink / raw)
  To: Jingoo Han, linux-fbdev
  Cc: Tomasz Figa, linux-samsung-soc, FlorianSchandinat, thomas.ab,
	devicetree-discuss
In-Reply-To: <000601cd9d0d$c3296ca0$497c45e0$%han@samsung.com>

On Fri, Sep 28, 2012 at 5:41 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> On Thursday, September 27, 2012 10:45 PM Sylwester Nawrocki wrote
>>
>> Hi,
>>
>> Cc: devicetree-discuss@lists.ozlabs.org
>>
>> On 09/24/2012 09:36 PM, Ajay Kumar wrote:
>> > This patch enables device tree based discovery support for DP driver.
>> > The driver is modified to handle platform data in both the cases:
>> > with DT and non-DT.
>> > Documentation is also added for the DT bindings.
>> >
>> > DP-PHY should be regarded as a seperate device node while
>> > being passed from device tree list, and device node for
>> > DP should contain DP-PHY as child node with property name "dp-phy"
>> > associated with it.
>> >
>> > Signed-off-by: Ajay Kumar<ajaykumar.rs@samsung.com>
>> > ---
>> >   .../devicetree/bindings/video/exynos_dp.txt        |   83 ++++++++++
>> >   drivers/video/exynos/exynos_dp_core.c              |  168 ++++++++++++++++++--
>> >   drivers/video/exynos/exynos_dp_core.h              |    2 +
>> >   3 files changed, 239 insertions(+), 14 deletions(-)
>> >   create mode 100644 Documentation/devicetree/bindings/video/exynos_dp.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/video/exynos_dp.txt
>> b/Documentation/devicetree/bindings/video/exynos_dp.txt
>> > new file mode 100644
>> > index 0000000..c27f892
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/video/exynos_dp.txt
>> > @@ -0,0 +1,83 @@
>> > +Exynos Displayport driver should configure the displayport interface
>>
>> Don't we need a whitespace between 'display' and 'port' ?
>
> Either 'display port' or 'DisplayPort' is commonly used.
> For clarity, display port would be better.
Ok. I will change it.
>>
>> > +based on the type of panel connected to it.
>> > +
>> > +We use two nodes:
>> > +   -dptx_phy node
>> > +   -display-port-controller node
>> > +
>> > +For the dp-phy initialization, we use a dptx_phy node.
>> > +Required properties for dptx_phy:
>> > +   -compatible:
>> > +           Should be "samsung,dp-phy".
>> > +   -samsung,dptx_phy_reg:
>> > +           Base address of DP PHY register.
>>
>> Couldn't just 'reg' be used for this one ?
>>
>> > +   -samsung,enable_bit:
>> > +           The bit used to enable/disable DP PHY.
>>
>> Is this the bit mask or the bit index ? In the code it's used as
>> a bitmask. But from description it is not clear whether it is
>> an index or a mask. Is it different across various SoCs ?
>>
>> Perhaps it's better to name it samsung,enable_mask (in case some
>> SoC need more than one bit) ?
>
> It's the bit mask.
> It is different across various SoCs.
> OK, enable_mask would be better.
Ok. I will change it.
>>
>> > +
>> > +For the Panel initialization, we read data from display-port-controller node.
>> > +Required properties for display-port-controller:
>> > +   -compatible:
>> > +           Should be "samsung,exynos5-dp".
>> > +   -reg:
>> > +           physical base address of the controller and length
>> > +           of memory mapped region.
>> > +   -interrupts:
>> > +           Internet combiner values.
>>
>> what? :)
>
> Ajay: Please, fix it.
My bad :(...I will fix it.
>>
>> > +   -interrupt-parent:
>> > +           Address of Interrupt combiner node.
>> > +   -dp_phy:
>> > +           Address of dptx_phy node.
>>
>> "A phandle to dptx_phy node" ?
>
> Yes, you're right.
>
> Ajay: Please, replace 'Address of dptx_phy node'
>        with 'phandle of dptx_phy node'.
Ok.
>>
>> > +   -samsung,color_space:
>> > +           input video data format.
>> > +                   COLOR_RGB = 0, COLOR_YCBCR422 = 1, COLOR_YCBCR444 = 2
>>
>> Can this be changed at run time ?
>
> No, it is not changeable.
> It's a default setting according to eDP panel.
>
>>
>> > +   -samsung,dynamic_range:
>> > +           dynamic range for input video data.
>> > +                   VESA = 0, CEA = 1
>>
>> Why is it in the device tree ? Shouldn't it be configurable at runtime ?
>> My apologies if this an obvious question, I don't have much experience
>> with DP.
>
> Same above.
>
>>
>> > +   -samsung,ycbcr_coeff:
>> > +           YCbCr co-efficients for input video.
>> > +                   COLOR_YCBCR601 = 0, COLOR_YCBCR709 = 1
>> > +   -samsung,color_depth:
>> > +           Bit per color component.
>>
>> "Number of bits per colour component" ? Also same remark as above.
>
> Same above.
>
>>
>> > +                   COLOR_6 = 0, COLOR_8 = 1, COLOR_10 = 2, COLOR_12 = 3
>> > +   -samsung,link_rate:
>> > +           link rates supportd by the panel.
>>
>> typo: supportd -> supported
> Ajay: Please fix it.
Ok.
>
>>
>> > +                   LINK_RATE_1_62GBPS = 0x6, LINK_RATE_2_70GBPS = 0x0A
>>
>> Is this really a property of a panel ? Why it is in the PHY node ?
>> Also I can see this is just a single property, so "link rates" is a bit
>> misleading.
>>
>
> Yes, It's a property of LCD panel.
> It is not in the 'PHY' node, but 'display-port-controller' node.
>
> Ajay: Please replace 'link rates' with 'link rate'.
Ok.
>> > +   -samsung,lane_count:
>> > +           number of lanes supported by the panel.
>> > +                   LANE_COUNT1 = 1, LANE_COUNT2 = 2, LANE_COUNT4 = 4
>>
>> What do these symbolic names are needed for ? Is lane_count a number or a
>> mask, is this really a _maximum_ number of lanes ? What are the valid values,
>> 1, 2 and 4 ? Or maybe 0x3 is also valid which  would indicate that we can
>> use 1 or 2 data lanes ?
>>
>
> These symbolic names are defined in './include/video/exynos_dp.h'.
> It seems that Ajay used these definitions.
Yes. I referred  include/video/exynos_dp.h.
> 'lane_count' is a number of used lanes of main link which transfer data
> streams as video. The number of lanes of Main Link is 1, 2, or 4 lanes.
> It is dependent on eDP panel.
>
>
>> > +   -samsung,interlaced:
>> > +           Interlace scan mode.
>> > +                   Progressive if defined, Interlaced if not defined
>>
>> Why do we need this in the device tree ? Is this really a default scan mode ?
>> Can it be the changed at runtime ?
>
> No, it is not changeable.
> It's a default setting according to eDP panel.
>
>>
>> > +   -samsung,v_sync_polarity:
>> > +           VSYNC polarity configuration.
>> > +                   High if defined, Low if not defined
>> > +   -samsung,h_sync_polarity:
>> > +           HSYNC polarity configuration.
>> > +                   High if defined, Low if not defined
>> > +
>> > +Example:
>> > +
>> > +SOC specific portion:
>> > +   dptx_phy: dptx_phy@0x10040720 {
>> > +           compatible = "samsung,dp-phy";
>> > +           samsung,dptx_phy_reg =<0x10040720>;
>> > +           samsung,enable_bit =<1>;
>> > +   };
>> > +
>> > +   display-port-controller {
>> > +           compatible = "samsung,exynos5-dp";
>> > +           reg =<0x145B0000 0x10000>;
>> > +           interrupts =<10 3>;
>> > +           interrupt-parent =<&combiner>;
>> > +           dp_phy =<&dptx_phy>;
>>
>> Shouldn't it be "samsung,dp_phy" ?
>
> Do you mean this ? It is not working.
> +               dp_phy = "samsung,dp_phy";
I din't get this.
What do I need to change here?
>>
>> > +        };
>> > +
>> > +Board Specific portion:
>> > +   display-port-controller {
>> > +           samsung,color_space =<0>;
>> > +           samsung,dynamic_range =<0>;
>> > +           samsung,ycbcr_coeff =<0>;
>> > +           samsung,color_depth =<1>;
>> > +           samsung,link_rate =<0x0a>;
>> > +           samsung,lane_count =<2>;
>> > +   };
>>
>> --
>>
>> Regards,
>> Sylwester
>
Thanks and regards,
Ajay Kumar
> --
> 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

* [PATCH V5 2/5] OMAPDSS: Create new debug config options
From: Chandrabhanu Mahapatra @ 2012-10-01  7:51 UTC (permalink / raw)
  To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Chandrabhanu Mahapatra
In-Reply-To: <af097b99b4d2eab96850fd6d8a0c260917aa4f98.1348914940.git.cmahapatra@ti.com>

The config option CONFIG_OMAP2_DSS_DEBUG_SUPPORT has been removed and replaced
with CONFIG_OMAP2_DSS_DEBUG and CONFIG_OMAP2_DSS_DEBUGFS. CONFIG_OMAP2_DSS_DEBUG
enables DEBUG flag and CONFIG_OMAP2_DSS_DEBUGFS enables creation of debugfs for
OMAPDSS. Both the config options are disabled by default and can be enabled
independently of one another as per convenience.

Signed-off-by: Chandrabhanu Mahapatra <cmahapatra@ti.com>
---
changes from V4 to V5
 * added "depends on DEBUG_FS" to OMAP2_DSS_DEBUGFS defintion in Kconfig so that
   CONFIG_OMAP2_DSS_DEBUGFS check become cleaner

 drivers/video/omap2/dss/Kconfig  |   22 ++++++++++++++++------
 drivers/video/omap2/dss/Makefile |    2 +-
 drivers/video/omap2/dss/core.c   |    6 +++---
 drivers/video/omap2/dss/dss.c    |    2 +-
 drivers/video/omap2/dss/dss.h    |    2 +-
 5 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/video/omap2/dss/Kconfig b/drivers/video/omap2/dss/Kconfig
index 80f5390..7052487 100644
--- a/drivers/video/omap2/dss/Kconfig
+++ b/drivers/video/omap2/dss/Kconfig
@@ -18,16 +18,26 @@ config OMAP2_VRAM_SIZE
 	  You can also set this with "vram=<bytes>" kernel argument, or
 	  in the board file.
 
-config OMAP2_DSS_DEBUG_SUPPORT
-        bool "Debug support"
-	default y
+config OMAP2_DSS_DEBUG
+	bool "Debug support"
+	default n
+	help
+	  This enables printing of debug messages. Alternatively, debug messages
+	  can also be enabled by setting CONFIG_DYNAMIC_DEBUG and then setting
+	  appropriate flags in <debugfs>/dynamic_debug/control.
+
+config OMAP2_DSS_DEBUGFS
+	bool "Debugfs filesystem support"
+	depends on DEBUG_FS
+	default n
 	help
-	  This enables debug messages. You need to enable printing
-	  with 'debug' module parameter.
+	  This enables debugfs for OMAPDSS at <debugfs>/omapdss. This enables
+	  querying about clock configuration and register configuration of dss,
+	  dispc, dsi, hdmi and rfbi.
 
 config OMAP2_DSS_COLLECT_IRQ_STATS
 	bool "Collect DSS IRQ statistics"
-	depends on OMAP2_DSS_DEBUG_SUPPORT
+	depends on OMAP2_DSS_DEBUGFS
 	default n
 	help
 	  Collect DSS IRQ statistics, printable via debugfs.
diff --git a/drivers/video/omap2/dss/Makefile b/drivers/video/omap2/dss/Makefile
index 86493e3..4070191 100644
--- a/drivers/video/omap2/dss/Makefile
+++ b/drivers/video/omap2/dss/Makefile
@@ -8,4 +8,4 @@ omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
 omapdss-$(CONFIG_OMAP2_DSS_DSI) += dsi.o
 omapdss-$(CONFIG_OMAP4_DSS_HDMI) += hdmi.o \
 				    hdmi_panel.o ti_hdmi_4xxx_ip.o
-ccflags-$(CONFIG_OMAP2_DSS_DEBUG_SUPPORT) += -DDEBUG
+ccflags-$(CONFIG_OMAP2_DSS_DEBUG) += -DDEBUG
diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index b2af72d..826d64f 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -138,7 +138,7 @@ int dss_set_min_bus_tput(struct device *dev, unsigned long tput)
 		return 0;
 }
 
-#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUG_SUPPORT)
+#if defined(CONFIG_OMAP2_DSS_DEBUGFS)
 static int dss_debug_show(struct seq_file *s, void *unused)
 {
 	void (*func)(struct seq_file *) = s->private;
@@ -193,7 +193,7 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
 
 	return 0;
 }
-#else /* CONFIG_DEBUG_FS && CONFIG_OMAP2_DSS_DEBUG_SUPPORT */
+#else /* CONFIG_OMAP2_DSS_DEBUGFS */
 static inline int dss_initialize_debugfs(void)
 {
 	return 0;
@@ -205,7 +205,7 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
 {
 	return 0;
 }
-#endif /* CONFIG_DEBUG_FS && CONFIG_OMAP2_DSS_DEBUG_SUPPORT */
+#endif /* CONFIG_OMAP2_DSS_DEBUGFS */
 
 /* PLATFORM DEVICE */
 static int omap_dss_pm_notif(struct notifier_block *b, unsigned long v, void *d)
diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 2ab1c3e..3954742 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -746,7 +746,7 @@ static void dss_runtime_put(void)
 }
 
 /* DEBUGFS */
-#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUG_SUPPORT)
+#if defined(CONFIG_OMAP2_DSS_DEBUGFS)
 void dss_debug_dump_clocks(struct seq_file *s)
 {
 	dss_dump_clocks(s);
diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
index ffbba7e..685275e 100644
--- a/drivers/video/omap2/dss/dss.h
+++ b/drivers/video/omap2/dss/dss.h
@@ -301,7 +301,7 @@ enum dss_hdmi_venc_clk_source_select dss_get_hdmi_venc_clk_source(void);
 const char *dss_get_generic_clk_source_name(enum omap_dss_clk_source clk_src);
 void dss_dump_clocks(struct seq_file *s);
 
-#if defined(CONFIG_DEBUG_FS) && defined(CONFIG_OMAP2_DSS_DEBUG_SUPPORT)
+#if defined(CONFIG_OMAP2_DSS_DEBUGFS)
 void dss_debug_dump_clocks(struct seq_file *s);
 #endif
 
-- 
1.7.10


^ permalink raw reply related

* Re: [PATCH 1/2] of: add helper to parse display specs
From: Stephen Warren @ 2012-10-01 16:53 UTC (permalink / raw)
  To: Steffen Trumtrar
  Cc: devicetree-discuss, linux-fbdev, dri-devel, Hans Verkuil,
	Tomi Valkeinen, Laurent Pinchart, kernel, linux-media
In-Reply-To: <1348500924-8551-2-git-send-email-s.trumtrar@pengutronix.de>

On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
> Parse a display-node with timings and hardware-specs from devictree.

> diff --git a/Documentation/devicetree/bindings/video/display b/Documentation/devicetree/bindings/video/display
> new file mode 100644
> index 0000000..722766a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/video/display

This should be display.txt.

> @@ -0,0 +1,208 @@
> +display bindings
> +=========
> +
> +display-node
> +------------

I'm not personally convinced about the direction this is going. While I
think it's reasonable to define DT bindings for displays, and DT
bindings for display modes, I'm not sure that it's reasonable to couple
them together into a single binding.

I think creating a well-defined timing binding first will be much
simpler than doing so within the context of a display binding; the
scope/content of a general display binding seems much less well-defined
to me at least, for reasons I mentioned before.

> +required properties:
> + - none
> +
> +optional properties:
> + - default-timing: the default timing value
> + - width-mm, height-mm: Display dimensions in mm

> + - hsync-active-high (bool): Hsync pulse is active high
> + - vsync-active-high (bool): Vsync pulse is active high

At least those two properties should exist in the display timing instead
(or perhaps as well). There are certainly cases where different similar
display modes are differentiated by hsync/vsync polarity more than
anything else. This is probably more likely with analog display
connectors than digital, but I see no reason why a DT binding for
display timing shouldn't cover both.

> + - de-active-high (bool): Data-Enable pulse is active high
> + - pixelclk-inverted (bool): pixelclock is inverted

> + - pixel-per-clk

pixel-per-clk is probably something that should either be part of the
timing definition, or something computed internally to the display
driver based on rules for the signal type, rather than something
represented in DT.

The above comment assumes this property is intended to represent DVI's
requirement for pixel clock doubling for low-pixel-clock-rate modes. If
it's something to do with e.g. a single-data-rate vs. double-data-rate
property of the underlying physical connection, that's most likely
something that should be defined in a binding specific to e.g. LVDS,
rather than something generic.

> + - link-width: number of channels (e.g. LVDS)
> + - bpp: bits-per-pixel
> +
> +timings-subnode
> +---------------
> +
> +required properties:
> +subnodes that specify
> + - hactive, vactive: Display resolution
> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
> +   in pixels
> +   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
> +   lines
> + - clock: displayclock in Hz
> +
> +There are different ways of describing a display and its capabilities. The devicetree
> +representation corresponds to the one commonly found in datasheets for displays.
> +The description of the display and its timing is split in two parts: first the display
> +properties like size in mm and (optionally) multiple subnodes with the supported timings.
> +If a display supports multiple signal timings, the default-timing can be specified.
> +
> +Example:
> +
> +	display@0 {
> +		width-mm = <800>;
> +		height-mm = <480>;
> +		default-timing = <&timing0>;
> +		timings {
> +			timing0: timing@0 {

If you're going to use a unit address ("@0") to ensure that node names
are unique (which is not mandatory), then each node also needs a reg
property with matching value, and #address-cells/#size-cells in the
parent. Instead, you could name the nodes something unique based on the
mode name to avoid this, e.g. 1080p24 { ... }.


^ permalink raw reply

* Re: [PATCH 1/2] of: add helper to parse display specs
From: Mitch Bradley @ 2012-10-01 19:16 UTC (permalink / raw)
  To: Stephen Warren
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Hans Verkuil,
	Tomi Valkeinen, Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Steffen Trumtrar, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5069CA74.7040409-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

On 10/1/2012 6:53 AM, Stephen Warren wrote:
> On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
>> Parse a display-node with timings and hardware-specs from devictree.
> 
>> diff --git a/Documentation/devicetree/bindings/video/display b/Documentation/devicetree/bindings/video/display
>> new file mode 100644
>> index 0000000..722766a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/video/display
> 
> This should be display.txt.
> 
>> @@ -0,0 +1,208 @@
>> +display bindings
>> +=========
>> +
>> +display-node
>> +------------
> 
> I'm not personally convinced about the direction this is going. While I
> think it's reasonable to define DT bindings for displays, and DT
> bindings for display modes, I'm not sure that it's reasonable to couple
> them together into a single binding.
> 
> I think creating a well-defined timing binding first will be much
> simpler than doing so within the context of a display binding; the
> scope/content of a general display binding seems much less well-defined
> to me at least, for reasons I mentioned before.
> 
>> +required properties:
>> + - none
>> +
>> +optional properties:
>> + - default-timing: the default timing value
>> + - width-mm, height-mm: Display dimensions in mm
> 
>> + - hsync-active-high (bool): Hsync pulse is active high
>> + - vsync-active-high (bool): Vsync pulse is active high
> 
> At least those two properties should exist in the display timing instead
> (or perhaps as well). There are certainly cases where different similar
> display modes are differentiated by hsync/vsync polarity more than
> anything else. This is probably more likely with analog display
> connectors than digital, but I see no reason why a DT binding for
> display timing shouldn't cover both.
> 
>> + - de-active-high (bool): Data-Enable pulse is active high
>> + - pixelclk-inverted (bool): pixelclock is inverted
> 
>> + - pixel-per-clk
> 
> pixel-per-clk is probably something that should either be part of the
> timing definition, or something computed internally to the display
> driver based on rules for the signal type, rather than something
> represented in DT.
> 
> The above comment assumes this property is intended to represent DVI's
> requirement for pixel clock doubling for low-pixel-clock-rate modes. If
> it's something to do with e.g. a single-data-rate vs. double-data-rate
> property of the underlying physical connection, that's most likely
> something that should be defined in a binding specific to e.g. LVDS,
> rather than something generic.
> 
>> + - link-width: number of channels (e.g. LVDS)
>> + - bpp: bits-per-pixel
>> +
>> +timings-subnode
>> +---------------
>> +
>> +required properties:
>> +subnodes that specify
>> + - hactive, vactive: Display resolution
>> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
>> +   in pixels
>> +   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
>> +   lines
>> + - clock: displayclock in Hz
>> +
>> +There are different ways of describing a display and its capabilities. The devicetree
>> +representation corresponds to the one commonly found in datasheets for displays.
>> +The description of the display and its timing is split in two parts: first the display
>> +properties like size in mm and (optionally) multiple subnodes with the supported timings.
>> +If a display supports multiple signal timings, the default-timing can be specified.
>> +
>> +Example:
>> +
>> +	display@0 {
>> +		width-mm = <800>;
>> +		height-mm = <480>;
>> +		default-timing = <&timing0>;
>> +		timings {
>> +			timing0: timing@0 {
> 
> If you're going to use a unit address ("@0") to ensure that node names
> are unique (which is not mandatory), then each node also needs a reg
> property with matching value, and #address-cells/#size-cells in the
> parent. Instead, you could name the nodes something unique based on the
> mode name to avoid this, e.g. 1080p24 { ... }.


I'm concerned that numbered nodes are being misused as arrays.

It's easy to make real arrays by including multiple cells in the value
of each timing parameter, and easy to choose a cell by saying the array
index instead of using the phandle.



> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
> 

^ permalink raw reply

* [GIT PULL] OMAP DSS for v3.7
From: Tomi Valkeinen @ 2012-10-01 19:45 UTC (permalink / raw)
  To: Florian Tobias Schandinat; +Cc: linux-omap, linux-fbdev

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

Hi Florian,

Here are omapdss changes for 3.7 merge window.

There's something funny with the diff stat, though. I can see a few
changes there that I have not made, for example to
drivers/video/console/fbcon.c. I believe they come from my "Merge branch
'fbdev-for-linus' of git://github.com/schandinat/linux-2.6", although I
don't know why they show up in the diff stat.

Merge with v3.6 goes without conflicts, but there will be some conflicts
with OMAP platform stuff, at least according to linux-next reports.
Those conflicts are trivial, though.

 Tomi

The following changes since commit 4cbe5a555fa58a79b6ecbb6c531b8bab0650778d:

  Linux 3.6-rc4 (2012-09-01 10:39:58 -0700)

are available in the git repository at:

  git://gitorious.org/linux-omap-dss2/linux.git tags/omapdss-for-3.7

for you to fetch changes up to 13b1ba7de8d0ecc42e4f9c002d5b0c1a48f05e58:

  OMAPDSS: add missing include for string.h (2012-09-28 10:03:03 +0300)

----------------------------------------------------------------
Omapdss driver changes for the 3.7 merge window.

Notable changes:

* Basic writeback support for DISPC level. Writeback is not yet usable, though,
  as we need higher level code to actually expose the writeback feature to
  userspace.
* Rewriting the omapdss output drivers. We're trying to remove the hard links
  between the omapdss and the panels, and this rewrite work moves us closer to
  that goal.
* Cleanup and restructuring patches that have been made while working on device
  tree support for omapdss. Device tree support is still some way ahead, but
  these patches are good cleanups in themselves.
* Basic OMAP5 DSS support for DPI and DSI outputs.
* Workaround for the problem that GFX overlay's fifo is too small for high
  resolution scenarios, causing underflows.
* Cleanups that remove dependencies to omap platform code.

----------------------------------------------------------------
Archit Taneja (70):
      OMAPDSS: APPLY: Constify timings argument in dss_mgr_set_timings
      OMAPDSS: DPI: Add locking for DPI interface
      OMAPDSS: Displays: Add locking in generic DPI panel driver
      OMAPDSS: DPI: Maintain our own timings field in driver data
      OMAPDSS: DPI displays: Take care of panel timings in the driver itself
      OMAPDSS: DSI: Maintain own copy of timings in driver data
      OMAPDSS: DSI: Add function to set panel size for command mode panels
      OMAPDSS: DSI: Update manager timings on a manual update
      OMAPDSS: HDMI: Use our own omap_video_timings field when setting interface timings
      OMAPDSS: HDMI: Add locking for hdmi interface set timing functions
      OMAPDSS: SDI: Create a function to set timings
      OMAPDSS: SDI: Maintain our own timings field in driver data
      OMAPDSS: VENC: Split VENC into interface and panel driver
      OMAPDSS: VENC: Maintain our own timings field in driver data
      OMAPDSS: RFBI: Remove partial update support
      OMAPDSS: RFBI: Add function to set panel size
      OMAPDSS: DSI: Maintain copy of pixel format in driver data
      OMAPDSS: RFBI: Maintain copy of pixel size in driver data
      OMAPDSS: RFBI: Maintain copy of number of data lines in driver data
      OMAPDSS: DPI: Maintain copy of number of data lines in driver data
      OMAPDSS: SDI: Maintain copy of data pairs in driver data
      OMAPDSS: DSI: Maintain copy of operation mode in driver data
      OMAPDSS: DSI: Rename dsi_videomode_data to dsi_videomode_timings
      OMAPDSS: DSI: Maintain copy of video mode timings in driver data
      OMAPDSS: RFBI: Maitain copy of rfbi timings in driver data
      OMAPDSS: VENC: Maintain copy of venc type in driver data
      OMAPDSS: VENC: Maintian copy of video output polarity info in private data
      OMAPFB: Clear framebuffers before they are registered
      OMAPDSS: Add basic omap5 features to dss and dispc
      OMAPDSS: DSI: Pass dsi platform device wherever possible
      OMAPDSS: APPLY: Remove omap_dss_device references in wait_for_go functions
      OMAPDSS: outputs: Create a new entity called outputs
      OMAPDSS: outputs: Create and register output instances
      OMAPDSS: output: Add set/unset device ops for omap_dss_output
      OMAPDSS: APPLY: Add manager set/unset output ops for omap_overlay_manager
      OMAPDSS: Remove manager->device references
      OMAP_VOUT: Remove manager->device references
      OMAPFB: remove manager->device references
      OMAPDRM: Remove manager->device references
      OMAPDSS: Create links between managers, outputs and devices
      OMAPDSS: DPI: Replace dssdev->manager with dssdev->output->manager references
      OMAPDSS: DSI: Remove dsi_pdev_map global struct
      OMAPDSS: DSI: Replace dssdev->manager with dssdev->output->manager references
      OMAPDSS: SDI: Replace dssdev->manager with dssdev->output->manager references
      OMAPDSS: RFBI: Replace dssdev->manager with dssdev->output->manager references
      OMAPDSS: VENC: Replace dssdev->manager with dssdev->output->manager references
      OMAPDSS: HDMI: Replace dssdev->manager with dssdev->output->manager references
      OMAPFB: Change dssdev->manager references
      OMAPDSS: MANAGER: Update display sysfs store
      OMAPDSS: OVERLAY/MANAGER: Get device via output
      OMAPDSS: APPLY: Remove omap_dss_device references from dss_ovl_enable/disable
      OMAPDSS: Remove old way of setting manager and device links
      OMAPDSS: DISPC: Constify omap_overlay_info in dispc_ovl_setup()
      OMAPDSS: DISPC: Simplify function names for setting pipeline input and output sizes
      OMAPDSS: DISPC: Pass overlay caps as a parameter to dispc plane functions
      OMAPDSS: OVERLAY: Add position and replication as overlay caps
      OMAPDSS: DISPC: Make dispc_ovl_setup call dispc_ovl_setup_common
      OMAPDSS: DISPC: Don't pass channel out when configuring overlays
      OMAPDSS: DIPSC: Relax scaling limitations when in memory to memory mode
      OMAPDSS: DISPC: Allow both upscaling and downscaling of chroma
      OMAPDSS: DISPC: Add writeback register offsets and dss features structs
      OMAPDSS: DISPC: Configure input and output sizes for writeback
      OMAPDSS: DISPC: Downscale chroma if plane is writeback
      OMAPDSS: DISPC: Don't set chroma resampling bit for writeback
      OMAPDSS: DISPC: Add function to set channel in for writeback
      OMAPDSS: DISPC: Configure overlay-like parameters in dispc_wb_setup
      OMAPDSS: DISPC: Configure writeback specific parameters in dispc_wb_setup()
      OMAPDSS: DISPC: Configure writeback FIFOs
      OMAPDSS: DISPC: Add manager like functions for writeback
      OMAPDSS: DISPC: Configure color conversion coefficients for writeback

Chandrabhanu Mahapatra (8):
      OMAPDSS: DISPC: Cleanup cpu_is_xxxx checks
      OMAPDSS: DSS: Remove redundant functions
      OMAPDSS: DSS: Cleanup cpu_is_xxxx checks
      ARM: OMAP: Disable venc for OMAP4
      OMAPDSS: VENC: Remove cpu_is_xxxx checks
      OMAPDSS: DPI: Remove cpu_is_xxxx checks
      OMAPDSS: Correct DISPC_IRQ bit definitions for LCD3
      OMAPDSS: DISPC: Add predecimation limit for TILER based rotations

Grazvydas Ignotas (1):
      OMAPFB: fix framebuffer console colors

Jassi Brar (1):
      OMAPDSS: DISPC: Use msleep instead of blocking mdelay

Raphaël Assénat (1):
      OMAPDSS: Do not require a VDDS_DSI regulator on AM35xx

Ricardo Neri (2):
      OMAPDSS: HDMI: Disable PLL properly in case of error at power_on
      OMAPDSS: DISPC: Improvements to DIGIT sync signal selection

Tomi Valkeinen (53):
      Merge output work from Archit
      OMAPDSS: HDMI: fix initial HDMI enable
      Merge tag 'v3.6-rc4'
      OMAPDSS: HDMI: Move GPIO handling to HDMI driver
      OMAPDSS: HDMI: Add delay to wait for 5V power
      OMAP4: TWL: add vdda_hdmi_dac regulator supply
      OMAPDSS: HDMI: use vdda_hdmi_dac
      OMAPDSS: Add DSI fclk maximum to dss_features
      OMAPDSS: DSI: calculate dsi clock
      OMAP: 4430SDP: remove DSI clock config from board file
      OMAPDSS: fix use of dssdev->caps
      OMAPDSS: Taal: use devm_* functions
      OMAPFB1: remove unnecessary includes
      OMAPFB1: remove a non-used table
      OMAPDSS: remove unnecessary includes
      OMAPFB: clear framebuffers with CPU
      OMAPDSS: VRAM: Remove clearing with sDMA
      OMAPDSS: Taal: Reogranize for device tree
      OMAPDSS: TFP410: use devm_gpio_request_one
      OMAPDSS: split overlay sysfs code
      OMAPDSS: split manager sysfs code
      OMAPDSS: clean up dss_mgr_set_lcd_config
      OMAPDSS: clean up dss_mgr_set_timings
      Revert "OMAPDSS: APPLY: add fifo-merge support"
      Revert "OMAPDSS: APPLY: add fifo merge support funcs"
      OMAPDSS: Improve fifo management code
      OMAPDSS: Use WB fifo for GFX overlay
      OMAPDSS: fix set_timings
      OMAPDSS: fix dss_ovl_unset_manager
      Merge miscellaneous omapdss changes
      Merge branch 'fbdev-for-linus' of git://github.com/schandinat/linux-2.6
      OMAPDSS: omap_dss_register_device() doesn't take display index
      OMAPDSS: Add dss_get_default_display_name()
      OMAPDSS: register only one display device per output
      OMAPDSS: explicitely initialize dssdev->channel for new displays
      OMAPDSS: handle errors in dss_init_device
      OMAPDSS: cleanup dss_recheck_connections
      OMAPDSS: cleanup dss_recheck_connections further
      OMAPDSS: alloc dssdevs dynamically
      OMAPDSS: DSI: improve DSI clock calcs for DISPC
      OMAPDSS: move dss feats to the end of dss.c
      OMAPDSS: Add support for DPI source selection
      OMAPDSS: DSI: Add FEAT_DSI_PLL_SELFREQDCO
      OMAPDSS: DSI: Add FEAT_DSI_PLL_REFSEL
      OMAPDSS: DSI: Add new linebuffer size for OMAP5
      OMAPDSS: DSI: Add code to disable PHY DCC
      OMAPDSS: DSI: make OMAP2_DSS_DSI depend on ARCH_OMAP5
      Merge omapdss single-dssdev series
      Merge OMAP5 DSS changes to omapdss
      OMAPDSS: DSI: fix tlpx_half reg field length
      Merge branch 'archit/outputs-for-3.7'
      Merge branch 'archit/wb-dispc-for-3.7'
      OMAPDSS: add missing include for string.h

 arch/arm/mach-omap2/board-4430sdp.c                |   73 +-
 arch/arm/mach-omap2/board-omap4panda.c             |   27 +-
 arch/arm/mach-omap2/display.c                      |    1 -
 arch/arm/mach-omap2/twl-common.c                   |    6 +
 drivers/media/video/omap/omap_vout.c               |   75 +-
 drivers/staging/omapdrm/omap_drv.c                 |    5 +-
 drivers/video/auo_k190x.c                          |    2 -
 drivers/video/console/bitblit.c                    |    2 +-
 drivers/video/console/fbcon.c                      |    2 +-
 drivers/video/mb862xx/mb862xxfbdrv.c               |    2 +
 drivers/video/omap/hwa742.c                        |    1 -
 drivers/video/omap/lcd_ams_delta.c                 |    1 -
 drivers/video/omap/lcd_palmte.c                    |    1 -
 drivers/video/omap/omapfb_main.c                   |    9 -
 drivers/video/omap2/displays/panel-acx565akm.c     |   14 +-
 drivers/video/omap2/displays/panel-generic-dpi.c   |   76 +-
 .../omap2/displays/panel-lgphilips-lb035q02.c      |    3 +
 drivers/video/omap2/displays/panel-n8x0.c          |   31 +-
 .../omap2/displays/panel-nec-nl8048hl11-01b.c      |    3 +
 drivers/video/omap2/displays/panel-picodlp.c       |    4 +
 .../video/omap2/displays/panel-sharp-ls037v7dw01.c |    3 +
 drivers/video/omap2/displays/panel-taal.c          |  239 ++---
 drivers/video/omap2/displays/panel-tfp410.c        |   20 +-
 .../video/omap2/displays/panel-tpo-td043mtea1.c    |    7 +-
 drivers/video/omap2/dss/Kconfig                    |    2 +-
 drivers/video/omap2/dss/Makefile                   |    4 +-
 drivers/video/omap2/dss/apply.c                    |  330 ++-----
 drivers/video/omap2/dss/core.c                     |   91 +-
 drivers/video/omap2/dss/dispc.c                    | 1019 ++++++++++++++------
 drivers/video/omap2/dss/dispc.h                    |   37 +
 drivers/video/omap2/dss/display.c                  |  108 ++-
 drivers/video/omap2/dss/dpi.c                      |  181 +++-
 drivers/video/omap2/dss/dsi.c                      |  675 ++++++++++---
 drivers/video/omap2/dss/dss.c                      |  257 +++--
 drivers/video/omap2/dss/dss.h                      |   79 +-
 drivers/video/omap2/dss/dss_features.c             |  278 +++++-
 drivers/video/omap2/dss/dss_features.h             |    7 +
 drivers/video/omap2/dss/hdmi.c                     |  247 +++--
 drivers/video/omap2/dss/hdmi_panel.c               |   31 +-
 drivers/video/omap2/dss/manager-sysfs.c            |  512 ++++++++++
 drivers/video/omap2/dss/manager.c                  |  473 +--------
 drivers/video/omap2/dss/output.c                   |  148 +++
 drivers/video/omap2/dss/overlay-sysfs.c            |  456 +++++++++
 drivers/video/omap2/dss/overlay.c                  |  492 +---------
 drivers/video/omap2/dss/rfbi.c                     |  222 +++--
 drivers/video/omap2/dss/sdi.c                      |  136 ++-
 drivers/video/omap2/dss/venc.c                     |  337 +++----
 drivers/video/omap2/dss/venc_panel.c               |  251 +++++
 drivers/video/omap2/omapfb/omapfb-ioctl.c          |    7 +-
 drivers/video/omap2/omapfb/omapfb-main.c           |   34 +-
 drivers/video/omap2/omapfb/omapfb.h                |    5 +-
 drivers/video/omap2/vram.c                         |   56 --
 include/video/omapdss.h                            |  112 ++-
 53 files changed, 4592 insertions(+), 2602 deletions(-)
 create mode 100644 drivers/video/omap2/dss/manager-sysfs.c
 create mode 100644 drivers/video/omap2/dss/output.c
 create mode 100644 drivers/video/omap2/dss/overlay-sysfs.c
 create mode 100644 drivers/video/omap2/dss/venc_panel.c


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] of: add helper to parse display specs
From: Stephen Warren @ 2012-10-01 20:25 UTC (permalink / raw)
  To: Mitch Bradley
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Hans Verkuil,
	Tomi Valkeinen, Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Steffen Trumtrar, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5069EC1C.2050506-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org>

On 10/01/2012 01:16 PM, Mitch Bradley wrote:
> On 10/1/2012 6:53 AM, Stephen Warren wrote:
>> On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
>>> Parse a display-node with timings and hardware-specs from devictree.
>>
>>> diff --git a/Documentation/devicetree/bindings/video/display b/Documentation/devicetree/bindings/video/display
>>> new file mode 100644
>>> index 0000000..722766a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/video/display
>>
>> This should be display.txt.
>>
>>> @@ -0,0 +1,208 @@
>>> +display bindings
>>> +=========
>>> +
>>> +display-node
>>> +------------
>>
>> I'm not personally convinced about the direction this is going. While I
>> think it's reasonable to define DT bindings for displays, and DT
>> bindings for display modes, I'm not sure that it's reasonable to couple
>> them together into a single binding.
>>
>> I think creating a well-defined timing binding first will be much
>> simpler than doing so within the context of a display binding; the
>> scope/content of a general display binding seems much less well-defined
>> to me at least, for reasons I mentioned before.
>>
>>> +required properties:
>>> + - none
>>> +
>>> +optional properties:
>>> + - default-timing: the default timing value
>>> + - width-mm, height-mm: Display dimensions in mm
>>
>>> + - hsync-active-high (bool): Hsync pulse is active high
>>> + - vsync-active-high (bool): Vsync pulse is active high
>>
>> At least those two properties should exist in the display timing instead
>> (or perhaps as well). There are certainly cases where different similar
>> display modes are differentiated by hsync/vsync polarity more than
>> anything else. This is probably more likely with analog display
>> connectors than digital, but I see no reason why a DT binding for
>> display timing shouldn't cover both.
>>
>>> + - de-active-high (bool): Data-Enable pulse is active high
>>> + - pixelclk-inverted (bool): pixelclock is inverted
>>
>>> + - pixel-per-clk
>>
>> pixel-per-clk is probably something that should either be part of the
>> timing definition, or something computed internally to the display
>> driver based on rules for the signal type, rather than something
>> represented in DT.
>>
>> The above comment assumes this property is intended to represent DVI's
>> requirement for pixel clock doubling for low-pixel-clock-rate modes. If
>> it's something to do with e.g. a single-data-rate vs. double-data-rate
>> property of the underlying physical connection, that's most likely
>> something that should be defined in a binding specific to e.g. LVDS,
>> rather than something generic.
>>
>>> + - link-width: number of channels (e.g. LVDS)
>>> + - bpp: bits-per-pixel
>>> +
>>> +timings-subnode
>>> +---------------
>>> +
>>> +required properties:
>>> +subnodes that specify
>>> + - hactive, vactive: Display resolution
>>> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
>>> +   in pixels
>>> +   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
>>> +   lines
>>> + - clock: displayclock in Hz
>>> +
>>> +There are different ways of describing a display and its capabilities. The devicetree
>>> +representation corresponds to the one commonly found in datasheets for displays.
>>> +The description of the display and its timing is split in two parts: first the display
>>> +properties like size in mm and (optionally) multiple subnodes with the supported timings.
>>> +If a display supports multiple signal timings, the default-timing can be specified.
>>> +
>>> +Example:
>>> +
>>> +	display@0 {
>>> +		width-mm = <800>;
>>> +		height-mm = <480>;
>>> +		default-timing = <&timing0>;
>>> +		timings {
>>> +			timing0: timing@0 {
>>
>> If you're going to use a unit address ("@0") to ensure that node names
>> are unique (which is not mandatory), then each node also needs a reg
>> property with matching value, and #address-cells/#size-cells in the
>> parent. Instead, you could name the nodes something unique based on the
>> mode name to avoid this, e.g. 1080p24 { ... }.
> 
> 
> I'm concerned that numbered nodes are being misused as arrays.
> 
> It's easy to make real arrays by including multiple cells in the value
> of each timing parameter, and easy to choose a cell by saying the array
> index instead of using the phandle.

In this case though, arrays don't work out so well in my opinion:

We want to describe a set of unrelated display modes that the display
can handle. These are logically separate entities. I don't think
combining the values that represent say 5 different modes into a single
set of properties really makes sense here; a separate node or property
per display mode really does make sense because they're separate objects.

Related, each display timing parameter (e.g. hsync length, position,
...) has a range, so min/typical/max values. These are already
represented as a 3-cell property as I believe you're proposing.
Combining that with a cell that represents n different modes in a single
array seems like it'd end up with something rather hard to read, at
least for humans even if it would be admittedly trivial for a CPU.

^ permalink raw reply

* Re: [PATCH 1/2] of: add helper to parse display specs
From: Mitch Bradley @ 2012-10-01 21:08 UTC (permalink / raw)
  To: Stephen Warren
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Hans Verkuil,
	Tomi Valkeinen, Laurent Pinchart, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	Steffen Trumtrar, linux-media-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5069FC20.8060708-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

On 10/1/2012 10:25 AM, Stephen Warren wrote:
> On 10/01/2012 01:16 PM, Mitch Bradley wrote:
>> On 10/1/2012 6:53 AM, Stephen Warren wrote:
>>> On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
>>>> Parse a display-node with timings and hardware-specs from devictree.
>>>
>>>> diff --git a/Documentation/devicetree/bindings/video/display b/Documentation/devicetree/bindings/video/display
>>>> new file mode 100644
>>>> index 0000000..722766a
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/video/display
>>>
>>> This should be display.txt.
>>>
>>>> @@ -0,0 +1,208 @@
>>>> +display bindings
>>>> +=========
>>>> +
>>>> +display-node
>>>> +------------
>>>
>>> I'm not personally convinced about the direction this is going. While I
>>> think it's reasonable to define DT bindings for displays, and DT
>>> bindings for display modes, I'm not sure that it's reasonable to couple
>>> them together into a single binding.
>>>
>>> I think creating a well-defined timing binding first will be much
>>> simpler than doing so within the context of a display binding; the
>>> scope/content of a general display binding seems much less well-defined
>>> to me at least, for reasons I mentioned before.
>>>
>>>> +required properties:
>>>> + - none
>>>> +
>>>> +optional properties:
>>>> + - default-timing: the default timing value
>>>> + - width-mm, height-mm: Display dimensions in mm
>>>
>>>> + - hsync-active-high (bool): Hsync pulse is active high
>>>> + - vsync-active-high (bool): Vsync pulse is active high
>>>
>>> At least those two properties should exist in the display timing instead
>>> (or perhaps as well). There are certainly cases where different similar
>>> display modes are differentiated by hsync/vsync polarity more than
>>> anything else. This is probably more likely with analog display
>>> connectors than digital, but I see no reason why a DT binding for
>>> display timing shouldn't cover both.
>>>
>>>> + - de-active-high (bool): Data-Enable pulse is active high
>>>> + - pixelclk-inverted (bool): pixelclock is inverted
>>>
>>>> + - pixel-per-clk
>>>
>>> pixel-per-clk is probably something that should either be part of the
>>> timing definition, or something computed internally to the display
>>> driver based on rules for the signal type, rather than something
>>> represented in DT.
>>>
>>> The above comment assumes this property is intended to represent DVI's
>>> requirement for pixel clock doubling for low-pixel-clock-rate modes. If
>>> it's something to do with e.g. a single-data-rate vs. double-data-rate
>>> property of the underlying physical connection, that's most likely
>>> something that should be defined in a binding specific to e.g. LVDS,
>>> rather than something generic.
>>>
>>>> + - link-width: number of channels (e.g. LVDS)
>>>> + - bpp: bits-per-pixel
>>>> +
>>>> +timings-subnode
>>>> +---------------
>>>> +
>>>> +required properties:
>>>> +subnodes that specify
>>>> + - hactive, vactive: Display resolution
>>>> + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
>>>> +   in pixels
>>>> +   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
>>>> +   lines
>>>> + - clock: displayclock in Hz
>>>> +
>>>> +There are different ways of describing a display and its capabilities. The devicetree
>>>> +representation corresponds to the one commonly found in datasheets for displays.
>>>> +The description of the display and its timing is split in two parts: first the display
>>>> +properties like size in mm and (optionally) multiple subnodes with the supported timings.
>>>> +If a display supports multiple signal timings, the default-timing can be specified.
>>>> +
>>>> +Example:
>>>> +
>>>> +	display@0 {
>>>> +		width-mm = <800>;
>>>> +		height-mm = <480>;
>>>> +		default-timing = <&timing0>;
>>>> +		timings {
>>>> +			timing0: timing@0 {
>>>
>>> If you're going to use a unit address ("@0") to ensure that node names
>>> are unique (which is not mandatory), then each node also needs a reg
>>> property with matching value, and #address-cells/#size-cells in the
>>> parent. Instead, you could name the nodes something unique based on the
>>> mode name to avoid this, e.g. 1080p24 { ... }.
>>
>>
>> I'm concerned that numbered nodes are being misused as arrays.
>>
>> It's easy to make real arrays by including multiple cells in the value
>> of each timing parameter, and easy to choose a cell by saying the array
>> index instead of using the phandle.
> 
> In this case though, arrays don't work out so well in my opinion:
> 
> We want to describe a set of unrelated display modes that the display
> can handle. These are logically separate entities. I don't think
> combining the values that represent say 5 different modes into a single
> set of properties really makes sense here; a separate node or property
> per display mode really does make sense because they're separate objects.

That argument seems pretty dependent on how you choose to look at things.


> 
> Related, each display timing parameter (e.g. hsync length, position,
> ...) has a range, so min/typical/max values. These are already
> represented as a 3-cell property as I believe you're proposing.
> Combining that with a cell that represents n different modes in a single
> array seems like it'd end up with something rather hard to read, at
> least for humans even if it would be admittedly trivial for a CPU.


That argument is better.

> 

^ permalink raw reply

* [PATCH] video: s3c-fb: use clk_prepare_enable and clk_disable_unprepare
From: Thomas Abraham @ 2012-10-02 23:57 UTC (permalink / raw)
  To: linux-fbdev; +Cc: FlorianSchandinat, jg1.han, kgene.kim, linux-samsung-soc

Convert clk_enable/clk_disable to clk_prepare_enable/clk_disable_unprepare
calls as required by common clock framework.

Signed-off-by: Thomas Abraham <thomas.abraham@linaro.org>
---
 drivers/video/s3c-fb.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 52b744f..2ed7b63 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -1404,7 +1404,7 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
 		return PTR_ERR(sfb->bus_clk);
 	}
 
-	clk_enable(sfb->bus_clk);
+	clk_prepare_enable(sfb->bus_clk);
 
 	if (!sfb->variant.has_clksel) {
 		sfb->lcd_clk = devm_clk_get(dev, "sclk_fimd");
@@ -1414,7 +1414,7 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
 			goto err_bus_clk;
 		}
 
-		clk_enable(sfb->lcd_clk);
+		clk_prepare_enable(sfb->lcd_clk);
 	}
 
 	pm_runtime_enable(sfb->dev);
@@ -1504,10 +1504,10 @@ err_lcd_clk:
 	pm_runtime_disable(sfb->dev);
 
 	if (!sfb->variant.has_clksel)
-		clk_disable(sfb->lcd_clk);
+		clk_disable_unprepare(sfb->lcd_clk);
 
 err_bus_clk:
-	clk_disable(sfb->bus_clk);
+	clk_disable_unprepare(sfb->bus_clk);
 
 	return ret;
 }
@@ -1531,9 +1531,9 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
 			s3c_fb_release_win(sfb, sfb->windows[win]);
 
 	if (!sfb->variant.has_clksel)
-		clk_disable(sfb->lcd_clk);
+		clk_disable_unprepare(sfb->lcd_clk);
 
-	clk_disable(sfb->bus_clk);
+	clk_disable_unprepare(sfb->bus_clk);
 
 	pm_runtime_put_sync(sfb->dev);
 	pm_runtime_disable(sfb->dev);
@@ -1561,9 +1561,9 @@ static int s3c_fb_suspend(struct device *dev)
 	}
 
 	if (!sfb->variant.has_clksel)
-		clk_disable(sfb->lcd_clk);
+		clk_disable_unprepare(sfb->lcd_clk);
 
-	clk_disable(sfb->bus_clk);
+	clk_disable_unprepare(sfb->bus_clk);
 
 	pm_runtime_put_sync(sfb->dev);
 
@@ -1581,10 +1581,10 @@ static int s3c_fb_resume(struct device *dev)
 
 	pm_runtime_get_sync(sfb->dev);
 
-	clk_enable(sfb->bus_clk);
+	clk_prepare_enable(sfb->bus_clk);
 
 	if (!sfb->variant.has_clksel)
-		clk_enable(sfb->lcd_clk);
+		clk_prepare_enable(sfb->lcd_clk);
 
 	/* setup gpio and output polarity controls */
 	pd->setup_gpio();
@@ -1640,9 +1640,9 @@ static int s3c_fb_runtime_suspend(struct device *dev)
 	struct s3c_fb *sfb = platform_get_drvdata(pdev);
 
 	if (!sfb->variant.has_clksel)
-		clk_disable(sfb->lcd_clk);
+		clk_disable_unprepare(sfb->lcd_clk);
 
-	clk_disable(sfb->bus_clk);
+	clk_disable_unprepare(sfb->bus_clk);
 
 	return 0;
 }
@@ -1653,10 +1653,10 @@ static int s3c_fb_runtime_resume(struct device *dev)
 	struct s3c_fb *sfb = platform_get_drvdata(pdev);
 	struct s3c_fb_platdata *pd = sfb->pdata;
 
-	clk_enable(sfb->bus_clk);
+	clk_prepare_enable(sfb->bus_clk);
 
 	if (!sfb->variant.has_clksel)
-		clk_enable(sfb->lcd_clk);
+		clk_prepare_enable(sfb->lcd_clk);
 
 	/* setup gpio and output polarity controls */
 	pd->setup_gpio();
-- 
1.7.4.1


^ permalink raw reply related

* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-10-03  8:24 UTC (permalink / raw)
  To: Stephen Warren
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Stephen Warren, linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Leela Krishna Amudala,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Brown,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
	Anton Vorontsov, Tomi Valkeinen,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David Woodhouse,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <5051FAC5.40501-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

On 09/14/2012 12:24 AM, Stephen Warren wrote:
> On 09/13/2012 01:29 AM, Mark Brown wrote:
>> On Thu, Sep 13, 2012 at 04:26:34PM +0900, Alex Courbot wrote:
>>> On Thursday 13 September 2012 15:19:30 Mark Brown wrote:
>>>>> On Thursday 13 September 2012 14:25:53 Mark Brown wrote:
>>>>>> It would be sensible to make sure that the framework is done in such a
>>>>>> way that drivers can use it - there will be drivers (perhaps not display
>>>>>> ones) that have a known power sequence and which could benefit from the
>>>>>> ability to use library code to implement it based on the user simply
>>>>>> supplying named resources.
>>
>>>>> Not sure I understand what you mean, but things should be working this way
>>>>> already - regulators and PWMs are acquired by name using the standard
>>>>> regulator_get() and pwm_get() functions. GPIOs do not, AFAIK, have a way
>>>>> to be referenced by name so their number is used instead.
>>
>>>> Right, but the sequencing for enabling them is currently open coded in
>>>> each driver.
>>
>>> Mmm then I'm afraid I don't see what you wanted to say initially - could you
>>> elaborate?
>>
>> The driver knows the power sequence.  Having to type the same sequence
>> into the DT or platform data for each board using the device wouuld be
>> retarded so we need the drivers to be able to give the sequence to the
>> library if they're going to be able to reuse it (which is a lot of what
>> Tomi is talking about).
>
> I believe that's trivial to implement. The relevant function is:
>
> struct power_seq_set *devm_power_seq_set_build(struct device *dev,
> 		   struct platform_power_seq_set *pseq);
>
> It's up to the driver whether pseq comes from platform data or is
> hard-coded into the driver (or not provided at all, for the DT case).
> So, the only change needed to convert a "hard-coded" driver to this API
> is to convert the current custom data structure (or code) that describes
> the sequence into a struct platform_power_seq_set.

If we go this way (which looks good IMO!), then maybe we should abandon 
that "platform" denomination and merge platform_power_seq* structures 
with the currently private power_seq*, and also replace the "building" 
step with a resources acquisition one. Calling these structures 
"platform" implies they are for platform data while they can be used to 
perform more flexible things as Mark mentioned. Also making the resolved 
resource visible would allow drivers to "patch" generic sequences with 
the proper GPIO numbers at runtime. We would also avoid a few memory 
copies and both design and usage would be simplified, at the cost of 
having more things exposed. How does that sound?

Alex.


^ permalink raw reply

* Re: [PATCH 1/2] of: add helper to parse display specs
From: Steffen Trumtrar @ 2012-10-03 11:06 UTC (permalink / raw)
  To: Stephen Warren
  Cc: devicetree-discuss, linux-fbdev, dri-devel, Tomi Valkeinen,
	Laurent Pinchart, kernel, linux-media
In-Reply-To: <5069CA74.7040409@wwwdotorg.org>

On Mon, Oct 01, 2012 at 10:53:08AM -0600, Stephen Warren wrote:
> On 09/24/2012 09:35 AM, Steffen Trumtrar wrote:
> > Parse a display-node with timings and hardware-specs from devictree.
> 
> > diff --git a/Documentation/devicetree/bindings/video/display b/Documentation/devicetree/bindings/video/display
> > new file mode 100644
> > index 0000000..722766a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/video/display
> 
> This should be display.txt.
> 
Okay

> > @@ -0,0 +1,208 @@
> > +display bindings
> > +=========
> > +
> > +display-node
> > +------------
> 
> I'm not personally convinced about the direction this is going. While I
> think it's reasonable to define DT bindings for displays, and DT
> bindings for display modes, I'm not sure that it's reasonable to couple
> them together into a single binding.
> 
> I think creating a well-defined timing binding first will be much
> simpler than doing so within the context of a display binding; the
> scope/content of a general display binding seems much less well-defined
> to me at least, for reasons I mentioned before.
> 

Yes, you are right. I'm in the middle of moving things around a little.
It seems best, to have bindings only for the timings at the moment and
get people to agree on those and use them, instead of all the adhoc solutions
based on of_videomode v2.

Then, the of_get_display_timings and the conversion via videomode to fb_videomode
etc can be combined with Laurent Pincharts panel proposal.

> > +required properties:
> > + - none
> > +
> > +optional properties:
> > + - default-timing: the default timing value
> > + - width-mm, height-mm: Display dimensions in mm
> 
> > + - hsync-active-high (bool): Hsync pulse is active high
> > + - vsync-active-high (bool): Vsync pulse is active high
> 
> At least those two properties should exist in the display timing instead
> (or perhaps as well). There are certainly cases where different similar
> display modes are differentiated by hsync/vsync polarity more than
> anything else. This is probably more likely with analog display
> connectors than digital, but I see no reason why a DT binding for
> display timing shouldn't cover both.
> 

Yes. Will do.

> > + - de-active-high (bool): Data-Enable pulse is active high
> > + - pixelclk-inverted (bool): pixelclock is inverted
> 
> > + - pixel-per-clk
> 
> pixel-per-clk is probably something that should either be part of the
> timing definition, or something computed internally to the display
> driver based on rules for the signal type, rather than something
> represented in DT.
> 
> The above comment assumes this property is intended to represent DVI's
> requirement for pixel clock doubling for low-pixel-clock-rate modes. If
> it's something to do with e.g. a single-data-rate vs. double-data-rate
> property of the underlying physical connection, that's most likely
> something that should be defined in a binding specific to e.g. LVDS,
> rather than something generic.
> 
> > + - link-width: number of channels (e.g. LVDS)
> > + - bpp: bits-per-pixel
> > +
> > +timings-subnode
> > +---------------
> > +
> > +required properties:
> > +subnodes that specify
> > + - hactive, vactive: Display resolution
> > + - hfront-porch, hback-porch, hsync-len: Horizontal Display timing parameters
> > +   in pixels
> > +   vfront-porch, vback-porch, vsync-len: Vertical display timing parameters in
> > +   lines
> > + - clock: displayclock in Hz
> > +
> > +There are different ways of describing a display and its capabilities. The devicetree
> > +representation corresponds to the one commonly found in datasheets for displays.
> > +The description of the display and its timing is split in two parts: first the display
> > +properties like size in mm and (optionally) multiple subnodes with the supported timings.
> > +If a display supports multiple signal timings, the default-timing can be specified.
> > +
> > +Example:
> > +
> > +	display@0 {
> > +		width-mm = <800>;
> > +		height-mm = <480>;
> > +		default-timing = <&timing0>;
> > +		timings {
> > +			timing0: timing@0 {
> 
> If you're going to use a unit address ("@0") to ensure that node names
> are unique (which is not mandatory), then each node also needs a reg
> property with matching value, and #address-cells/#size-cells in the
> parent. Instead, you could name the nodes something unique based on the
> mode name to avoid this, e.g. 1080p24 { ... }.
> 

Ah, okay. Wasn't sure that was valid. I prefer to not use unit addresses.

 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* [PATCH 1/2] drivers/video/savage/savagefb_driver.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-03 12:40 UTC (permalink / raw)
  To: Antonino Daplas
  Cc: kernel-janitors, Florian Tobias Schandinat, linux-fbdev,
	linux-kernel

From: Peter Senna Tschudin <peter.senna@gmail.com>

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
    when != &ret
*if(...)
{
  ... when != ret = e2
      when forall
 return ret;
}
// </smpl>


Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

---
 drivers/video/savage/savagefb_driver.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/video/savage/savagefb_driver.c b/drivers/video/savage/savagefb_driver.c
index 0d0f52c..f4f53b0 100644
--- a/drivers/video/savage/savagefb_driver.c
+++ b/drivers/video/savage/savagefb_driver.c
@@ -2266,8 +2266,10 @@ static int __devinit savagefb_probe(struct pci_dev* dev,
 	lpitch = info->var.xres_virtual*((info->var.bits_per_pixel + 7) >> 3);
 	info->var.yres_virtual = info->fix.smem_len/lpitch;
 
-	if (info->var.yres_virtual < info->var.yres)
+	if (info->var.yres_virtual < info->var.yres) {
+		err = -ENOMEM;
 		goto failed;
+	}
 
 #if defined(CONFIG_FB_SAVAGE_ACCEL)
 	/*


^ permalink raw reply related

* [PATCH 2/2] drivers/video/exynos/exynos_mipi_dsi.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-03 12:40 UTC (permalink / raw)
  To: Inki Dae
  Cc: kernel-janitors, Donghwa Lee, Kyungmin Park,
	Florian Tobias Schandinat, linux-fbdev, linux-kernel

From: Peter Senna Tschudin <peter.senna@gmail.com>

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
    when != &ret
*if(...)
{
  ... when != ret = e2
      when forall
 return ret;
}
// </smpl>


Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

---
 drivers/video/exynos/exynos_mipi_dsi.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/exynos/exynos_mipi_dsi.c b/drivers/video/exynos/exynos_mipi_dsi.c
index c4f25de..07d70a3 100644
--- a/drivers/video/exynos/exynos_mipi_dsi.c
+++ b/drivers/video/exynos/exynos_mipi_dsi.c
@@ -375,6 +375,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 	dsim->clock = clk_get(&pdev->dev, "dsim0");
 	if (IS_ERR(dsim->clock)) {
 		dev_err(&pdev->dev, "failed to get dsim clock source\n");
+		ret = -ENODEV;
 		goto err_clock_get;
 	}
 
@@ -383,6 +384,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(&pdev->dev, "failed to get io memory region\n");
+		ret = -ENODEV;
 		goto err_platform_get;
 	}
 
@@ -407,6 +409,7 @@ static int exynos_mipi_dsi_probe(struct platform_device *pdev)
 	dsim_ddi = exynos_mipi_dsi_bind_lcd_ddi(dsim, dsim_pd->lcd_panel_name);
 	if (!dsim_ddi) {
 		dev_err(&pdev->dev, "mipi_dsim_ddi object not found.\n");
+		ret = -EINVAL;
 		goto err_bind;
 	}
 


^ permalink raw reply related

* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Stephen Warren @ 2012-10-03 15:30 UTC (permalink / raw)
  To: Alex Courbot
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Stephen Warren, linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Leela Krishna Amudala,
	linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Brown,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
	Anton Vorontsov, Tomi Valkeinen,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David Woodhouse,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <506BF62F.6040308-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

On 10/03/2012 02:24 AM, Alex Courbot wrote:
> On 09/14/2012 12:24 AM, Stephen Warren wrote:
>> On 09/13/2012 01:29 AM, Mark Brown wrote:
>>> On Thu, Sep 13, 2012 at 04:26:34PM +0900, Alex Courbot wrote:
>>>> On Thursday 13 September 2012 15:19:30 Mark Brown wrote:
>>>>>> On Thursday 13 September 2012 14:25:53 Mark Brown wrote:
>>>>>>> It would be sensible to make sure that the framework is done in
>>>>>>> such a
>>>>>>> way that drivers can use it - there will be drivers (perhaps not
>>>>>>> display
>>>>>>> ones) that have a known power sequence and which could benefit
>>>>>>> from the
>>>>>>> ability to use library code to implement it based on the user simply
>>>>>>> supplying named resources.
>>>
>>>>>> Not sure I understand what you mean, but things should be working
>>>>>> this way
>>>>>> already - regulators and PWMs are acquired by name using the standard
>>>>>> regulator_get() and pwm_get() functions. GPIOs do not, AFAIK, have
>>>>>> a way
>>>>>> to be referenced by name so their number is used instead.
>>>
>>>>> Right, but the sequencing for enabling them is currently open coded in
>>>>> each driver.
>>>
>>>> Mmm then I'm afraid I don't see what you wanted to say initially -
>>>> could you
>>>> elaborate?
>>>
>>> The driver knows the power sequence.  Having to type the same sequence
>>> into the DT or platform data for each board using the device wouuld be
>>> retarded so we need the drivers to be able to give the sequence to the
>>> library if they're going to be able to reuse it (which is a lot of what
>>> Tomi is talking about).
>>
>> I believe that's trivial to implement. The relevant function is:
>>
>> struct power_seq_set *devm_power_seq_set_build(struct device *dev,
>>            struct platform_power_seq_set *pseq);
>>
>> It's up to the driver whether pseq comes from platform data or is
>> hard-coded into the driver (or not provided at all, for the DT case).
>> So, the only change needed to convert a "hard-coded" driver to this API
>> is to convert the current custom data structure (or code) that describes
>> the sequence into a struct platform_power_seq_set.
> 
> If we go this way (which looks good IMO!), then maybe we should abandon
> that "platform" denomination and merge platform_power_seq* structures
> with the currently private power_seq*, and also replace the "building"
> step with a resources acquisition one. Calling these structures
> "platform" implies they are for platform data while they can be used to
> perform more flexible things as Mark mentioned.

That all seems reasonable.

> Also making the resolved
> resource visible would allow drivers to "patch" generic sequences with
> the proper GPIO numbers at runtime.

That doesn't sound like a great idea to me, but we can simply avoid
doing this even though it's technically possible.

> We would also avoid a few memory
> copies and both design and usage would be simplified, at the cost of
> having more things exposed. How does that sound?

Sounds fine to me at least.

^ permalink raw reply

* Re: [PATCH 0/6] OMAPDSS: remove cpu_is_* calls
From: Jingoo Han @ 2012-10-04  1:33 UTC (permalink / raw)
  To: 'Tomi Valkeinen'
  Cc: linux-omap, linux-fbdev, 'Archit Taneja',
	'Chandrabhanu Mahapatra',
	'Raphaël Assénat', 'Tony Lindgren',
	'Jingoo Han'
In-Reply-To: <1348828527-13309-1-git-send-email-tomi.valkeinen@ti.com>

On Friday, September 28, 2012 7:35 PM Tomi Valkeinen wrote
> 
> Hi,
> 
> This series adds an omapdss_version enum that is passed via platform data to
> omapdss driver. This version identifier is then used instead of cpu_is_*()
> calls.

Hi Tomi,

As you mentioned, cpu_is_*() is not preferable in driver.
Actually, I thought so, when I saw the OMAPDSS driver a few months ago.
Anyway, it looks good. :)

Best regards,
Jingoo Han

> 
> After these, omapdss no longer contains any plat/ or mach/ includes. omapfb,
> vrfb and vram still do, though.
> 
>  Tomi
> 
> Tomi Valkeinen (6):
>   OMAPDSS: add omapdss_version
>   OMAPDSS: use omapdss_version in dss_features.c
>   OMAPDSS: DISPC: use omapdss_version
>   OMAPDSS: DSS: use omapdss_version
>   OMAPDSS: HDMI: use omapdss_version
>   OMAPDSS: remove <plat/cpu.h> includes
> 
>  arch/arm/mach-omap2/display.c          |   38 +++++++++++++++++++
>  drivers/video/omap2/dss/core.c         |    2 +-
>  drivers/video/omap2/dss/dispc.c        |   41 +++++++++++++-------
>  drivers/video/omap2/dss/dss.c          |   39 +++++++++++++------
>  drivers/video/omap2/dss/dss_features.c |   64 ++++++++++++++++++++++----------
>  drivers/video/omap2/dss/dss_features.h |    5 ++-
>  drivers/video/omap2/dss/hdmi.c         |    3 +-
>  include/video/omapdss.h                |   14 +++++++
>  8 files changed, 157 insertions(+), 49 deletions(-)
> 
> --
> 1.7.9.5
> 
> --
> 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 V3] video: exynos_dp: Add device tree support to DP driver
From: Jingoo Han @ 2012-10-04  1:50 UTC (permalink / raw)
  To: 'Ajay kumar', linux-fbdev
  Cc: linux-samsung-soc, FlorianSchandinat, thomas.ab,
	'devicetree-discuss', 'Sylwester Nawrocki',
	'Tomasz Figa', 'Jingoo Han'
In-Reply-To: <CAEC9eQNFos4Sw57eubX-USEjKBnKf9ETeWCU_sT1vdX3MgN4dA@mail.gmail.com>

On Monday, October 01, 2012 2:40 PM Ajay kumar wrote
> 
> On Fri, Sep 28, 2012 at 5:41 AM, Jingoo Han <jg1.han@samsung.com> wrote:
> > On Thursday, September 27, 2012 10:45 PM Sylwester Nawrocki wrote
> >> On 09/24/2012 09:36 PM, Ajay Kumar wrote:

[...]

> >> > +Example:
> >> > +
> >> > +SOC specific portion:
> >> > +   dptx_phy: dptx_phy@0x10040720 {
> >> > +           compatible = "samsung,dp-phy";
> >> > +           samsung,dptx_phy_reg =<0x10040720>;
> >> > +           samsung,enable_bit =<1>;
> >> > +   };
> >> > +
> >> > +   display-port-controller {
> >> > +           compatible = "samsung,exynos5-dp";
> >> > +           reg =<0x145B0000 0x10000>;
> >> > +           interrupts =<10 3>;
> >> > +           interrupt-parent =<&combiner>;
> >> > +           dp_phy =<&dptx_phy>;
> >>
> >> Shouldn't it be "samsung,dp_phy" ?
> >
> > Do you mean this ? It is not working.
> > +               dp_phy = "samsung,dp_phy";
> I din't get this.
> What do I need to change here?

As Tomasz Figa mentioned, it means that

-+		dp_phy =<&dptx_phy>;
++		samsung,dp_phy =<&dptx_phy>;

It is because this is a Samsung-specific property.

Best regards,
Jingoo Han




^ permalink raw reply


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