From: Claudiu Beznea claudiu.beznea@microchip.com
Hi,
These patches adds support for SAM9X60's LCD controller.
First patches add option to specify if controller clock source is fixed. Second patch avoid a variable initialization in atmel_hlcdc_crtc_mode_set_nofb(). The 3rd one adds specific bindings for SAM9X60 LCD controller. The 4th and 5th add compatibles in the driver. The 6th patch enables sys_clk in probe since SAM9X60 needs this. Specific support was added also in suspend/resume hooks. The work in the 6th patch was done based on support added in 1st patch. The 7th patch adds SAM9X60's LCD configuration and enabled it.
I kept a big series including PWM, MFD, LCD changes due to shared DT bindings. If you prefer, I'm available to send them separately. Please let me know.
Thank you, Claudiu Beznea
Claudiu Beznea (5): drm: atmel-hlcdc: add config option for clock selection drm: atmel-hlcdc: avoid initializing cfg with zero dt-bindings: mfd: add bindings for SAM9X60 HLCD controller mfd: atmel-hlcdc: add compatible for SAM9X60 HLCD controller pwm: atmel-hlcdc: add compatible for SAM9X60 HLCDC's PWM
Sandeep Sheriker Mallikarjun (2): drm: atmel-hlcdc: enable sys_clk during initalization. drm: atmel-hlcdc: add sam9x60 LCD controller
.../devicetree/bindings/mfd/atmel-hlcdc.txt | 1 + drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 17 ++- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 120 ++++++++++++++++++++- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 2 + drivers/mfd/atmel-hlcdc.c | 1 + drivers/pwm/pwm-atmel-hlcdc.c | 3 + 6 files changed, 134 insertions(+), 10 deletions(-)
From: Claudiu Beznea claudiu.beznea@microchip.com
SAM9x60 LCD Controller has no option to select clock source as previous controllers have. To be able to use the same driver even for this LCD controller add a config option to know if controller supports this.
Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 12 +++++++----- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 2 ++ 2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c index 8070a558d7b1..17a7a18f6a07 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c @@ -78,7 +78,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c) unsigned long mode_rate; struct videomode vm; unsigned long prate; - unsigned int cfg; + unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL; + unsigned int cfg = 0; int div;
vm.vfront_porch = adj->crtc_vsync_start - adj->crtc_vdisplay; @@ -101,7 +102,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c) (adj->crtc_hdisplay - 1) | ((adj->crtc_vdisplay - 1) << 16));
- cfg = ATMEL_HLCDC_CLKSEL; + if (!crtc->dc->desc->fixed_clksrc) { + cfg = ATMEL_HLCDC_CLKSEL; + mask |= ATMEL_HLCDC_CLKSEL; + }
prate = 2 * clk_get_rate(crtc->dc->hlcdc->sys_clk); mode_rate = adj->crtc_clock * 1000; @@ -132,9 +136,7 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
cfg |= ATMEL_HLCDC_CLKDIV(div);
- regmap_update_bits(regmap, ATMEL_HLCDC_CFG(0), - ATMEL_HLCDC_CLKSEL | ATMEL_HLCDC_CLKDIV_MASK | - ATMEL_HLCDC_CLKPOL, cfg); + regmap_update_bits(regmap, ATMEL_HLCDC_CFG(0), mask, cfg);
cfg = 0;
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h index 70bd540d644e..0155efb9c443 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h @@ -328,6 +328,7 @@ atmel_hlcdc_layer_to_plane(struct atmel_hlcdc_layer *layer) * @max_hpw: maximum horizontal back/front porch width * @conflicting_output_formats: true if RGBXXX output formats conflict with * each other. + * @fixed_clksrc: true if clock source is fixed * @layers: a layer description table describing available layers * @nlayers: layer description table size */ @@ -340,6 +341,7 @@ struct atmel_hlcdc_dc_desc { int max_vpw; int max_hpw; bool conflicting_output_formats; + bool fixed_clksrc; const struct atmel_hlcdc_layer_desc *layers; int nlayers; };
Hi Claudiu
On Wed, Feb 27, 2019 at 04:24:16PM +0000, Claudiu.Beznea@microchip.com wrote:
From: Claudiu Beznea claudiu.beznea@microchip.com
SAM9x60 LCD Controller has no option to select clock source as previous controllers have. To be able to use the same driver even for this LCD controller add a config option to know if controller supports this.
Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 12 +++++++----- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 2 ++ 2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c index 8070a558d7b1..17a7a18f6a07 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c @@ -78,7 +78,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c) unsigned long mode_rate; struct videomode vm; unsigned long prate;
- unsigned int cfg;
- unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
- unsigned int cfg = 0; int div;
Here cfg and mask has initial values.
vm.vfront_porch = adj->crtc_vsync_start - adj->crtc_vdisplay; @@ -101,7 +102,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c) (adj->crtc_hdisplay - 1) | ((adj->crtc_vdisplay - 1) << 16));
- cfg = ATMEL_HLCDC_CLKSEL;
- if (!crtc->dc->desc->fixed_clksrc) {
cfg = ATMEL_HLCDC_CLKSEL;mask |= ATMEL_HLCDC_CLKSEL;- }
And here mask is bitwise or with ATMEL_HLCDC_CLKSEL - fine. But cfg is directly assigned. If cfg was bitwise or the ATMEL_HLCDC_CLKSEL value then the code was prepared to have extra flags added above these lines.
Only a small change and no functional difference.
With or without this change: Reviewed-by: Sam Ravnborg sam@ravnborg.org
On 28.02.2019 23:25, Sam Ravnborg wrote:
Hi Claudiu
On Wed, Feb 27, 2019 at 04:24:16PM +0000, Claudiu.Beznea@microchip.com wrote:
From: Claudiu Beznea claudiu.beznea@microchip.com
SAM9x60 LCD Controller has no option to select clock source as previous controllers have. To be able to use the same driver even for this LCD controller add a config option to know if controller supports this.
Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 12 +++++++----- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 2 ++ 2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c index 8070a558d7b1..17a7a18f6a07 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c @@ -78,7 +78,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c) unsigned long mode_rate; struct videomode vm; unsigned long prate;
- unsigned int cfg;
- unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
- unsigned int cfg = 0; int div;
Here cfg and mask has initial values.
vm.vfront_porch = adj->crtc_vsync_start - adj->crtc_vdisplay; @@ -101,7 +102,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c) (adj->crtc_hdisplay - 1) | ((adj->crtc_vdisplay - 1) << 16));
- cfg = ATMEL_HLCDC_CLKSEL;
- if (!crtc->dc->desc->fixed_clksrc) {
cfg = ATMEL_HLCDC_CLKSEL;mask |= ATMEL_HLCDC_CLKSEL;- }
And here mask is bitwise or with ATMEL_HLCDC_CLKSEL - fine. But cfg is directly assigned.
I directly assigned it here because it was the first time it was used. I will use a "|" operation as you suggest in next version.
If cfg was bitwise or the ATMEL_HLCDC_CLKSEL value then the code was prepared to have extra flags added above these lines.
Only a small change and no functional difference.
With or without this change: Reviewed-by: Sam Ravnborg sam@ravnborg.org
Hi Claudiu
One more reply to this patch...
On Wed, Feb 27, 2019 at 04:24:16PM +0000, Claudiu.Beznea@microchip.com wrote:
From: Claudiu Beznea claudiu.beznea@microchip.com
SAM9x60 LCD Controller has no option to select clock source as previous controllers have. To be able to use the same driver even for this LCD controller add a config option to know if controller supports this.
Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 12 +++++++----- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 2 ++ 2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c index 8070a558d7b1..17a7a18f6a07 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c @@ -78,7 +78,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c) unsigned long mode_rate; struct videomode vm; unsigned long prate;
- unsigned int cfg;
unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
unsigned int cfg = 0; int div;
vm.vfront_porch = adj->crtc_vsync_start - adj->crtc_vdisplay;
@@ -101,7 +102,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c) (adj->crtc_hdisplay - 1) | ((adj->crtc_vdisplay - 1) << 16));
- cfg = ATMEL_HLCDC_CLKSEL;
- if (!crtc->dc->desc->fixed_clksrc) {
cfg = ATMEL_HLCDC_CLKSEL;mask |= ATMEL_HLCDC_CLKSEL;- }
Maybe this is just too late to look at code, but I do not get this. If the sam9x60 do not support selecting the clk rate then I assume it is either fixed to used system clock or 2 x system clock.
And we have in the driver code to adjust the "div" value. The value of div depends on the CLKSEL configuration, so there must be one part of this that is not valid when fixed_clksrc is true.
prate = 2 * clk_get_rate(crtc->dc->hlcdc->sys_clk); mode_rate = adj->crtc_clock * 1000; @@ -132,9 +136,7 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
cfg |= ATMEL_HLCDC_CLKDIV(div);
It is the following code (not visible in this patch I talk about: if (div < 2) { div = 2; } else if (ATMEL_HLCDC_CLKDIV(div) & ~ATMEL_HLCDC_CLKDIV_MASK) { /* The divider ended up too big, try a lower base rate. */ cfg &= ~ATMEL_HLCDC_CLKSEL; prate /= 2; 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; }
Am I missing something obvious?
- regmap_update_bits(regmap, ATMEL_HLCDC_CFG(0),
ATMEL_HLCDC_CLKSEL | ATMEL_HLCDC_CLKDIV_MASK |ATMEL_HLCDC_CLKPOL, cfg);
regmap_update_bits(regmap, ATMEL_HLCDC_CFG(0), mask, cfg);
cfg = 0;
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h index 70bd540d644e..0155efb9c443 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h @@ -328,6 +328,7 @@ atmel_hlcdc_layer_to_plane(struct atmel_hlcdc_layer *layer)
- @max_hpw: maximum horizontal back/front porch width
- @conflicting_output_formats: true if RGBXXX output formats conflict with
each other.
- @fixed_clksrc: true if clock source is fixed
Be specific here. Tell if "fixed_clksrc == true" equals system clock or 2 x system clock.
Sam
On 28.02.2019 23:38, Sam Ravnborg wrote:
Hi Claudiu
One more reply to this patch...
On Wed, Feb 27, 2019 at 04:24:16PM +0000, Claudiu.Beznea@microchip.com wrote:
From: Claudiu Beznea claudiu.beznea@microchip.com
SAM9x60 LCD Controller has no option to select clock source as previous controllers have. To be able to use the same driver even for this LCD controller add a config option to know if controller supports this.
Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 12 +++++++----- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 2 ++ 2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c index 8070a558d7b1..17a7a18f6a07 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c @@ -78,7 +78,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c) unsigned long mode_rate; struct videomode vm; unsigned long prate;
- unsigned int cfg;
unsigned int mask = ATMEL_HLCDC_CLKDIV_MASK | ATMEL_HLCDC_CLKPOL;
unsigned int cfg = 0; int div;
vm.vfront_porch = adj->crtc_vsync_start - adj->crtc_vdisplay;
@@ -101,7 +102,10 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c) (adj->crtc_hdisplay - 1) | ((adj->crtc_vdisplay - 1) << 16));
- cfg = ATMEL_HLCDC_CLKSEL;
- if (!crtc->dc->desc->fixed_clksrc) {
cfg = ATMEL_HLCDC_CLKSEL;mask |= ATMEL_HLCDC_CLKSEL;- }
Maybe this is just too late to look at code, but I do not get this. If the sam9x60 do not support selecting the clk rate then I assume it is either fixed to used system clock or 2 x system clock.
SAM9X60's LCD controller has no support for selecting clock source (and for the moment, yes, as Alexandre specified, the datasheet is not public). If you look at e.g. SAM9X35 datasheet , chapter 45.7, at LCD Controller Configuration Register 0 [1], you will see this CLKSEL bit. On SAM9X60 you don't have this option, but you have only one clock source, fixed one. You still have the option to divide this clock to get LCD pixel clock using CLKDIV field of LCD Controller Configuration Register 0, and the formula is the same as in SAM9X35 case: the only difference is that the formula uses the fixed clock in case of SAM9X60 apart from SAM9X35 which uses the selected clock: SAM9X60: LCDPCLK = source clock / (CLKDIV+2) SAM9X35: pixel_clock = selected_clock / (CLKDIV + 2).
[1] http://ww1.microchip.com/downloads/en/DeviceDoc/Atmel-11055-32-bit-ARM926EJ-...
And we have in the driver code to adjust the "div" value. The value of div depends on the CLKSEL configuration, so there must be one part of this that is not valid when fixed_clksrc is true.
When fixed_clksrc is true you still have to option to divide the source clock.
I hope I was clear.
prate = 2 * clk_get_rate(crtc->dc->hlcdc->sys_clk); mode_rate = adj->crtc_clock * 1000; @@ -132,9 +136,7 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
cfg |= ATMEL_HLCDC_CLKDIV(div);
It is the following code (not visible in this patch I talk about: if (div < 2) { div = 2; } else if (ATMEL_HLCDC_CLKDIV(div) & ~ATMEL_HLCDC_CLKDIV_MASK) { /* The divider ended up too big, try a lower base rate. */ cfg &= ~ATMEL_HLCDC_CLKSEL; prate /= 2; 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; }Am I missing something obvious?
- regmap_update_bits(regmap, ATMEL_HLCDC_CFG(0),
ATMEL_HLCDC_CLKSEL | ATMEL_HLCDC_CLKDIV_MASK |ATMEL_HLCDC_CLKPOL, cfg);
regmap_update_bits(regmap, ATMEL_HLCDC_CFG(0), mask, cfg);
cfg = 0;
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h index 70bd540d644e..0155efb9c443 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h @@ -328,6 +328,7 @@ atmel_hlcdc_layer_to_plane(struct atmel_hlcdc_layer *layer)
- @max_hpw: maximum horizontal back/front porch width
- @conflicting_output_formats: true if RGBXXX output formats conflict with
each other.
- @fixed_clksrc: true if clock source is fixed
Be specific here. Tell if "fixed_clksrc == true" equals system clock or 2 x system clock.
In case of SAM9X60, as I specified above, there is no option to select b/w MCK and 2xMCK as in case of, e.g. SAM9X35, but only one clock source.
Sam
From: Claudiu Beznea claudiu.beznea@microchip.com
Remove cfg initialization with zero and read state with drm_crtc_state_to_atmel_hlcdc_crtc_state() so that cfg to be initialized with state's output_mode.
Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c index 17a7a18f6a07..7b0c6683a690 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c @@ -138,7 +138,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)
regmap_update_bits(regmap, ATMEL_HLCDC_CFG(0), mask, cfg);
- cfg = 0; + 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; @@ -146,9 +147,6 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c) if (adj->flags & DRM_MODE_FLAG_NHSYNC) cfg |= ATMEL_HLCDC_HSPOL;
- state = drm_crtc_state_to_atmel_hlcdc_crtc_state(c->state); - cfg |= state->output_mode << 8; - regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5), ATMEL_HLCDC_HSPOL | ATMEL_HLCDC_VSPOL | ATMEL_HLCDC_VSPDLYS | ATMEL_HLCDC_VSPDLYE |
On Wed, Feb 27, 2019 at 04:24:21PM +0000, Claudiu.Beznea@microchip.com wrote:
From: Claudiu Beznea claudiu.beznea@microchip.com
Remove cfg initialization with zero and read state with drm_crtc_state_to_atmel_hlcdc_crtc_state() so that cfg to be initialized with state's output_mode.
Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com
Looks good, good to have simpler code. Reviewed-by: Sam Ravnborg sam@ravnborg.org
From: Claudiu Beznea claudiu.beznea@microchip.com
Add new compatible string for the HLCD controller on SAM9X60 SoC.
Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com --- Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt | 1 + 1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt index 3f643ef121ff..5f8880cc757e 100644 --- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt +++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt @@ -7,6 +7,7 @@ Required properties: "atmel,sama5d2-hlcdc" "atmel,sama5d3-hlcdc" "atmel,sama5d4-hlcdc" + "microchip,sam9x60-hlcdc" - reg: base address and size of the HLCDC device registers. - clock-names: the name of the 3 clocks requested by the HLCDC device. Should contain "periph_clk", "sys_clk" and "slow_clk".
From: Claudiu Beznea claudiu.beznea@microchip.com
Add compatible for SAM9X60 HLCD controller.
Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com --- drivers/mfd/atmel-hlcdc.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c index e82543bcfdc8..35a9e16f9902 100644 --- a/drivers/mfd/atmel-hlcdc.c +++ b/drivers/mfd/atmel-hlcdc.c @@ -141,6 +141,7 @@ static const struct of_device_id atmel_hlcdc_match[] = { { .compatible = "atmel,sama5d2-hlcdc" }, { .compatible = "atmel,sama5d3-hlcdc" }, { .compatible = "atmel,sama5d4-hlcdc" }, + { .compatible = "microchip,sam9x60-hlcdc" }, { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, atmel_hlcdc_match);
From: Claudiu Beznea claudiu.beznea@microchip.com
Add compatible string for SAM9X60 HLCDC's PWM.
Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com --- drivers/pwm/pwm-atmel-hlcdc.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/pwm/pwm-atmel-hlcdc.c b/drivers/pwm/pwm-atmel-hlcdc.c index 54c6633d9b5d..331ca0233d9e 100644 --- a/drivers/pwm/pwm-atmel-hlcdc.c +++ b/drivers/pwm/pwm-atmel-hlcdc.c @@ -246,6 +246,9 @@ static const struct of_device_id atmel_hlcdc_dt_ids[] = { .compatible = "atmel,sama5d4-hlcdc", .data = &atmel_hlcdc_pwm_sama5d3_errata, }, + { + .compatible = "microchip,sam9x60-hlcdc", + }, { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, atmel_hlcdc_dt_ids);
On Wed, Feb 27, 2019 at 04:24:35PM +0000, Claudiu.Beznea@microchip.com wrote:
From: Claudiu Beznea claudiu.beznea@microchip.com
Add compatible string for SAM9X60 HLCDC's PWM.
Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com
drivers/pwm/pwm-atmel-hlcdc.c | 3 +++ 1 file changed, 3 insertions(+)
I guess it makes more sense for this to go in with the MFD changes rather than through the PWM tree, so:
Acked-by: Thierry Reding thierry.reding@gmail.com
From: Sandeep Sheriker Mallikarjun sandeepsheriker.mallikarjun@microchip.com
For SAM9X60 SoC, sys_clk is through lcd_gclk clock source and this needs to be enabled before enabling lcd_clk.
Signed-off-by: Sandeep Sheriker Mallikarjun sandeepsheriker.mallikarjun@microchip.com [claudiu.beznea@microchip.com: add fixed_clksrc checks] Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index 0be13eceedba..8bf51f853721 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -625,10 +625,18 @@ 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_destroy_wq; + goto err_sys_clk_disable; }
pm_runtime_enable(dev->dev); @@ -664,6 +672,9 @@ 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); @@ -688,6 +699,8 @@ 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); }
@@ -805,6 +818,8 @@ 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; } @@ -814,6 +829,8 @@ 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);
Hi Claudiu
On Wed, Feb 27, 2019 at 04:24:40PM +0000, Claudiu.Beznea@microchip.com wrote:
From: Sandeep Sheriker Mallikarjun sandeepsheriker.mallikarjun@microchip.com
For SAM9X60 SoC, sys_clk is through lcd_gclk clock source and this needs to be enabled before enabling lcd_clk.
We have "ownership" of the clocks in the mfd device. So it would make more sense to let the mfd device handle the base clocks. In other words - what about pushing enable of perigh_clk and sys_clk to the mfd driver.
This may have the nice side-effect that we avoid that both the drm driver and the pwm driver enable/disable the periph_clk as it is today.
Another comment - fixed_clksrc is used to determine if sys_clk is enabled. But that flag is about the clksource selection, and it is just a coincidence that the same flag can be used here. Why we cannot always enable sys_clk? Do we need to do this only for sam9x60? IF yes, then add a new falg. If no, then skip the flag.
Sam
Signed-off-by: Sandeep Sheriker Mallikarjun sandeepsheriker.mallikarjun@microchip.com [claudiu.beznea@microchip.com: add fixed_clksrc checks] Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index 0be13eceedba..8bf51f853721 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -625,10 +625,18 @@ 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_destroy_wq;
goto err_sys_clk_disable;}
pm_runtime_enable(dev->dev);
@@ -664,6 +672,9 @@ 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); @@ -688,6 +699,8 @@ 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);}
@@ -805,6 +818,8 @@ 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;
} @@ -814,6 +829,8 @@ 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);-- 2.7.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 28.02.2019 23:55, Sam Ravnborg wrote:
Hi Claudiu
On Wed, Feb 27, 2019 at 04:24:40PM +0000, Claudiu.Beznea@microchip.com wrote:
From: Sandeep Sheriker Mallikarjun sandeepsheriker.mallikarjun@microchip.com
For SAM9X60 SoC, sys_clk is through lcd_gclk clock source and this needs to be enabled before enabling lcd_clk.
We have "ownership" of the clocks in the mfd device. So it would make more sense to let the mfd device handle the base clocks. In other words - what about pushing enable of perigh_clk and sys_clk to the mfd driver.
I think this could be achieved, but how is better? To have individual drivers (in this care I would say, child drivers, like e.g. this lcd driver) taking decisions on their own for their resources (even if shared) or to have parents taking decisions for them? Just asking.
This may have the nice side-effect that we avoid that both the drm driver and the pwm driver enable/disable the periph_clk as it is today.
Another comment - fixed_clksrc is used to determine if sys_clk is enabled. But that flag is about the clksource selection, and it is just a coincidence that the same flag can be used here.
Yes, agree on this, I took advantage of fixed_clksrc to also enable the sys_clk based on fixed_clksrc to avoid introducing another member in struct atmel_hlcdc_dc_desc. At this moment this is valid only for SAM9X60 and I was thinking that in case some other scenario will appear I will do the appropriate changes.
Why we cannot always enable sys_clk? Do we need to do this only for sam9x60?
Only SAM9X60 requires this explicitly at probe.
IF yes, then add a new falg. If no, then skip the flag.
Sam
Signed-off-by: Sandeep Sheriker Mallikarjun sandeepsheriker.mallikarjun@microchip.com [claudiu.beznea@microchip.com: add fixed_clksrc checks] Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index 0be13eceedba..8bf51f853721 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -625,10 +625,18 @@ 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_destroy_wq;
goto err_sys_clk_disable;}
pm_runtime_enable(dev->dev);
@@ -664,6 +672,9 @@ 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); @@ -688,6 +699,8 @@ 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);}
@@ -805,6 +818,8 @@ 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;
} @@ -814,6 +829,8 @@ 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);-- 2.7.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
From: Sandeep Sheriker Mallikarjun sandeepsheriker.mallikarjun@microchip.com
Add the LCD controller for SAM9X60.
Signed-off-by: Sandeep Sheriker Mallikarjun sandeepsheriker.mallikarjun@microchip.com [claudiu.beznea@microchip.com: add fixed_clksrc option to atmel_hlcdc_dc_sam9x60] Signed-off-by: Claudiu Beznea claudiu.beznea@microchip.com --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 101 +++++++++++++++++++++++++++ 1 file changed, 101 insertions(+)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index 8bf51f853721..fb2e7646daeb 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -364,6 +364,103 @@ static const struct atmel_hlcdc_dc_desc atmel_hlcdc_dc_sama5d4 = { .nlayers = ARRAY_SIZE(atmel_hlcdc_sama5d4_layers), .layers = atmel_hlcdc_sama5d4_layers, }; + +static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sam9x60_layers[] = { + { + .name = "base", + .formats = &atmel_hlcdc_plane_rgb_formats, + .regs_offset = 0x60, + .id = 0, + .type = ATMEL_HLCDC_BASE_LAYER, + .cfgs_offset = 0x2c, + .layout = { + .xstride = { 2 }, + .default_color = 3, + .general_config = 4, + .disc_pos = 5, + .disc_size = 6, + }, + .clut_offset = 0x600, + }, + { + .name = "overlay1", + .formats = &atmel_hlcdc_plane_rgb_formats, + .regs_offset = 0x160, + .id = 1, + .type = ATMEL_HLCDC_OVERLAY_LAYER, + .cfgs_offset = 0x2c, + .layout = { + .pos = 2, + .size = 3, + .xstride = { 4 }, + .pstride = { 5 }, + .default_color = 6, + .chroma_key = 7, + .chroma_key_mask = 8, + .general_config = 9, + }, + .clut_offset = 0xa00, + }, + { + .name = "overlay2", + .formats = &atmel_hlcdc_plane_rgb_formats, + .regs_offset = 0x260, + .id = 2, + .type = ATMEL_HLCDC_OVERLAY_LAYER, + .cfgs_offset = 0x2c, + .layout = { + .pos = 2, + .size = 3, + .xstride = { 4 }, + .pstride = { 5 }, + .default_color = 6, + .chroma_key = 7, + .chroma_key_mask = 8, + .general_config = 9, + }, + .clut_offset = 0xe00, + }, + { + .name = "high-end-overlay", + .formats = &atmel_hlcdc_plane_rgb_and_yuv_formats, + .regs_offset = 0x360, + .id = 3, + .type = ATMEL_HLCDC_OVERLAY_LAYER, + .cfgs_offset = 0x4c, + .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, + .scaler_config = 13, + .phicoeffs = { + .x = 17, + .y = 33, + }, + .csc = 14, + }, + .clut_offset = 0x1200, + }, +}; + +static const struct atmel_hlcdc_dc_desc atmel_hlcdc_dc_sam9x60 = { + .min_width = 0, + .min_height = 0, + .max_width = 2048, + .max_height = 2048, + .max_spw = 0xff, + .max_vpw = 0xff, + .max_hpw = 0x3ff, + .fixed_clksrc = true, + .nlayers = ARRAY_SIZE(atmel_hlcdc_sam9x60_layers), + .layers = atmel_hlcdc_sam9x60_layers, +}; + static const struct of_device_id atmel_hlcdc_of_match[] = { { .compatible = "atmel,at91sam9n12-hlcdc", @@ -385,6 +482,10 @@ static const struct of_device_id atmel_hlcdc_of_match[] = { .compatible = "atmel,sama5d4-hlcdc", .data = &atmel_hlcdc_dc_sama5d4, }, + { + .compatible = "microchip,sam9x60-hlcdc", + .data = &atmel_hlcdc_dc_sam9x60, + }, { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, atmel_hlcdc_of_match);
Hi Claudiu
On Wed, Feb 27, 2019 at 04:24:11PM +0000, Claudiu.Beznea@microchip.com wrote:
From: Claudiu Beznea claudiu.beznea@microchip.com
Hi,
These patches adds support for SAM9X60's LCD controller.
Can you elaborate a little more which chips that are relevant. To be able to look into the right data-sheet, while reviewing. Link to data-sheet would be nice too.
Thanks in advance, Sam
On 28/02/2019 19:21:19+0100, Sam Ravnborg wrote:
Hi Claudiu
On Wed, Feb 27, 2019 at 04:24:11PM +0000, Claudiu.Beznea@microchip.com wrote:
From: Claudiu Beznea claudiu.beznea@microchip.com
Hi,
These patches adds support for SAM9X60's LCD controller.
Can you elaborate a little more which chips that are relevant. To be able to look into the right data-sheet, while reviewing. Link to data-sheet would be nice too.
sam9x60 is the chip name (i.e. x is not a wildcard). I don't think the datasheet is finished/public yet.
Hi Alexandre.
These patches adds support for SAM9X60's LCD controller.
Can you elaborate a little more which chips that are relevant. To be able to look into the right data-sheet, while reviewing. Link to data-sheet would be nice too.
sam9x60 is the chip name (i.e. x is not a wildcard). I don't think the datasheet is finished/public yet.
Thanks. I will take a look at the patches but not knowing the specifics of the implementation/IP will makes this less valueable.
Sam
On 28.02.2019 23:13, Sam Ravnborg wrote:
Hi Alexandre.
These patches adds support for SAM9X60's LCD controller.
Can you elaborate a little more which chips that are relevant. To be able to look into the right data-sheet, while reviewing. Link to data-sheet would be nice too.
sam9x60 is the chip name (i.e. x is not a wildcard). I don't think the datasheet is finished/public yet.
Thanks. I will take a look at the patches but not knowing the specifics of the implementation/IP will makes this less valueable.
At this moment, as Alexandre specified, the datasheet is not published.
Thank you, Claudiu Beznea
Sam
dri-devel@lists.freedesktop.org
-
Alexandre Belloni -
Claudiu.Beznea＠microchip.com -
Sam Ravnborg -
Thierry Reding