[PATCH v2 08/12] drm: dsi: Add "enter idle" & "exit idle" dcs functions
On 07/01/2015 01:51 PM, Mark Zhang wrote:
> Signed-off-by: Mark Zhang
> ---
> drivers/gpu/drm/drm_mipi_dsi.c | 36
> include/drm/drm_mipi_dsi.h |2 ++
> 2 files changed, 38 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 2d5ca8eec13a..9bc6ff75eb8f 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -862,6 +862,42 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device
> *dsi, u8 format)
> }
> EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format);
>
> +/**
> + * mipi_dsi_dcs_enter_idle_mode()
> + * @dsi: DSI peripheral device
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int mipi_dsi_dcs_enter_idle_mode(struct mipi_dsi_device *dsi)
> +{
> + ssize_t err;
> +
> + err = mipi_dsi_dcs_write(dsi, MIPI_DCS_ENTER_IDLE_MODE, NULL, 0);
> + if (err < 0)
> + return err;
> +
> + return 0;
> +}
This we can do simply as:
return mipi_dsi_dcs_write(dsi, MIPI_DCS_ENTER_IDLE_MODE, NULL, 0);
> +EXPORT_SYMBOL(mipi_dsi_dcs_enter_idle_mode);
> +
> +/**
> + * mipi_dsi_dcs_exit_idle_mode()
> + * @dsi: DSI peripheral device
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int mipi_dsi_dcs_exit_idle_mode(struct mipi_dsi_device *dsi)
> +{
> + ssize_t err;
> +
> + err = mipi_dsi_dcs_write(dsi, MIPI_DCS_EXIT_IDLE_MODE, NULL, 0);
> + if (err < 0)
> + return err;
> +
> + return 0;
For this one also: return mipi_dsi_dcs_write(dsi, MIPI_DCS_EXIT_IDLE_MODE,
NULL, 0);
--
Varka Bhadram.
[v3 5/7] pwm: crc: Add Crystalcove (CRC) PWM driver
On 06/26/2015 02:32 PM, Shobhit Kumar wrote: > The Crystalcove PMIC provides three PWM signals and this driver exports > one of them on the BYT platform which is used to control backlight for > DSI panel. This is platform device implementation of the drivers/mfd > cell device for CRC PMIC. > > CC: Samuel Ortiz > Cc: Linus Walleij > Cc: Alexandre Courbot > Cc: Thierry Reding > Cc: Paul Bolle > Cc: Paul Gortmaker > Tested-by: Ville Syrjälä > Signed-off-by: Shobhit Kumar > --- > v2: Use the existing config callback with duty_ns and period_ns(Thierry) > > v3: Correct the subject line (Lee jones) > > v4: Address comment by Thierry & Paul > - Commit message update and fixes for few syntax errors > - Add PWM_CRC in Kconfig and Makefile sorted alphabetically > - Use the PWM_BASE_CLK as 600 for better code readability > - Remove the redundant rule of three while calculating pwm level > - Use the platform_device in pwm_chip > - Use builin_platform_driver > > v5: Address comments by Varka > - Make the bit definition using BIT macro > - reorder probe and remove code to directly return from > pwmchip_add/remove > > drivers/pwm/Kconfig | 7 +++ > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-crc.c | 143 > ++ > 3 files changed, 151 insertions(+) > create mode 100644 drivers/pwm/pwm-crc.c Reviewed-by: Varka Bhadram -- Best regards, Varka Bhadram.
[PATCH v7 05/15] drm/exynos: add Exynos5433 decon driver
On 06/22/2015 05:27 PM, Inki Dae wrote: > On 2015ë 06ì 22ì¼ 20:41, Varka Bhadram wrote: > (..) > +platform_set_drvdata(pdev, ctx); >> You are setting the driver data as ctx.. >> >> But no where you are using it...? >> >> Am i missing anything ? > See decon_bind and decon_unbind functions. :) Cleared. Thanks -- Best regards, Varka Bhadram.
[PATCH v7 05/15] drm/exynos: add Exynos5433 decon driver
Hi,
On 06/22/2015 04:46 PM, Inki Dae wrote:
> From: Joonyoung Shim
>
> DECON(Display and Enhancement Controller) is new IP replacing FIMD in
> Exynos5433. This patch adds Exynos5433 decon driver.
>
> Changelog v7:
> - Rebased on top of exynos-drm-next.
> - Added runtime pm support.
>
> Signed-off-by: Joonyoung Shim
> Signed-off-by: Hyungwon Hwang
> Signed-off-by: Inki Dae
> ---
>
(...)
> +static int exynos5433_decon_probe(struct platform_device *pdev)
> +{
> + struct device *dev = >dev;
> + struct decon_context *ctx;
> + struct resource *res;
> + int ret;
> + int i;
> +
> + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> +
> + ctx->default_win = 0;
> + ctx->suspended = true;
> + ctx->dev = dev;
> + if (of_get_child_by_name(dev->of_node, "i80-if-timings"))
> + ctx->i80_if = true;
> +
> + for (i = 0; i < ARRAY_SIZE(decon_clks_name); i++) {
> + struct clk *clk;
> +
> + clk = devm_clk_get(ctx->dev, decon_clks_name[i]);
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + ctx->clks[i] = clk;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(dev, "cannot find IO resource\n");
> + return -ENXIO;
> + }
> +
You people promised me to remove this check :-)
http://lists.freedesktop.org/archives/dri-devel/2015-April/081077.html
> + ctx->addr = devm_ioremap_resource(dev, res);
> + if (IS_ERR(ctx->addr)) {
> + dev_err(dev, "ioremap failed\n");
> + return PTR_ERR(ctx->addr);
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_IRQ,
> + ctx->i80_if ? "lcd_sys" : "vsync");
> + if (!res) {
> + dev_err(dev, "cannot find IRQ resource\n");
> + return -ENXIO;
> + }
> +
> + ret = devm_request_irq(dev, res->start, ctx->i80_if ?
> + decon_lcd_sys_irq_handler : decon_vsync_irq_handler, 0,
> + "drm_decon", ctx);
> + if (ret < 0) {
> + dev_err(dev, "lcd_sys irq request failed\n");
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, ctx);
You are setting the driver data as ctx..
But no where you are using it...?
Am i missing anything ?
--
Best regards,
Varka Bhadram.
[v2 5/7] pwm: crc: Add Crystalcove (CRC) PWM driver
Hi Shobhit Kumar,
On 06/22/2015 04:24 PM, Shobhit Kumar wrote:
> The Crystalcove PMIC provides three PWM signals and this driver exports
> one of them on the BYT platform which is used to control backlight for
> DSI panel. This is platform device implementation of the drivers/mfd
> cell device for CRC PMIC.
>
> v2: Use the existing config callback with duty_ns and period_ns(Thierry)
>
> v3: Correct the subject line (Lee jones)
>
> v4: Address comment by Thierry & Paul
> - Commit message update and fixes for few syntax errors
> - Add PWM_CRC in Kconfig and Makefile sorted alphabetically
> - Use the PWM_BASE_CLK as 600 for better code readability
> - Remove the redundant rule of three while calculating pwm level
> - Use the platform_device in pwm_chip
> - Use builin_platform_driver
>
> CC: Samuel Ortiz
> Cc: Linus Walleij
> Cc: Alexandre Courbot
> Cc: Thierry Reding
> Cc: Paul Bolle
> Cc: Paul Gortmaker
> Signed-off-by: Shobhit Kumar
(...)
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#define PWM0_CLK_DIV 0x4B
> +#define PWM_OUTPUT_ENABLE (1 << 7)
Can't be BIT() macro ?
> +#define PWM_DIV_CLK_0 0x00 /* DIVIDECLK = BASECLK */
> +#define PWM_DIV_CLK_100 0x63 /* DIVIDECLK = BASECLK/100 */
> +#define PWM_DIV_CLK_128 0x7F /* DIVIDECLK = BASECLK/128 */
> +
> +#define PWM0_DUTY_CYCLE 0x4E
> +#define BACKLIGHT_EN 0x51
(...)
> +static int crystalcove_pwm_probe(struct platform_device *pdev)
> +{
> + struct crystalcove_pwm *pwm;
> + int retval;
> + struct device *dev = pdev->dev.parent;
> + struct intel_soc_pmic *pmic = dev_get_drvdata(dev);
> +
> + pwm = devm_kzalloc(>dev, sizeof(*pwm), GFP_KERNEL);
> + if (!pwm)
> + return -ENOMEM;
> +
> + pwm->chip.dev = >dev;
> + pwm->chip.ops = _pwm_ops;
> + pwm->chip.base = -1;
> + pwm->chip.npwm = 1;
> +
> + /* get the PMIC regmap */
> + pwm->regmap = pmic->regmap;
> +
> + retval = pwmchip_add(>chip);
> + if (retval < 0)
> + return retval;
> +
> + platform_set_drvdata(pdev, pwm);
> +
If you can change this oder we can simply do something like this:
platform_set_drvdata(pdev, pwm);
return pwmchip_add(>chip);
> + return 0;
> +}
> +
> +static int crystalcove_pwm_remove(struct platform_device *pdev)
> +{
> + struct crystalcove_pwm *pwm = platform_get_drvdata(pdev);
> + int retval;
> +
> + retval = pwmchip_remove(>chip);
> + if (retval < 0)
> + return retval;
> +
> + dev_dbg(>dev, "crc-pwm driver removed\n");
This debug message may not be required :-)
you can directly do:
return pwmchip_remove(>chip);
> +
> + return 0;
> +}
> +
> +static struct platform_driver crystalcove_pwm_driver = {
> + .probe = crystalcove_pwm_probe,
> + .remove = crystalcove_pwm_remove,
> + .driver = {
> + .name = "crystal_cove_pwm",
> + },
> +};
> +
> +builtin_platform_driver(crystalcove_pwm_driver);
--
Best regards,
Varka Bhadram.
[v2 4/7] mfd: intel_soc_pmic_core: ADD PWM lookup table for CRC PMIC based PWM
Hi Shobhit Kumar, On 06/22/2015 04:24 PM, Shobhit Kumar wrote: > On some BYT PLatform the PWM is controlled using CRC PMIC. Add a lookup > entry for the same to be used by the consumer (Intel GFX) > > v2: Remove the lookup table on driver unload (Thierry) > > v3: Correct the subject line (Lee jones) This part should only describe what this is about.. Don't put this patch change history over here. Include this change history after ... Signed-off-by: Author --- > CC: Samuel Ortiz > Cc: Linus Walleij > Cc: Alexandre Courbot > Cc: Thierry Reding > Acked-by: Lee Jones > Signed-off-by: Shobhit Kumar > --- Here you add this change history so that after applying this will not be the part of your commit description. This comment is applicable for all of your patches. -- Best regards, Varka Bhadram.
[PATCH v5 01/12] drm/exynos: add Exynos5433 decon driver
On 04/10/2015 11:25 AM, Hyungwon Hwang wrote:
> From: Joonyoung Shim
>
> DECON(Display and Enhancement Controller) is new IP replacing FIMD in
> Exynos5433. This patch adds Exynos5433 decon driver.
>
> Signed-off-by: Joonyoung Shim
> Signed-off-by: Hyungwon Hwang
> ---
> Changes for v2:
> - change file names and variable names of decon to represnt exynos5433 instead
> of exynos to distinguish them from exynos7 decon
>
> Changes for v3:
> - fail fast when the proper image format is not set
> - remove unnecessary checking code
> - add and modify the function to make DPMS work well
>
> Changes for v4:
> - rebased to exynos-drm-next with the clean-up patchset by Gustavo Padovan.
>
> Changes for v5:
> - None
> .../devicetree/bindings/video/exynos5433-decon.txt | 65 +++
> drivers/gpu/drm/exynos/Kconfig | 6 +
> drivers/gpu/drm/exynos/Makefile| 1 +
> drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 617
> +
> drivers/gpu/drm/exynos/exynos_drm_drv.c| 3 +
> drivers/gpu/drm/exynos/exynos_drm_drv.h| 1 +
> include/video/exynos5433_decon.h | 163 ++
> 7 files changed, 856 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/video/exynos5433-decon.txt
> create mode 100644 drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> create mode 100644 include/video/exynos5433_decon.h
>
(...)
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(dev, "cannot find IO resource\n");
> + return -ENXIO;
> + }
> +
Remove the above check. Check one *res* will be done by
*devm_ioremap_resource()*
> + ctx->addr = devm_ioremap_resource(dev, res);
> + if (IS_ERR(ctx->addr)) {
> + dev_err(dev, "ioremap failed\n");
> + return PTR_ERR(ctx->addr);
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_IRQ,
> + ctx->i80_if ? "lcd_sys" : "vsync");
> + if (!res) {
> + dev_err(dev, "cannot find IRQ resource\n");
> + return -ENXIO;
> + }
> +
> + ret = devm_request_irq(dev, res->start, ctx->i80_if ?
> + decon_lcd_sys_irq_handler : decon_vsync_irq_handler, 0,
> + "drm_decon", ctx);
> + if (ret < 0) {
> + dev_err(dev, "lcd_sys irq request failed\n");
> + return ret;
> + }
> +
> + ret = exynos_drm_component_add(dev, EXYNOS_DEVICE_TYPE_CRTC,
> +EXYNOS_DISPLAY_TYPE_LCD);
> + if (ret < 0)
> + return ret;
> +
> + platform_set_drvdata(pdev, ctx);
> +
> + ret = component_add(dev, _component_ops);
> + if (ret < 0) {
> + exynos_drm_component_del(dev, EXYNOS_DEVICE_TYPE_CRTC);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int exynos5433_decon_remove(struct platform_device *pdev)
> +{
> + component_del(>dev, _component_ops);
> + exynos_drm_component_del(>dev, EXYNOS_DEVICE_TYPE_CRTC);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id exynos5433_decon_driver_dt_match[] = {
> + { .compatible = "samsung,exynos5433-decon" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, exynos5433_decon_driver_dt_match);
> +
> +struct platform_driver exynos5433_decon_driver = {
> + .probe = exynos5433_decon_probe,
> + .remove = exynos5433_decon_remove,
> + .driver = {
> + .name = "exynos5433-decon",
> + .owner = THIS_MODULE,
Remove this field. It will be updated by platform core.
> + .of_match_table = exynos5433_decon_driver_dt_match,
> + },
> +};
--
Varka Bhadram
[PATCH v4 04/11] pwm: add DT bindings documentation for atmel-hlcdc-pwm driver
On Tuesday 22 July 2014 06:41 PM, Boris BREZILLON wrote: > The HLCDC IP available in some Atmel SoCs (i.e. sam9x5i.e. at91sam9n12, > at91sam9x5 family or sama5d3 family) provide a PWM device. > > The DT bindings used for this PWM device is following the default 3 cells > bindings described in Documentation/devicetree/bindings/pwm/pwm.txt. > > Signed-off-by: Boris BREZILLON > --- > .../devicetree/bindings/pwm/atmel-hlcdc-pwm.txt| 55 > ++ > 1 file changed, 55 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt > > diff --git a/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt > b/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt > new file mode 100644 > index 000..86ad3e2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/pwm/atmel-hlcdc-pwm.txt > @@ -0,0 +1,55 @@ > +Device-Tree bindings for Atmel's HLCDC (High LCD Controller) PWM driver > + > +The Atmel HLCDC PWM is subdevice of the HLCDC MFD device. > +See ../mfd/atmel-hlcdc.txt for more details. > + > +Required properties: > + - compatible: value should be one of the following: > + "atmel,hlcdc-pwm" > + - pinctr-names: the pin control state names. Should contain "default". > + - pinctrl-0: should contain the pinctrl states described by pinctrl > + default. > + - #pwm-cells: should be set to 3. This PWM chip use the default 3 cells > + bindings defined in Documentation/devicetree/bindings/pwm/pwm.txt. > + The first cell encodes the PWM id (0 is the only acceptable value here, > + because the chip only provide one PWM). > + The second cell encodes the PWM period in nanoseconds. > + The third cell encodes the PWM flags (the only supported flag is > + PWM_POLARITY_INVERTED) It will be readable if: Required properties: - compatible : value should be one of the following: "atmel,hlcdc-pwm" - pinctr-names: the pin control state names. Should contain "default". - pinctrl-0 : should contain the pinctrl states described by pinctrl default. - #pwm-cells : should be set to 3. This PWM chip use the default 3 cells bindings defined in Documentation/devicetree/bindings/pwm/pwm.txt. The first cell encodes the PWM id (0 is the only acceptable value here, because the chip only provide one PWM). The second cell encodes the PWM period in nanoseconds. The third cell encodes the PWM flags (the only supported flag is PWM_POLARITY_INVERTED) -- Regards, Varka Bhadram
[PATCH v4 02/11] mfd: add documentation for atmel-hlcdc DT bindings
On Tuesday 22 July 2014 06:41 PM, Boris BREZILLON wrote: > The HLCDC IP available on some Atmel SoCs (i.e. at91sam9n12, at91sam9x5 > family or sama5d3 family) exposes 2 subdevices: > - a display controller (controlled by a DRM driver) > - a PWM chip > > This patch adds documentation for atmel-hlcdc DT bindings. > > Signed-off-by: Boris BREZILLON > --- > .../devicetree/bindings/mfd/atmel-hlcdc.txt| 50 > ++ > 1 file changed, 50 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt > > diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt > b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt > new file mode 100644 > index 000..e9cc1b2 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt > @@ -0,0 +1,50 @@ > +Device-Tree bindings for Atmel's HLCDC (High LCD Controller) MFD driver > + > +Required properties: > + - compatible: value should be one of the following: > + "atmel,sama5d3-hlcdc" > + - reg: base address and size of the HLCDC device registers. > + - clock-names: the name of the 3 clocks requested by the HLCDC device. > + Should contain "periph_clk", "sys_clk" and "slow_clk". > + - clocks: should contain the 3 clocks requested by the HLCDC device. > + These bindings not clearly readable. It would be readable if Required properties: - compatible : value should be one of the following:"atmel,sama5d3-hlcdc" - reg : base address and size of the HLCDC device registers. - clock-names : the name of the 3 clocks requested by the HLCDC device. Should contain "periph_clk", "sys_clk" and "slow_clk". - clocks : should contain the 3 clocks requested by the HLCDC device. .. -- Regards, Varka Bhadram
[PATCH v6 05/14] drm/exynos: dsi: add TE interrupt handler to support LCD I80 interface
On 07/22/2014 05:23 PM, YoungJun Cho wrote: > Hi Varka, > > On 07/22/2014 08:14 PM, Varka Bhadram wrote: >> On 07/22/2014 04:40 PM, YoungJun Cho wrote: >>> Hi Varka, >>> >>> This irq handler should be registered in attach() and unregistered in >>> detach(). >>> >>> The devm_* APIs are released(freed) in remove(), right? >>> >>> Logically the panel could be attached and detached several times after >>> dsi is probed and not removed. >>> So I don't use devm_* APIs. >> >> You meant to say that in-case of GPIOs also you are following the same >> thing ..? >> >> Means requesting the GPIOs and Releasing several times ..? >> > > Yes, this TE gpio is came from panel. > So it should be refresh whenever a (new) panel is attached. > In this case it would be fine. Thanks for the clarity. -- Regards, Varka Bhadram.
[PATCH v6 05/14] drm/exynos: dsi: add TE interrupt handler to support LCD I80 interface
On 07/22/2014 04:40 PM, YoungJun Cho wrote: > Hi Varka, > > This irq handler should be registered in attach() and unregistered in > detach(). > > The devm_* APIs are released(freed) in remove(), right? > > Logically the panel could be attached and detached several times after > dsi is probed and not removed. > So I don't use devm_* APIs. You meant to say that in-case of GPIOs also you are following the same thing ..? Means requesting the GPIOs and Releasing several times ..? > >> > -- Regards, Varka Bhadram.
[PATCH v6 05/14] drm/exynos: dsi: add TE interrupt handler to support LCD I80 interface
On 07/22/2014 04:19 PM, YoungJun Cho wrote:
(...)
> + ret = gpio_request_one(dsi->te_gpio, GPIOF_IN, "te_gpio");
devm_* APIs..?
> + if (ret) {
> + dev_err(dsi->dev, "gpio request failed with %d\n", ret);
> + goto out;
> + }
> +
> + /*
> + * This TE GPIO IRQ should not be set to IRQ_NOAUTOEN, because panel
> + * calls drm_panel_init() first then calls mipi_dsi_attach() in probe().
> + * It means that te_gpio is invalid when exynos_dsi_enable_irq() is
> + * called by drm_panel_init() before panel is attached.
> + */
> + ret = request_threaded_irq(gpio_to_irq(dsi->te_gpio),
> + exynos_dsi_te_irq_handler, NULL,
> + IRQF_TRIGGER_RISING, "TE", dsi);
why don't we use devm_request_threaded_irq()..?
--
Regards,
Varka Bhadram.
