Re: [PATCH 1/3] drm/bridge: microchip-lvds: Revert clk_prepare_enable() in case of failure

2024-09-02 Thread claudiu beznea



On 02.09.2024 07:39, dharm...@microchip.com wrote:
> On 27/08/24 9:42 pm, Claudiu Beznea wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
>> content is safe
>>
>> In case pm_runtime_get_sync() fails the clock remains enabled. Disable
>> it to save power on this failure scenario.
>>
>> Fixes: 179b0769fc5f ("drm/bridge: add lvds controller support for sam9x7")
>> Signed-off-by: Claudiu Beznea 
> Reviewed-and-tested-by: Dharma Balasubiramani 

This tag is not valid, AFAIK. You should use 2 different tags: Reviewed-by,
Tested-by

>> ---
>>   drivers/gpu/drm/bridge/microchip-lvds.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c 
>> b/drivers/gpu/drm/bridge/microchip-lvds.c
>> index b8313dad6072..027292ab0197 100644
>> --- a/drivers/gpu/drm/bridge/microchip-lvds.c
>> +++ b/drivers/gpu/drm/bridge/microchip-lvds.c
>> @@ -125,6 +125,7 @@ static void mchp_lvds_enable(struct drm_bridge *bridge)
>>
>>  ret = pm_runtime_get_sync(lvds->dev);
>>  if (ret < 0) {
>> +   clk_disable_unprepare(lvds->pclk);
>>  dev_err(lvds->dev, "failed to get pm runtime: %d\n", ret);
>>  return;
>>  }
>> --
>> 2.39.2
>>
> 
> 


[PATCH 3/3] drm/bridge: microchip-lvds: Use devm_platform_ioremap_resource()

2024-08-27 Thread Claudiu Beznea
The devm_platform_ioremap_resouce() does exactly what
devm_ioremap_resource() combined with platform_get_resouce() does.
Thus use it.

Signed-off-by: Claudiu Beznea 
---
 drivers/gpu/drm/bridge/microchip-lvds.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c 
b/drivers/gpu/drm/bridge/microchip-lvds.c
index f04831106eea..85aff8c5aaf4 100644
--- a/drivers/gpu/drm/bridge/microchip-lvds.c
+++ b/drivers/gpu/drm/bridge/microchip-lvds.c
@@ -154,8 +154,7 @@ static int mchp_lvds_probe(struct platform_device *pdev)
 
lvds->dev = dev;
 
-   lvds->regs = devm_ioremap_resource(lvds->dev,
-   platform_get_resource(pdev, IORESOURCE_MEM, 0));
+   lvds->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(lvds->regs))
return PTR_ERR(lvds->regs);
 
-- 
2.39.2



[PATCH 2/3] drm/bridge: microchip-lvds: Drop unused headers

2024-08-27 Thread Claudiu Beznea
Drop unused headers. With this code becomes simpler.

Signed-off-by: Claudiu Beznea 
---
 drivers/gpu/drm/bridge/microchip-lvds.c | 11 +--
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c 
b/drivers/gpu/drm/bridge/microchip-lvds.c
index 027292ab0197..f04831106eea 100644
--- a/drivers/gpu/drm/bridge/microchip-lvds.c
+++ b/drivers/gpu/drm/bridge/microchip-lvds.c
@@ -8,25 +8,16 @@
  */
 
 #include 
-#include 
 #include 
 #include 
-#include 
+#include 
 #include 
-#include 
-#include 
 #include 
 #include 
-#include 
-#include 
 
-#include 
 #include 
 #include 
 #include 
-#include 
-#include 
-#include 
 
 #define LVDS_POLL_TIMEOUT_MS 1000
 
-- 
2.39.2



[PATCH 1/3] drm/bridge: microchip-lvds: Revert clk_prepare_enable() in case of failure

2024-08-27 Thread Claudiu Beznea
In case pm_runtime_get_sync() fails the clock remains enabled. Disable
it to save power on this failure scenario.

Fixes: 179b0769fc5f ("drm/bridge: add lvds controller support for sam9x7")
Signed-off-by: Claudiu Beznea 
---
 drivers/gpu/drm/bridge/microchip-lvds.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/microchip-lvds.c 
b/drivers/gpu/drm/bridge/microchip-lvds.c
index b8313dad6072..027292ab0197 100644
--- a/drivers/gpu/drm/bridge/microchip-lvds.c
+++ b/drivers/gpu/drm/bridge/microchip-lvds.c
@@ -125,6 +125,7 @@ static void mchp_lvds_enable(struct drm_bridge *bridge)
 
ret = pm_runtime_get_sync(lvds->dev);
if (ret < 0) {
+   clk_disable_unprepare(lvds->pclk);
dev_err(lvds->dev, "failed to get pm runtime: %d\n", ret);
return;
}
-- 
2.39.2



[PATCH 0/3] drm/bridge: microchip-lvds: Cleanups

2024-08-27 Thread Claudiu Beznea
Hi,

Series adds few cleanups identified while looking though the driver.
Please note that these are compile test only.

Thank you,
Claudiu Beznea

Claudiu Beznea (3):
  drm/bridge: microchip-lvds: Revert clk_prepare_enable() in case of
failure
  drm/bridge: microchip-lvds: Drop unused headers
  drm/bridge: microchip-lvds: Use devm_platform_ioremap_resource()

 drivers/gpu/drm/bridge/microchip-lvds.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

-- 
2.39.2



Re: [PATCH 2/4] drm/bridge: add Microchip DSI controller support for sam9x7 SoC series

2024-07-14 Thread claudiu beznea



On 04.07.2024 11:48, Manikandan Muralidharan wrote:
> Add the Microchip's DSI controller wrapper driver that uses
> the Synopsys DesignWare MIPI DSI host controller bridge.
> 
> Signed-off-by: Manikandan Muralidharan 
> ---
>  drivers/gpu/drm/bridge/Kconfig|   8 +
>  drivers/gpu/drm/bridge/Makefile   |   1 +
>  drivers/gpu/drm/bridge/dw-mipi-dsi-mchp.c | 544 ++
>  3 files changed, 553 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/dw-mipi-dsi-mchp.c
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index c621be1a99a8..459ad9768234 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -196,6 +196,14 @@ config DRM_MICROCHIP_LVDS_SERIALIZER
>   help
> Support for Microchip's LVDS serializer.
>  
> +config DRM_MICROCHIP_DW_MIPI_DSI
> + tristate "Microchip specific extensions for Synopsys DW MIPI DSI"
> + depends on DRM_ATMEL_HLCDC
> + select DRM_DW_MIPI_DSI
> + help
> +   This selects support for Microchip's SoC specific extensions
> +   for the Synopsys DesignWare dsi driver.
> +
>  config DRM_NWL_MIPI_DSI
>   tristate "Northwest Logic MIPI DSI Host controller"
>   depends on DRM
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 7df87b582dca..aff5052100b9 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_DRM_LONTIUM_LT9611UXC) += lontium-lt9611uxc.o
>  obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o
>  obj-$(CONFIG_DRM_MEGACHIPS_STDP_GE_B850V3_FW) += 
> megachips-stdp-ge-b850v3-fw.o
>  obj-$(CONFIG_DRM_MICROCHIP_LVDS_SERIALIZER) += microchip-lvds.o
> +obj-$(CONFIG_DRM_MICROCHIP_DW_MIPI_DSI) += dw-mipi-dsi-mchp.o
>  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
>  obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
>  obj-$(CONFIG_DRM_PARADE_PS8640) += parade-ps8640.o
> diff --git a/drivers/gpu/drm/bridge/dw-mipi-dsi-mchp.c 
> b/drivers/gpu/drm/bridge/dw-mipi-dsi-mchp.c
> new file mode 100644
> index ..d2c4525677ab
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/dw-mipi-dsi-mchp.c
> @@ -0,0 +1,544 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2024 Microchip Technology Inc. and its subsidiaries
> + *
> + * Author: Manikandan Muralidharan 
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DSI_PLL_REF_CLK  2400
> +
> +#define DSI_PWR_UP   0x04
> +#define HOST_RESET   BIT(0)
> +#define HOST_PWRUP   0
> +
> +#define DSI_PHY_RSTZ 0xa0
> +#define PHY_SHUTDOWNZ0
> +
> +#define DSI_PHY_TST_CTRL00xb4
> +#define PHY_TESTCLK  BIT(1)
> +#define PHY_UNTESTCLK0
> +#define PHY_TESTCLR  BIT(0)
> +#define PHY_UNTESTCLR0
> +
> +#define DSI_PHY_TST_CTRL10xb8
> +#define PHY_TESTEN   BIT(16)
> +#define PHY_UNTESTEN 0
> +#define PHY_TESTDOUT(n)  (((n) & 0xff) << 8)
> +#define PHY_TESTDIN(n)   (((n) & 0xff) << 0)

These 2 looks like FIELD_PREP() candidates.

> +
> +#define BYPASS_VCO_RANGE BIT(7)
> +#define VCO_RANGE_CON_SEL(val)   (((val) & 0x7) << 3)

This, too.

> +#define VCO_IN_CAP_CON_LOW   BIT(1)
> +
> +#define CP_CURRENT_0 0x2
> +#define CP_CURRENT_1 0x4
> +#define CP_CURRENT_2 0x5
> +#define CP_CURRENT_3 0x6
> +#define CP_CURRENT_4 0x7
> +#define CP_CURRENT_5 0x8
> +#define CP_CURRENT_6 0xc
> +#define CP_CURRENT_SEL(val)  ((val) & 0xf)

This, too.

> +#define CP_PROGRAM_ENBIT(7)
> +
> +#define LPF_RESISTORS_18KOHM 0x0
> +#define LPF_RESISTORS_15_6KOHM   0x1
> +#define LPF_RESISTORS_15KOHM 0x2
> +#define LPF_RESISTORS_14_4KOHM   0x4
> +#define LPF_RESISTORS_12_8KOHM   0x8
> +#define LPF_RESISTORS_11_4KOHM   0x10
> +#define LPF_RESISTORS_10_5KOHM   0x20

Some of these are unsused.

> +#define LPF_PROGRAM_EN   BIT(6)
> +#define LPF_RESISTORS_SEL(val)   ((val) & 0x3f)

This, too

> +
> +#define HSFREQRANGE_SEL(val) (((val) & 0x3f) << 1)

Unused

> +
> +#define INPUT_DIVIDER(val)   (((val) - 1) & 0x7f)
> +#define LOW_PROGRAM_EN   0
> +#define HIGH_PROGRAM_EN  BIT(7)
> +#define LOOP_DIV_LOW_SEL(val)(((val) - 1) & 0x1f)
> +#define LOOP_DIV_HIGH_SEL(val)   val) - 1) >> 5) & 0xf)
> +#define PLL_LOOP_DIV_EN  BIT(5)
> 

Re: [PATCH v8 4/4] ARM: configs: at91: Enable LVDS serializer support

2024-06-09 Thread claudiu beznea



On 21.04.2024 04:10, Dharma Balasubiramani wrote:
> Enable LVDS serializer support for display pipeline.
> 
> Signed-off-by: Dharma Balasubiramani 
> Acked-by: Hari Prasath Gujulan Elango 
> Acked-by: Nicolas Ferre 

Applied to at91-defconfig, thanks!


Re: [PATCH v8 0/7] Add support for XLCDC to sam9x7 SoC family.

2024-02-24 Thread claudiu beznea
Hi, Manikandan,

On 21.02.2024 07:35, Manikandan Muralidharan wrote:
> This patch series aims to add support for XLCDC IP of sam9x7 SoC family
> to the DRM subsystem.XLCDC IP has additional registers and new
> configuration bits compared to the existing register set of HLCDC IP.
> The new compatible string "microchip,sam9x75-xlcdc" is defined for sam9x75
> variant of the sam9x7 SoC family.The is_xlcdc flag under driver data and
> IP specific driver ops helps to differentiate the XLCDC and existing HLCDC
> code within the same driver.
> 
> changes in v8:
> * Re-arrange the patch set to prepare and update the current HLCDC
> code base with the new LCDC IP based driver ops and then add support
> for XLCDC code changes.
> * Fix Cosmetic issues.
> 
> changes in v7:
> * LCDC IP driver ops functions are declared static and its 
> declaration are removed.
> 
> changes in v6:
> * Fixed Cosmetic defects.
> * Added comments for readability.
> 
> changes in v5:
> * return value of regmap_read_poll_timeout is checked in failure
> case.
> * HLCDC and XLCDC specific driver functions are now invoked
> using its IP specific driver ops w/o the need of checking is_xlcdc flag.
> * Removed empty spaces and blank lines.
> 
> changes in v4:
> * fixed kernel warnings reported by kernel test robot.
> 
> changes in v3:
> * Removed de-referencing the value of is_xlcdc flag multiple times in
> a single function.
> * Removed cpu_relax() call when using regmap_read_poll_timeout.
> * Updated xfactor and yfactor equations using shift operators
> * Defined CSC co-efficients in an array for code readability.
> 
> changes in v2:
> * Change the driver compatible name from "microchip,sam9x7-xlcdc" to
> "microchip,sam9x75-xlcdc".
> * Move is_xlcdc flag to driver data.
> * Remove unsed Macro definitions.
> * Add co-developed-bys tags
> * Replace regmap_read() with regmap_read_poll_timeout() call
> * Split code into two helpers for code readablitity.
> ---
> 
> Durai Manickam KR (1):
>   drm: atmel-hlcdc: Define XLCDC specific registers
> 
> Manikandan Muralidharan (6):
>   drm: atmel-hlcdc: add driver ops to differentiate HLCDC and XLCDC IP
>   drm: atmel_hlcdc: Add support for XLCDC using IP specific driver ops
>   drm: atmel-hlcdc: add DPI mode support for XLCDC
>   drm: atmel-hlcdc: add vertical and horizontal scaling support for
> XLCDC
>   drm: atmel-hlcdc: add support for DSI output formats
>   drm: atmel-hlcdc: add LCD controller layer definition for sam9x75
> 

Only minor comments from me (check individual patches). W/ or w/o those
addressed you can add:

Reviewed-by: Claudiu Beznea 



Re: [PATCH v8 3/7] drm: atmel_hlcdc: Add support for XLCDC using IP specific driver ops

2024-02-24 Thread claudiu beznea



On 21.02.2024 07:35, Manikandan Muralidharan wrote:
> Add XLCDC specific driver ops and is_xlcdc flag to separate the
> functionality and to access the controller registers.
> HEO scaling, window resampling, Alpha blending, YUV-to-RGB
> conversion in XLCDC is derived and handled using additional
> configuration bits and registers. Writing one to the Enable fields
> of each layer in LCD_ATTRE is required to reflect the values set
> in Configuration, FBA, Enable registers of each layer.
> 
> Signed-off-by: Manikandan Muralidharan 
> Co-developed-by: Hari Prasath Gujulan Elango 
> Signed-off-by: Hari Prasath Gujulan Elango 
> Co-developed-by: Durai Manickam KR 
> Signed-off-by: Durai Manickam KR 
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c|  81 +---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h  |   3 +
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 182 +-
>  3 files changed, 242 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index cc5cf4c2faf7..98a98b5fca85 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -164,11 +164,13 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct 
> drm_crtc *c)
>   state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
>   cfg = state->output_mode << 8;
>  
> - if (adj->flags & DRM_MODE_FLAG_NVSYNC)
> - cfg |= ATMEL_HLCDC_VSPOL;
> + if (!crtc->dc->desc->is_xlcdc) {
> + if (adj->flags & DRM_MODE_FLAG_NVSYNC)
> + cfg |= ATMEL_HLCDC_VSPOL;
>  
> - if (adj->flags & DRM_MODE_FLAG_NHSYNC)
> - cfg |= ATMEL_HLCDC_HSPOL;
> + if (adj->flags & DRM_MODE_FLAG_NHSYNC)
> + cfg |= ATMEL_HLCDC_HSPOL;
> + }
>  
>   regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
>  ATMEL_HLCDC_HSPOL | ATMEL_HLCDC_VSPOL |
> @@ -202,20 +204,37 @@ static void atmel_hlcdc_crtc_atomic_disable(struct 
> drm_crtc *c,
>  
>   pm_runtime_get_sync(dev->dev);
>  
> + if (crtc->dc->desc->is_xlcdc) {
> + regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_CM);
> + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  !(status & ATMEL_XLCDC_CM),
> +  10, 1000))
> + dev_warn(dev->dev, "Atmel LCDC status register CMSTS 
> timeout\n");
> +
> + regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
> + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  status & ATMEL_XLCDC_SD,
> +  10, 1000))
> + dev_warn(dev->dev, "Atmel LCDC status register SDSTS 
> timeout\n");
> + }
> +
>   regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
> - while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
> -(status & ATMEL_HLCDC_DISP))
> - cpu_relax();
> + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  !(status & ATMEL_HLCDC_DISP),
> + 10, 1000))
> + dev_warn(dev->dev, "Atmel LCDC status register DISPSTS 
> timeout\n");

This is different thing than what the commit states it does but if
maintainer wants to keep it like this I don't have anything against. Valid
for the rest of regmap_read_poll_timeout() additions.

>  
>   regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_SYNC);
> - while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
> -(status & ATMEL_HLCDC_SYNC))
> - cpu_relax();
> + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  !(status & ATMEL_HLCDC_SYNC),
> + 10, 1000))
> + dev_warn(dev->dev, "Atmel LCDC status register LCDSTS 
> timeout\n");
>  
>   regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_PIXEL_CLK);
> - while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
> -(status & ATMEL_HLCDC_PIXEL_CLK))
> - cpu_relax();
> + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  !(status & ATMEL_HLCDC_PIXEL_CLK),
> + 10, 1000))
> + dev_warn(dev->dev, "Atmel LCDC status register CLKSTS 
> timeout\n");
>  
>   clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
>   pinctrl_pm_select_sleep_state(dev->dev);
> @@ -241,20 +260,36 @@ static void atmel_hlcdc_crtc_atomic_enable(struct 
> drm_crtc *c,
>   clk_prepare_enable(crtc->dc->hlcdc->sys_clk);
>  
>   regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_HLCDC_PIXEL_CLK);
> - while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
> -!(status & ATMEL_HLCDC_P

Re: [PATCH v8 6/7] drm: atmel-hlcdc: add support for DSI output formats

2024-02-24 Thread claudiu beznea



On 21.02.2024 07:35, Manikandan Muralidharan wrote:
> Add support for the following DPI mode if the encoder type
> is DSI as per the XLCDC IP datasheet:
> - 16BPPCFG1
> - 16BPPCFG2
> - 16BPPCFG3
> - 18BPPCFG1
> - 18BPPCFG2
> - 24BPP
> 
> Signed-off-by: Manikandan Muralidharan 
> [durai.manicka...@microchip.com: update output format using is_xlcdc flag]
> Signed-off-by: Durai Manickam KR 
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 74 +--
>  1 file changed, 68 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index fdd3a6bc0f79..0dbe85686fc2 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -301,11 +301,64 @@ static void atmel_hlcdc_crtc_atomic_enable(struct 
> drm_crtc *c,
>  
>  }
>  
> -#define ATMEL_HLCDC_RGB444_OUTPUTBIT(0)
> -#define ATMEL_HLCDC_RGB565_OUTPUTBIT(1)
> -#define ATMEL_HLCDC_RGB666_OUTPUTBIT(2)
> -#define ATMEL_HLCDC_RGB888_OUTPUTBIT(3)
> -#define ATMEL_HLCDC_OUTPUT_MODE_MASK GENMASK(3, 0)
> +#define ATMEL_HLCDC_RGB444_OUTPUTBIT(0)
> +#define ATMEL_HLCDC_RGB565_OUTPUTBIT(1)
> +#define ATMEL_HLCDC_RGB666_OUTPUTBIT(2)
> +#define ATMEL_HLCDC_RGB888_OUTPUTBIT(3)
> +#define ATMEL_HLCDC_DPI_RGB565C1_OUTPUT  BIT(4)
> +#define ATMEL_HLCDC_DPI_RGB565C2_OUTPUT  BIT(5)
> +#define ATMEL_HLCDC_DPI_RGB565C3_OUTPUT  BIT(6)
> +#define ATMEL_HLCDC_DPI_RGB666C1_OUTPUT  BIT(7)
> +#define ATMEL_HLCDC_DPI_RGB666C2_OUTPUT  BIT(8)
> +#define ATMEL_HLCDC_DPI_RGB888_OUTPUTBIT(9)
> +#define ATMEL_HLCDC_OUTPUT_MODE_MASK GENMASK(3, 0)
> +#define ATMEL_XLCDC_OUTPUT_MODE_MASK GENMASK(9, 0)
> +
> +static int atmel_hlcdc_connector_output_dsi(struct drm_encoder *encoder,
> + struct drm_display_info *info)
> +{
> + int j;
> + unsigned int supported_fmts = 0;
> +
> + switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
> + case 0:
> + break;
> + case MEDIA_BUS_FMT_RGB565_1X16:
> + return ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;
> + case MEDIA_BUS_FMT_RGB666_1X18:
> + return ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;
> + case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
> + return ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;
> + case MEDIA_BUS_FMT_RGB888_1X24:
> + return ATMEL_HLCDC_DPI_RGB888_OUTPUT;
> + default:
> + return -EINVAL;
> + }
> +
> + for (j = 0; j < info->num_bus_formats; j++) {
> + switch (info->bus_formats[j]) {
> + case MEDIA_BUS_FMT_RGB565_1X16:
> + supported_fmts |=
> + ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;

This can fit on the above line

> + break;
> + case MEDIA_BUS_FMT_RGB666_1X18:
> + supported_fmts |=
> + ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;

Ditto

> + break;
> + case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
> + supported_fmts |=
> + ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;

Ditto

> + break;
> + case MEDIA_BUS_FMT_RGB888_1X24:
> + supported_fmts |=
> + ATMEL_HLCDC_DPI_RGB888_OUTPUT;

Ditto

> + break;
> + default:
> + break;
> + }
> + }
> + return supported_fmts;
> +}
>  
>  static int atmel_hlcdc_connector_output_mode(struct drm_connector_state 
> *state)
>  {
> @@ -318,6 +371,13 @@ static int atmel_hlcdc_connector_output_mode(struct 
> drm_connector_state *state)
>   encoder = state->best_encoder;
>   if (!encoder)
>   encoder = connector->encoder;
> + /*
> +  * atmel-hlcdc to support DSI formats with DSI video pipeline
> +  * when DRM_MODE_ENCODER_DSI type is set by
> +  * connector driver component.
> +  */
> + if (encoder->encoder_type == DRM_MODE_ENCODER_DSI)
> + return atmel_hlcdc_connector_output_dsi(encoder, info);
>  
>   switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
>   case 0:
> @@ -358,7 +418,7 @@ static int atmel_hlcdc_connector_output_mode(struct 
> drm_connector_state *state)
>  
>  static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
>  {
> - unsigned int output_fmts = ATMEL_HLCDC_OUTPUT_MODE_MASK;
> + unsigned int output_fmts;
>   struct atmel_hlcdc_crtc_state *hstate;
>   struct drm_connector_state *cstate;
>   struct drm_connector *connector;
> @@ -366,6 +426,8 @@ static int atmel_hlcdc_crtc_select_output_mode(struct 
> drm_crtc_state *state)
>   int i;
>  
>   crtc = drm_crtc_to_atmel_hlcdc_crtc(state->crtc);
> + output_fmts = crtc->dc-

Re: [PATCH v8 1/7] drm: atmel-hlcdc: add driver ops to differentiate HLCDC and XLCDC IP

2024-02-24 Thread claudiu beznea



On 21.02.2024 07:35, Manikandan Muralidharan wrote:
> Add LCD IP specific ops in driver data to differentiate
> HLCDC and XLCDC code within the atmel-hlcdc driver files.
> XLCDC in SAM9X7 has different sets of registers and additional
> configuration bits when compared to previous HLCDC IP. Read/write
> operation on the controller register and functionality is now
> separated using the LCD IP specific ops.
> 
> Signed-off-by: Manikandan Muralidharan 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c  |   5 +
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h  |  84 ++---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 167 +++---
>  3 files changed, 173 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 84c54e8622d1..b09df821cbc0 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -58,6 +58,7 @@ static const struct atmel_hlcdc_dc_desc 
> atmel_hlcdc_dc_at91sam9n12 = {
>   .conflicting_output_formats = true,
>   .nlayers = ARRAY_SIZE(atmel_hlcdc_at91sam9n12_layers),
>   .layers = atmel_hlcdc_at91sam9n12_layers,
> + .ops = &atmel_hlcdc_ops,
>  };
>  
>  static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = 
> {
> @@ -151,6 +152,7 @@ static const struct atmel_hlcdc_dc_desc 
> atmel_hlcdc_dc_at91sam9x5 = {
>   .conflicting_output_formats = true,
>   .nlayers = ARRAY_SIZE(atmel_hlcdc_at91sam9x5_layers),
>   .layers = atmel_hlcdc_at91sam9x5_layers,
> + .ops = &atmel_hlcdc_ops,
>  };
>  
>  static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = {
> @@ -269,6 +271,7 @@ static const struct atmel_hlcdc_dc_desc 
> atmel_hlcdc_dc_sama5d3 = {
>   .conflicting_output_formats = true,
>   .nlayers = ARRAY_SIZE(atmel_hlcdc_sama5d3_layers),
>   .layers = atmel_hlcdc_sama5d3_layers,
> + .ops = &atmel_hlcdc_ops,
>  };
>  
>  static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = {
> @@ -364,6 +367,7 @@ static const struct atmel_hlcdc_dc_desc 
> atmel_hlcdc_dc_sama5d4 = {
>   .max_hpw = 0x3ff,
>   .nlayers = ARRAY_SIZE(atmel_hlcdc_sama5d4_layers),
>   .layers = atmel_hlcdc_sama5d4_layers,
> + .ops = &atmel_hlcdc_ops,
>  };
>  
>  static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sam9x60_layers[] = {
> @@ -460,6 +464,7 @@ static const struct atmel_hlcdc_dc_desc 
> atmel_hlcdc_dc_sam9x60 = {
>   .fixed_clksrc = true,
>   .nlayers = ARRAY_SIZE(atmel_hlcdc_sam9x60_layers),
>   .layers = atmel_hlcdc_sam9x60_layers,
> + .ops = &atmel_hlcdc_ops,
>  };
>  
>  static const struct of_device_id atmel_hlcdc_of_match[] = {
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index 5b5c774e0edf..ad732edfef0b 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -288,6 +288,64 @@ atmel_hlcdc_layer_to_plane(struct atmel_hlcdc_layer 
> *layer)
>   return container_of(layer, struct atmel_hlcdc_plane, layer);
>  }
>  
> +/**
> + * Atmel HLCDC Display Controller.

This doens't comply w/ recommendation at
https://docs.kernel.org/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation

> + *
> + * @desc: HLCDC Display Controller description
> + * @dscrpool: DMA coherent pool used to allocate DMA descriptors
> + * @hlcdc: pointer to the atmel_hlcdc structure provided by the MFD device
> + * @fbdev: framebuffer device attached to the Display Controller
> + * @crtc: CRTC provided by the display controller
> + * @planes: instantiated planes
> + * @layers: active HLCDC layers
> + * @suspend: used to store the HLCDC state when entering suspend

Check this for nested struct documentation:
https://docs.kernel.org/doc-guide/kernel-doc.html#nested-structs-unions

> + */
> +struct atmel_hlcdc_dc {
> + const struct atmel_hlcdc_dc_desc *desc;
> + struct dma_pool *dscrpool;
> + struct atmel_hlcdc *hlcdc;
> + struct drm_crtc *crtc;
> + struct atmel_hlcdc_layer *layers[ATMEL_HLCDC_MAX_LAYERS];
> + struct {
> + u32 imr;
> + struct drm_atomic_state *state;
> + } suspend;
> +};
> +
> +struct atmel_hlcdc_plane_state;
> +
> +/**
> + * struct atmel_lcdc_dc_ops - describes atmel_lcdc ops group
> + * to differentiate HLCDC and XLCDC IP code support
> + * @plane_setup_scaler: update the vertical and horizontal scaling factors
> + * @update_lcdc_buffers: update the each LCDC layers DMA registers
> + * @lcdc_atomic_disable: disable LCDC interrupts and layers
> + * @lcdc_update_general_settings: update each LCDC layers general
> + * confiugration register

s/confiugration/configuration

> + * @lcdc_atomic_update: enable the LCDC layers and interrupts
> + * @lcdc_csc_init: update the color space conversion co-efficient of
> + * High-end overlay register
> +

Re: [PATCH RESEND v7 7/7] drm: atmel-hlcdc: add support for DSI output formats

2024-02-03 Thread claudiu beznea



On 29.01.2024 11:23, Manikandan Muralidharan wrote:
> Add support for the following DPI mode if the encoder type
> is DSI as per the XLCDC IP datasheet:
> - 16BPPCFG1
> - 16BPPCFG2
> - 16BPPCFG3
> - 18BPPCFG1
> - 18BPPCFG2
> - 24BPP
> 
> Signed-off-by: Manikandan Muralidharan 
> [durai.manicka...@microchip.com: update output format using is_xlcdc flag]
> Signed-off-by: Durai Manickam KR 
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 123 +-
>  1 file changed, 88 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 1899be2eb6a3..6f529769b036 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -295,11 +295,18 @@ static void atmel_hlcdc_crtc_atomic_enable(struct 
> drm_crtc *c,
>  
>  }
>  
> -#define ATMEL_HLCDC_RGB444_OUTPUTBIT(0)
> -#define ATMEL_HLCDC_RGB565_OUTPUTBIT(1)
> -#define ATMEL_HLCDC_RGB666_OUTPUTBIT(2)
> -#define ATMEL_HLCDC_RGB888_OUTPUTBIT(3)
> -#define ATMEL_HLCDC_OUTPUT_MODE_MASK GENMASK(3, 0)
> +#define ATMEL_HLCDC_RGB444_OUTPUTBIT(0)
> +#define ATMEL_HLCDC_RGB565_OUTPUTBIT(1)
> +#define ATMEL_HLCDC_RGB666_OUTPUTBIT(2)
> +#define ATMEL_HLCDC_RGB888_OUTPUTBIT(3)
> +#define ATMEL_HLCDC_DPI_RGB565C1_OUTPUT  BIT(4)
> +#define ATMEL_HLCDC_DPI_RGB565C2_OUTPUT  BIT(5)
> +#define ATMEL_HLCDC_DPI_RGB565C3_OUTPUT  BIT(6)
> +#define ATMEL_HLCDC_DPI_RGB666C1_OUTPUT  BIT(7)
> +#define ATMEL_HLCDC_DPI_RGB666C2_OUTPUT  BIT(8)
> +#define ATMEL_HLCDC_DPI_RGB888_OUTPUTBIT(9)
> +#define ATMEL_HLCDC_OUTPUT_MODE_MASK GENMASK(3, 0)
> +#define ATMEL_XLCDC_OUTPUT_MODE_MASK GENMASK(9, 0)
>  
>  static int atmel_hlcdc_connector_output_mode(struct drm_connector_state 
> *state)
>  {
> @@ -313,53 +320,99 @@ static int atmel_hlcdc_connector_output_mode(struct 
> drm_connector_state *state)
>   if (!encoder)
>   encoder = connector->encoder;
>  
> - switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
> - case 0:
> - break;
> - case MEDIA_BUS_FMT_RGB444_1X12:
> - return ATMEL_HLCDC_RGB444_OUTPUT;
> - case MEDIA_BUS_FMT_RGB565_1X16:
> - return ATMEL_HLCDC_RGB565_OUTPUT;
> - case MEDIA_BUS_FMT_RGB666_1X18:
> - return ATMEL_HLCDC_RGB666_OUTPUT;
> - case MEDIA_BUS_FMT_RGB888_1X24:
> - return ATMEL_HLCDC_RGB888_OUTPUT;
> - default:
> - return -EINVAL;
> - }
> -
> - for (j = 0; j < info->num_bus_formats; j++) {
> - switch (info->bus_formats[j]) {
> - case MEDIA_BUS_FMT_RGB444_1X12:
> - supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;

To generate less diff here and have a cleaner code you can move all this
DSI specific code in a different function and have here something like:

if (encoder->encoder_type == DRM_MODE_ENCODER_DSI)
return atmel_hlcdc_connector_output_dsi();

> + if (encoder->encoder_type == DRM_MODE_ENCODER_DSI) {
> + /*
> +  * atmel-hlcdc to support DSI formats with DSI video pipeline
> +  * when DRM_MODE_ENCODER_DSI type is set by
> +  * connector driver component.
> +  */
> + switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
> + case 0:
>   break;
>   case MEDIA_BUS_FMT_RGB565_1X16:
> - supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
> - break;
> + return ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;
>   case MEDIA_BUS_FMT_RGB666_1X18:
> - supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
> - break;
> + return ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;
> + case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
> + return ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;
>   case MEDIA_BUS_FMT_RGB888_1X24:
> - supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
> - break;
> + return ATMEL_HLCDC_DPI_RGB888_OUTPUT;
>   default:
> + return -EINVAL;
> + }
> +
> + for (j = 0; j < info->num_bus_formats; j++) {
> + switch (info->bus_formats[j]) {
> + case MEDIA_BUS_FMT_RGB565_1X16:
> + supported_fmts |=
> + ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;
> + break;
> + case MEDIA_BUS_FMT_RGB666_1X18:
> + supported_fmts |=
> + ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;
> + break;
> + case MEDIA_BUS_FMT_RGB666_1X2

Re: [PATCH RESEND v7 4/7] drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver

2024-02-03 Thread claudiu beznea



On 29.01.2024 11:23, Manikandan Muralidharan wrote:
> XLCDC in SAM9X7 has different sets of registers and additional
> configuration bits when compared to previous HLCDC IP. Read/write
> operation on the controller registers is now separated using the
> XLCDC status flag and with HLCDC and XLCDC IP specific ops.
> HEO scaling, window resampling, Alpha blending, YUV-to-RGB
> conversion in XLCDC is derived and handled using additional
> configuration bits and registers. Writing one to the Enable fields
> of each layer in LCD_ATTRE is required to reflect the values set
> in Configuration, FBA, Enable registers of each layer.
> 
> Signed-off-by: Manikandan Muralidharan 
> Co-developed-by: Hari Prasath Gujulan Elango 
> Signed-off-by: Hari Prasath Gujulan Elango 
> Co-developed-by: Durai Manickam KR 
> Signed-off-by: Durai Manickam KR 
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c|  33 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c  |   6 +
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h  |   3 +
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 349 +++---
>  4 files changed, 329 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index cc5cf4c2faf7..1ac31c0c474a 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -79,6 +79,7 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
> *c)
>   unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
>   unsigned int cfg = 0;
>   int div, ret;
> + bool is_xlcdc = crtc->dc->desc->is_xlcdc;

You may want to keep reverse christmass tree order, with this, though the
mask variable breaks it.

>  
>   /* get encoder from crtc */
>   drm_for_each_encoder(en_iter, ddev) {
> @@ -164,10 +165,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct 
> drm_crtc *c)
>   state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
>   cfg = state->output_mode << 8;
>  
> - if (adj->flags & DRM_MODE_FLAG_NVSYNC)
> + if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
>   cfg |= ATMEL_HLCDC_VSPOL;
>  
> - if (adj->flags & DRM_MODE_FLAG_NHSYNC)
> + if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NHSYNC))
>   cfg |= ATMEL_HLCDC_HSPOL;

Instead of checking 2 times the !is_xlcdc you can have on check: e.g.:

if (!is_xlcdc) {
if (adj->flags & DRM_MODE_FLAG_NVSYNC)
cfg |= ATMEL_HLCDC_VSPOL;

if (adj->flags & DRM_MODE_FLAG_NHSYNC)
cfg |= ATMEL_HLCDC_HSPOL;
}

>  
>   regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
> @@ -202,6 +203,20 @@ static void atmel_hlcdc_crtc_atomic_disable(struct 
> drm_crtc *c,
>  
>   pm_runtime_get_sync(dev->dev);
>  
> + if (crtc->dc->desc->is_xlcdc) {
> + regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_CM);
> + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  !(status & ATMEL_XLCDC_CM),
> +  10, 1000))
> + dev_warn(dev->dev, "Atmel LCDC status register CMSTS 
> timeout\n");
> +
> + regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
> + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  status & ATMEL_XLCDC_SD,
> +  10, 1000))
> + dev_warn(dev->dev, "Atmel LCDC status register SDSTS 
> timeout\n");
> + }
> +
>   regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
>   while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&

Not related to this patch: it may worth implementing the same approach
accross all bits polling instructions in this function.

>  (status & ATMEL_HLCDC_DISP))
> @@ -256,6 +271,20 @@ static void atmel_hlcdc_crtc_atomic_enable(struct 
> drm_crtc *c,
>  !(status & ATMEL_HLCDC_DISP))
>   cpu_relax();
>  
> + if (crtc->dc->desc->is_xlcdc) {
> + regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_CM);
> + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  status & ATMEL_XLCDC_CM,
> +  10, 1000))
> + dev_warn(dev->dev, "Atmel LCDC status register CMSTS 
> timeout\n");
> +
> + regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_SD);
> + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  !(status & ATMEL_XLCDC_SD),
> +  10, 1000))
> + dev_warn(dev->dev, "Atmel LCDC status register SDSTS 
> timeout\n");
> + }
> +
>   pm_runtime_put_sync(dev->dev);
>  
>  }
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel

Re: [PATCH RESEND v7 1/7] drm: atmel-hlcdc: add flag and driver ops to differentiate XLCDC and HLCDC IP

2024-02-03 Thread claudiu beznea
Hi, Manikandan,

On 29.01.2024 11:23, Manikandan Muralidharan wrote:
> Add is_xlcdc flag and LCD IP specific ops in driver data to differentiate
> XLCDC and HLCDC code within the atmel-hlcdc driver files.

I would first prepare the current code base for the addition of XLCDC by
first adding the struct atmel_lcdc_dc_ops, update current code to use it
and after that add XLCDC.

> 
> Signed-off-by: Manikandan Muralidharan 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 37 
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index 5b5c774e0edf..d5e01ff8c7f9 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -177,6 +177,9 @@ struct atmel_hlcdc_layer_cfg_layout {
>   int csc;
>  };
>  
> +struct atmel_hlcdc_plane_state;

You can move this forward declaration close the the structure that needs it
(struct atmel_lcdc_dc_ops).

> +struct atmel_hlcdc_dc;

And you can get rid if this one if you move struct atmel_lcdc_dc_ops after
struct atmel_hlcdc_dc definition.

> +
>  /**
>   * Atmel HLCDC DMA descriptor structure
>   *
> @@ -288,6 +291,36 @@ atmel_hlcdc_layer_to_plane(struct atmel_hlcdc_layer 
> *layer)
>   return container_of(layer, struct atmel_hlcdc_plane, layer);
>  }
>  
> +/**
> + * struct atmel_lcdc_dc_ops - describes atmel_lcdc ops group
> + * to differentiate HLCDC and XLCDC IP code support.
> + * @plane_setup_scaler: update the vertical and horizontal scaling factors
> + * @update_lcdc_buffers: update the each LCDC layers DMA registers.
> + * @lcdc_atomic_disable: disable LCDC interrupts and layers
> + * @lcdc_update_general_settings: update each LCDC layers general
> + * confiugration register.
s/confiugration/configuration

> + * @lcdc_atomic_update: enable the LCDC layers and interrupts.

You may want to keep consistency by adding or not '.' at the end of the
documentation statement (I consider '.' is useless)

> + * @lcdc_csc_init: update the color space conversion co-efficient of
> + * High-end overlay register.
> + * @lcdc_irq_dbg: to raise alert incase of interrupt overrun in any LCDC 
> layer.

s/incase/in case

> + */
> +struct atmel_lcdc_dc_ops {
> + void (*plane_setup_scaler)(struct atmel_hlcdc_plane *plane,
> +struct atmel_hlcdc_plane_state *state);
> + void (*update_lcdc_buffers)(struct atmel_hlcdc_plane *plane,
> + struct atmel_hlcdc_plane_state *state,
> + u32 sr, int i);
> + void (*lcdc_atomic_disable)(struct atmel_hlcdc_plane *plane);
> + void (*lcdc_update_general_settings)(struct atmel_hlcdc_plane *plane,
> +  struct atmel_hlcdc_plane_state 
> *state);
> + void (*lcdc_atomic_update)(struct atmel_hlcdc_plane *plane,
> +struct atmel_hlcdc_dc *dc);
> + void (*lcdc_csc_init)(struct atmel_hlcdc_plane *plane,
> +   const struct atmel_hlcdc_layer_desc *desc);
> + void (*lcdc_irq_dbg)(struct atmel_hlcdc_plane *plane,
> +  const struct atmel_hlcdc_layer_desc *desc);
> +};
> +
>  /**
>   * Atmel HLCDC Display Controller description structure.
>   *
> @@ -304,8 +337,10 @@ atmel_hlcdc_layer_to_plane(struct atmel_hlcdc_layer 
> *layer)
>   * @conflicting_output_formats: true if RGBXXX output formats conflict with
>   *   each other.
>   * @fixed_clksrc: true if clock source is fixed
> + * @is_xlcdc: true if XLCDC IP is supported
>   * @layers: a layer description table describing available layers
>   * @nlayers: layer description table size
> + * @ops: atmel lcdc dc ops
>   */
>  struct atmel_hlcdc_dc_desc {
>   int min_width;
> @@ -317,8 +352,10 @@ struct atmel_hlcdc_dc_desc {
>   int max_hpw;
>   bool conflicting_output_formats;
>   bool fixed_clksrc;
> + bool is_xlcdc;
>   const struct atmel_hlcdc_layer_desc *layers;
>   int nlayers;
> + const struct atmel_lcdc_dc_ops *ops;
>  };
>  
>  /**


Re: [PATCH RESEND v7 2/7] drm: atmel-hlcdc: add LCD controller layer definition for sam9x75

2024-02-03 Thread claudiu beznea



On 29.01.2024 11:23, Manikandan Muralidharan wrote:
> Add the LCD controller layer definition and descriptor structure for
> sam9x75 for the following layers:
> - Base Layer
> - Overlay1 Layer
> - Overlay2 Layer
> - High End Overlay
> 
> Signed-off-by: Manikandan Muralidharan 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 97 
>  1 file changed, 97 insertions(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index fa0f9a93d50d..d30aec174aa2 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -462,6 +462,99 @@ static const struct atmel_hlcdc_dc_desc 
> atmel_hlcdc_dc_sam9x60 = {
>   .layers = atmel_hlcdc_sam9x60_layers,
>  };
>  
> +static const struct atmel_hlcdc_layer_desc atmel_xlcdc_sam9x75_layers[] = {
> + {
> + .name = "base",
> + .formats = &atmel_hlcdc_plane_rgb_formats,
> + .regs_offset = 0x60,
> + .id = 0,
> + .type = ATMEL_HLCDC_BASE_LAYER,
> + .cfgs_offset = 0x1c,
> + .layout = {
> + .xstride = { 2 },
> + .default_color = 3,
> + .general_config = 4,
> + .disc_pos = 5,
> + .disc_size = 6,
> + },
> + .clut_offset = 0x700,
> + },
> + {
> + .name = "overlay1",
> + .formats = &atmel_hlcdc_plane_rgb_formats,
> + .regs_offset = 0x160,
> + .id = 1,
> + .type = ATMEL_HLCDC_OVERLAY_LAYER,
> + .cfgs_offset = 0x1c,
> + .layout = {
> + .pos = 2,
> + .size = 3,
> + .xstride = { 4 },
> + .pstride = { 5 },
> + .default_color = 6,
> + .chroma_key = 7,
> + .chroma_key_mask = 8,
> + .general_config = 9,
> + },
> + .clut_offset = 0xb00,
> + },
> + {
> + .name = "overlay2",
> + .formats = &atmel_hlcdc_plane_rgb_formats,
> + .regs_offset = 0x260,
> + .id = 2,
> + .type = ATMEL_HLCDC_OVERLAY_LAYER,
> + .cfgs_offset = 0x1c,
> + .layout = {
> + .pos = 2,
> + .size = 3,
> + .xstride = { 4 },
> + .pstride = { 5 },
> + .default_color = 6,
> + .chroma_key = 7,
> + .chroma_key_mask = 8,
> + .general_config = 9,
> + },
> + .clut_offset = 0xf00,
> + },
> + {
> + .name = "high-end-overlay",
> + .formats = &atmel_hlcdc_plane_rgb_and_yuv_formats,
> + .regs_offset = 0x360,
> + .id = 3,
> + .type = ATMEL_HLCDC_OVERLAY_LAYER,
> + .cfgs_offset = 0x30,
> + .layout = {
> + .pos = 2,
> + .size = 3,
> + .memsize = 4,
> + .xstride = { 5, 7 },
> + .pstride = { 6, 8 },
> + .default_color = 9,
> + .chroma_key = 10,
> + .chroma_key_mask = 11,
> + .general_config = 12,
> + .csc = 16,
> + .scaler_config = 23,
> + },
> + .clut_offset = 0x1300,
> + },
> +};
> +
> +static const struct atmel_hlcdc_dc_desc atmel_xlcdc_dc_sam9x75 = {
> + .min_width = 0,
> + .min_height = 0,
> + .max_width = 2048,
> + .max_height = 2048,
> + .max_spw = 0xff,
> + .max_vpw = 0xff,
> + .max_hpw = 0x3ff,
> + .fixed_clksrc = true,
> + .is_xlcdc = true,
> + .nlayers = ARRAY_SIZE(atmel_xlcdc_sam9x75_layers),
> + .layers = atmel_xlcdc_sam9x75_layers,
> +};
> +
>  static const struct of_device_id atmel_hlcdc_of_match[] = {
>   {
>   .compatible = "atmel,at91sam9n12-hlcdc",
> @@ -487,6 +580,10 @@ static const struct of_device_id atmel_hlcdc_of_match[] 
> = {
>   .compatible = "microchip,sam9x60-hlcdc",
>   .data = &atmel_hlcdc_dc_sam9x60,
>   },
> + {
> + .compatible = "microchip,sam9x75-xlcdc",
> + .data = &atmel_xlcdc_dc_sam9x75,

Will SAM9X75 XLCDC work only with this patch and the previous one? If not
then, organize the patches such that the SAM9X75 will work when introduced.

> + },
>   { /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, atmel_hlcdc_of_match);


Re: [PATCH v6 1/7] drm: atmel-hlcdc: add flag and driver ops to differentiate XLCDC and HLCDC IP

2023-10-03 Thread claudiu beznea



On 03.10.2023 07:18, manikanda...@microchip.com wrote:
> On 28/09/23 11:31 am, claudiu beznea wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
>> content is safe
>>
>> Hi, Manikandan,
>>
>> On 27.09.2023 12:47, Manikandan Muralidharan wrote:
>>> +void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>>> + struct atmel_hlcdc_plane_state *state);
>>> +void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
>>> + struct atmel_hlcdc_plane_state *state);
>>> +void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,
>>> +   struct atmel_hlcdc_plane_state *state,
>>> +   u32 sr, int i);
>>> +void update_xlcdc_buffers(struct atmel_hlcdc_plane *plane,
>>> +   struct atmel_hlcdc_plane_state *state,
>>> +   u32 sr, int i);
>>> +void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane);
>>> +void xlcdc_atomic_disable(struct atmel_hlcdc_plane *plane);
>>> +void
>>> +atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>>> +   struct atmel_hlcdc_plane_state 
>>> *state);
>>> +void
>>> +atmel_xlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
>>> +   struct atmel_hlcdc_plane_state 
>>> *state);
>>> +void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
>>> +  struct atmel_hlcdc_dc *dc);
>>> +void xlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
>>> +  struct atmel_hlcdc_dc *dc);
>>> +void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
>>> + const struct atmel_hlcdc_layer_desc *desc);
>>> +void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
>>> + const struct atmel_hlcdc_layer_desc *desc);
>>> +void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
>>> +const struct atmel_hlcdc_layer_desc *desc);
>>> +void xlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
>>> +const struct atmel_hlcdc_layer_desc *desc);
>>> +
>>
>> These are still here... Isn't the solution I proposed to you in the
>> previous version good enough?
> Hi Claudiu
> 
> These changes were integrated in the current patch set based on the 
> solution which you proposed in the previous series.
> The XLCDC and HLCDC functions calls are moved to IP specific driver->ops
> and their function declarations are made here in atmel_hlcdc_dc.h
> Rest of the changes are integrated in Patch 4/7.

I still think (and I've checked it last time) you can remove these
declaration. See comment from previous version:

"You can get rid of these and keep the function definitions static to
atmel_hlcdc_plane.c if you define struct atmel_lcdc_dc_ops objects directly
to atmel_hlcdc_plane.c. In atmel_hlcdc_dc.c you can have something like:

extern const struct atmel_lcdc_dc_ops  atmel_hlcdc_ops;
extern const struct atmel_lcdc_dc_ops  atmel_xlcdc_ops;
"

>>
>> Thank you,
>> Claudiu Beznea
> 


Re: [PATCH v6 1/7] drm: atmel-hlcdc: add flag and driver ops to differentiate XLCDC and HLCDC IP

2023-09-27 Thread claudiu beznea
Hi, Manikandan,

On 27.09.2023 12:47, Manikandan Muralidharan wrote:
> +void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
> + struct atmel_hlcdc_plane_state *state);
> +void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
> + struct atmel_hlcdc_plane_state *state);
> +void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,
> +   struct atmel_hlcdc_plane_state *state,
> +   u32 sr, int i);
> +void update_xlcdc_buffers(struct atmel_hlcdc_plane *plane,
> +   struct atmel_hlcdc_plane_state *state,
> +   u32 sr, int i);
> +void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane);
> +void xlcdc_atomic_disable(struct atmel_hlcdc_plane *plane);
> +void
> +atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
> +   struct atmel_hlcdc_plane_state 
> *state);
> +void
> +atmel_xlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
> +   struct atmel_hlcdc_plane_state 
> *state);
> +void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
> +  struct atmel_hlcdc_dc *dc);
> +void xlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
> +  struct atmel_hlcdc_dc *dc);
> +void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
> + const struct atmel_hlcdc_layer_desc *desc);
> +void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
> + const struct atmel_hlcdc_layer_desc *desc);
> +void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
> +const struct atmel_hlcdc_layer_desc *desc);
> +void xlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
> +const struct atmel_hlcdc_layer_desc *desc);
> +

These are still here... Isn't the solution I proposed to you in the
previous version good enough?

Thank you,
Claudiu Beznea


Re: [PATCH v5 5/8] drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver

2023-09-25 Thread claudiu beznea



On 15.09.2023 13:48, Manikandan Muralidharan wrote:
> - XLCDC in SAM9X7 has different sets of registers and additional
> configuration bits when compared to previous HLCDC IP. Read/write
> operation on the controller registers is now separated using the
> XLCDC status flag and with HLCDC and XLCDC IP specific ops.
>   - HEO scaling, window resampling, Alpha blending, YUV-to-RGB
> conversion in XLCDC is derived and handled using additional
> configuration bits and registers.
>   - Writing one to the Enable fields of each layer in LCD_ATTRE
> is required to reflect the values set in Configuration, FBA, Enable
> registers of each layer

Having "-" may be interpreted as you are doing multiple things in this patch.

> 
> Signed-off-by: Manikandan Muralidharan 
> Co-developed-by: Hari Prasath Gujulan Elango 
> Signed-off-by: Hari Prasath Gujulan Elango 
> Co-developed-by: Durai Manickam KR 
> Signed-off-by: Durai Manickam KR 
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c|  33 +-
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c  |  26 ++
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 317 ++
>  3 files changed, 315 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index cc5cf4c2faf7..1ac31c0c474a 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -79,6 +79,7 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
> *c)
>   unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
>   unsigned int cfg = 0;
>   int div, ret;
> + bool is_xlcdc = crtc->dc->desc->is_xlcdc;
>  
>   /* get encoder from crtc */
>   drm_for_each_encoder(en_iter, ddev) {
> @@ -164,10 +165,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct 
> drm_crtc *c)
>   state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
>   cfg = state->output_mode << 8;
>  
> - if (adj->flags & DRM_MODE_FLAG_NVSYNC)
> + if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
>   cfg |= ATMEL_HLCDC_VSPOL;
>  
> - if (adj->flags & DRM_MODE_FLAG_NHSYNC)
> + if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NHSYNC))
>   cfg |= ATMEL_HLCDC_HSPOL;
>  
>   regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
> @@ -202,6 +203,20 @@ static void atmel_hlcdc_crtc_atomic_disable(struct 
> drm_crtc *c,
>  
>   pm_runtime_get_sync(dev->dev);
>  
> + if (crtc->dc->desc->is_xlcdc) {
> + regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_CM);
> + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  !(status & ATMEL_XLCDC_CM),
> +  10, 1000))
> + dev_warn(dev->dev, "Atmel LCDC status register CMSTS 
> timeout\n");
> +
> + regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
> + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  status & ATMEL_XLCDC_SD,
> +  10, 1000))
> + dev_warn(dev->dev, "Atmel LCDC status register SDSTS 
> timeout\n");
> + }
> +
>   regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
>   while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
>  (status & ATMEL_HLCDC_DISP))
> @@ -256,6 +271,20 @@ static void atmel_hlcdc_crtc_atomic_enable(struct 
> drm_crtc *c,
>  !(status & ATMEL_HLCDC_DISP))
>   cpu_relax();
>  
> + if (crtc->dc->desc->is_xlcdc) {
> + regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_CM);
> + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  status & ATMEL_XLCDC_CM,
> +  10, 1000))
> + dev_warn(dev->dev, "Atmel LCDC status register CMSTS 
> timeout\n");
> +
> + regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_SD);
> + if (regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  !(status & ATMEL_XLCDC_SD),
> +  10, 1000))
> + dev_warn(dev->dev, "Atmel LCDC status register SDSTS 
> timeout\n");
> + }
> +
>   pm_runtime_put_sync(dev->dev);
>  
>  }
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index d30aec174aa2..7702c2f16178 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -30,6 +30,26 @@
>  
>  #define ATMEL_HLCDC_LAYER_IRQS_OFFSET8
>  
> +static const struct atmel_lcdc_dc_ops  atmel_hlcdc_ops = {

there are 2 spaces b/w atmel_lcdc_dc_ops and atmel_hlcdc_ops.

> + .plane_setup_scaler = atmel_

Re: [PATCH v5 4/8] drm: atmel-hlcdc: Define SAM9X7 SoC XLCDC specific registers

2023-09-25 Thread claudiu beznea



On 15.09.2023 13:48, Manikandan Muralidharan wrote:
> From: Durai Manickam KR 
> 
> The register address of the XLCDC IP used in SAM9X7 SoC family
> are different from the previous HLCDC.Defining those address

Add a space after .

> space with valid macros.
> 
> Signed-off-by: Durai Manickam KR 
> [manikanda...@microchip.com: Remove unused macro definitions]
> Signed-off-by: Manikandan Muralidharan 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 42 
>  include/linux/mfd/atmel-hlcdc.h  | 10 +
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index c61fa1733da4..9965c7cc5bf8 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -15,6 +15,7 @@
>  
>  #include 
>  
> +/* LCD controller common registers */
>  #define ATMEL_HLCDC_LAYER_CHER   0x0
>  #define ATMEL_HLCDC_LAYER_CHDR   0x4
>  #define ATMEL_HLCDC_LAYER_CHSR   0x8
> @@ -128,6 +129,47 @@
>  
>  #define ATMEL_HLCDC_MAX_LAYERS   6
>  
> +/* XLCDC controller specific registers */
> +#define ATMEL_XLCDC_LAYER_ENR0x10
> +#define ATMEL_XLCDC_LAYER_EN BIT(0)
> +
> +#define ATMEL_XLCDC_LAYER_IER0x0
> +#define ATMEL_XLCDC_LAYER_IDR0x4
> +#define ATMEL_XLCDC_LAYER_ISR0xc
> +#define ATMEL_XLCDC_LAYER_OVR_IRQ(p) BIT(2 + (8 * (p)))
> +
> +#define ATMEL_XLCDC_LAYER_PLANE_ADDR(p)  (((p) * 0x4) + 0x18)
> +
> +#define ATMEL_XLCDC_LAYER_DMA_CFG0
> +
> +#define ATMEL_XLCDC_LAYER_DMABIT(0)
> +#define ATMEL_XLCDC_LAYER_REPBIT(1)
> +#define ATMEL_XLCDC_LAYER_DISCEN BIT(4)
> +
> +#define ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS  (4 << 6)
> +#define ATMEL_XLCDC_LAYER_SFACTA_ONE BIT(9)
> +#define ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS(6 << 11)
> +#define ATMEL_XLCDC_LAYER_DFACTA_ONE BIT(14)
> +
> +#define ATMEL_XLCDC_LAYER_A0_SHIFT   16
> +#define ATMEL_XLCDC_LAYER_A0(x)  \
> + ((x) << ATMEL_XLCDC_LAYER_A0_SHIFT)
> +
> +#define ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLEBIT(0)
> +#define ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE  BIT(1)
> +#define ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLEBIT(4)
> +#define ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE  BIT(5)
> +
> +#define ATMEL_XLCDC_LAYER_VXSYCFG_ONEBIT(0)
> +#define ATMEL_XLCDC_LAYER_VXSYTAP2_ENABLEBIT(4)
> +#define ATMEL_XLCDC_LAYER_VXSCCFG_ONEBIT(16)
> +#define ATMEL_XLCDC_LAYER_VXSCTAP2_ENABLEBIT(20)
> +
> +#define ATMEL_XLCDC_LAYER_HXSYCFG_ONEBIT(0)
> +#define ATMEL_XLCDC_LAYER_HXSYTAP2_ENABLEBIT(4)
> +#define ATMEL_XLCDC_LAYER_HXSCCFG_ONEBIT(16)
> +#define ATMEL_XLCDC_LAYER_HXSCTAP2_ENABLEBIT(20)
> +
>  /**
>   * Atmel HLCDC Layer registers layout structure
>   *
> diff --git a/include/linux/mfd/atmel-hlcdc.h b/include/linux/mfd/atmel-hlcdc.h
> index a186119a49b5..80d675a03b39 100644
> --- a/include/linux/mfd/atmel-hlcdc.h
> +++ b/include/linux/mfd/atmel-hlcdc.h
> @@ -22,6 +22,8 @@
>  #define ATMEL_HLCDC_DITHER   BIT(6)
>  #define ATMEL_HLCDC_DISPDLY  BIT(7)
>  #define ATMEL_HLCDC_MODE_MASKGENMASK(9, 8)
> +#define ATMEL_XLCDC_MODE_MASKGENMASK(10, 8)
> +#define ATMEL_XLCDC_DPI  BIT(11)
>  #define ATMEL_HLCDC_PP   BIT(10)
>  #define ATMEL_HLCDC_VSPSUBIT(12)
>  #define ATMEL_HLCDC_VSPHOBIT(13)
> @@ -34,6 +36,12 @@
>  #define ATMEL_HLCDC_IDR  0x30
>  #define ATMEL_HLCDC_IMR  0x34
>  #define ATMEL_HLCDC_ISR  0x38
> +#define ATMEL_XLCDC_ATTRE0x3c
> +
> +#define ATMEL_XLCDC_BASE_UPDATE  BIT(0)
> +#define ATMEL_XLCDC_OVR1_UPDATE  BIT(1)
> +#define ATMEL_XLCDC_OVR3_UPDATE  BIT(2)
> +#define ATMEL_XLCDC_HEO_UPDATE   BIT(3)
>  
>  #define ATMEL_HLCDC_CLKPOL   BIT(0)
>  #define ATMEL_HLCDC_CLKSEL   BIT(2)
> @@ -48,6 +56,8 @@
>  #define ATMEL_HLCDC_DISP BIT(2)
>  #define ATMEL_HLCDC_PWM  BIT(3)
>  #define ATMEL_HLCDC_SIP  BIT(4)
> +#define ATMEL_XLCDC_SD   BIT(5)
> +#define ATMEL_XLCDC_CM   BIT(6)
>  
>  #define ATMEL_HLCDC_SOF  BIT(0)
>  #define ATMEL_HLCDC_SYNCDIS  BIT(1)


Re: [PATCH v5 3/8] drm: atmel-hlcdc: add LCD controller layer definition for sam9x75

2023-09-25 Thread claudiu beznea



On 15.09.2023 13:48, Manikandan Muralidharan wrote:
> Add the LCD controller layer definition and descriptor structure for
> sam9x75 for the following layers,

s/,/:

> - Base Layer
> - Overlay1 Layer
> - Overlay2 Layer
> - High End Overlay
> 
> Signed-off-by: Manikandan Muralidharan 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 97 
>  1 file changed, 97 insertions(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index fa0f9a93d50d..d30aec174aa2 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -462,6 +462,99 @@ static const struct atmel_hlcdc_dc_desc 
> atmel_hlcdc_dc_sam9x60 = {
>   .layers = atmel_hlcdc_sam9x60_layers,
>  };
>  
> +static const struct atmel_hlcdc_layer_desc atmel_xlcdc_sam9x75_layers[] = {
> + {
> + .name = "base",
> + .formats = &atmel_hlcdc_plane_rgb_formats,
> + .regs_offset = 0x60,
> + .id = 0,
> + .type = ATMEL_HLCDC_BASE_LAYER,
> + .cfgs_offset = 0x1c,
> + .layout = {
> + .xstride = { 2 },
> + .default_color = 3,
> + .general_config = 4,
> + .disc_pos = 5,
> + .disc_size = 6,
> + },
> + .clut_offset = 0x700,
> + },
> + {
> + .name = "overlay1",
> + .formats = &atmel_hlcdc_plane_rgb_formats,
> + .regs_offset = 0x160,
> + .id = 1,
> + .type = ATMEL_HLCDC_OVERLAY_LAYER,
> + .cfgs_offset = 0x1c,
> + .layout = {
> + .pos = 2,
> + .size = 3,
> + .xstride = { 4 },
> + .pstride = { 5 },
> + .default_color = 6,
> + .chroma_key = 7,
> + .chroma_key_mask = 8,
> + .general_config = 9,
> + },
> + .clut_offset = 0xb00,
> + },
> + {
> + .name = "overlay2",
> + .formats = &atmel_hlcdc_plane_rgb_formats,
> + .regs_offset = 0x260,
> + .id = 2,
> + .type = ATMEL_HLCDC_OVERLAY_LAYER,
> + .cfgs_offset = 0x1c,
> + .layout = {
> + .pos = 2,
> + .size = 3,
> + .xstride = { 4 },
> + .pstride = { 5 },
> + .default_color = 6,
> + .chroma_key = 7,
> + .chroma_key_mask = 8,
> + .general_config = 9,
> + },
> + .clut_offset = 0xf00,
> + },
> + {
> + .name = "high-end-overlay",
> + .formats = &atmel_hlcdc_plane_rgb_and_yuv_formats,
> + .regs_offset = 0x360,
> + .id = 3,
> + .type = ATMEL_HLCDC_OVERLAY_LAYER,
> + .cfgs_offset = 0x30,
> + .layout = {
> + .pos = 2,
> + .size = 3,
> + .memsize = 4,
> + .xstride = { 5, 7 },
> + .pstride = { 6, 8 },
> + .default_color = 9,
> + .chroma_key = 10,
> + .chroma_key_mask = 11,
> + .general_config = 12,
> + .csc = 16,
> + .scaler_config = 23,
> + },
> + .clut_offset = 0x1300,
> + },
> +};
> +
> +static const struct atmel_hlcdc_dc_desc atmel_xlcdc_dc_sam9x75 = {
> + .min_width = 0,
> + .min_height = 0,
> + .max_width = 2048,
> + .max_height = 2048,
> + .max_spw = 0xff,
> + .max_vpw = 0xff,
> + .max_hpw = 0x3ff,
> + .fixed_clksrc = true,
> + .is_xlcdc = true,
> + .nlayers = ARRAY_SIZE(atmel_xlcdc_sam9x75_layers),
> + .layers = atmel_xlcdc_sam9x75_layers,
> +};
> +
>  static const struct of_device_id atmel_hlcdc_of_match[] = {
>   {
>   .compatible = "atmel,at91sam9n12-hlcdc",
> @@ -487,6 +580,10 @@ static const struct of_device_id atmel_hlcdc_of_match[] 
> = {
>   .compatible = "microchip,sam9x60-hlcdc",
>   .data = &atmel_hlcdc_dc_sam9x60,
>   },
> + {
> + .compatible = "microchip,sam9x75-xlcdc",
> + .data = &atmel_xlcdc_dc_sam9x75,
> + },
>   { /* sentinel */ },
>  };
>  MODULE_DEVICE_TABLE(of, atmel_hlcdc_of_match);


Re: [PATCH v5 6/8] drm: atmel-hlcdc: add DPI mode support for XLCDC

2023-09-25 Thread claudiu beznea



On 15.09.2023 13:48, Manikandan Muralidharan wrote:
> Add support for Display Pixel Interface (DPI) Compatible Mode
> support in atmel-hlcdc driver for XLCDC IP along with legacy
> pixel mapping.DPI mode BIT is configured in LCDC_CFG5 register.

Space after .

> 
> Signed-off-by: Manikandan Muralidharan 
> [durai.manicka...@microchip.com: update DPI mode bit using is_xlcdc flag]
> Signed-off-by: Durai Manickam KR 
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 22 ---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 1ac31c0c474a..b0051ec02f7f 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -30,10 +30,12 @@
>   *
>   * @base: base CRTC state
>   * @output_mode: RGBXXX output mode
> + * @dpi: output DPI mode
>   */
>  struct atmel_hlcdc_crtc_state {
>   struct drm_crtc_state base;
>   unsigned int output_mode;
> + u8 dpi;
>  };
>  
>  static inline struct atmel_hlcdc_crtc_state *
> @@ -164,6 +166,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct 
> drm_crtc *c)
>  
>   state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
>   cfg = state->output_mode << 8;
> + if (is_xlcdc)
> + cfg |= state->dpi << 11;
>  
>   if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
>   cfg |= ATMEL_HLCDC_VSPOL;
> @@ -176,7 +180,9 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct 
> drm_crtc *c)
>  ATMEL_HLCDC_VSPDLYS | ATMEL_HLCDC_VSPDLYE |
>  ATMEL_HLCDC_DISPPOL | ATMEL_HLCDC_DISPDLY |
>  ATMEL_HLCDC_VSPSU | ATMEL_HLCDC_VSPHO |
> -ATMEL_HLCDC_GUARDTIME_MASK | ATMEL_HLCDC_MODE_MASK,
> +ATMEL_HLCDC_GUARDTIME_MASK |
> +(is_xlcdc ? ATMEL_XLCDC_MODE_MASK |
> +ATMEL_XLCDC_DPI : ATMEL_HLCDC_MODE_MASK),
>  cfg);
>  
>   clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
> @@ -374,7 +380,15 @@ static int atmel_hlcdc_crtc_select_output_mode(struct 
> drm_crtc_state *state)
>  
>   hstate = drm_crtc_state_to_atmel_hlcdc_crtc_state(state);
>   hstate->output_mode = fls(output_fmts) - 1;
> -
> + if (crtc->dc->desc->is_xlcdc) {
> + /* check if MIPI DPI bit needs to be set */
> + if (fls(output_fmts) > 3) {
> + hstate->output_mode -= 4;
> + hstate->dpi = 1;
> + } else {
> + hstate->dpi = 0;
> + }
> + }
>   return 0;
>  }
>  
> @@ -478,7 +492,7 @@ static struct drm_crtc_state *
>  atmel_hlcdc_crtc_duplicate_state(struct drm_crtc *crtc)
>  {
>   struct atmel_hlcdc_crtc_state *state, *cur;
> -
> + struct atmel_hlcdc_crtc *c = drm_crtc_to_atmel_hlcdc_crtc(crtc);

Keep the blank line here.

>   if (WARN_ON(!crtc->state))
>   return NULL;
>  
> @@ -489,6 +503,8 @@ atmel_hlcdc_crtc_duplicate_state(struct drm_crtc *crtc)
>  
>   cur = drm_crtc_state_to_atmel_hlcdc_crtc_state(crtc->state);
>   state->output_mode = cur->output_mode;
> + if (c->dc->desc->is_xlcdc)
> + state->dpi = cur->dpi;
>  
>   return &state->base;
>  }


Re: [PATCH v4 5/8] drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver

2023-09-17 Thread claudiu beznea



On 12.09.2023 13:44, manikanda...@microchip.com wrote:
> On 09/09/23 9:50 pm, claudiu beznea wrote:
>> [You don't often get email from claudiu.bez...@tuxon.dev. Learn why this is 
>> important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
>> content is safe
>>
>> Hi, Manikandan,
>>
>> On 8/25/23 15:54, Manikandan Muralidharan wrote:
>>> - XLCDC in SAM9X7 has different sets of registers and additional
>>> configuration bits when compared to previous HLCDC IP. Read/write
>>> operation on the controller registers is now separated using the
>>> XLCDC status flag.
>>>- HEO scaling, window resampling, Alpha blending, YUV-to-RGB
>>> conversion in XLCDC is derived and handled using additional
>>> configuration bits and registers.
>>>- Writing one to the Enable fields of each layer in LCD_ATTRE
>>> is required to reflect the values set in Configuration, FBA, Enable
>>> registers of each layer
>>>
>>> Signed-off-by: Manikandan Muralidharan 
>>> Co-developed-by: Hari Prasath Gujulan Elango 
>>> Signed-off-by: Hari Prasath Gujulan Elango 
>>> Co-developed-by: Durai Manickam KR 
>>> Signed-off-by: Durai Manickam KR 
>>> ---
>>>   .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c|  25 +-
>>>   .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 333 +++---
>>>   2 files changed, 299 insertions(+), 59 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
>>> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>>> index cc5cf4c2faf7..4b11a1de8af4 100644
>>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
>>> @@ -79,6 +79,7 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct 
>>> drm_crtc *c)
>>>unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
>>>unsigned int cfg = 0;
>>>int div, ret;
>>> + bool is_xlcdc = crtc->dc->desc->is_xlcdc;
>>>
>>>/* get encoder from crtc */
>>>drm_for_each_encoder(en_iter, ddev) {
>>> @@ -164,10 +165,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct 
>>> drm_crtc *c)
>>>state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
>>>cfg = state->output_mode << 8;
>>>
>>> - if (adj->flags & DRM_MODE_FLAG_NVSYNC)
>>> + if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
>>>cfg |= ATMEL_HLCDC_VSPOL;
>>>
>>> - if (adj->flags & DRM_MODE_FLAG_NHSYNC)
>>> + if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NHSYNC))
>>>cfg |= ATMEL_HLCDC_HSPOL;
>>>
>>>regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
>>> @@ -202,6 +203,16 @@ static void atmel_hlcdc_crtc_atomic_disable(struct 
>>> drm_crtc *c,
>>>
>>>pm_runtime_get_sync(dev->dev);
>>>
>>> + if (crtc->dc->desc->is_xlcdc) {
>>> + regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_CM);
>>> + regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>>> +  !(status & ATMEL_XLCDC_CM), 10, 0);
>>
>> You may want to check the return value of regmap_read_poll_timeout().
>> Otherwise your setup may fail.
>>
>> Also, regmap_read_poll_timeout() may sleep, the other settings in this
>> functions are done with bussy looping, is there a reason for that?
>> Hi Claudiu
> 
> Not sure if a non-zero timeout_us coud be sufficient for this operation, 
> considering the next power-up and power-down sequence following the 
> current step.
> Any suggestion on the value of timeout_us is appreciable.

Ok, haven't noticed that the timeout is zero.

>>> +
>>> + regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
>>> + regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
>>> +  status & ATMEL_XLCDC_SD, 10, 0);
>>
>> Same here.
>>
>>> + }
>>> +
>>>regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
>>>while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
>>>   (status & ATMEL_HLCDC_DISP))
>>> @@ -256,6 +267,16 @@ static void atmel_hlcdc_crtc_

Re: [PATCH v4 5/8] drm: atmel_hlcdc: Add support for XLCDC in atmel LCD driver

2023-09-09 Thread claudiu beznea
Hi, Manikandan,

On 8/25/23 15:54, Manikandan Muralidharan wrote:
> - XLCDC in SAM9X7 has different sets of registers and additional
> configuration bits when compared to previous HLCDC IP. Read/write
> operation on the controller registers is now separated using the
> XLCDC status flag.
>   - HEO scaling, window resampling, Alpha blending, YUV-to-RGB
> conversion in XLCDC is derived and handled using additional
> configuration bits and registers.
>   - Writing one to the Enable fields of each layer in LCD_ATTRE
> is required to reflect the values set in Configuration, FBA, Enable
> registers of each layer
> 
> Signed-off-by: Manikandan Muralidharan 
> Co-developed-by: Hari Prasath Gujulan Elango 
> Signed-off-by: Hari Prasath Gujulan Elango 
> Co-developed-by: Durai Manickam KR 
> Signed-off-by: Durai Manickam KR 
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c|  25 +-
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 333 +++---
>  2 files changed, 299 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index cc5cf4c2faf7..4b11a1de8af4 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -79,6 +79,7 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
> *c)
>   unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
>   unsigned int cfg = 0;
>   int div, ret;
> + bool is_xlcdc = crtc->dc->desc->is_xlcdc;
>  
>   /* get encoder from crtc */
>   drm_for_each_encoder(en_iter, ddev) {
> @@ -164,10 +165,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct 
> drm_crtc *c)
>   state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
>   cfg = state->output_mode << 8;
>  
> - if (adj->flags & DRM_MODE_FLAG_NVSYNC)
> + if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
>   cfg |= ATMEL_HLCDC_VSPOL;
>  
> - if (adj->flags & DRM_MODE_FLAG_NHSYNC)
> + if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NHSYNC))
>   cfg |= ATMEL_HLCDC_HSPOL;
>  
>   regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
> @@ -202,6 +203,16 @@ static void atmel_hlcdc_crtc_atomic_disable(struct 
> drm_crtc *c,
>  
>   pm_runtime_get_sync(dev->dev);
>  
> + if (crtc->dc->desc->is_xlcdc) {
> + regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_CM);
> + regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  !(status & ATMEL_XLCDC_CM), 10, 0);

You may want to check the return value of regmap_read_poll_timeout().
Otherwise your setup may fail.

Also, regmap_read_poll_timeout() may sleep, the other settings in this
functions are done with bussy looping, is there a reason for that?

> +
> + regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_XLCDC_SD);
> + regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  status & ATMEL_XLCDC_SD, 10, 0);

Same here.

> + }
> +
>   regmap_write(regmap, ATMEL_HLCDC_DIS, ATMEL_HLCDC_DISP);
>   while (!regmap_read(regmap, ATMEL_HLCDC_SR, &status) &&
>  (status & ATMEL_HLCDC_DISP))
> @@ -256,6 +267,16 @@ static void atmel_hlcdc_crtc_atomic_enable(struct 
> drm_crtc *c,
>  !(status & ATMEL_HLCDC_DISP))
>   cpu_relax();
>  
> + if (crtc->dc->desc->is_xlcdc) {
> + regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_CM);
> + regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  status & ATMEL_XLCDC_CM, 10, 0);
> +
> + regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_XLCDC_SD);
> + regmap_read_poll_timeout(regmap, ATMEL_HLCDC_SR, status,
> +  !(status & ATMEL_XLCDC_SD), 10, 0);
> + }
> +
>   pm_runtime_put_sync(dev->dev);
>  
>  }
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index daa508504f47..26caf4cddfa4 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -330,11 +330,59 @@ static void atmel_hlcdc_plane_setup_scaler(struct 
> atmel_hlcdc_plane *plane,
>yfactor));
>  }
>  
> +static void atmel_xlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
> +struct atmel_hlcdc_plane_state 
> *state)
> +{
> + const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> + u32 xfactor, yfactor;
> +
> + if (!desc->layout.scaler_config)
> + return;
> +
> + if (state->crtc_w == state->src_w && state->crtc_h == state->src_h) {
> + atmel_hlcdc_layer_write_cfg(&plane->layer,
> + desc->layout.scaler_config, 0)

Re: [PATCH v4 7/8] drm: atmel-hlcdc: add vertical and horizontal scaling support for XLCDC

2023-09-09 Thread claudiu beznea



On 8/25/23 15:54, Manikandan Muralidharan wrote:
> update the LCDC_HEOCFG30 and LCDC_HEOCFG31 registers of XLCDC IP which

s/update/Update

> supports vertical and horizontal scaling with Bilinear and Bicubic
> co-efficients taps for Chroma and Luma componenets of the Pixel.
> 
> Signed-off-by: Manikandan Muralidharan 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c  |  2 ++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h  |  4 
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c   | 20 +++
>  3 files changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index d30aec174aa2..ae3e1a813482 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -536,6 +536,8 @@ static const struct atmel_hlcdc_layer_desc 
> atmel_xlcdc_sam9x75_layers[] = {
>   .general_config = 12,
>   .csc = 16,
>   .scaler_config = 23,
> + .vxs_config = 30,
> + .hxs_config = 31,
>   },
>   .clut_offset = 0x1300,
>   },
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index 8b05a54b5fd0..27074a4c5aec 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -198,6 +198,8 @@
>   * @disc_pos: discard area position register
>   * @disc_size: discard area size register
>   * @csc: color space conversion register
> + * @vxs_config: vertical scalar filter taps control register
> + * @hxs_config: horizontal scalar filter taps control register
>   */
>  struct atmel_hlcdc_layer_cfg_layout {
>   int xstride[ATMEL_HLCDC_LAYER_MAX_PLANES];
> @@ -217,6 +219,8 @@ struct atmel_hlcdc_layer_cfg_layout {
>   int disc_pos;
>   int disc_size;
>   int csc;
> + int vxs_config;
> + int hxs_config;
>  };
>  
>  /**
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index 26caf4cddfa4..a06ae2dc5909 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -972,6 +972,26 @@ static void xlcdc_csc_init(struct atmel_hlcdc_plane 
> *plane,
>   atmel_hlcdc_layer_write_cfg(&plane->layer,
>   desc->layout.csc + i,
>   xlcdc_csc_coeffs[i]);
> +
> + if (desc->layout.vxs_config && desc->layout.hxs_config) {
> + /*
> +  * Updating vxs.config and hxs.config fixes the
> +  * Green Color Issue in SAM9X7 EGT Video Player App
> +  */
> + atmel_hlcdc_layer_write_cfg(&plane->layer,
> + desc->layout.vxs_config,
> + ATMEL_XLCDC_LAYER_VXSYCFG_ONE |
> + ATMEL_XLCDC_LAYER_VXSYTAP2_ENABLE |
> + ATMEL_XLCDC_LAYER_VXSCCFG_ONE |
> + ATMEL_XLCDC_LAYER_VXSCTAP2_ENABLE);
> +
> + atmel_hlcdc_layer_write_cfg(&plane->layer,
> + desc->layout.hxs_config,
> + ATMEL_XLCDC_LAYER_HXSYCFG_ONE |
> + ATMEL_XLCDC_LAYER_HXSYTAP2_ENABLE |
> + ATMEL_XLCDC_LAYER_HXSCCFG_ONE |
> + ATMEL_XLCDC_LAYER_HXSCTAP2_ENABLE);
> + }
>  }
>  
>  static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)


Re: [PATCH v4 6/8] drm: atmel-hlcdc: add DPI mode support for XLCDC

2023-09-09 Thread claudiu beznea



On 8/25/23 15:54, Manikandan Muralidharan wrote:
> Add support for Display Pixel Interface (DPI) Compatible Mode
> support in atmel-hlcdc driver for XLCDC IP along with legacy
> pixel mapping.DPI mode BIT is configured in LCDC_CFG5 register.
> 
> Signed-off-by: Manikandan Muralidharan 
> [durai.manicka...@microchip.com: update DPI mode bit using is_xlcdc flag]
> Signed-off-by: Durai Manickam KR 
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 22 ---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 4b11a1de8af4..c3d0c60ba419 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -30,10 +30,12 @@
>   *
>   * @base: base CRTC state
>   * @output_mode: RGBXXX output mode
> + * @dpi: output DPI mode
>   */
>  struct atmel_hlcdc_crtc_state {
>   struct drm_crtc_state base;
>   unsigned int output_mode;
> + bool dpi;

To avoid confusion, better use u8 to avoid shifting a boolean value in
configuration phase.

>  };
>  
>  static inline struct atmel_hlcdc_crtc_state *
> @@ -164,6 +166,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct 
> drm_crtc *c)
>  
>   state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state);
>   cfg = state->output_mode << 8;
> + if (is_xlcdc)
> + cfg |= state->dpi << 11;
>  
>   if (!is_xlcdc && (adj->flags & DRM_MODE_FLAG_NVSYNC))
>   cfg |= ATMEL_HLCDC_VSPOL;
> @@ -176,7 +180,9 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct 
> drm_crtc *c)
>  ATMEL_HLCDC_VSPDLYS | ATMEL_HLCDC_VSPDLYE |
>  ATMEL_HLCDC_DISPPOL | ATMEL_HLCDC_DISPDLY |
>  ATMEL_HLCDC_VSPSU | ATMEL_HLCDC_VSPHO |
> -ATMEL_HLCDC_GUARDTIME_MASK | ATMEL_HLCDC_MODE_MASK,
> +ATMEL_HLCDC_GUARDTIME_MASK |
> +(is_xlcdc ? ATMEL_XLCDC_MODE_MASK |
> +ATMEL_XLCDC_DPI : ATMEL_HLCDC_MODE_MASK),
>  cfg);
>  
>   clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
> @@ -366,7 +372,15 @@ static int atmel_hlcdc_crtc_select_output_mode(struct 
> drm_crtc_state *state)
>  
>   hstate = drm_crtc_state_to_atmel_hlcdc_crtc_state(state);
>   hstate->output_mode = fls(output_fmts) - 1;
> -
> + if (crtc->dc->desc->is_xlcdc) {
> + /* check if MIPI DPI bit needs to be set */
> + if (fls(output_fmts) > 3) {
> + hstate->output_mode -= 4;
> + hstate->dpi = true;
> + } else {
> + hstate->dpi = false;
> + }
> + }
>   return 0;
>  }
>  
> @@ -470,7 +484,7 @@ static struct drm_crtc_state *
>  atmel_hlcdc_crtc_duplicate_state(struct drm_crtc *crtc)
>  {
>   struct atmel_hlcdc_crtc_state *state, *cur;
> -
> + struct atmel_hlcdc_crtc *c = drm_crtc_to_atmel_hlcdc_crtc(crtc);
>   if (WARN_ON(!crtc->state))
>   return NULL;
>  
> @@ -481,6 +495,8 @@ atmel_hlcdc_crtc_duplicate_state(struct drm_crtc *crtc)
>  
>   cur = drm_crtc_state_to_atmel_hlcdc_crtc_state(crtc->state);
>   state->output_mode = cur->output_mode;
> + if (c->dc->desc->is_xlcdc)
> + state->dpi = cur->dpi;
>  
>   return &state->base;
>  }


Re: [PATCH v4 4/8] drm: atmel-hlcdc: Define SAM9X7 SoC XLCDC specific registers

2023-09-09 Thread claudiu beznea



On 8/25/23 15:54, Manikandan Muralidharan wrote:
> From: Durai Manickam KR 
> 
> The register address of the XLCDC IP used in SAM9X7 SoC family
> are different from the previous HLCDC.Defining those address
> space with valid macros.
> 
> Signed-off-by: Durai Manickam KR 
> [manikanda...@microchip.com: Remove unused macro definitions]
> Signed-off-by: Manikandan Muralidharan 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 42 
>  include/linux/mfd/atmel-hlcdc.h  | 10 +
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index d68c79a6eae7..8b05a54b5fd0 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -15,6 +15,7 @@
>  
>  #include 
>  
> +/* LCD controller common registers */
>  #define ATMEL_HLCDC_LAYER_CHER   0x0
>  #define ATMEL_HLCDC_LAYER_CHDR   0x4
>  #define ATMEL_HLCDC_LAYER_CHSR   0x8
> @@ -128,6 +129,47 @@
>  
>  #define ATMEL_HLCDC_MAX_LAYERS   6
>  
> +/* XLCDC controller specific registers */
> +#define ATMEL_XLCDC_LAYER_ENR0x10
> +#define ATMEL_XLCDC_LAYER_EN BIT(0)
> +
> +#define ATMEL_XLCDC_LAYER_IER0x0
> +#define ATMEL_XLCDC_LAYER_IDR0x4
> +#define ATMEL_XLCDC_LAYER_ISR0xc
> +#define ATMEL_XLCDC_LAYER_OVR_IRQ(p) BIT(2 + (8 * (p)))
> +
> +#define ATMEL_XLCDC_LAYER_PLANE_ADDR(p)  (((p) * 0x4) + 0x18)
> +
> +#define ATMEL_XLCDC_LAYER_DMA_CFG0
> +
> +#define ATMEL_XLCDC_LAYER_DMABIT(0)
> +#define ATMEL_XLCDC_LAYER_REPBIT(1)
> +#define ATMEL_XLCDC_LAYER_DISCENBIT(4)

You have spaces after macro name

> +
> +#define ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS  (4 << 6)
> +#define ATMEL_XLCDC_LAYER_SFACTA_ONE BIT(9)
> +#define ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS(6 << 11)
> +#define ATMEL_XLCDC_LAYER_DFACTA_ONE BIT(14)
> +
> +#define ATMEL_XLCDC_LAYER_A0_SHIFT   16
> +#define ATMEL_XLCDC_LAYER_A0(x)  \
> + ((x) << ATMEL_XLCDC_LAYER_A0_SHIFT)
> +
> +#define ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLEBIT(0)
> +#define ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE  BIT(1)
> +#define ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLEBIT(4)
> +#define ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE  BIT(5)
> +
> +#define ATMEL_XLCDC_LAYER_VXSYCFG_ONEBIT(0)
> +#define ATMEL_XLCDC_LAYER_VXSYTAP2_ENABLEBIT(4)
> +#define ATMEL_XLCDC_LAYER_VXSCCFG_ONEBIT(16)
> +#define ATMEL_XLCDC_LAYER_VXSCTAP2_ENABLEBIT(20)
> +
> +#define ATMEL_XLCDC_LAYER_HXSYCFG_ONEBIT(0)
> +#define ATMEL_XLCDC_LAYER_HXSYTAP2_ENABLEBIT(4)
> +#define ATMEL_XLCDC_LAYER_HXSCCFG_ONEBIT(16)
> +#define ATMEL_XLCDC_LAYER_HXSCTAP2_ENABLEBIT(20)
> +
>  /**
>   * Atmel HLCDC Layer registers layout structure
>   *
> diff --git a/include/linux/mfd/atmel-hlcdc.h b/include/linux/mfd/atmel-hlcdc.h
> index a186119a49b5..80d675a03b39 100644
> --- a/include/linux/mfd/atmel-hlcdc.h
> +++ b/include/linux/mfd/atmel-hlcdc.h
> @@ -22,6 +22,8 @@
>  #define ATMEL_HLCDC_DITHER   BIT(6)
>  #define ATMEL_HLCDC_DISPDLY  BIT(7)
>  #define ATMEL_HLCDC_MODE_MASKGENMASK(9, 8)
> +#define ATMEL_XLCDC_MODE_MASKGENMASK(10, 8)
> +#define ATMEL_XLCDC_DPI  BIT(11)
>  #define ATMEL_HLCDC_PP   BIT(10)
>  #define ATMEL_HLCDC_VSPSUBIT(12)
>  #define ATMEL_HLCDC_VSPHOBIT(13)
> @@ -34,6 +36,12 @@
>  #define ATMEL_HLCDC_IDR  0x30
>  #define ATMEL_HLCDC_IMR  0x34
>  #define ATMEL_HLCDC_ISR  0x38
> +#define ATMEL_XLCDC_ATTRE0x3c
> +
> +#define ATMEL_XLCDC_BASE_UPDATE  BIT(0)
> +#define ATMEL_XLCDC_OVR1_UPDATE  BIT(1)
> +#define ATMEL_XLCDC_OVR3_UPDATE  BIT(2)
> +#define ATMEL_XLCDC_HEO_UPDATE   BIT(3)
>  
>  #define ATMEL_HLCDC_CLKPOL   BIT(0)
>  #define ATMEL_HLCDC_CLKSEL   BIT(2)
> @@ -48,6 +56,8 @@
>  #define ATMEL_HLCDC_DISP BIT(2)
>  #define ATMEL_HLCDC_PWM  BIT(3)
>  #define ATMEL_HLCDC_SIP  BIT(4)
> +#define ATMEL_XLCDC_SD   BIT(5)
> +#define ATMEL_XLCDC_CM   BIT(6)
>  
>  #define ATMEL_HLCDC_SOF  BIT(0)
>  #define ATMEL_HLCDC_SYNCDIS  BIT(1)


Re: [PATCH v4 8/8] drm: atmel-hlcdc: add support for DSI output formats

2023-09-09 Thread claudiu beznea



On 8/25/23 15:54, Manikandan Muralidharan wrote:
> Add support for the following DPI mode if the encoder type
> is DSI as per the XLCDC IP datasheet:
> - 16BPPCFG1
> - 16BPPCFG2
> - 16BPPCFG3
> - 18BPPCFG1
> - 18BPPCFG2
> - 24BPP
> 
> Signed-off-by: Manikandan Muralidharan 
> [durai.manicka...@microchip.com: update output format using is_xlcdc flag]
> Signed-off-by: Durai Manickam KR 
> ---
>  .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 123 +-
>  1 file changed, 89 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index c3d0c60ba419..0d10f84c82d8 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -287,11 +287,18 @@ static void atmel_hlcdc_crtc_atomic_enable(struct 
> drm_crtc *c,
>  
>  }
>  
> -#define ATMEL_HLCDC_RGB444_OUTPUTBIT(0)
> -#define ATMEL_HLCDC_RGB565_OUTPUTBIT(1)
> -#define ATMEL_HLCDC_RGB666_OUTPUTBIT(2)
> -#define ATMEL_HLCDC_RGB888_OUTPUTBIT(3)
> -#define ATMEL_HLCDC_OUTPUT_MODE_MASK GENMASK(3, 0)
> +#define ATMEL_HLCDC_RGB444_OUTPUTBIT(0)
> +#define ATMEL_HLCDC_RGB565_OUTPUTBIT(1)
> +#define ATMEL_HLCDC_RGB666_OUTPUTBIT(2)
> +#define ATMEL_HLCDC_RGB888_OUTPUTBIT(3)
> +#define ATMEL_HLCDC_DPI_RGB565C1_OUTPUT  BIT(4)
> +#define ATMEL_HLCDC_DPI_RGB565C2_OUTPUT  BIT(5)
> +#define ATMEL_HLCDC_DPI_RGB565C3_OUTPUT  BIT(6)
> +#define ATMEL_HLCDC_DPI_RGB666C1_OUTPUT  BIT(7)
> +#define ATMEL_HLCDC_DPI_RGB666C2_OUTPUT  BIT(8)
> +#define ATMEL_HLCDC_DPI_RGB888_OUTPUTBIT(9)
> +#define ATMEL_HLCDC_OUTPUT_MODE_MASK GENMASK(3, 0)
> +#define ATMEL_XLCDC_OUTPUT_MODE_MASK GENMASK(9, 0)
>  
>  static int atmel_hlcdc_connector_output_mode(struct drm_connector_state 
> *state)
>  {
> @@ -305,37 +312,83 @@ static int atmel_hlcdc_connector_output_mode(struct 
> drm_connector_state *state)
>   if (!encoder)
>   encoder = connector->encoder;
>  
> - switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
> - case 0:
> - break;
> - case MEDIA_BUS_FMT_RGB444_1X12:
> - return ATMEL_HLCDC_RGB444_OUTPUT;
> - case MEDIA_BUS_FMT_RGB565_1X16:
> - return ATMEL_HLCDC_RGB565_OUTPUT;
> - case MEDIA_BUS_FMT_RGB666_1X18:
> - return ATMEL_HLCDC_RGB666_OUTPUT;
> - case MEDIA_BUS_FMT_RGB888_1X24:
> - return ATMEL_HLCDC_RGB888_OUTPUT;
> - default:
> - return -EINVAL;
> - }
> -
> - for (j = 0; j < info->num_bus_formats; j++) {
> - switch (info->bus_formats[j]) {
> - case MEDIA_BUS_FMT_RGB444_1X12:
> - supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
> + if (encoder->encoder_type == DRM_MODE_ENCODER_DSI) {
> + /*
> +  * atmel-hlcdc to support DSI formats with DSI video pipeline
> +  * when DRM_MODE_ENCODER_DSI type is set by
> +  * connector driver component.
> +  */
> + switch (atmel_hlcdc_encoder_get_bus_fmt(encoder)) {
> + case 0:
>   break;
>   case MEDIA_BUS_FMT_RGB565_1X16:
> - supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
> - break;
> + return ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;
>   case MEDIA_BUS_FMT_RGB666_1X18:
> - supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
> - break;
> + return ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;
> + case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
> + return ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;
>   case MEDIA_BUS_FMT_RGB888_1X24:
> - supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
> - break;
> + return ATMEL_HLCDC_DPI_RGB888_OUTPUT;
>   default:
> + return -EINVAL;
> + }
> +
> + for (j = 0; j < info->num_bus_formats; j++) {
> + switch (info->bus_formats[j]) {
> + case MEDIA_BUS_FMT_RGB565_1X16:
> + supported_fmts |=
> + ATMEL_HLCDC_DPI_RGB565C1_OUTPUT;
> + break;
> + case MEDIA_BUS_FMT_RGB666_1X18:
> + supported_fmts |=
> + ATMEL_HLCDC_DPI_RGB666C1_OUTPUT;
> + break;
> + case MEDIA_BUS_FMT_RGB666_1X24_CPADHI:
> + supported_fmts |=
> + ATMEL_HLCDC_DPI_RGB666C2_OUTPUT;
> + break;
> + case MEDIA_BUS_FMT_RGB888_1X24:
> + supported

[PATCH] drm/stm: ltdc: check memory returned by devm_kzalloc()

2023-06-01 Thread Claudiu Beznea
devm_kzalloc() can fail and return NULL pointer. Check its return status.
Identified with Coccinelle (kmerr.cocci script).

Fixes: 484e72d3146b ("drm/stm: ltdc: add support of ycbcr pixel formats")
Signed-off-by: Claudiu Beznea 
---

Hi,

This has been addressed using kmerr.cocci script proposed for update
at [1].

Thank you,
Claudiu Beznea

[1] 
https://lore.kernel.org/all/20230530074044.1603426-1-claudiu.bez...@microchip.com/

 drivers/gpu/drm/stm/ltdc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 03c6becda795..9f3ac54d4cb3 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -1579,6 +1579,8 @@ static struct drm_plane *ltdc_plane_create(struct 
drm_device *ddev,
   ARRAY_SIZE(ltdc_drm_fmt_ycbcr_sp) +
   ARRAY_SIZE(ltdc_drm_fmt_ycbcr_fp)) *
   sizeof(*formats), GFP_KERNEL);
+   if (!formats)
+   return NULL;
 
for (i = 0; i < ldev->caps.pix_fmt_nb; i++) {
drm_fmt = ldev->caps.pix_fmt_drm[i];
-- 
2.34.1



[PATCH v3 3/6] mfd: atmel-hlcdc: add struct device member to struct atmel_hlcdc_regmap

2019-12-18 Thread Claudiu Beznea
Add struct device member to struct atmel_hlcdc_regmap to be
able to use dev_*() specific logging functions.

Signed-off-by: Claudiu Beznea 
Acked-for-MFD-by: Lee Jones 
---
 drivers/mfd/atmel-hlcdc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c
index 64013c57a920..92bfcaa62ace 100644
--- a/drivers/mfd/atmel-hlcdc.c
+++ b/drivers/mfd/atmel-hlcdc.c
@@ -19,6 +19,7 @@
 
 struct atmel_hlcdc_regmap {
void __iomem *regs;
+   struct device *dev;
 };
 
 static const struct mfd_cell atmel_hlcdc_cells[] = {
@@ -90,6 +91,8 @@ static int atmel_hlcdc_probe(struct platform_device *pdev)
if (IS_ERR(hregmap->regs))
return PTR_ERR(hregmap->regs);
 
+   hregmap->dev = &pdev->dev;
+
hlcdc->irq = platform_get_irq(pdev, 0);
if (hlcdc->irq < 0)
return hlcdc->irq;
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 5/6] drm: atmel-hlcdc: prefer a lower pixel-clock than requested

2019-12-18 Thread Claudiu Beznea
From: Peter Rosin 

The intention was to only select a higher pixel-clock rate than the
requested, if a slight overclocking would result in a rate significantly
closer to the requested rate than if the conservative lower pixel-clock
rate is selected. The fixed patch has the logic the other way around and
actually prefers the higher frequency. Fix that.

Fixes: f6f7ad323461 ("drm/atmel-hlcdc: allow selecting a higher pixel-clock 
than requested")
Reported-by: Claudiu Beznea 
Tested-by: Claudiu Beznea 
Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 721fa88bf71d..10985134ce0b 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -121,8 +121,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
int div_low = prate / mode_rate;
 
if (div_low >= 2 &&
-   ((prate / div_low - mode_rate) <
-10 * (mode_rate - prate / div)))
+   (10 * (prate / div_low - mode_rate) <
+(mode_rate - prate / div)))
/*
 * At least 10 times better when using a higher
 * frequency than requested, instead of a lower.
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 1/6] drm: atmel-hlcdc: use double rate for pixel clock only if supported

2019-12-18 Thread Claudiu Beznea
Doubled system clock should be used as pixel cock source only if this
is supported. This is emphasized by the value of
atmel_hlcdc_crtc::dc::desc::fixed_clksrc.

Fixes: a6eca2abdd42 ("drm: atmel-hlcdc: add config option for clock selection")
Signed-off-by: Claudiu Beznea 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index f2e73e6d46b8..5040ed8d0871 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -95,14 +95,14 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
 (adj->crtc_hdisplay - 1) |
 ((adj->crtc_vdisplay - 1) << 16));
 
+   prate = clk_get_rate(crtc->dc->hlcdc->sys_clk);
+   mode_rate = adj->crtc_clock * 1000;
if (!crtc->dc->desc->fixed_clksrc) {
+   prate *= 2;
cfg |= ATMEL_HLCDC_CLKSEL;
mask |= ATMEL_HLCDC_CLKSEL;
}
 
-   prate = 2 * clk_get_rate(crtc->dc->hlcdc->sys_clk);
-   mode_rate = adj->crtc_clock * 1000;
-
div = DIV_ROUND_UP(prate, mode_rate);
if (div < 2) {
div = 2;
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 4/6] mfd: atmel-hlcdc: return in case of error

2019-12-18 Thread Claudiu Beznea
For HLCDC timing engine configurations bit ATMEL_HLCDC_SIP of
ATMEL_HLCDC_SR needs to be polled before applying new config. In case of
timeout there is no indicator about this, so, return in case of timeout
and also print a message about this.

Signed-off-by: Claudiu Beznea 
Acked-for-MFD-by: Lee Jones 
---
 drivers/mfd/atmel-hlcdc.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c
index 92bfcaa62ace..a1e46c87b956 100644
--- a/drivers/mfd/atmel-hlcdc.c
+++ b/drivers/mfd/atmel-hlcdc.c
@@ -40,10 +40,17 @@ static int regmap_atmel_hlcdc_reg_write(void *context, 
unsigned int reg,
 
if (reg <= ATMEL_HLCDC_DIS) {
u32 status;
-
-   readl_poll_timeout_atomic(hregmap->regs + ATMEL_HLCDC_SR,
- status, !(status & ATMEL_HLCDC_SIP),
- 1, 100);
+   int ret;
+
+   ret = readl_poll_timeout_atomic(hregmap->regs + ATMEL_HLCDC_SR,
+   status,
+   !(status & ATMEL_HLCDC_SIP),
+   1, 100);
+   if (ret) {
+   dev_err(hregmap->dev,
+   "Timeout! Clock domain synchronization is in 
progress!\n");
+   return ret;
+   }
}
 
writel(val, hregmap->regs + reg);
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 0/6] fixes for atmel-hlcdc

2019-12-18 Thread Claudiu Beznea
Hi,

I have few fixes for atmel-hlcdc driver in this series as well
as two reverts.
Revert "drm: atmel-hlcdc: enable sys_clk during initalization." is
due to the fix in in patch 2/5.

Thank you,
Claudiu Beznea

Changes in v3:
- changes dev_err() message in patch 4/6
- collect Acked-by tags

Changes in v2:
- introduce patch 3/6
- use dev_err() inpatch 4/6
- introduce patch 5/6 instead of reverting commit f6f7ad323461
  ("drm/atmel-hlcdc: allow selecting a higher pixel-clock than requested")

Claudiu Beznea (5):
  drm: atmel-hlcdc: use double rate for pixel clock only if supported
  drm: atmel-hlcdc: enable clock before configuring timing engine
  mfd: atmel-hlcdc: add struct device member to struct
atmel_hlcdc_regmap
  mfd: atmel-hlcdc: return in case of error
  Revert "drm: atmel-hlcdc: enable sys_clk during initalization."

Peter Rosin (1):
  drm: atmel-hlcdc: prefer a lower pixel-clock than requested

 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 18 --
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c   | 19 +--
 drivers/mfd/atmel-hlcdc.c  | 18 ++
 3 files changed, 27 insertions(+), 28 deletions(-)

-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 2/6] drm: atmel-hlcdc: enable clock before configuring timing engine

2019-12-18 Thread Claudiu Beznea
Changing pixel clock source without having this clock source enabled
will block the timing engine and the next operations after (in this case
setting ATMEL_HLCDC_CFG(5) settings in atmel_hlcdc_crtc_mode_set_nofb()
will fail). It is recomended (although in datasheet this is not present)
to actually enabled pixel clock source before doing any changes on timing
enginge (only SAM9X60 datasheet specifies that the peripheral clock and
pixel clock must be enabled before using LCD controller).

Fixes: 1a396789f65a ("drm: add Atmel HLCDC Display Controller support")
Signed-off-by: Claudiu Beznea 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 5040ed8d0871..721fa88bf71d 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -73,7 +73,11 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
unsigned long prate;
unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
unsigned int cfg = 0;
-   int div;
+   int div, ret;
+
+   ret = clk_prepare_enable(crtc->dc->hlcdc->sys_clk);
+   if (ret)
+   return;
 
vm.vfront_porch = adj->crtc_vsync_start - adj->crtc_vdisplay;
vm.vback_porch = adj->crtc_vtotal - adj->crtc_vsync_end;
@@ -147,6 +151,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
   ATMEL_HLCDC_VSPSU | ATMEL_HLCDC_VSPHO |
   ATMEL_HLCDC_GUARDTIME_MASK | ATMEL_HLCDC_MODE_MASK,
   cfg);
+
+   clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
 }
 
 static enum drm_mode_status
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 6/6] Revert "drm: atmel-hlcdc: enable sys_clk during initalization."

2019-12-18 Thread Claudiu Beznea
This reverts commit d2c755e66617620b729041c625a6396c81d1231c
("drm: atmel-hlcdc: enable sys_clk during initalization."). With
commit "drm: atmel-hlcdc: enable clock before configuring timing engine"
there is no need for this patch. Code is also simpler.

Cc: Sandeep Sheriker Mallikarjun 
Signed-off-by: Claudiu Beznea 
---

Hi Sam,

I still kept this as a patch as I didn't got any answer from you at my
last email up to this moment.

If you think it is better to squash this one with patch 2/6 in this seris
let me know.

Thank you,
Claudiu Beznea

 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 19 +--
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 8dc917a1270b..112aa5066cee 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -721,18 +721,10 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
dc->hlcdc = dev_get_drvdata(dev->dev->parent);
dev->dev_private = dc;
 
-   if (dc->desc->fixed_clksrc) {
-   ret = clk_prepare_enable(dc->hlcdc->sys_clk);
-   if (ret) {
-   dev_err(dev->dev, "failed to enable sys_clk\n");
-   goto err_destroy_wq;
-   }
-   }
-
ret = clk_prepare_enable(dc->hlcdc->periph_clk);
if (ret) {
dev_err(dev->dev, "failed to enable periph_clk\n");
-   goto err_sys_clk_disable;
+   goto err_destroy_wq;
}
 
pm_runtime_enable(dev->dev);
@@ -768,9 +760,6 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
 err_periph_clk_disable:
pm_runtime_disable(dev->dev);
clk_disable_unprepare(dc->hlcdc->periph_clk);
-err_sys_clk_disable:
-   if (dc->desc->fixed_clksrc)
-   clk_disable_unprepare(dc->hlcdc->sys_clk);
 
 err_destroy_wq:
destroy_workqueue(dc->wq);
@@ -795,8 +784,6 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev)
 
pm_runtime_disable(dev->dev);
clk_disable_unprepare(dc->hlcdc->periph_clk);
-   if (dc->desc->fixed_clksrc)
-   clk_disable_unprepare(dc->hlcdc->sys_clk);
destroy_workqueue(dc->wq);
 }
 
@@ -910,8 +897,6 @@ static int atmel_hlcdc_dc_drm_suspend(struct device *dev)
regmap_read(regmap, ATMEL_HLCDC_IMR, &dc->suspend.imr);
regmap_write(regmap, ATMEL_HLCDC_IDR, dc->suspend.imr);
clk_disable_unprepare(dc->hlcdc->periph_clk);
-   if (dc->desc->fixed_clksrc)
-   clk_disable_unprepare(dc->hlcdc->sys_clk);
 
return 0;
 }
@@ -921,8 +906,6 @@ static int atmel_hlcdc_dc_drm_resume(struct device *dev)
struct drm_device *drm_dev = dev_get_drvdata(dev);
struct atmel_hlcdc_dc *dc = drm_dev->dev_private;
 
-   if (dc->desc->fixed_clksrc)
-   clk_prepare_enable(dc->hlcdc->sys_clk);
clk_prepare_enable(dc->hlcdc->periph_clk);
regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IER, dc->suspend.imr);
 
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 6/6] Revert "drm: atmel-hlcdc: enable sys_clk during initalization."

2019-12-14 Thread Claudiu Beznea
This reverts commit d2c755e66617620b729041c625a6396c81d1231c
("drm: atmel-hlcdc: enable sys_clk during initalization."). With
commit "drm: atmel-hlcdc: enable clock before configuring timing engine"
there is no need for this patch. Code is also simpler.

Cc: Sandeep Sheriker Mallikarjun 
Signed-off-by: Claudiu Beznea 
---

Hi Sam,

I still kept this as a patch as I didn't got any answer from you at my
last email up to this moment.

If you think it is better to squash this one with patch 2/6 in this seris
let me know.

Thank you,
Claudiu Beznea

 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 19 +--
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 8dc917a1270b..112aa5066cee 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -721,18 +721,10 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
dc->hlcdc = dev_get_drvdata(dev->dev->parent);
dev->dev_private = dc;
 
-   if (dc->desc->fixed_clksrc) {
-   ret = clk_prepare_enable(dc->hlcdc->sys_clk);
-   if (ret) {
-   dev_err(dev->dev, "failed to enable sys_clk\n");
-   goto err_destroy_wq;
-   }
-   }
-
ret = clk_prepare_enable(dc->hlcdc->periph_clk);
if (ret) {
dev_err(dev->dev, "failed to enable periph_clk\n");
-   goto err_sys_clk_disable;
+   goto err_destroy_wq;
}
 
pm_runtime_enable(dev->dev);
@@ -768,9 +760,6 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
 err_periph_clk_disable:
pm_runtime_disable(dev->dev);
clk_disable_unprepare(dc->hlcdc->periph_clk);
-err_sys_clk_disable:
-   if (dc->desc->fixed_clksrc)
-   clk_disable_unprepare(dc->hlcdc->sys_clk);
 
 err_destroy_wq:
destroy_workqueue(dc->wq);
@@ -795,8 +784,6 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev)
 
pm_runtime_disable(dev->dev);
clk_disable_unprepare(dc->hlcdc->periph_clk);
-   if (dc->desc->fixed_clksrc)
-   clk_disable_unprepare(dc->hlcdc->sys_clk);
destroy_workqueue(dc->wq);
 }
 
@@ -910,8 +897,6 @@ static int atmel_hlcdc_dc_drm_suspend(struct device *dev)
regmap_read(regmap, ATMEL_HLCDC_IMR, &dc->suspend.imr);
regmap_write(regmap, ATMEL_HLCDC_IDR, dc->suspend.imr);
clk_disable_unprepare(dc->hlcdc->periph_clk);
-   if (dc->desc->fixed_clksrc)
-   clk_disable_unprepare(dc->hlcdc->sys_clk);
 
return 0;
 }
@@ -921,8 +906,6 @@ static int atmel_hlcdc_dc_drm_resume(struct device *dev)
struct drm_device *drm_dev = dev_get_drvdata(dev);
struct atmel_hlcdc_dc *dc = drm_dev->dev_private;
 
-   if (dc->desc->fixed_clksrc)
-   clk_prepare_enable(dc->hlcdc->sys_clk);
clk_prepare_enable(dc->hlcdc->periph_clk);
regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IER, dc->suspend.imr);
 
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 3/6] mfd: atmel-hlcdc: add struct device member to struct atmel_hlcdc_regmap

2019-12-14 Thread Claudiu Beznea
Add struct device member to struct atmel_hlcdc_regmap to be
able to use dev_*() specific logging functions.

Signed-off-by: Claudiu Beznea 
---
 drivers/mfd/atmel-hlcdc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c
index 64013c57a920..92bfcaa62ace 100644
--- a/drivers/mfd/atmel-hlcdc.c
+++ b/drivers/mfd/atmel-hlcdc.c
@@ -19,6 +19,7 @@
 
 struct atmel_hlcdc_regmap {
void __iomem *regs;
+   struct device *dev;
 };
 
 static const struct mfd_cell atmel_hlcdc_cells[] = {
@@ -90,6 +91,8 @@ static int atmel_hlcdc_probe(struct platform_device *pdev)
if (IS_ERR(hregmap->regs))
return PTR_ERR(hregmap->regs);
 
+   hregmap->dev = &pdev->dev;
+
hlcdc->irq = platform_get_irq(pdev, 0);
if (hlcdc->irq < 0)
return hlcdc->irq;
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 2/6] drm: atmel-hlcdc: enable clock before configuring timing engine

2019-12-14 Thread Claudiu Beznea
Changing pixel clock source without having this clock source enabled
will block the timing engine and the next operations after (in this case
setting ATMEL_HLCDC_CFG(5) settings in atmel_hlcdc_crtc_mode_set_nofb()
will fail). It is recomended (although in datasheet this is not present)
to actually enabled pixel clock source before doing any changes on timing
enginge (only SAM9X60 datasheet specifies that the peripheral clock and
pixel clock must be enabled before using LCD controller).

Fixes: 1a396789f65a ("drm: add Atmel HLCDC Display Controller support")
Signed-off-by: Claudiu Beznea 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 5040ed8d0871..721fa88bf71d 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -73,7 +73,11 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
unsigned long prate;
unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
unsigned int cfg = 0;
-   int div;
+   int div, ret;
+
+   ret = clk_prepare_enable(crtc->dc->hlcdc->sys_clk);
+   if (ret)
+   return;
 
vm.vfront_porch = adj->crtc_vsync_start - adj->crtc_vdisplay;
vm.vback_porch = adj->crtc_vtotal - adj->crtc_vsync_end;
@@ -147,6 +151,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
   ATMEL_HLCDC_VSPSU | ATMEL_HLCDC_VSPHO |
   ATMEL_HLCDC_GUARDTIME_MASK | ATMEL_HLCDC_MODE_MASK,
   cfg);
+
+   clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
 }
 
 static enum drm_mode_status
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 1/6] drm: atmel-hlcdc: use double rate for pixel clock only if supported

2019-12-14 Thread Claudiu Beznea
Doubled system clock should be used as pixel cock source only if this
is supported. This is emphasized by the value of
atmel_hlcdc_crtc::dc::desc::fixed_clksrc.

Fixes: a6eca2abdd42 ("drm: atmel-hlcdc: add config option for clock selection")
Signed-off-by: Claudiu Beznea 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index f2e73e6d46b8..5040ed8d0871 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -95,14 +95,14 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
 (adj->crtc_hdisplay - 1) |
 ((adj->crtc_vdisplay - 1) << 16));
 
+   prate = clk_get_rate(crtc->dc->hlcdc->sys_clk);
+   mode_rate = adj->crtc_clock * 1000;
if (!crtc->dc->desc->fixed_clksrc) {
+   prate *= 2;
cfg |= ATMEL_HLCDC_CLKSEL;
mask |= ATMEL_HLCDC_CLKSEL;
}
 
-   prate = 2 * clk_get_rate(crtc->dc->hlcdc->sys_clk);
-   mode_rate = adj->crtc_clock * 1000;
-
div = DIV_ROUND_UP(prate, mode_rate);
if (div < 2) {
div = 2;
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 4/6] mfd: atmel-hlcdc: return in case of error

2019-12-14 Thread Claudiu Beznea
For HLCDC timing engine configurations bit ATMEL_HLCDC_SIP of
ATMEL_HLCDC_SR needs to be polled before applying new config. In case of
timeout there is no indicator about this, so, return in case of timeout
and also print a message about this.

Signed-off-by: Claudiu Beznea 
---
 drivers/mfd/atmel-hlcdc.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c
index 92bfcaa62ace..a1e46c87b956 100644
--- a/drivers/mfd/atmel-hlcdc.c
+++ b/drivers/mfd/atmel-hlcdc.c
@@ -40,10 +40,17 @@ static int regmap_atmel_hlcdc_reg_write(void *context, 
unsigned int reg,
 
if (reg <= ATMEL_HLCDC_DIS) {
u32 status;
-
-   readl_poll_timeout_atomic(hregmap->regs + ATMEL_HLCDC_SR,
- status, !(status & ATMEL_HLCDC_SIP),
- 1, 100);
+   int ret;
+
+   ret = readl_poll_timeout_atomic(hregmap->regs + ATMEL_HLCDC_SR,
+   status,
+   !(status & ATMEL_HLCDC_SIP),
+   1, 100);
+   if (ret) {
+   dev_err(hregmap->dev,
+   "Timeout waiting for ATMEL_HLCDC_SIP\n");
+   return ret;
+   }
}
 
writel(val, hregmap->regs + reg);
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 0/6] fixes for atmel-hlcdc

2019-12-14 Thread Claudiu Beznea
Hi,

I have few fixes for atmel-hlcdc driver in this series as well
as two reverts.
Revert "drm: atmel-hlcdc: enable sys_clk during initalization." is
due to the fix in in patch 2/5.

Thank you,
Claudiu Beznea

Changes in v2:
- introduce patch 3/6
- use dev_err() inpatch 4/6
- introduce patch 5/6 instead of reverting commit f6f7ad323461
  ("drm/atmel-hlcdc: allow selecting a higher pixel-clock than requested")

Claudiu Beznea (5):
  drm: atmel-hlcdc: use double rate for pixel clock only if supported
  drm: atmel-hlcdc: enable clock before configuring timing engine
  mfd: atmel-hlcdc: add struct device member to struct
atmel_hlcdc_regmap
  mfd: atmel-hlcdc: return in case of error
  Revert "drm: atmel-hlcdc: enable sys_clk during initalization."

Peter Rosin (1):
  drm: atmel-hlcdc: prefer a lower pixel-clock than requested

 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 18 --
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c   | 19 +--
 drivers/mfd/atmel-hlcdc.c  | 18 ++
 3 files changed, 27 insertions(+), 28 deletions(-)

-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 5/6] drm: atmel-hlcdc: prefer a lower pixel-clock than requested

2019-12-14 Thread Claudiu Beznea
From: Peter Rosin 

The intention was to only select a higher pixel-clock rate than the
requested, if a slight overclocking would result in a rate significantly
closer to the requested rate than if the conservative lower pixel-clock
rate is selected. The fixed patch has the logic the other way around and
actually prefers the higher frequency. Fix that.

Fixes: f6f7ad323461 ("drm/atmel-hlcdc: allow selecting a higher pixel-clock 
than requested")
Reported-by: Claudiu Beznea 
Tested-by: Claudiu Beznea 
Signed-off-by: Peter Rosin 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 721fa88bf71d..10985134ce0b 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -121,8 +121,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
int div_low = prate / mode_rate;
 
if (div_low >= 2 &&
-   ((prate / div_low - mode_rate) <
-10 * (mode_rate - prate / div)))
+   (10 * (prate / div_low - mode_rate) <
+(mode_rate - prate / div)))
/*
 * At least 10 times better when using a higher
 * frequency than requested, instead of a lower.
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 4/5] Revert "drm/atmel-hlcdc: allow selecting a higher pixel-clock than requested"

2019-12-11 Thread Claudiu Beznea
This reverts commit f6f7ad3234613f6f7f27c25036aaf078de07e9b0.
("drm/atmel-hlcdc: allow selecting a higher pixel-clock than requested")
because allowing selecting a higher pixel clock may overclock
LCD devices, not all of them being capable of this.

Cc: Peter Rosin 
Signed-off-by: Claudiu Beznea 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 12 
 1 file changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 721fa88bf71d..1a70dff1a417 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -117,18 +117,6 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
div = DIV_ROUND_UP(prate, mode_rate);
if (ATMEL_HLCDC_CLKDIV(div) & ~ATMEL_HLCDC_CLKDIV_MASK)
div = ATMEL_HLCDC_CLKDIV_MASK;
-   } else {
-   int div_low = prate / mode_rate;
-
-   if (div_low >= 2 &&
-   ((prate / div_low - mode_rate) <
-10 * (mode_rate - prate / div)))
-   /*
-* At least 10 times better when using a higher
-* frequency than requested, instead of a lower.
-* So, go with that.
-*/
-   div = div_low;
}
 
cfg |= ATMEL_HLCDC_CLKDIV(div);
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 1/5] drm: atmel-hlcdc: use double rate for pixel clock only if supported

2019-12-11 Thread Claudiu Beznea
Doubled system clock should be used as pixel cock source only if this
is supported. This is emphasized by the value of
atmel_hlcdc_crtc::dc::desc::fixed_clksrc.

Fixes: a6eca2abdd42 ("drm: atmel-hlcdc: add config option for clock selection")
Signed-off-by: Claudiu Beznea 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index f2e73e6d46b8..5040ed8d0871 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -95,14 +95,14 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
 (adj->crtc_hdisplay - 1) |
 ((adj->crtc_vdisplay - 1) << 16));
 
+   prate = clk_get_rate(crtc->dc->hlcdc->sys_clk);
+   mode_rate = adj->crtc_clock * 1000;
if (!crtc->dc->desc->fixed_clksrc) {
+   prate *= 2;
cfg |= ATMEL_HLCDC_CLKSEL;
mask |= ATMEL_HLCDC_CLKSEL;
}
 
-   prate = 2 * clk_get_rate(crtc->dc->hlcdc->sys_clk);
-   mode_rate = adj->crtc_clock * 1000;
-
div = DIV_ROUND_UP(prate, mode_rate);
if (div < 2) {
div = 2;
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 3/5] mfd: atmel-hlcdc: return in case of error

2019-12-11 Thread Claudiu Beznea
For HLCDC timing engine configurations bit ATMEL_HLCDC_SIP of
ATMEL_HLCDC_SR needs to checked if it is equal with zero before applying
new configuration to timing engine. In case of timeout there is no
indicator about this, so, return with error in case of timeout in
regmap_atmel_hlcdc_reg_write() and also print a message about this.

Signed-off-by: Claudiu Beznea 
---
 drivers/mfd/atmel-hlcdc.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c
index 64013c57a920..19f1dbeb8bcd 100644
--- a/drivers/mfd/atmel-hlcdc.c
+++ b/drivers/mfd/atmel-hlcdc.c
@@ -39,10 +39,16 @@ static int regmap_atmel_hlcdc_reg_write(void *context, 
unsigned int reg,
 
if (reg <= ATMEL_HLCDC_DIS) {
u32 status;
-
-   readl_poll_timeout_atomic(hregmap->regs + ATMEL_HLCDC_SR,
- status, !(status & ATMEL_HLCDC_SIP),
- 1, 100);
+   int ret;
+
+   ret = readl_poll_timeout_atomic(hregmap->regs + ATMEL_HLCDC_SR,
+   status,
+   !(status & ATMEL_HLCDC_SIP),
+   1, 100);
+   if (ret) {
+   pr_err("Timeout waiting for ATMEL_HLCDC_SIP\n");
+   return ret;
+   }
}
 
writel(val, hregmap->regs + reg);
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 0/5] fixes for atmel-hlcdc

2019-12-11 Thread Claudiu Beznea
Hi,

I have few fixes for atmel-hlcdc driver in this series as well
as two reverts.
Revert "drm: atmel-hlcdc: enable sys_clk during initalization." is
due to the fix in in patch 2/5.

Thank you,
Claudiu Beznea

Claudiu Beznea (5):
  drm: atmel-hlcdc: use double rate for pixel clock only if supported
  drm: atmel-hlcdc: enable clock before configuring timing engine
  mfd: atmel-hlcdc: return in case of error
  Revert "drm/atmel-hlcdc: allow selecting a higher pixel-clock than
requested"
  Revert "drm: atmel-hlcdc: enable sys_clk during initalization."

 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 26 ++
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c   | 19 +--
 drivers/mfd/atmel-hlcdc.c  | 14 ++
 3 files changed, 21 insertions(+), 38 deletions(-)

-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 5/5] Revert "drm: atmel-hlcdc: enable sys_clk during initalization."

2019-12-11 Thread Claudiu Beznea
This reverts commit d2c755e66617620b729041c625a6396c81d1231c.
("drm: atmel-hlcdc: enable sys_clk during initalization."). With
commit "drm: atmel-hlcdc: enable clock before configuring timing engine"
there is no need for this patch. Code is also simpler.

Cc: Sandeep Sheriker Mallikarjun 
Signed-off-by: Claudiu Beznea 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 19 +--
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 8dc917a1270b..112aa5066cee 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -721,18 +721,10 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
dc->hlcdc = dev_get_drvdata(dev->dev->parent);
dev->dev_private = dc;
 
-   if (dc->desc->fixed_clksrc) {
-   ret = clk_prepare_enable(dc->hlcdc->sys_clk);
-   if (ret) {
-   dev_err(dev->dev, "failed to enable sys_clk\n");
-   goto err_destroy_wq;
-   }
-   }
-
ret = clk_prepare_enable(dc->hlcdc->periph_clk);
if (ret) {
dev_err(dev->dev, "failed to enable periph_clk\n");
-   goto err_sys_clk_disable;
+   goto err_destroy_wq;
}
 
pm_runtime_enable(dev->dev);
@@ -768,9 +760,6 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
 err_periph_clk_disable:
pm_runtime_disable(dev->dev);
clk_disable_unprepare(dc->hlcdc->periph_clk);
-err_sys_clk_disable:
-   if (dc->desc->fixed_clksrc)
-   clk_disable_unprepare(dc->hlcdc->sys_clk);
 
 err_destroy_wq:
destroy_workqueue(dc->wq);
@@ -795,8 +784,6 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev)
 
pm_runtime_disable(dev->dev);
clk_disable_unprepare(dc->hlcdc->periph_clk);
-   if (dc->desc->fixed_clksrc)
-   clk_disable_unprepare(dc->hlcdc->sys_clk);
destroy_workqueue(dc->wq);
 }
 
@@ -910,8 +897,6 @@ static int atmel_hlcdc_dc_drm_suspend(struct device *dev)
regmap_read(regmap, ATMEL_HLCDC_IMR, &dc->suspend.imr);
regmap_write(regmap, ATMEL_HLCDC_IDR, dc->suspend.imr);
clk_disable_unprepare(dc->hlcdc->periph_clk);
-   if (dc->desc->fixed_clksrc)
-   clk_disable_unprepare(dc->hlcdc->sys_clk);
 
return 0;
 }
@@ -921,8 +906,6 @@ static int atmel_hlcdc_dc_drm_resume(struct device *dev)
struct drm_device *drm_dev = dev_get_drvdata(dev);
struct atmel_hlcdc_dc *dc = drm_dev->dev_private;
 
-   if (dc->desc->fixed_clksrc)
-   clk_prepare_enable(dc->hlcdc->sys_clk);
clk_prepare_enable(dc->hlcdc->periph_clk);
regmap_write(dc->hlcdc->regmap, ATMEL_HLCDC_IER, dc->suspend.imr);
 
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 2/5] drm: atmel-hlcdc: enable clock before configuring timing engine

2019-12-11 Thread Claudiu Beznea
Changing pixel clock source without having this clock source enabled
will block the timing engine and the next operations after (in this case
setting ATMEL_HLCDC_CFG(5) settings in atmel_hlcdc_crtc_mode_set_nofb()
will fail). It is recomended (although in datasheet this is not present)
to actually enabled pixel clock source before doing any changes on timing
enginge (only SAM9X60 datasheet specifies that the peripheral clock and
pixel clock must be enabled before using LCD controller).

Fixes: 1a396789f65a ("drm: add Atmel HLCDC Display Controller support")
Signed-off-by: Claudiu Beznea 
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 5040ed8d0871..721fa88bf71d 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -73,7 +73,11 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
unsigned long prate;
unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
unsigned int cfg = 0;
-   int div;
+   int div, ret;
+
+   ret = clk_prepare_enable(crtc->dc->hlcdc->sys_clk);
+   if (ret)
+   return;
 
vm.vfront_porch = adj->crtc_vsync_start - adj->crtc_vdisplay;
vm.vback_porch = adj->crtc_vtotal - adj->crtc_vsync_end;
@@ -147,6 +151,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc 
*c)
   ATMEL_HLCDC_VSPSU | ATMEL_HLCDC_VSPHO |
   ATMEL_HLCDC_GUARDTIME_MASK | ATMEL_HLCDC_MODE_MASK,
   cfg);
+
+   clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
 }
 
 static enum drm_mode_status
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

2018-03-02 Thread Claudiu Beznea


On 28.02.2018 21:44, Thierry Reding wrote:
> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>> were adapted to this change.
>>
>> Signed-off-by: Claudiu Beznea 
>> ---
>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +--
>>  drivers/bus/ts-nbus.c|  2 +-
>>  drivers/clk/clk-pwm.c|  3 ++-
>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++---
>>  drivers/hwmon/pwm-fan.c  |  2 +-
>>  drivers/input/misc/max77693-haptic.c |  2 +-
>>  drivers/input/misc/max8997_haptic.c  |  6 +-
>>  drivers/leds/leds-pwm.c  |  5 -
>>  drivers/media/rc/ir-rx51.c   |  5 -
>>  drivers/media/rc/pwm-ir-tx.c |  5 -
>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>>  drivers/video/backlight/lp8788_bl.c  |  5 -
>>  drivers/video/backlight/pwm_bl.c | 11 +--
>>  drivers/video/fbdev/ssd1307fb.c  |  3 ++-
>>  include/linux/pwm.h  |  6 --
>>  16 files changed, 70 insertions(+), 21 deletions(-)
> 
> I don't think it makes sense to leak mode support into the legacy API.
> The pwm_config() function is considered legacy
I missed this aspect.

 and should eventually go
> away. As such it doesn't make sense to integrate a new feature such as
> PWM modes into it. 
Agree.

All users of pwm_config() assume normal mode, and
> that's what pwm_config() should provide.
Agree.

> 
> Anyone that needs something other than normal mode should use the new
> atomic PWM API.
Agree.

> 
> Thierry
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

2018-03-02 Thread Claudiu Beznea


On 28.02.2018 22:04, Jani Nikula wrote:
> On Wed, 28 Feb 2018, Thierry Reding  wrote:
>> Anyone that needs something other than normal mode should use the new
>> atomic PWM API.
> 
> At the risk of revealing my true ignorance, what is the new atomic PWM
> API? Where? Examples of how one would convert old code over to the new
> API?
As far as I know, the old PWM core code uses config(), set_polarity(),
enable(), disable() methods of driver, registered as pwm_ops:
struct pwm_ops {

int (*request)(struct pwm_chip *chip, struct pwm_device *pwm);

void (*free)(struct pwm_chip *chip, struct pwm_device *pwm);

int (*config)(struct pwm_chip *chip, struct pwm_device *pwm,

  int duty_ns, int period_ns);

int (*set_polarity)(struct pwm_chip *chip, struct pwm_device *pwm,

enum pwm_polarity polarity);

int (*capture)(struct pwm_chip *chip, struct pwm_device *pwm,

   struct pwm_capture *result, unsigned long timeout);

int (*enable)(struct pwm_chip *chip, struct pwm_device *pwm);

void (*disable)(struct pwm_chip *chip, struct pwm_device *pwm);

int (*apply)(struct pwm_chip *chip, struct pwm_device *pwm,

 struct pwm_state *state);

void (*get_state)(struct pwm_chip *chip, struct pwm_device *pwm,

  struct pwm_state *state);

#ifdef CONFIG_DEBUG_FS

void (*dbg_show)(struct pwm_chip *chip, struct seq_file *s);

#endif

struct module *owner;

};


to do settings on hardware. In order to so settings on a PWM the users
should have been follow the below steps:
->config()
->set_polarity()
->enable()
Moreover, if the PWM was previously enabled it should have been first
disable and then to follow the above steps in order to apply a new settings
on hardware.
The driver should have been provide, at probe, all the above function:
->config(), ->set_polarity(), ->disable(), ->enable(), function that were
used by PWM core.

Now, having atomic PWM, the driver should provide one function to PWM core,
which is ->apply() function. Every PWM has a state associated, which keeps
the period, duty cycle, polarity and enable/disable status. The driver's
->apply() function takes as argument the state that should be applied and
it takes care of applying this new state directly without asking user to
call ->disable(), then ->config()/->set_polarity(), then ->enable() to
apply new hardware settings.

The PWM consumer could set a new state for PWM it uses, using
pwm_apply_state(pwm, new_state);

Regarding the models to switch on atomic PWM, on the controller side you
can check for drivers that registers apply function at probe time.
Regarding the PWM users, you can look for pwm_apply_state()
(drivers/hwmon/pwm-fan.c or drivers/input/misc/pwm-beeper.c are some examples).

Thierry, please correct me if I'm wrong.

Thank you,
Claudiu Beznea

> 
> BR,
> Jani.
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

2018-02-28 Thread Claudiu Beznea


On 27.02.2018 17:38, Daniel Thompson wrote:
> On Tue, Feb 27, 2018 at 01:40:58PM +0200, Claudiu Beznea wrote:
>> On 27.02.2018 12:54, Daniel Thompson wrote:
>>> On Mon, Feb 26, 2018 at 04:24:15PM +0200, Claudiu Beznea wrote:
>>>> On 26.02.2018 11:57, Jani Nikula wrote:
>>>>> On Thu, 22 Feb 2018, Daniel Thompson  wrote:
>>>>>> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>>>>>>> Add PWM mode to pwm_config() function. The drivers which uses 
>>>>>>> pwm_config()
>>>>>>> were adapted to this change.
>>>>>>>
>>>>>>> Signed-off-by: Claudiu Beznea 
>>>>>>> ---
>>>>>>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +--
>>>>>>>  drivers/bus/ts-nbus.c|  2 +-
>>>>>>>  drivers/clk/clk-pwm.c|  3 ++-
>>>>>>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++---
>>>>>>>  drivers/hwmon/pwm-fan.c  |  2 +-
>>>>>>>  drivers/input/misc/max77693-haptic.c |  2 +-
>>>>>>>  drivers/input/misc/max8997_haptic.c  |  6 +-
>>>>>>>  drivers/leds/leds-pwm.c  |  5 -
>>>>>>>  drivers/media/rc/ir-rx51.c   |  5 -
>>>>>>>  drivers/media/rc/pwm-ir-tx.c |  5 -
>>>>>>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>>>>>>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>>>>>>>  drivers/video/backlight/lp8788_bl.c  |  5 -
>>>>>>>  drivers/video/backlight/pwm_bl.c | 11 +--
>>>>>>>  drivers/video/fbdev/ssd1307fb.c  |  3 ++-
>>>>>>>  include/linux/pwm.h  |  6 --
>>>>>>>  16 files changed, 70 insertions(+), 21 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/video/backlight/lm3630a_bl.c 
>>>>>>> b/drivers/video/backlight/lm3630a_bl.c
>>>>>>> index 2030a6b77a09..696fa25dafd2 100644
>>>>>>> --- a/drivers/video/backlight/lm3630a_bl.c
>>>>>>> +++ b/drivers/video/backlight/lm3630a_bl.c
>>>>>>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip 
>>>>>>> *pchip, int br, int br_max)
>>>>>>>  {
>>>>>>> unsigned int period = pchip->pdata->pwm_period;
>>>>>>> unsigned int duty = br * period / br_max;
>>>>>>> +   struct pwm_caps caps = { };
>>>>>>>  
>>>>>>> -   pwm_config(pchip->pwmd, duty, period);
>>>>>>> +   pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
>>>>>>> +   pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
>>>>>>
>>>>>> Well... I admit I've only really looked at the patches that impact 
>>>>>> backlight but dispersing this really odd looking bit twiddling 
>>>>>> throughout the kernel doesn't strike me a great API design.
>>>>>>
>>>>>> IMHO callers should not be required to find the first set bit in
>>>>>> some specially crafted set of capability bits simply to get sane 
>>>>>> default behaviour.
>>>>>
>>>>> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
>>>>> error prone.
>>>>
>>>> Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be 
>>>> OK
>>>> from your side?
>>>>
>>>> Or, what about using a function like pwm_mode_first() to get the first 
>>>> supported
>>>> mode by PWM channel?
>>>>
>>>> Or, would you prefer to solve this inside pwm_config() function, let's 
>>>> say, in
>>>> case an invalid mode is passed as argument, to let pwm_config() to choose 
>>>> the
>>>> first available PWM mode for PWM channel passed as argument?
>>>
>>> What is it that actually needs solving?
>>>
>>> If a driver requests normal mode and the PWM driver cannot support it
>>> why not just return an error an move on.
>> Because, simply, I wasn't aware of what these PWM client drivers needs for.
> 
> I'm afraid you have confused me here.
> 
> Didn't you just 

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

2018-02-28 Thread Claudiu Beznea


On 27.02.2018 12:54, Daniel Thompson wrote:
> On Mon, Feb 26, 2018 at 04:24:15PM +0200, Claudiu Beznea wrote:
>> On 26.02.2018 11:57, Jani Nikula wrote:
>>> On Thu, 22 Feb 2018, Daniel Thompson  wrote:
>>>> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>>>>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>>>>> were adapted to this change.
>>>>>
>>>>> Signed-off-by: Claudiu Beznea 
>>>>> ---
>>>>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +--
>>>>>  drivers/bus/ts-nbus.c|  2 +-
>>>>>  drivers/clk/clk-pwm.c|  3 ++-
>>>>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++---
>>>>>  drivers/hwmon/pwm-fan.c  |  2 +-
>>>>>  drivers/input/misc/max77693-haptic.c |  2 +-
>>>>>  drivers/input/misc/max8997_haptic.c  |  6 +-
>>>>>  drivers/leds/leds-pwm.c  |  5 -
>>>>>  drivers/media/rc/ir-rx51.c   |  5 -
>>>>>  drivers/media/rc/pwm-ir-tx.c |  5 -
>>>>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>>>>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>>>>>  drivers/video/backlight/lp8788_bl.c  |  5 -
>>>>>  drivers/video/backlight/pwm_bl.c | 11 +--
>>>>>  drivers/video/fbdev/ssd1307fb.c  |  3 ++-
>>>>>  include/linux/pwm.h  |  6 --
>>>>>  16 files changed, 70 insertions(+), 21 deletions(-)
>>>>>
>>>>> diff --git a/drivers/video/backlight/lm3630a_bl.c 
>>>>> b/drivers/video/backlight/lm3630a_bl.c
>>>>> index 2030a6b77a09..696fa25dafd2 100644
>>>>> --- a/drivers/video/backlight/lm3630a_bl.c
>>>>> +++ b/drivers/video/backlight/lm3630a_bl.c
>>>>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip 
>>>>> *pchip, int br, int br_max)
>>>>>  {
>>>>>   unsigned int period = pchip->pdata->pwm_period;
>>>>>   unsigned int duty = br * period / br_max;
>>>>> + struct pwm_caps caps = { };
>>>>>  
>>>>> - pwm_config(pchip->pwmd, duty, period);
>>>>> + pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
>>>>> + pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
>>>>
>>>> Well... I admit I've only really looked at the patches that impact 
>>>> backlight but dispersing this really odd looking bit twiddling 
>>>> throughout the kernel doesn't strike me a great API design.
>>>>
>>>> IMHO callers should not be required to find the first set bit in
>>>> some specially crafted set of capability bits simply to get sane 
>>>> default behaviour.
>>>
>>> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
>>> error prone.
>>
>> Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
>> from your side?
>>
>> Or, what about using a function like pwm_mode_first() to get the first 
>> supported
>> mode by PWM channel?
>>
>> Or, would you prefer to solve this inside pwm_config() function, let's say, 
>> in
>> case an invalid mode is passed as argument, to let pwm_config() to choose the
>> first available PWM mode for PWM channel passed as argument?
> 
> What is it that actually needs solving?
> 
> If a driver requests normal mode and the PWM driver cannot support it
> why not just return an error an move on.
Because, simply, I wasn't aware of what these PWM client drivers needs for.

> 
> Put another way, what is the use case for secretly adopting a mode the
> caller didn't want? Under what circumstances is this a good thing?
No one... But I wasn't aware of what the PWM clients needs for from their PWM
controllers. At this moment having BIT(ffs(caps.modes)) instead of
PWM_MODE(NORMAL) is mostly the same since all the driver that has not explicitly
registered PWM caps will use PWM normal mode.

I will use PWM_MODE(NORMAL) instead of this in all the cases if this is OK from
your side.

Thank you,
Claudiu Beznea
> 
> 
> Daniel.
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

2018-02-27 Thread Claudiu Beznea


On 26.02.2018 11:57, Jani Nikula wrote:
> On Thu, 22 Feb 2018, Daniel Thompson  wrote:
>> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>>> were adapted to this change.
>>>
>>> Signed-off-by: Claudiu Beznea 
>>> ---
>>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +--
>>>  drivers/bus/ts-nbus.c|  2 +-
>>>  drivers/clk/clk-pwm.c|  3 ++-
>>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++---
>>>  drivers/hwmon/pwm-fan.c  |  2 +-
>>>  drivers/input/misc/max77693-haptic.c |  2 +-
>>>  drivers/input/misc/max8997_haptic.c  |  6 +-
>>>  drivers/leds/leds-pwm.c  |  5 -
>>>  drivers/media/rc/ir-rx51.c   |  5 -
>>>  drivers/media/rc/pwm-ir-tx.c |  5 -
>>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>>>  drivers/video/backlight/lp8788_bl.c  |  5 -
>>>  drivers/video/backlight/pwm_bl.c | 11 +--
>>>  drivers/video/fbdev/ssd1307fb.c  |  3 ++-
>>>  include/linux/pwm.h  |  6 --
>>>  16 files changed, 70 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/video/backlight/lm3630a_bl.c 
>>> b/drivers/video/backlight/lm3630a_bl.c
>>> index 2030a6b77a09..696fa25dafd2 100644
>>> --- a/drivers/video/backlight/lm3630a_bl.c
>>> +++ b/drivers/video/backlight/lm3630a_bl.c
>>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip 
>>> *pchip, int br, int br_max)
>>>  {
>>> unsigned int period = pchip->pdata->pwm_period;
>>> unsigned int duty = br * period / br_max;
>>> +   struct pwm_caps caps = { };
>>>  
>>> -   pwm_config(pchip->pwmd, duty, period);
>>> +   pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
>>> +   pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
>>
>> Well... I admit I've only really looked at the patches that impact 
>> backlight but dispersing this really odd looking bit twiddling 
>> throughout the kernel doesn't strike me a great API design.
>>
>> IMHO callers should not be required to find the first set bit in
>> some specially crafted set of capability bits simply to get sane 
>> default behaviour.
> 
> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
> error prone.

Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
from your side?

Or, what about using a function like pwm_mode_first() to get the first supported
mode by PWM channel?

Or, would you prefer to solve this inside pwm_config() function, let's say, in
case an invalid mode is passed as argument, to let pwm_config() to choose the
first available PWM mode for PWM channel passed as argument?

Thank you,
Claudiu Beznea

> 
> BR,
> Jani.
> 
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 01/10] pwm: extend PWM framework with PWM modes

2018-02-26 Thread Claudiu Beznea
I'll rebase it on latest for-next in next version.

Thank you,
Claudiu Beznea

On 24.02.2018 22:49, kbuild test robot wrote:
> Hi Claudiu,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> [auto build test WARNING on pwm/for-next]
> [also build test WARNING on v4.16-rc2 next-20180223]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Claudiu-Beznea/extend-PWM-framework-to-support-PWM-modes/20180225-024011
> base:   
> https://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git 
> for-next
> config: xtensa-allmodconfig (attached as .config)
> compiler: xtensa-linux-gcc (GCC) 7.2.0
> reproduce:
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=xtensa 
> 
> All warnings (new ones prefixed by >>):
> 
>>> drivers//pwm/pwm-sun4i.c:36:0: warning: "PWM_MODE" redefined
> #define PWM_MODE  BIT(7)
> 
>In file included from drivers//pwm/pwm-sun4i.c:19:0:
>include/linux/pwm.h:40:0: note: this is the location of the previous 
> definition
> #define PWM_MODE(name)  BIT(PWM_MODE_##name##_BIT)
> 
> 
> vim +/PWM_MODE +36 drivers//pwm/pwm-sun4i.c
> 
> 09853ce7 Alexandre Belloni 2014-12-17  29  
> 09853ce7 Alexandre Belloni 2014-12-17  30  #define PWMCH_OFFSET   
> 15
> 09853ce7 Alexandre Belloni 2014-12-17  31  #define PWM_PRESCAL_MASK   
> GENMASK(3, 0)
> 09853ce7 Alexandre Belloni 2014-12-17  32  #define PWM_PRESCAL_OFF> 0
> 09853ce7 Alexandre Belloni 2014-12-17  33  #define PWM_EN 
> BIT(4)
> 09853ce7 Alexandre Belloni 2014-12-17  34  #define PWM_ACT_STATE  
> BIT(5)
> 09853ce7 Alexandre Belloni 2014-12-17  35  #define PWM_CLK_GATING 
> BIT(6)
> 09853ce7 Alexandre Belloni 2014-12-17 @36  #define PWM_MODE   BIT(7)
> 09853ce7 Alexandre Belloni 2014-12-17  37  #define PWM_PULSE  BIT(8)
> 09853ce7 Alexandre Belloni 2014-12-17  38  #define PWM_BYPASS BIT(9)
> 09853ce7 Alexandre Belloni 2014-12-17  39  
> 
> :: The code at line 36 was first introduced by commit
> :: 09853ce7bc1003a490c7ee74a5705d7a7cf16b7d pwm: Add Allwinner SoC support
> 
> :: TO: Alexandre Belloni 
> :: CC: Thierry Reding 
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 00/10] extend PWM framework to support PWM modes

2018-02-23 Thread Claudiu Beznea
Hi all,

Please give feedback on these patches which extends the PWM framework in
order to support multiple PWM modes of operations. This series is a rework
of [1] and [2].

The current patch series add the following PWM modes:
- PWM mode normal
- PWM mode complementary
- PWM mode push-pull

Normal mode - for PWM channels with one output; output waveforms looks like
this:
 ________
PWM   __|  |__|  |__|  |__|  |__
<--T-->

Where T is the signal period

Since PWMs with more than one output per channel could be used as one
output PWM the normal mode is the default mode for all PWMs (if not
specified otherwise).

Complementary mode - for PWM channels with two outputs; output waveforms
for a PWM channel in complementary mode looks line this:
 ________
PWMH1 __|  |__|  |__|  |__|  |__
  __________
PWML1   |__|  |__|  |__|  |__|
<--T-->

Where T is the signal period.

Push-pull mode - for PWM channels with two outputs; output waveforms for a
PWM channel in push-pull mode with normal polarity looks like this:
__  __
PWMH __|  ||  |
  __  __
PWML |  ||  |__
   <--T-->

If polarity is inversed:
 __
PWMH   |__||__|
 __
PWML |__||__|
   <--T-->

Where T is the signal period.

The PWM working modes are per PWM channel registered as PWM's capabilities.
The driver registers itself to PWM core a get_caps() function, in
struct pwm_ops, that will be used by PWM core to retrieve PWM capabilities.
If this function is not registered in driver's probe, a default function
will be used to retrieve PWM capabilities. Currently, the default
capabilities includes only PWM normal mode.

PWM state has been updated to keep PWM mode. PWM mode could be configured
via sysfs or via DT. pwm_apply_state() will do the preliminary validation
for PWM mode to be applied.

In sysfs, user could get PWM modes by reading mode file of PWM device:
root@sama5d2-xplained:/sys/class/pwm/pwmchip0/pwm2# ls -l
total 0
-r--r--r-- 1 root root 4096 Oct  9 09:07 capture
lrwxrwxrwx 1 root root0 Oct  9 09:07 device -> ../../pwmchip0
-rw-r--r-- 1 root root 4096 Oct  9 08:42 duty_cycle
-rw-r--r-- 1 root root 4096 Oct  9 08:44 enable
--w--- 1 root root 4096 Oct  9 09:07 export
-rw-r--r-- 1 root root 4096 Oct  9 08:43 mode
-r--r--r-- 1 root root 4096 Oct  9 09:07 npwm
-rw-r--r-- 1 root root 4096 Oct  9 08:42 period
-rw-r--r-- 1 root root 4096 Oct  9 08:44 polarity
drwxr-xr-x 2 root root0 Oct  9 09:07 power
lrwxrwxrwx 1 root root0 Oct  9 09:07 subsystem -> 
../../../../../../../../class/pwm
-rw-r--r-- 1 root root 4096 Oct  9 08:42 uevent
--w--- 1 root root 4096 Oct  9 09:07 unexport
root@sama5d2-xplained:/sys/class/pwm/pwmchip0/pwm2# cat mode
normal complementary [push-pull]

The mode enclosed in bracket is the currently active mode.

The mode could be set, via sysfs, by writing to mode file one of the modes
displayed at read:
root@sama5d2-xplained:/sys/class/pwm/pwmchip0/pwm2# echo normal > mode
root@sama5d2-xplained:/sys/class/pwm/pwmchip0/pwm2# cat mode
[normal] complementary push-pull 

The PWM push-pull mode could be usefull in applications like half bridge
converters.

This series also add support for PWM modes on Atmel/Microchip SoCs.

Thank you,
Claudiu Beznea

[1] https://www.spinics.net/lists/arm-kernel/msg580275.html
[2] https://lkml.org/lkml/2018/1/12/359

Changes in v3:
- removed changes related to only one of_xlate function for all PWM drivers
- switch to PWM capabilities per PWM channel nor per PWM chip
- squash documentation and bindings patches as requeted by reviewer
- introduced PWM_MODE(name) macro and used a bit enum for pwm modes
- related to DT bindings, used flags cell also for PWM modes
- updated of_xlate specific functions with "state->mode = mode;"
  instructions to avoid pwm_apply_state() failures
- use available modes for PWM channel in pwm_config() by first calling
  pwm_get_caps() to get caps.modes
- use loops through available modes in mode_store()/mode_show() and also in
  of_pwm_xlate_with_flags() instead of "if else" instructions; in this way,
  the addition of a new mode is independent of this code sections
- use DTLI=1, DTHI=0 register settings to obtain push-pull mode waveforms
  for Atmel/Microchip PWM controller.

Changes in v2:
- remove of_xlate and of_pwm_n_cells and use generic functions to pharse DT
  inputs; this is done in patches 1, 2, 3, 4, 5, 6, 7 of this series; this will
  make easy the addition of PWM mode support from DT
- add PWM mode normal
- register PWM modes as capabilities of PWM chips at driver probe and, in case
  driver doesn't provide these capabilities use default ones
- change the way PWM mode is ph

[PATCH v3 09/10] pwm: add documentation for pwm push-pull mode

2018-02-23 Thread Claudiu Beznea
Add documentation for PWM push-pull mode.

Signed-off-by: Claudiu Beznea 
Reviewed-by: Rob Herring 
---
 Documentation/devicetree/bindings/pwm/pwm.txt |  2 ++
 Documentation/pwm.txt | 16 
 include/dt-bindings/pwm/pwm.h |  1 +
 3 files changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt 
b/Documentation/devicetree/bindings/pwm/pwm.txt
index c2b4b9ba0e3f..968dd786ad7c 100644
--- a/Documentation/devicetree/bindings/pwm/pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm.txt
@@ -49,6 +49,8 @@ Optionally, the pwm-specifier can encode a number of flags 
(defined in
 - PWM_DTMODE_COMPLEMENTARY: PWM complementary working mode (for PWM
 channels two outputs); if not specified, the default for PWM channel will
 be used
+- PWM_DTMODE_PUSH_PULL: PWM push-pull working modes (for PWM channels with
+two outputs); if not specified the default for PWM channel will be used
 
 Example with optional PWM specifier for inverse polarity and complementary
 mode:
diff --git a/Documentation/pwm.txt b/Documentation/pwm.txt
index 59592f8cc381..174c371583b6 100644
--- a/Documentation/pwm.txt
+++ b/Documentation/pwm.txt
@@ -128,6 +128,22 @@ channel that was exported. The following properties will 
then be available:
 PWML1   |__|  |__|  |__|  |__|
 <--T-->
 
+Push-pull mode - for PWM channels with two outputs; output waveforms
+for a PWM channel with 2 outputs, in push-pull mode, with normal
+polarity looks like this:
+__  __
+PWMH __|  ||  |
+  __  __
+PWML |  ||  |__
+   <--T-->
+
+If polarity is inversed:
+ __
+PWMH   |__||__|
+ __
+PWML |__||__|
+   <--T-->
+
 Where T is the signal period.
 
 Implementing a PWM driver
diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h
index b4d61269aced..674dfdd59595 100644
--- a/include/dt-bindings/pwm/pwm.h
+++ b/include/dt-bindings/pwm/pwm.h
@@ -12,5 +12,6 @@
 
 #define PWM_POLARITY_INVERTED  (1 << 0)
 #define PWM_DTMODE_COMPLEMENTARY   (1 << 1)
+#define PWM_DTMODE_PUSH_PULL   (1 << 2)
 
 #endif
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 08/10] pwm: add push-pull mode support

2018-02-23 Thread Claudiu Beznea
Add push-pull mode support. In push-pull mode the channels' outputs have
same polarities and the edges are complementary delayed for one period.

Signed-off-by: Claudiu Beznea 
---
 include/linux/pwm.h | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 0ba416ab2772..765d760ef82d 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -29,11 +29,14 @@ enum pwm_polarity {
  * PWM modes
  * @PWM_MODE_NORMAL_BIT: PWM has one output
  * @PWM_MODE_COMPLEMENTARY_BIT: PWM has 2 outputs with opposite polarities
+ * @PWM_MODE_PUSH_PULL_BIT: PWM has 2 outputs with same polarities and the 
edges
+ * are complementary delayed for one period
  * @PWM_MODE_CNT: PWM modes count
  */
 enum {
PWM_MODE_NORMAL_BIT,
PWM_MODE_COMPLEMENTARY_BIT,
+   PWM_MODE_PUSH_PULL_BIT,
PWM_MODE_CNT,
 };
 
@@ -481,7 +484,11 @@ static inline bool pwm_caps_valid(struct pwm_caps caps)
 
 static inline const char * const pwm_mode_desc(unsigned long mode)
 {
-   static const char * const modes[] = { "normal", "complementary" };
+   static const char * const modes[] = {
+   "normal",
+   "complementary",
+   "push-pull",
+   };
 
if (!pwm_mode_valid(mode))
return "invalid";
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

2018-02-23 Thread Claudiu Beznea


On 22.02.2018 15:01, Sean Young wrote:
> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>> were adapted to this change.
>>
>> Signed-off-by: Claudiu Beznea 
>> ---
> 
> -snip-
> 
>> diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
>> index 49265f02e772..a971b02ea021 100644
>> --- a/drivers/media/rc/ir-rx51.c
>> +++ b/drivers/media/rc/ir-rx51.c
>> @@ -55,10 +55,13 @@ static int init_timing_params(struct ir_rx51 *ir_rx51)
>>  {
>>  struct pwm_device *pwm = ir_rx51->pwm;
>>  int duty, period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, ir_rx51->freq);
>> +struct pwm_caps caps = { };
>>  
>>  duty = DIV_ROUND_CLOSEST(ir_rx51->duty_cycle * period, 100);
>>  
>> -pwm_config(pwm, duty, period);
>> +pwm_get_caps(pwm->chip, pwm, &caps);
>> +
>> +pwm_config(pwm, duty, period, BIT(ffs(caps.modes) - 1));
>>  
>>  return 0;
>>  }
>> diff --git a/drivers/media/rc/pwm-ir-tx.c b/drivers/media/rc/pwm-ir-tx.c
>> index 27d0f5837a76..c630e1b450a3 100644
>> --- a/drivers/media/rc/pwm-ir-tx.c
>> +++ b/drivers/media/rc/pwm-ir-tx.c
>> @@ -61,6 +61,7 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int 
>> *txbuf,
>>  {
>>  struct pwm_ir *pwm_ir = dev->priv;
>>  struct pwm_device *pwm = pwm_ir->pwm;
>> +struct pwm_caps caps = { };
>>  int i, duty, period;
>>  ktime_t edge;
>>  long delta;
>> @@ -68,7 +69,9 @@ static int pwm_ir_tx(struct rc_dev *dev, unsigned int 
>> *txbuf,
>>  period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, pwm_ir->carrier);
>>  duty = DIV_ROUND_CLOSEST(pwm_ir->duty_cycle * period, 100);
>>  
>> -pwm_config(pwm, duty, period);
>> +pwm_get_caps(pwm->chip, pwm, &caps);
>> +
>> +pwm_config(pwm, duty, period, BIT(ffs(caps.modes) - 1));
>>  
>>  edge = ktime_get();
>>  
> 
> The two PWM rc-core drivers need PWM_MODE(NORMAL), not the first available
> mode that the device supports. If mode normal is not supported, then probe
> should fail.
OK, thank you for your inputs. I will address this in next version.


Thank you,
Claudiu Beznea
> 
> 
> Sean
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 01/10] pwm: extend PWM framework with PWM modes

2018-02-23 Thread Claudiu Beznea
Add basic PWM modes: normal and complementary. These modes should
differentiate the single output PWM channels from two outputs PWM
channels. These modes could be set as follow:
1. PWM channels with one output per channel:
- normal mode
2. PWM channels with two outputs per channel:
- normal mode
- complementary mode
Since users could use a PWM channel with two output as one output PWM
channel, the PWM normal mode is allowed to be set for PWM channels with
two outputs; in fact PWM normal mode should be supported by all PWMs.

The PWM capabilities were implemented per PWM channel. Every PWM controller
will register a function to get PWM capabilities. If this is not explicitly
set by the driver a default function will be used to retrieve the the PWM
capabilities (in this case the PWM capabilities will contain only PWM
normal mode). This function is set in pwmchip_add_with_polarity() as a
member of "struct pwm_chip". To retrieve capabilities the pwm_get_caps()
function could be used.

Every PWM channel will have associated a mode in the PWM state. Proper
helper functions were added to get/set PWM mode. The mode could also be set
from DT via flag cells. The valid DT modes could be located in
include/dt-bindings/pwm/pwm.h. Only modes supported by PWM channel could be
set. If nothing is specified for a PWM channel, via DT, the first available
mode will be used (normally, this will be PWM normal mode).

Signed-off-by: Claudiu Beznea 
---
 drivers/pwm/core.c  | 98 ++---
 drivers/pwm/sysfs.c | 56 ++
 include/linux/pwm.h | 87 +++
 3 files changed, 237 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 1581f6ab1b1f..16a409d452c0 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -136,6 +136,8 @@ struct pwm_device *
 of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args 
*args)
 {
struct pwm_device *pwm;
+   struct pwm_caps caps;
+   int modebit;
 
/* check, whether the driver supports a third cell for flags */
if (pc->of_pwm_n_cells < 3)
@@ -152,11 +154,23 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct 
of_phandle_args *args)
if (IS_ERR(pwm))
return pwm;
 
+   pwm_get_caps(pc, pwm, &caps);
+
pwm->args.period = args->args[1];
pwm->args.polarity = PWM_POLARITY_NORMAL;
+   pwm->args.mode = BIT(ffs(caps.modes) - 1);
+
+   if (args->args_count > 2) {
+   if (args->args[2] & PWM_POLARITY_INVERTED)
+   pwm->args.polarity = PWM_POLARITY_INVERSED;
 
-   if (args->args_count > 2 && args->args[2] & PWM_POLARITY_INVERTED)
-   pwm->args.polarity = PWM_POLARITY_INVERSED;
+   for (modebit = PWM_MODE_COMPLEMENTARY_BIT;
+modebit < PWM_MODE_CNT; modebit++)
+   if (args->args[2] & BIT(modebit)) {
+   pwm->args.mode = BIT(modebit);
+   break;
+   }
+   }
 
return pwm;
 }
@@ -166,6 +180,7 @@ static struct pwm_device *
 of_pwm_simple_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
 {
struct pwm_device *pwm;
+   struct pwm_caps caps;
 
/* sanity check driver support */
if (pc->of_pwm_n_cells < 2)
@@ -182,7 +197,9 @@ of_pwm_simple_xlate(struct pwm_chip *pc, const struct 
of_phandle_args *args)
if (IS_ERR(pwm))
return pwm;
 
+   pwm_get_caps(pc, pwm, &caps);
pwm->args.period = args->args[1];
+   pwm->args.mode = BIT(ffs(caps.modes) - 1);
 
return pwm;
 }
@@ -250,6 +267,39 @@ static bool pwm_ops_check(const struct pwm_ops *ops)
 }
 
 /**
+ * pwm_get_caps() - get PWM capabilities
+ * @chip: PWM chip
+ * @pwm: PWM device to get the capabilities for
+ * @caps: returned capabilities
+ *
+ * Retrievers capabilities for PWM device.
+ */
+void pwm_get_caps(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_caps *caps)
+{
+   if (!chip || !pwm || !caps)
+   return;
+
+   if (chip->ops && chip->ops->get_caps)
+   pwm->chip->ops->get_caps(chip, pwm, caps);
+   else if (chip->get_default_caps)
+   chip->get_default_caps(caps);
+}
+EXPORT_SYMBOL_GPL(pwm_get_caps);
+
+static void pwmchip_get_default_caps(struct pwm_caps *caps)
+{
+   static const struct pwm_caps default_caps = {
+   .modes = PWM_MODE(NORMAL),
+   };
+
+   if (!caps)
+   return;
+
+   *caps = default_caps;
+}
+
+/**
  * pwmchip_add_with_polarity() - register a new PWM chip
  * @chip: the PWM chip to add
  * @polarity: initial polarity of PWM channels
@@ -264,7 +314,8 @@ int pwmc

[PATCH v3 07/10] pwm: atmel: add pwm capabilities

2018-02-23 Thread Claudiu Beznea
Add pwm capabilities for Atmel/Microchip PWM controllers.

Signed-off-by: Claudiu Beznea 
---
 drivers/pwm/pwm-atmel.c | 80 -
 1 file changed, 52 insertions(+), 28 deletions(-)

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index 530d7dc5f1b5..d2482fe28cfa 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -65,11 +65,16 @@ struct atmel_pwm_registers {
u8 duty_upd;
 };
 
+struct atmel_pwm_data {
+   struct atmel_pwm_registers regs;
+   struct pwm_caps caps;
+};
+
 struct atmel_pwm_chip {
struct pwm_chip chip;
struct clk *clk;
void __iomem *base;
-   const struct atmel_pwm_registers *regs;
+   const struct atmel_pwm_data *data;
 
unsigned int updated_pwms;
/* ISR is cleared when read, ensure only one thread does that */
@@ -150,15 +155,15 @@ static void atmel_pwm_update_cdty(struct pwm_chip *chip, 
struct pwm_device *pwm,
struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
u32 val;
 
-   if (atmel_pwm->regs->duty_upd ==
-   atmel_pwm->regs->period_upd) {
+   if (atmel_pwm->data->regs.duty_upd ==
+   atmel_pwm->data->regs.period_upd) {
val = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm, PWM_CMR);
val &= ~PWM_CMR_UPD_CDTY;
atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, val);
}
 
atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm,
-   atmel_pwm->regs->duty_upd, cdty);
+   atmel_pwm->data->regs.duty_upd, cdty);
 }
 
 static void atmel_pwm_set_cprd_cdty(struct pwm_chip *chip,
@@ -168,9 +173,9 @@ static void atmel_pwm_set_cprd_cdty(struct pwm_chip *chip,
struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
 
atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm,
-   atmel_pwm->regs->duty, cdty);
+   atmel_pwm->data->regs.duty, cdty);
atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm,
-   atmel_pwm->regs->period, cprd);
+   atmel_pwm->data->regs.period, cprd);
 }
 
 static void atmel_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -225,7 +230,7 @@ static int atmel_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
cstate.polarity == state->polarity &&
cstate.period == state->period) {
cprd = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm,
- atmel_pwm->regs->period);
+ atmel_pwm->data->regs.period);
atmel_pwm_calculate_cdty(state, cprd, &cdty);
atmel_pwm_update_cdty(chip, pwm, cdty);
return 0;
@@ -272,32 +277,51 @@ static int atmel_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
return 0;
 }
 
+static void atmel_pwm_get_caps(struct pwm_chip *chip, struct pwm_device *pwm,
+  struct pwm_caps *caps)
+{
+   struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
+
+   *caps = atmel_pwm->data->caps;
+}
+
 static const struct pwm_ops atmel_pwm_ops = {
.apply = atmel_pwm_apply,
+   .get_caps = atmel_pwm_get_caps,
.owner = THIS_MODULE,
 };
 
-static const struct atmel_pwm_registers atmel_pwm_regs_v1 = {
-   .period = PWMV1_CPRD,
-   .period_upd = PWMV1_CUPD,
-   .duty   = PWMV1_CDTY,
-   .duty_upd   = PWMV1_CUPD,
+static const struct atmel_pwm_data atmel_pwm_data_v1 = {
+   .regs = {
+   .period = PWMV1_CPRD,
+   .period_upd = PWMV1_CUPD,
+   .duty   = PWMV1_CDTY,
+   .duty_upd   = PWMV1_CUPD,
+   },
+   .caps = {
+   .modes = PWM_MODE(NORMAL),
+   },
 };
 
-static const struct atmel_pwm_registers atmel_pwm_regs_v2 = {
-   .period = PWMV2_CPRD,
-   .period_upd = PWMV2_CPRDUPD,
-   .duty   = PWMV2_CDTY,
-   .duty_upd   = PWMV2_CDTYUPD,
+static const struct atmel_pwm_data atmel_pwm_data_v2 = {
+   .regs = {
+   .period = PWMV2_CPRD,
+   .period_upd = PWMV2_CPRDUPD,
+   .duty   = PWMV2_CDTY,
+   .duty_upd   = PWMV2_CDTYUPD,
+   },
+   .caps = {
+   .modes = PWM_MODE(NORMAL) | PWM_MODE(COMPLEMENTARY),
+   },
 };
 
 static const struct platform_device_id atmel_pwm_devtypes[] = {
{
.name = "at91sam9rl-pwm",
-   .driver_data = (kernel_ulong_t)&atmel_pwm_regs_v1,
+   .driver_data = (kernel_ulong_t)&atmel_pwm_data_v1,
}, {

[PATCH v3 04/10] pwm: pxa: populate PWM mode in of_xlate function

2018-02-23 Thread Claudiu Beznea
Populate PWM mode in of_xlate function to avoid pwm_apply_state() failure.

Signed-off-by: Claudiu Beznea 
---
 drivers/pwm/pwm-pxa.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
index 4143a46684d2..7a035716e054 100644
--- a/drivers/pwm/pwm-pxa.c
+++ b/drivers/pwm/pwm-pxa.c
@@ -155,12 +155,16 @@ static struct pwm_device *
 pxa_pwm_of_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
 {
struct pwm_device *pwm;
+   struct pwm_caps caps;
 
pwm = pwm_request_from_chip(pc, 0, NULL);
if (IS_ERR(pwm))
return pwm;
 
+   pwm_get_caps(pc, pwm, &caps);
+
pwm->args.period = args->args[0];
+   pwm->args.mode = BIT(ffs(caps.modes) - 1);
 
return pwm;
 }
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 02/10] pwm: clps711x: populate PWM mode in of_xlate function

2018-02-23 Thread Claudiu Beznea
Populate PWM mode in of_xlate function to avoid pwm_apply_state() failure.

Signed-off-by: Claudiu Beznea 
---
 drivers/pwm/pwm-clps711x.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-clps711x.c b/drivers/pwm/pwm-clps711x.c
index 26ec24e457b1..2a4d31ab3af0 100644
--- a/drivers/pwm/pwm-clps711x.c
+++ b/drivers/pwm/pwm-clps711x.c
@@ -109,10 +109,20 @@ static const struct pwm_ops clps711x_pwm_ops = {
 static struct pwm_device *clps711x_pwm_xlate(struct pwm_chip *chip,
 const struct of_phandle_args *args)
 {
+   struct pwm_device *pwm;
+   struct pwm_caps caps;
+
if (args->args[0] >= chip->npwm)
return ERR_PTR(-EINVAL);
 
-   return pwm_request_from_chip(chip, args->args[0], NULL);
+   pwm = pwm_request_from_chip(chip, args->args[0], NULL);
+   if (IS_ERR(pwm))
+   return pwm;
+
+   pwm_get_caps(chip, pwm, &caps);
+   pwm->args.mode = BIT(ffs(caps.modes) - 1);
+
+   return pwm;
 }
 
 static int clps711x_pwm_probe(struct platform_device *pdev)
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 10/10] pwm: atmel: add push-pull mode support

2018-02-23 Thread Claudiu Beznea
Add support for PWM push-pull mode. This is only supported by SAMA5D2 SoCs.

Signed-off-by: Claudiu Beznea 
---
 drivers/pwm/pwm-atmel.c | 40 
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/drivers/pwm/pwm-atmel.c b/drivers/pwm/pwm-atmel.c
index d2482fe28cfa..da4b58c1ecf2 100644
--- a/drivers/pwm/pwm-atmel.c
+++ b/drivers/pwm/pwm-atmel.c
@@ -33,8 +33,11 @@
 
 #define PWM_CMR0x0
 /* Bit field in CMR */
-#define PWM_CMR_CPOL   (1 << 9)
-#define PWM_CMR_UPD_CDTY   (1 << 10)
+#define PWM_CMR_CPOL   BIT(9)
+#define PWM_CMR_UPD_CDTY   BIT(10)
+#define PWM_CMR_DTHI   BIT(17)
+#define PWM_CMR_DTLI   BIT(18)
+#define PWM_CMR_PPMBIT(19)
 #define PWM_CMR_CPRE_MSK   0xF
 
 /* The following registers for PWM v1 */
@@ -219,16 +222,19 @@ static int atmel_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
 {
struct atmel_pwm_chip *atmel_pwm = to_atmel_pwm_chip(chip);
struct pwm_state cstate;
+   struct pwm_caps caps;
unsigned long cprd, cdty;
u32 pres, val;
int ret;
 
pwm_get_state(pwm, &cstate);
+   pwm_get_caps(chip, pwm, &caps);
 
if (state->enabled) {
if (cstate.enabled &&
cstate.polarity == state->polarity &&
-   cstate.period == state->period) {
+   cstate.period == state->period &&
+   cstate.mode == state->mode) {
cprd = atmel_pwm_ch_readl(atmel_pwm, pwm->hwpwm,
  atmel_pwm->data->regs.period);
atmel_pwm_calculate_cdty(state, cprd, &cdty);
@@ -263,6 +269,18 @@ static int atmel_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
val &= ~PWM_CMR_CPOL;
else
val |= PWM_CMR_CPOL;
+
+   /* PWM mode. */
+   if (caps.modes & PWM_MODE(PUSH_PULL)) {
+   if (state->mode == PWM_MODE(PUSH_PULL)) {
+   val |= PWM_CMR_PPM | PWM_CMR_DTLI;
+   val &= ~PWM_CMR_DTHI;
+   } else {
+   val &= ~(PWM_CMR_PPM | PWM_CMR_DTLI |
+PWM_CMR_DTHI);
+   }
+   }
+
atmel_pwm_ch_writel(atmel_pwm, pwm->hwpwm, PWM_CMR, val);
atmel_pwm_set_cprd_cdty(chip, pwm, cprd, cdty);
mutex_lock(&atmel_pwm->isr_lock);
@@ -315,6 +333,20 @@ static const struct atmel_pwm_data atmel_pwm_data_v2 = {
},
 };
 
+static const struct atmel_pwm_data atmel_pwm_data_v3 = {
+   .regs = {
+   .period = PWMV2_CPRD,
+   .period_upd = PWMV2_CPRDUPD,
+   .duty   = PWMV2_CDTY,
+   .duty_upd   = PWMV2_CDTYUPD,
+   },
+   .caps = {
+   .modes = PWM_MODE(NORMAL) |
+PWM_MODE(COMPLEMENTARY) |
+PWM_MODE(PUSH_PULL),
+   },
+};
+
 static const struct platform_device_id atmel_pwm_devtypes[] = {
{
.name = "at91sam9rl-pwm",
@@ -337,7 +369,7 @@ static const struct of_device_id atmel_pwm_dt_ids[] = {
.data = &atmel_pwm_data_v2,
}, {
.compatible = "atmel,sama5d2-pwm",
-   .data = &atmel_pwm_data_v2,
+   .data = &atmel_pwm_data_v3,
}, {
/* sentinel */
},
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 06/10] pwm: add PWM modes

2018-02-23 Thread Claudiu Beznea


On 22.02.2018 19:28, Andy Shevchenko wrote:
> On Thu, Feb 22, 2018 at 2:01 PM, Claudiu Beznea
>  wrote:
>> Add PWM normal and complementary modes.
> 
>> +- PWM_DTMODE_COMPLEMENTARY: PWM complementary working mode (for PWM
>> +channels two outputs); if not specified, the default for PWM channel will
>> +be used
> 
> What DT stands for?
It stands for Device Tree. It remained this way from the previous version. In
the previous version I had modes described in an enum, to be used by PWM core,
as follows:
enum pwm_mode {
PWM_MODE_NORMAL,
PWM_MODE_COMPLEMENTARY,
};

and, to avoid conflict b/w these defines and the one from
include/dt-bindings/pwm/pwm.h I introduced this DT in the define names from
dt-bindings. But now the DT might be removed since I've changed the way the PWM
mode is identified in PWM core. I will remove the DT in the next version, if not
requested otherwise.

Thank you,
Claudiu Benea
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 06/10] pwm: add PWM modes

2018-02-23 Thread Claudiu Beznea
Add PWM normal and complementary modes.

Signed-off-by: Claudiu Beznea 
---
 Documentation/devicetree/bindings/pwm/pwm.txt |  9 +++--
 Documentation/pwm.txt | 26 +++---
 include/dt-bindings/pwm/pwm.h |  1 +
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/pwm/pwm.txt 
b/Documentation/devicetree/bindings/pwm/pwm.txt
index 8556263b8502..c2b4b9ba0e3f 100644
--- a/Documentation/devicetree/bindings/pwm/pwm.txt
+++ b/Documentation/devicetree/bindings/pwm/pwm.txt
@@ -46,11 +46,16 @@ period in nanoseconds.
 Optionally, the pwm-specifier can encode a number of flags (defined in
 ) in a third cell:
 - PWM_POLARITY_INVERTED: invert the PWM signal polarity
+- PWM_DTMODE_COMPLEMENTARY: PWM complementary working mode (for PWM
+channels two outputs); if not specified, the default for PWM channel will
+be used
 
-Example with optional PWM specifier for inverse polarity
+Example with optional PWM specifier for inverse polarity and complementary
+mode:
 
bl: backlight {
-   pwms = <&pwm 0 500 PWM_POLARITY_INVERTED>;
+   pwms = <&pwm 0 500
+   (PWM_DTMODE_COMPLEMENTARY | PWM_POLARITY_INVERTED)>;
pwm-names = "backlight";
};
 
diff --git a/Documentation/pwm.txt b/Documentation/pwm.txt
index 8fbf0aa3ba2d..59592f8cc381 100644
--- a/Documentation/pwm.txt
+++ b/Documentation/pwm.txt
@@ -61,9 +61,9 @@ In addition to the PWM state, the PWM API also exposes PWM 
arguments, which
 are the reference PWM config one should use on this PWM.
 PWM arguments are usually platform-specific and allows the PWM user to only
 care about dutycycle relatively to the full period (like, duty = 50% of the
-period). struct pwm_args contains 2 fields (period and polarity) and should
-be used to set the initial PWM config (usually done in the probe function
-of the PWM user). PWM arguments are retrieved with pwm_get_args().
+period). struct pwm_args contains 3 fields (period, polarity and mode) and
+should be used to set the initial PWM config (usually done in the probe
+function of the PWM user). PWM arguments are retrieved with pwm_get_args().
 
 Using PWMs with the sysfs interface
 ---
@@ -110,6 +110,26 @@ channel that was exported. The following properties will 
then be available:
- 0 - disabled
- 1 - enabled
 
+  mode
+Get/set PWM channel working mode.
+
+Normal mode - for PWM channels with one output; this should be the
+default working mode for every PWM channel; output waveforms looks
+like this:
+ ________
+PWM   __|  |__|  |__|  |__|  |__
+<--T-->
+
+Complementary mode - for PWM channels two outputs; output waveforms
+looks line this:
+ ________
+PWMH1 __|  |__|  |__|  |__|  |__
+  __________
+PWML1   |__|  |__|  |__|  |__|
+<--T-->
+
+Where T is the signal period.
+
 Implementing a PWM driver
 -
 
diff --git a/include/dt-bindings/pwm/pwm.h b/include/dt-bindings/pwm/pwm.h
index ab9a077e3c7d..b4d61269aced 100644
--- a/include/dt-bindings/pwm/pwm.h
+++ b/include/dt-bindings/pwm/pwm.h
@@ -11,5 +11,6 @@
 #define _DT_BINDINGS_PWM_PWM_H
 
 #define PWM_POLARITY_INVERTED  (1 << 0)
+#define PWM_DTMODE_COMPLEMENTARY   (1 << 1)
 
 #endif
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

2018-02-23 Thread Claudiu Beznea


On 22.02.2018 14:33, Daniel Thompson wrote:
> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
>> were adapted to this change.
>>
>> Signed-off-by: Claudiu Beznea 
>> ---
>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +--
>>  drivers/bus/ts-nbus.c|  2 +-
>>  drivers/clk/clk-pwm.c|  3 ++-
>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++---
>>  drivers/hwmon/pwm-fan.c  |  2 +-
>>  drivers/input/misc/max77693-haptic.c |  2 +-
>>  drivers/input/misc/max8997_haptic.c  |  6 +-
>>  drivers/leds/leds-pwm.c  |  5 -
>>  drivers/media/rc/ir-rx51.c   |  5 -
>>  drivers/media/rc/pwm-ir-tx.c |  5 -
>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>>  drivers/video/backlight/lp8788_bl.c  |  5 -
>>  drivers/video/backlight/pwm_bl.c | 11 +--
>>  drivers/video/fbdev/ssd1307fb.c  |  3 ++-
>>  include/linux/pwm.h  |  6 --
>>  16 files changed, 70 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/video/backlight/lm3630a_bl.c 
>> b/drivers/video/backlight/lm3630a_bl.c
>> index 2030a6b77a09..696fa25dafd2 100644
>> --- a/drivers/video/backlight/lm3630a_bl.c
>> +++ b/drivers/video/backlight/lm3630a_bl.c
>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip 
>> *pchip, int br, int br_max)
>>  {
>>  unsigned int period = pchip->pdata->pwm_period;
>>  unsigned int duty = br * period / br_max;
>> +struct pwm_caps caps = { };
>>  
>> -pwm_config(pchip->pwmd, duty, period);
>> +pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, &caps);
>> +pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
> 
> Well... I admit I've only really looked at the patches that impact 
> backlight but dispersing this really odd looking bit twiddling 
> throughout the kernel doesn't strike me a great API design.>
> IMHO callers should not be required to find the first set bit in
> some specially crafted set of capability bits simply to get sane 
> default behaviour.
Thank you for your inputs. I will try to have a fix for this in next version.

The idea with this was to locate first available PWM mode of PWM channel. If the
driver hasn't registered any PWM capabilities related ops the default
capabilities will include only PWM normal mode. In case the PWM channel will
register different capabilities taking the first available mode from caps.modes
and passing it as argument to pwm_config() will ensure the pwm_apply_state()
will not fail.

Thank you,
Claudiu Beznea

> 
> 
> Daniel.
> 
> 
> 
> 
>>  if (duty)
>>  pwm_enable(pchip->pwmd);
>>  else
>> diff --git a/drivers/video/backlight/lp855x_bl.c 
>> b/drivers/video/backlight/lp855x_bl.c
>> index 939f057836e1..3d274c604862 100644
>> --- a/drivers/video/backlight/lp855x_bl.c
>> +++ b/drivers/video/backlight/lp855x_bl.c
>> @@ -240,6 +240,7 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, 
>> int max_br)
>>  unsigned int period = lp->pdata->period_ns;
>>  unsigned int duty = br * period / max_br;
>>  struct pwm_device *pwm;
>> +struct pwm_caps caps = { };
>>  
>>  /* request pwm device with the consumer name */
>>  if (!lp->pwm) {
>> @@ -256,7 +257,8 @@ static void lp855x_pwm_ctrl(struct lp855x *lp, int br, 
>> int max_br)
>>  pwm_apply_args(pwm);
>>  }
>>  
>> -pwm_config(lp->pwm, duty, period);
>> +pwm_get_caps(lp->pwm->chip, lp->pwm, &caps);
>> +pwm_config(lp->pwm, duty, period, BIT(ffs(caps.modes) - 1));
>>  if (duty)
>>  pwm_enable(lp->pwm);
>>  else
>> diff --git a/drivers/video/backlight/lp8788_bl.c 
>> b/drivers/video/backlight/lp8788_bl.c
>> index cf869ec90cce..06de3163650d 100644
>> --- a/drivers/video/backlight/lp8788_bl.c
>> +++ b/drivers/video/backlight/lp8788_bl.c
>> @@ -128,6 +128,7 @@ static void lp8788_pwm_ctrl(struct lp8788_bl *bl, int 
>> br, int max_br)
>>  unsigned int duty;
>>  struct device *dev;
>>  struct pwm_device *pwm;
>> +struct pwm_caps caps = { };
>>  
>>  if (!bl->pdata)
>>  return;
>> @@ -153,7 +154,9 @@ static void lp8788_pwm_ctrl(struct lp8788_bl *bl, int 
>> br, int max_br)
>>  

[PATCH v3 03/10] pwm: cros-ec: populate PWM mode in of_xlate function

2018-02-23 Thread Claudiu Beznea
Populate PWM mode in of_xlate function to avoid pwm_apply_state() failure.

Signed-off-by: Claudiu Beznea 
---
 drivers/pwm/pwm-cros-ec.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/pwm/pwm-cros-ec.c b/drivers/pwm/pwm-cros-ec.c
index 9c13694eaa24..e54954c13323 100644
--- a/drivers/pwm/pwm-cros-ec.c
+++ b/drivers/pwm/pwm-cros-ec.c
@@ -137,6 +137,7 @@ static struct pwm_device *
 cros_ec_pwm_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
 {
struct pwm_device *pwm;
+   struct pwm_caps caps;
 
if (args->args[0] >= pc->npwm)
return ERR_PTR(-EINVAL);
@@ -145,8 +146,11 @@ cros_ec_pwm_xlate(struct pwm_chip *pc, const struct 
of_phandle_args *args)
if (IS_ERR(pwm))
return pwm;
 
+   pwm_get_caps(pc, pwm, &caps);
+
/* The EC won't let us change the period */
pwm->args.period = EC_PWM_MAX_DUTY;
+   pwm->args.mode = BIT(ffs(caps.modes) - 1);
 
return pwm;
 }
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 05/10] pwm: add PWM mode to pwm_config()

2018-02-23 Thread Claudiu Beznea
Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
were adapted to this change.

Signed-off-by: Claudiu Beznea 
---
 arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +--
 drivers/bus/ts-nbus.c|  2 +-
 drivers/clk/clk-pwm.c|  3 ++-
 drivers/gpu/drm/i915/intel_panel.c   | 17 ++---
 drivers/hwmon/pwm-fan.c  |  2 +-
 drivers/input/misc/max77693-haptic.c |  2 +-
 drivers/input/misc/max8997_haptic.c  |  6 +-
 drivers/leds/leds-pwm.c  |  5 -
 drivers/media/rc/ir-rx51.c   |  5 -
 drivers/media/rc/pwm-ir-tx.c |  5 -
 drivers/video/backlight/lm3630a_bl.c |  4 +++-
 drivers/video/backlight/lp855x_bl.c  |  4 +++-
 drivers/video/backlight/lp8788_bl.c  |  5 -
 drivers/video/backlight/pwm_bl.c | 11 +--
 drivers/video/fbdev/ssd1307fb.c  |  3 ++-
 include/linux/pwm.h  |  6 --
 16 files changed, 70 insertions(+), 21 deletions(-)

diff --git a/arch/arm/mach-s3c24xx/mach-rx1950.c 
b/arch/arm/mach-s3c24xx/mach-rx1950.c
index e86ad6a68a0b..6feae73dcc73 100644
--- a/arch/arm/mach-s3c24xx/mach-rx1950.c
+++ b/arch/arm/mach-s3c24xx/mach-rx1950.c
@@ -386,8 +386,13 @@ static void rx1950_lcd_power(int enable)
 {
int i;
static int enabled;
+   struct pwm_caps caps = { };
+
if (enabled == enable)
return;
+
+   pwm_get_caps(lcd_pwm->chip, lcd_pwm, &caps);
+
if (!enable) {
 
/* GPC11-GPC15->OUTPUT */
@@ -433,14 +438,16 @@ static void rx1950_lcd_power(int enable)
 
/* GPB1->OUTPUT, GPB1->0 */
gpio_direction_output(S3C2410_GPB(1), 0);
-   pwm_config(lcd_pwm, 0, LCD_PWM_PERIOD);
+   pwm_config(lcd_pwm, 0, LCD_PWM_PERIOD,
+  BIT(ffs(caps.modes) - 1));
pwm_disable(lcd_pwm);
 
/* GPC0->0, GPC10->0 */
gpio_direction_output(S3C2410_GPC(0), 0);
gpio_direction_output(S3C2410_GPC(10), 0);
} else {
-   pwm_config(lcd_pwm, LCD_PWM_DUTY, LCD_PWM_PERIOD);
+   pwm_config(lcd_pwm, LCD_PWM_DUTY, LCD_PWM_PERIOD,
+  BIT(ffs(caps.modes) - 1));
pwm_enable(lcd_pwm);
 
gpio_direction_output(S3C2410_GPC(0), 1);
diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
index 073fd9011154..dcd2ca3bcd99 100644
--- a/drivers/bus/ts-nbus.c
+++ b/drivers/bus/ts-nbus.c
@@ -316,7 +316,7 @@ static int ts_nbus_probe(struct platform_device *pdev)
 * the atomic PWM API.
 */
pwm_apply_args(pwm);
-   ret = pwm_config(pwm, pargs.period, pargs.period);
+   ret = pwm_config(pwm, pargs.period, pargs.period, pargs.mode);
if (ret < 0)
return ret;
 
diff --git a/drivers/clk/clk-pwm.c b/drivers/clk/clk-pwm.c
index 8cb9d117fdbf..605a6bffe893 100644
--- a/drivers/clk/clk-pwm.c
+++ b/drivers/clk/clk-pwm.c
@@ -92,7 +92,8 @@ static int clk_pwm_probe(struct platform_device *pdev)
 * atomic PWM API.
 */
pwm_apply_args(pwm);
-   ret = pwm_config(pwm, (pargs.period + 1) >> 1, pargs.period);
+   ret = pwm_config(pwm, (pargs.period + 1) >> 1, pargs.period,
+pargs.mode);
if (ret < 0)
return ret;
 
diff --git a/drivers/gpu/drm/i915/intel_panel.c 
b/drivers/gpu/drm/i915/intel_panel.c
index adc51e452e3e..960556261787 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -633,8 +633,12 @@ static void pwm_set_backlight(const struct 
drm_connector_state *conn_state, u32
 {
struct intel_panel *panel = 
&to_intel_connector(conn_state->connector)->panel;
int duty_ns = DIV_ROUND_UP(level * CRC_PMIC_PWM_PERIOD_NS, 100);
+   struct pwm_caps caps = { };
 
-   pwm_config(panel->backlight.pwm, duty_ns, CRC_PMIC_PWM_PERIOD_NS);
+   pwm_get_caps(panel->backlight.pwm->chip, panel->backlight.pwm, &caps);
+
+   pwm_config(panel->backlight.pwm, duty_ns, CRC_PMIC_PWM_PERIOD_NS,
+  BIT(ffs(caps.modes) - 1));
 }
 
 static void
@@ -821,9 +825,13 @@ static void pwm_disable_backlight(const struct 
drm_connector_state *old_conn_sta
 {
struct intel_connector *connector = 
to_intel_connector(old_conn_state->connector);
struct intel_panel *panel = &connector->panel;
+   struct pwm_caps caps = { };
+
+   pwm_get_caps(panel->backlight.pwm->chip, panel->backlight.pwm, &caps);
 
/* Disable the backlight */
-   pwm_config(panel->backlight.pwm, 0, CRC_PMIC_PWM_PERIOD_NS);
+   pwm_config(panel->backlight.pwm, 0, CRC_PMIC_PWM_PERIOD_NS,
+  BIT(ffs(caps.modes) - 1));
usleep_range(2000, 3000);
pwm_disable(panel->backlight.pwm);
 }
@@ -1754,6 +1762,7 @@ static int pwm_setup_