2024-02-21 05:36:06

by Manikandan Muralidharan

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

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

.../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 176 +++++++--
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 105 +++++
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 133 +++++--
.../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 369 +++++++++++++++---
include/linux/mfd/atmel-hlcdc.h | 10 +
5 files changed, 678 insertions(+), 115 deletions(-)

--
2.25.1



2024-02-21 05:36:18

by Manikandan Muralidharan

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

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 <[email protected]>
---
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.
+ *
+ * @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
+ */
+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
+ * @lcdc_atomic_update: enable the LCDC layers and interrupts
+ * @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
+ */
+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);
+};
+
+extern const struct atmel_lcdc_dc_ops atmel_hlcdc_ops;
+
/**
* Atmel HLCDC Display Controller description structure.
*
@@ -306,6 +364,7 @@ atmel_hlcdc_layer_to_plane(struct atmel_hlcdc_layer *layer)
* @fixed_clksrc: true if clock source is fixed
* @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;
@@ -319,30 +378,7 @@ struct atmel_hlcdc_dc_desc {
bool fixed_clksrc;
const struct atmel_hlcdc_layer_desc *layers;
int nlayers;
-};
-
-/**
- * Atmel HLCDC Display Controller.
- *
- * @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
- */
-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;
+ const struct atmel_lcdc_dc_ops *ops;
};

extern struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats;
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index daa508504f47..3427546cd58c 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -282,8 +282,9 @@ atmel_hlcdc_plane_scaler_set_phicoeff(struct atmel_hlcdc_plane *plane,
coeff_tab[i]);
}

-static void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
- struct atmel_hlcdc_plane_state *state)
+static
+void atmel_hlcdc_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;
@@ -335,6 +336,7 @@ atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
struct atmel_hlcdc_plane_state *state)
{
const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+ struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;

if (desc->layout.size)
atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.size,
@@ -352,12 +354,12 @@ atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
ATMEL_HLCDC_LAYER_POS(state->crtc_x,
state->crtc_y));

- atmel_hlcdc_plane_setup_scaler(plane, state);
+ dc->desc->ops->plane_setup_scaler(plane, state);
}

-static void
-atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
- struct atmel_hlcdc_plane_state *state)
+static
+void atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
+ struct atmel_hlcdc_plane_state *state)
{
unsigned int cfg = ATMEL_HLCDC_LAYER_DMA_BLEN_INCR16 | state->ahb_id;
const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
@@ -437,10 +439,33 @@ static void atmel_hlcdc_plane_update_clut(struct atmel_hlcdc_plane *plane,
}
}

+static void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,
+ struct atmel_hlcdc_plane_state *state,
+ u32 sr, int i)
+{
+ atmel_hlcdc_layer_write_reg(&plane->layer,
+ ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
+ state->dscrs[i]->self);
+
+ if (sr & ATMEL_HLCDC_LAYER_EN)
+ return;
+
+ atmel_hlcdc_layer_write_reg(&plane->layer,
+ ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
+ state->dscrs[i]->addr);
+ atmel_hlcdc_layer_write_reg(&plane->layer,
+ ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
+ state->dscrs[i]->ctrl);
+ atmel_hlcdc_layer_write_reg(&plane->layer,
+ ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
+ state->dscrs[i]->self);
+}
+
static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
- struct atmel_hlcdc_plane_state *state)
+ struct atmel_hlcdc_plane_state *state)
{
const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+ struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
struct drm_framebuffer *fb = state->base.fb;
u32 sr;
int i;
@@ -452,21 +477,7 @@ static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,

state->dscrs[i]->addr = gem->dma_addr + state->offsets[i];

- atmel_hlcdc_layer_write_reg(&plane->layer,
- ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
- state->dscrs[i]->self);
-
- if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
- atmel_hlcdc_layer_write_reg(&plane->layer,
- ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
- state->dscrs[i]->addr);
- atmel_hlcdc_layer_write_reg(&plane->layer,
- ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
- state->dscrs[i]->ctrl);
- atmel_hlcdc_layer_write_reg(&plane->layer,
- ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
- state->dscrs[i]->self);
- }
+ dc->desc->ops->update_lcdc_buffers(plane, state, sr, i);

if (desc->layout.xstride[i])
atmel_hlcdc_layer_write_cfg(&plane->layer,
@@ -712,11 +723,8 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,
return 0;
}

-static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
- struct drm_atomic_state *state)
+static void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane)
{
- struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
-
/* Disable interrupts */
atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IDR,
0xffffffff);
@@ -731,6 +739,34 @@ static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
}

+static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
+ struct drm_atomic_state *state)
+{
+ struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
+ struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
+
+ dc->desc->ops->lcdc_atomic_disable(plane);
+}
+
+static void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
+ struct atmel_hlcdc_dc *dc)
+{
+ u32 sr;
+
+ /* Enable the overrun interrupts. */
+ atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
+ ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
+ ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
+ ATMEL_HLCDC_LAYER_OVR_IRQ(2));
+
+ /* Apply the new config at the next SOF event. */
+ sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
+ atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
+ ATMEL_HLCDC_LAYER_UPDATE |
+ (sr & ATMEL_HLCDC_LAYER_EN ?
+ ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
+}
+
static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
struct drm_atomic_state *state)
{
@@ -739,7 +775,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
struct atmel_hlcdc_plane_state *hstate =
drm_plane_state_to_atmel_hlcdc_plane_state(new_s);
- u32 sr;
+ struct atmel_hlcdc_dc *dc = p->dev->dev_private;

if (!new_s->crtc || !new_s->fb)
return;
@@ -750,29 +786,39 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
}

atmel_hlcdc_plane_update_pos_and_size(plane, hstate);
- atmel_hlcdc_plane_update_general_settings(plane, hstate);
+ dc->desc->ops->lcdc_update_general_settings(plane, hstate);
atmel_hlcdc_plane_update_format(plane, hstate);
atmel_hlcdc_plane_update_clut(plane, hstate);
atmel_hlcdc_plane_update_buffers(plane, hstate);
atmel_hlcdc_plane_update_disc_area(plane, hstate);

- /* Enable the overrun interrupts. */
- atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
- ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
- ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
- ATMEL_HLCDC_LAYER_OVR_IRQ(2));
+ dc->desc->ops->lcdc_atomic_update(plane, dc);
+}

- /* Apply the new config at the next SOF event. */
- sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
- atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
- ATMEL_HLCDC_LAYER_UPDATE |
- (sr & ATMEL_HLCDC_LAYER_EN ?
- ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
+static void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
+ const struct atmel_hlcdc_layer_desc *desc)
+{
+ /*
+ * TODO: declare a "yuv-to-rgb-conv-factors" property to let
+ * userspace modify these factors (using a BLOB property ?).
+ */
+ static const u32 hlcdc_csc_coeffs[] = {
+ 0x4c900091,
+ 0x7a5f5090,
+ 0x40040890
+ };
+
+ for (int i = 0; i < ARRAY_SIZE(hlcdc_csc_coeffs); i++) {
+ atmel_hlcdc_layer_write_cfg(&plane->layer,
+ desc->layout.csc + i,
+ hlcdc_csc_coeffs[i]);
+ }
}

static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
{
const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+ struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;

if (desc->type == ATMEL_HLCDC_OVERLAY_LAYER ||
desc->type == ATMEL_HLCDC_CURSOR_LAYER) {
@@ -796,31 +842,16 @@ static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
return ret;
}

- if (desc->layout.csc) {
- /*
- * TODO: decare a "yuv-to-rgb-conv-factors" property to let
- * userspace modify these factors (using a BLOB property ?).
- */
- atmel_hlcdc_layer_write_cfg(&plane->layer,
- desc->layout.csc,
- 0x4c900091);
- atmel_hlcdc_layer_write_cfg(&plane->layer,
- desc->layout.csc + 1,
- 0x7a5f5090);
- atmel_hlcdc_layer_write_cfg(&plane->layer,
- desc->layout.csc + 2,
- 0x40040890);
- }
+ if (desc->layout.csc)
+ dc->desc->ops->lcdc_csc_init(plane, desc);

return 0;
}

-void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
+static void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
+ const struct atmel_hlcdc_layer_desc *desc)
{
- const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
- u32 isr;
-
- isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
+ u32 isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);

/*
* There's not much we can do in case of overrun except informing
@@ -834,6 +865,24 @@ void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
desc->name);
}

+void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
+{
+ const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+ struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
+
+ dc->desc->ops->lcdc_irq_dbg(plane, desc);
+}
+
+const struct atmel_lcdc_dc_ops atmel_hlcdc_ops = {
+ .plane_setup_scaler = atmel_hlcdc_plane_setup_scaler,
+ .update_lcdc_buffers = update_hlcdc_buffers,
+ .lcdc_atomic_disable = hlcdc_atomic_disable,
+ .lcdc_update_general_settings = atmel_hlcdc_plane_update_general_settings,
+ .lcdc_atomic_update = hlcdc_atomic_update,
+ .lcdc_csc_init = hlcdc_csc_init,
+ .lcdc_irq_dbg = hlcdc_irq_dbg,
+};
+
static const struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
.atomic_check = atmel_hlcdc_plane_atomic_check,
.atomic_update = atmel_hlcdc_plane_atomic_update,
--
2.25.1


2024-02-21 05:36:23

by Manikandan Muralidharan

[permalink] [raw]
Subject: [PATCH v8 2/7] drm: atmel-hlcdc: Define XLCDC specific registers

From: Durai Manickam KR <[email protected]>

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 <[email protected]>
[[email protected]: Remove unused macro definitions]
Signed-off-by: Manikandan Muralidharan <[email protected]>
---
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 ad732edfef0b..bc70a81b5275 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 <drm/drm_plane.h>

+/* 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_ENR 0x10
+#define ATMEL_XLCDC_LAYER_EN BIT(0)
+
+#define ATMEL_XLCDC_LAYER_IER 0x0
+#define ATMEL_XLCDC_LAYER_IDR 0x4
+#define ATMEL_XLCDC_LAYER_ISR 0xc
+#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_CFG 0
+
+#define ATMEL_XLCDC_LAYER_DMA BIT(0)
+#define ATMEL_XLCDC_LAYER_REP BIT(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_ENABLE BIT(0)
+#define ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE BIT(1)
+#define ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE BIT(4)
+#define ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE BIT(5)
+
+#define ATMEL_XLCDC_LAYER_VXSYCFG_ONE BIT(0)
+#define ATMEL_XLCDC_LAYER_VXSYTAP2_ENABLE BIT(4)
+#define ATMEL_XLCDC_LAYER_VXSCCFG_ONE BIT(16)
+#define ATMEL_XLCDC_LAYER_VXSCTAP2_ENABLE BIT(20)
+
+#define ATMEL_XLCDC_LAYER_HXSYCFG_ONE BIT(0)
+#define ATMEL_XLCDC_LAYER_HXSYTAP2_ENABLE BIT(4)
+#define ATMEL_XLCDC_LAYER_HXSCCFG_ONE BIT(16)
+#define ATMEL_XLCDC_LAYER_HXSCTAP2_ENABLE BIT(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_MASK GENMASK(9, 8)
+#define ATMEL_XLCDC_MODE_MASK GENMASK(10, 8)
+#define ATMEL_XLCDC_DPI BIT(11)
#define ATMEL_HLCDC_PP BIT(10)
#define ATMEL_HLCDC_VSPSU BIT(12)
#define ATMEL_HLCDC_VSPHO BIT(13)
@@ -34,6 +36,12 @@
#define ATMEL_HLCDC_IDR 0x30
#define ATMEL_HLCDC_IMR 0x34
#define ATMEL_HLCDC_ISR 0x38
+#define ATMEL_XLCDC_ATTRE 0x3c
+
+#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)
--
2.25.1


2024-02-21 05:36:42

by Manikandan Muralidharan

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

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 <[email protected]>
Co-developed-by: Hari Prasath Gujulan Elango <[email protected]>
Signed-off-by: Hari Prasath Gujulan Elango <[email protected]>
Co-developed-by: Durai Manickam KR <[email protected]>
Signed-off-by: Durai Manickam KR <[email protected]>
---
.../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");

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_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");

regmap_write(regmap, ATMEL_HLCDC_EN, 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_EN, 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");
+
+ 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.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index bc70a81b5275..777987b7873d 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
@@ -387,6 +387,7 @@ struct atmel_lcdc_dc_ops {
};

extern const struct atmel_lcdc_dc_ops atmel_hlcdc_ops;
+extern const struct atmel_lcdc_dc_ops atmel_xlcdc_ops;

/**
* Atmel HLCDC Display Controller description structure.
@@ -404,6 +405,7 @@ extern const struct atmel_lcdc_dc_ops atmel_hlcdc_ops;
* @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
@@ -418,6 +420,7 @@ 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;
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index 3427546cd58c..59ddd743ce92 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -331,6 +331,55 @@ 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);
+ return;
+ }
+
+ /* xfactor = round[(2^20 * XMEMSIZE)/XSIZE)] */
+ xfactor = (u32)(((1 << 20) * state->src_w) / state->crtc_w);
+
+ /* yfactor = round[(2^20 * YMEMSIZE)/YSIZE)] */
+ yfactor = (u32)(((1 << 20) * state->src_h) / state->crtc_h);
+
+ atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config,
+ ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE |
+ ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE |
+ ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE |
+ ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE);
+
+ atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 1,
+ yfactor);
+ atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 3,
+ xfactor);
+
+ /*
+ * With YCbCr 4:2:2 and YCbYcr 4:2:0 window resampling, configuration
+ * register LCDC_HEOCFG25.VXSCFACT and LCDC_HEOCFG27.HXSCFACT is half
+ * the value of yfactor and xfactor.
+ */
+ if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
+ yfactor /= 2;
+ xfactor /= 2;
+ }
+
+ atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
+ yfactor);
+ atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
+ xfactor);
+}
+
static void
atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
struct atmel_hlcdc_plane_state *state)
@@ -395,6 +444,40 @@ void atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
cfg);
}

+static
+void atmel_xlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
+ struct atmel_hlcdc_plane_state *state)
+{
+ const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
+ const struct drm_format_info *format = state->base.fb->format;
+ unsigned int cfg;
+
+ atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_XLCDC_LAYER_DMA_CFG,
+ ATMEL_HLCDC_LAYER_DMA_BLEN_INCR16 | state->ahb_id);
+
+ cfg = ATMEL_XLCDC_LAYER_DMA | ATMEL_XLCDC_LAYER_REP;
+
+ if (plane->base.type != DRM_PLANE_TYPE_PRIMARY) {
+ /*
+ * Alpha Blending bits specific to SAM9X7 SoC
+ */
+ cfg |= ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS |
+ ATMEL_XLCDC_LAYER_SFACTA_ONE |
+ ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS |
+ ATMEL_XLCDC_LAYER_DFACTA_ONE;
+ if (format->has_alpha)
+ cfg |= ATMEL_XLCDC_LAYER_A0(0xff);
+ else
+ cfg |= ATMEL_XLCDC_LAYER_A0(state->base.alpha);
+ }
+
+ if (state->disc_h && state->disc_w)
+ cfg |= ATMEL_XLCDC_LAYER_DISCEN;
+
+ atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.general_config,
+ cfg);
+}
+
static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
struct atmel_hlcdc_plane_state *state)
{
@@ -461,6 +544,15 @@ static void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,
state->dscrs[i]->self);
}

+static void update_xlcdc_buffers(struct atmel_hlcdc_plane *plane,
+ struct atmel_hlcdc_plane_state *state,
+ u32 sr, int i)
+{
+ atmel_hlcdc_layer_write_reg(&plane->layer,
+ ATMEL_XLCDC_LAYER_PLANE_ADDR(i),
+ state->dscrs[i]->addr);
+}
+
static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
struct atmel_hlcdc_plane_state *state)
{
@@ -470,7 +562,8 @@ static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
u32 sr;
int i;

- sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
+ if (!dc->desc->is_xlcdc)
+ sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);

for (i = 0; i < state->nplanes; i++) {
struct drm_gem_dma_object *gem = drm_fb_dma_get_gem_obj(fb, i);
@@ -739,6 +832,20 @@ static void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane)
atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
}

+static void xlcdc_atomic_disable(struct atmel_hlcdc_plane *plane)
+{
+ /* Disable interrupts */
+ atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IDR,
+ 0xffffffff);
+
+ /* Disable the layer */
+ atmel_hlcdc_layer_write_reg(&plane->layer,
+ ATMEL_XLCDC_LAYER_ENR, 0);
+
+ /* Clear all pending interrupts */
+ atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
+}
+
static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
struct drm_atomic_state *state)
{
@@ -767,6 +874,28 @@ static void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
}

+static void xlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
+ struct atmel_hlcdc_dc *dc)
+{
+ /* Enable the overrun interrupts. */
+ atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IER,
+ ATMEL_XLCDC_LAYER_OVR_IRQ(0) |
+ ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
+ ATMEL_XLCDC_LAYER_OVR_IRQ(2));
+
+ atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_ENR,
+ ATMEL_XLCDC_LAYER_EN);
+
+ /*
+ * Updating XLCDC_xxxCFGx, XLCDC_xxxFBA and XLCDC_xxxEN,
+ * (where xxx indicates each layer) requires writing one to the
+ * Update Attribute field for each layer in LCDC_ATTRE register for SAM9X7.
+ */
+ regmap_write(dc->hlcdc->regmap, ATMEL_XLCDC_ATTRE, ATMEL_XLCDC_BASE_UPDATE |
+ ATMEL_XLCDC_OVR1_UPDATE | ATMEL_XLCDC_OVR3_UPDATE |
+ ATMEL_XLCDC_HEO_UPDATE);
+}
+
static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
struct drm_atomic_state *state)
{
@@ -815,6 +944,30 @@ static void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
}
}

+static void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
+ const struct atmel_hlcdc_layer_desc *desc)
+{
+ /*
+ * yuv-to-rgb-conv-factors are now defined from LCDC_HEOCFG16 to
+ * LCDC_HEOCFG21 registers in SAM9X7.
+ */
+ static const u32 xlcdc_csc_coeffs[] = {
+ 0x00000488,
+ 0x00000648,
+ 0x1EA00480,
+ 0x00001D28,
+ 0x08100480,
+ 0x00000000,
+ 0x00000007
+ };
+
+ for (int i = 0; i < ARRAY_SIZE(xlcdc_csc_coeffs); i++) {
+ atmel_hlcdc_layer_write_cfg(&plane->layer,
+ desc->layout.csc + i,
+ xlcdc_csc_coeffs[i]);
+ }
+}
+
static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
{
const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
@@ -865,6 +1018,23 @@ static void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
desc->name);
}

+static void xlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
+ const struct atmel_hlcdc_layer_desc *desc)
+{
+ u32 isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
+
+ /*
+ * There's not much we can do in case of overrun except informing
+ * the user. However, we are in interrupt context here, hence the
+ * use of dev_dbg().
+ */
+ if (isr &
+ (ATMEL_XLCDC_LAYER_OVR_IRQ(0) | ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
+ ATMEL_XLCDC_LAYER_OVR_IRQ(2)))
+ dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
+ desc->name);
+}
+
void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
{
const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
@@ -883,6 +1053,16 @@ const struct atmel_lcdc_dc_ops atmel_hlcdc_ops = {
.lcdc_irq_dbg = hlcdc_irq_dbg,
};

+const struct atmel_lcdc_dc_ops atmel_xlcdc_ops = {
+ .plane_setup_scaler = atmel_xlcdc_plane_setup_scaler,
+ .update_lcdc_buffers = update_xlcdc_buffers,
+ .lcdc_atomic_disable = xlcdc_atomic_disable,
+ .lcdc_update_general_settings = atmel_xlcdc_plane_update_general_settings,
+ .lcdc_atomic_update = xlcdc_atomic_update,
+ .lcdc_csc_init = xlcdc_csc_init,
+ .lcdc_irq_dbg = xlcdc_irq_dbg,
+};
+
static const struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
.atomic_check = atmel_hlcdc_plane_atomic_check,
.atomic_update = atmel_hlcdc_plane_atomic_update,
--
2.25.1


2024-02-21 05:36:47

by Manikandan Muralidharan

[permalink] [raw]
Subject: [PATCH v8 4/7] drm: atmel-hlcdc: add DPI mode support for XLCDC

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 <[email protected]>
[[email protected]: update DPI mode bit using is_xlcdc flag]
Signed-off-by: Durai Manickam KR <[email protected]>
---
.../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 21 +++++++++++++++++--
1 file changed, 19 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 98a98b5fca85..fdd3a6bc0f79 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 *
@@ -170,6 +172,8 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c)

if (adj->flags & DRM_MODE_FLAG_NHSYNC)
cfg |= ATMEL_HLCDC_HSPOL;
+ } else {
+ cfg |= state->dpi << 11;
}

regmap_update_bits(regmap, ATMEL_HLCDC_CFG(5),
@@ -177,7 +181,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 |
+ (crtc->dc->desc->is_xlcdc ? ATMEL_XLCDC_MODE_MASK |
+ ATMEL_XLCDC_DPI : ATMEL_HLCDC_MODE_MASK),
cfg);

clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
@@ -380,7 +386,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;
}

@@ -484,6 +498,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;
@@ -495,6 +510,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;
}
--
2.25.1


2024-02-21 05:37:04

by Manikandan Muralidharan

[permalink] [raw]
Subject: [PATCH v8 5/7] drm: atmel-hlcdc: add vertical and horizontal scaling support for XLCDC

Update the vertical and horizontal scaler registers of XLCDC IP
with Bilinear and Bicubic co-efficients taps for Chroma and
Luma componenets of the Pixel.

Signed-off-by: Manikandan Muralidharan <[email protected]>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 4 ++++
.../gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 20 +++++++++++++++++++
2 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
index 777987b7873d..fd9d9af4332c 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 59ddd743ce92..a527badf865d 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -966,6 +966,26 @@ static void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
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)
--
2.25.1


2024-02-21 05:37:24

by Manikandan Muralidharan

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

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 <[email protected]>
[[email protected]: update output format using is_xlcdc flag]
Signed-off-by: Durai Manickam KR <[email protected]>
---
.../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_OUTPUT BIT(0)
-#define ATMEL_HLCDC_RGB565_OUTPUT BIT(1)
-#define ATMEL_HLCDC_RGB666_OUTPUT BIT(2)
-#define ATMEL_HLCDC_RGB888_OUTPUT BIT(3)
-#define ATMEL_HLCDC_OUTPUT_MODE_MASK GENMASK(3, 0)
+#define ATMEL_HLCDC_RGB444_OUTPUT BIT(0)
+#define ATMEL_HLCDC_RGB565_OUTPUT BIT(1)
+#define ATMEL_HLCDC_RGB666_OUTPUT BIT(2)
+#define ATMEL_HLCDC_RGB888_OUTPUT BIT(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_OUTPUT BIT(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;
+ 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_fmts |=
+ ATMEL_HLCDC_DPI_RGB888_OUTPUT;
+ 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->desc->is_xlcdc ? ATMEL_XLCDC_OUTPUT_MODE_MASK :
+ ATMEL_HLCDC_OUTPUT_MODE_MASK;

for_each_new_connector_in_state(state->state, connector, cstate, i) {
unsigned int supported_fmts = 0;
--
2.25.1


2024-02-21 05:37:27

by Manikandan Muralidharan

[permalink] [raw]
Subject: [PATCH v8 7/7] drm: atmel-hlcdc: add LCD controller layer definition for sam9x75

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 <[email protected]>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 100 +++++++++++++++++++
1 file changed, 100 insertions(+)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index b09df821cbc0..9ce429f889ca 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -467,6 +467,102 @@ static const struct atmel_hlcdc_dc_desc atmel_hlcdc_dc_sam9x60 = {
.ops = &atmel_hlcdc_ops,
};

+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,
+ .vxs_config = 30,
+ .hxs_config = 31,
+ },
+ .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 = 0x3ff,
+ .max_vpw = 0x3ff,
+ .max_hpw = 0x3ff,
+ .fixed_clksrc = true,
+ .is_xlcdc = true,
+ .nlayers = ARRAY_SIZE(atmel_xlcdc_sam9x75_layers),
+ .layers = atmel_xlcdc_sam9x75_layers,
+ .ops = &atmel_xlcdc_ops,
+};
+
static const struct of_device_id atmel_hlcdc_of_match[] = {
{
.compatible = "atmel,at91sam9n12-hlcdc",
@@ -492,6 +588,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);
--
2.25.1


2024-02-22 17:38:43

by Sam Ravnborg

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

On Wed, Feb 21, 2024 at 11:05:24AM +0530, 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

Hi Manikandan

thanks for your paitent follow-up on this patch set.
Everything looks good and all patches are:
Acked-by: Sam Ravnborg <[email protected]>

We are at v8 now and if there are additional comments we can handle them
in-tree.
I have asked Thomas Zimmermann to apply the patches to drm-misc as I
have lost my push infrastructure atm.

Sam

2024-02-23 17:05:03

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v8 2/7] drm: atmel-hlcdc: Define XLCDC specific registers

On Wed, 21 Feb 2024, Manikandan Muralidharan wrote:

> From: Durai Manickam KR <[email protected]>
>
> 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 <[email protected]>
> [[email protected]: Remove unused macro definitions]
> Signed-off-by: Manikandan Muralidharan <[email protected]>
> ---
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 42 ++++++++++++++++++++

> include/linux/mfd/atmel-hlcdc.h | 10 +++++

Acked-by: Lee Jones <[email protected]>

--
Lee Jones [李琼斯]

2024-02-24 12:50:16

by claudiu beznea

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



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 <[email protected]>
> ---
> 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
> + * @lcdc_irq_dbg: to raise alert incase of interrupt overrun in any LCDC layer
> + */
> +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);
> +};
> +
> +extern const struct atmel_lcdc_dc_ops atmel_hlcdc_ops;
> +
> /**
> * Atmel HLCDC Display Controller description structure.
> *
> @@ -306,6 +364,7 @@ atmel_hlcdc_layer_to_plane(struct atmel_hlcdc_layer *layer)
> * @fixed_clksrc: true if clock source is fixed
> * @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;
> @@ -319,30 +378,7 @@ struct atmel_hlcdc_dc_desc {
> bool fixed_clksrc;
> const struct atmel_hlcdc_layer_desc *layers;
> int nlayers;
> -};
> -
> -/**
> - * Atmel HLCDC Display Controller.
> - *
> - * @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
> - */
> -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;
> + const struct atmel_lcdc_dc_ops *ops;
> };
>
> extern struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats;
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index daa508504f47..3427546cd58c 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -282,8 +282,9 @@ atmel_hlcdc_plane_scaler_set_phicoeff(struct atmel_hlcdc_plane *plane,
> coeff_tab[i]);
> }
>
> -static void atmel_hlcdc_plane_setup_scaler(struct atmel_hlcdc_plane *plane,
> - struct atmel_hlcdc_plane_state *state)
> +static
> +void atmel_hlcdc_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;
> @@ -335,6 +336,7 @@ atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
> struct atmel_hlcdc_plane_state *state)
> {
> const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> + struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>
> if (desc->layout.size)
> atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.size,
> @@ -352,12 +354,12 @@ atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
> ATMEL_HLCDC_LAYER_POS(state->crtc_x,
> state->crtc_y));
>
> - atmel_hlcdc_plane_setup_scaler(plane, state);
> + dc->desc->ops->plane_setup_scaler(plane, state);
> }
>
> -static void
> -atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
> - struct atmel_hlcdc_plane_state *state)
> +static
> +void atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
> + struct atmel_hlcdc_plane_state *state)
> {
> unsigned int cfg = ATMEL_HLCDC_LAYER_DMA_BLEN_INCR16 | state->ahb_id;
> const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> @@ -437,10 +439,33 @@ static void atmel_hlcdc_plane_update_clut(struct atmel_hlcdc_plane *plane,
> }
> }
>
> +static void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,

would be good to keep in in the atmel_hlcdc/atmel namespace thus, rename it
something like:

*atmel_hlcdc*_update_buffers()

to avoid conflicting with any subsystem specific APIs, if any. And also
keep consistency with the rest of the HLCDC driver code.

> + struct atmel_hlcdc_plane_state *state,
> + u32 sr, int i)
> +{
> + atmel_hlcdc_layer_write_reg(&plane->layer,
> + ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
> + state->dscrs[i]->self);
> +
> + if (sr & ATMEL_HLCDC_LAYER_EN)
> + return;
> +
> + atmel_hlcdc_layer_write_reg(&plane->layer,
> + ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
> + state->dscrs[i]->addr);
> + atmel_hlcdc_layer_write_reg(&plane->layer,
> + ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
> + state->dscrs[i]->ctrl);
> + atmel_hlcdc_layer_write_reg(&plane->layer,
> + ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
> + state->dscrs[i]->self);
> +}
> +
> static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
> - struct atmel_hlcdc_plane_state *state)
> + struct atmel_hlcdc_plane_state *state)
> {
> const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> + struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
> struct drm_framebuffer *fb = state->base.fb;
> u32 sr;
> int i;
> @@ -452,21 +477,7 @@ static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
>
> state->dscrs[i]->addr = gem->dma_addr + state->offsets[i];
>
> - atmel_hlcdc_layer_write_reg(&plane->layer,
> - ATMEL_HLCDC_LAYER_PLANE_HEAD(i),
> - state->dscrs[i]->self);
> -
> - if (!(sr & ATMEL_HLCDC_LAYER_EN)) {
> - atmel_hlcdc_layer_write_reg(&plane->layer,
> - ATMEL_HLCDC_LAYER_PLANE_ADDR(i),
> - state->dscrs[i]->addr);
> - atmel_hlcdc_layer_write_reg(&plane->layer,
> - ATMEL_HLCDC_LAYER_PLANE_CTRL(i),
> - state->dscrs[i]->ctrl);
> - atmel_hlcdc_layer_write_reg(&plane->layer,
> - ATMEL_HLCDC_LAYER_PLANE_NEXT(i),
> - state->dscrs[i]->self);
> - }
> + dc->desc->ops->update_lcdc_buffers(plane, state, sr, i);
>
> if (desc->layout.xstride[i])
> atmel_hlcdc_layer_write_cfg(&plane->layer,
> @@ -712,11 +723,8 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,
> return 0;
> }
>
> -static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
> - struct drm_atomic_state *state)
> +static void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane)

Same here, e.g., *atmel_hlcdc*_atomic_disable()

> {
> - struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
> -
> /* Disable interrupts */
> atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IDR,
> 0xffffffff);
> @@ -731,6 +739,34 @@ static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
> atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
> }
>
> +static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
> + struct drm_atomic_state *state)
> +{
> + struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
> + struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
> +
> + dc->desc->ops->lcdc_atomic_disable(plane);
> +}
> +
> +static void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
> + struct atmel_hlcdc_dc *dc)

Same here, e.g., *atmel_hlcdc*_hlcdc_atomic_update()

> +{
> + u32 sr;
> +
> + /* Enable the overrun interrupts. */
> + atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
> + ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
> + ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
> + ATMEL_HLCDC_LAYER_OVR_IRQ(2));
> +
> + /* Apply the new config at the next SOF event. */
> + sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
> + atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
> + ATMEL_HLCDC_LAYER_UPDATE |
> + (sr & ATMEL_HLCDC_LAYER_EN ?
> + ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
> +}
> +
> static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
> struct drm_atomic_state *state)
> {
> @@ -739,7 +775,7 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
> struct atmel_hlcdc_plane *plane = drm_plane_to_atmel_hlcdc_plane(p);
> struct atmel_hlcdc_plane_state *hstate =
> drm_plane_state_to_atmel_hlcdc_plane_state(new_s);
> - u32 sr;
> + struct atmel_hlcdc_dc *dc = p->dev->dev_private;
>
> if (!new_s->crtc || !new_s->fb)
> return;
> @@ -750,29 +786,39 @@ static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
> }
>
> atmel_hlcdc_plane_update_pos_and_size(plane, hstate);
> - atmel_hlcdc_plane_update_general_settings(plane, hstate);
> + dc->desc->ops->lcdc_update_general_settings(plane, hstate);
> atmel_hlcdc_plane_update_format(plane, hstate);
> atmel_hlcdc_plane_update_clut(plane, hstate);
> atmel_hlcdc_plane_update_buffers(plane, hstate);
> atmel_hlcdc_plane_update_disc_area(plane, hstate);
>
> - /* Enable the overrun interrupts. */
> - atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_IER,
> - ATMEL_HLCDC_LAYER_OVR_IRQ(0) |
> - ATMEL_HLCDC_LAYER_OVR_IRQ(1) |
> - ATMEL_HLCDC_LAYER_OVR_IRQ(2));
> + dc->desc->ops->lcdc_atomic_update(plane, dc);
> +}
>
> - /* Apply the new config at the next SOF event. */
> - sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
> - atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHER,
> - ATMEL_HLCDC_LAYER_UPDATE |
> - (sr & ATMEL_HLCDC_LAYER_EN ?
> - ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
> +static void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
> + const struct atmel_hlcdc_layer_desc *desc)

Same here.

> +{
> + /*
> + * TODO: declare a "yuv-to-rgb-conv-factors" property to let
> + * userspace modify these factors (using a BLOB property ?).
> + */
> + static const u32 hlcdc_csc_coeffs[] = {
> + 0x4c900091,
> + 0x7a5f5090,
> + 0x40040890
> + };
> +
> + for (int i = 0; i < ARRAY_SIZE(hlcdc_csc_coeffs); i++) {
> + atmel_hlcdc_layer_write_cfg(&plane->layer,
> + desc->layout.csc + i,
> + hlcdc_csc_coeffs[i]);
> + }
> }
>
> static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
> {
> const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> + struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
>
> if (desc->type == ATMEL_HLCDC_OVERLAY_LAYER ||
> desc->type == ATMEL_HLCDC_CURSOR_LAYER) {
> @@ -796,31 +842,16 @@ static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
> return ret;
> }
>
> - if (desc->layout.csc) {
> - /*
> - * TODO: decare a "yuv-to-rgb-conv-factors" property to let
> - * userspace modify these factors (using a BLOB property ?).
> - */
> - atmel_hlcdc_layer_write_cfg(&plane->layer,
> - desc->layout.csc,
> - 0x4c900091);
> - atmel_hlcdc_layer_write_cfg(&plane->layer,
> - desc->layout.csc + 1,
> - 0x7a5f5090);
> - atmel_hlcdc_layer_write_cfg(&plane->layer,
> - desc->layout.csc + 2,
> - 0x40040890);
> - }
> + if (desc->layout.csc)
> + dc->desc->ops->lcdc_csc_init(plane, desc);
>
> return 0;
> }
>
> -void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
> +static void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
> + const struct atmel_hlcdc_layer_desc *desc)

same here.

> {
> - const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> - u32 isr;
> -
> - isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
> + u32 isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
>
> /*
> * There's not much we can do in case of overrun except informing
> @@ -834,6 +865,24 @@ void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
> desc->name);
> }
>
> +void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
> +{
> + const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> + struct atmel_hlcdc_dc *dc = plane->base.dev->dev_private;
> +
> + dc->desc->ops->lcdc_irq_dbg(plane, desc);
> +}
> +
> +const struct atmel_lcdc_dc_ops atmel_hlcdc_ops = {
> + .plane_setup_scaler = atmel_hlcdc_plane_setup_scaler,
> + .update_lcdc_buffers = update_hlcdc_buffers,
> + .lcdc_atomic_disable = hlcdc_atomic_disable,
> + .lcdc_update_general_settings = atmel_hlcdc_plane_update_general_settings,
> + .lcdc_atomic_update = hlcdc_atomic_update,
> + .lcdc_csc_init = hlcdc_csc_init,
> + .lcdc_irq_dbg = hlcdc_irq_dbg,
> +};
> +
> static const struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> .atomic_check = atmel_hlcdc_plane_atomic_check,
> .atomic_update = atmel_hlcdc_plane_atomic_update,

2024-02-24 12:50:27

by claudiu beznea

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



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 <[email protected]>
> [[email protected]: update output format using is_xlcdc flag]
> Signed-off-by: Durai Manickam KR <[email protected]>
> ---
> .../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_OUTPUT BIT(0)
> -#define ATMEL_HLCDC_RGB565_OUTPUT BIT(1)
> -#define ATMEL_HLCDC_RGB666_OUTPUT BIT(2)
> -#define ATMEL_HLCDC_RGB888_OUTPUT BIT(3)
> -#define ATMEL_HLCDC_OUTPUT_MODE_MASK GENMASK(3, 0)
> +#define ATMEL_HLCDC_RGB444_OUTPUT BIT(0)
> +#define ATMEL_HLCDC_RGB565_OUTPUT BIT(1)
> +#define ATMEL_HLCDC_RGB666_OUTPUT BIT(2)
> +#define ATMEL_HLCDC_RGB888_OUTPUT BIT(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_OUTPUT BIT(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->desc->is_xlcdc ? ATMEL_XLCDC_OUTPUT_MODE_MASK :
> + ATMEL_HLCDC_OUTPUT_MODE_MASK;
>
> for_each_new_connector_in_state(state->state, connector, cstate, i) {
> unsigned int supported_fmts = 0;

2024-02-24 12:50:39

by claudiu beznea

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



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 <[email protected]>
> Co-developed-by: Hari Prasath Gujulan Elango <[email protected]>
> Signed-off-by: Hari Prasath Gujulan Elango <[email protected]>
> Co-developed-by: Durai Manickam KR <[email protected]>
> Signed-off-by: Durai Manickam KR <[email protected]>
> ---
> .../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_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");
>
> regmap_write(regmap, ATMEL_HLCDC_EN, 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_EN, 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");
> +
> + 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.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> index bc70a81b5275..777987b7873d 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h
> @@ -387,6 +387,7 @@ struct atmel_lcdc_dc_ops {
> };
>
> extern const struct atmel_lcdc_dc_ops atmel_hlcdc_ops;
> +extern const struct atmel_lcdc_dc_ops atmel_xlcdc_ops;
>
> /**
> * Atmel HLCDC Display Controller description structure.
> @@ -404,6 +405,7 @@ extern const struct atmel_lcdc_dc_ops atmel_hlcdc_ops;
> * @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
> @@ -418,6 +420,7 @@ 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;
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index 3427546cd58c..59ddd743ce92 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -331,6 +331,55 @@ 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);
> + return;
> + }
> +
> + /* xfactor = round[(2^20 * XMEMSIZE)/XSIZE)] */
> + xfactor = (u32)(((1 << 20) * state->src_w) / state->crtc_w);
> +
> + /* yfactor = round[(2^20 * YMEMSIZE)/YSIZE)] */
> + yfactor = (u32)(((1 << 20) * state->src_h) / state->crtc_h);
> +
> + atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config,
> + ATMEL_XLCDC_LAYER_VSCALER_LUMA_ENABLE |
> + ATMEL_XLCDC_LAYER_VSCALER_CHROMA_ENABLE |
> + ATMEL_XLCDC_LAYER_HSCALER_LUMA_ENABLE |
> + ATMEL_XLCDC_LAYER_HSCALER_CHROMA_ENABLE);
> +
> + atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 1,
> + yfactor);
> + atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 3,
> + xfactor);
> +
> + /*
> + * With YCbCr 4:2:2 and YCbYcr 4:2:0 window resampling, configuration
> + * register LCDC_HEOCFG25.VXSCFACT and LCDC_HEOCFG27.HXSCFACT is half
> + * the value of yfactor and xfactor.
> + */
> + if (state->base.fb->format->format == DRM_FORMAT_YUV420) {
> + yfactor /= 2;
> + xfactor /= 2;
> + }
> +
> + atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 2,
> + yfactor);
> + atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.scaler_config + 4,
> + xfactor);
> +}
> +
> static void
> atmel_hlcdc_plane_update_pos_and_size(struct atmel_hlcdc_plane *plane,
> struct atmel_hlcdc_plane_state *state)
> @@ -395,6 +444,40 @@ void atmel_hlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
> cfg);
> }
>
> +static
> +void atmel_xlcdc_plane_update_general_settings(struct atmel_hlcdc_plane *plane,
> + struct atmel_hlcdc_plane_state *state)
> +{
> + const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> + const struct drm_format_info *format = state->base.fb->format;
> + unsigned int cfg;
> +
> + atmel_hlcdc_layer_write_cfg(&plane->layer, ATMEL_XLCDC_LAYER_DMA_CFG,
> + ATMEL_HLCDC_LAYER_DMA_BLEN_INCR16 | state->ahb_id);
> +
> + cfg = ATMEL_XLCDC_LAYER_DMA | ATMEL_XLCDC_LAYER_REP;
> +
> + if (plane->base.type != DRM_PLANE_TYPE_PRIMARY) {
> + /*
> + * Alpha Blending bits specific to SAM9X7 SoC
> + */
> + cfg |= ATMEL_XLCDC_LAYER_SFACTC_A0_MULT_AS |
> + ATMEL_XLCDC_LAYER_SFACTA_ONE |
> + ATMEL_XLCDC_LAYER_DFACTC_M_A0_MULT_AS |
> + ATMEL_XLCDC_LAYER_DFACTA_ONE;
> + if (format->has_alpha)
> + cfg |= ATMEL_XLCDC_LAYER_A0(0xff);
> + else
> + cfg |= ATMEL_XLCDC_LAYER_A0(state->base.alpha);
> + }
> +
> + if (state->disc_h && state->disc_w)
> + cfg |= ATMEL_XLCDC_LAYER_DISCEN;
> +
> + atmel_hlcdc_layer_write_cfg(&plane->layer, desc->layout.general_config,
> + cfg);
> +}
> +
> static void atmel_hlcdc_plane_update_format(struct atmel_hlcdc_plane *plane,
> struct atmel_hlcdc_plane_state *state)
> {
> @@ -461,6 +544,15 @@ static void update_hlcdc_buffers(struct atmel_hlcdc_plane *plane,
> state->dscrs[i]->self);
> }
>
> +static void update_xlcdc_buffers(struct atmel_hlcdc_plane *plane,
> + struct atmel_hlcdc_plane_state *state,
> + u32 sr, int i)

*atmel*_update_xlcdc_buffers()

> +{
> + atmel_hlcdc_layer_write_reg(&plane->layer,
> + ATMEL_XLCDC_LAYER_PLANE_ADDR(i),
> + state->dscrs[i]->addr);
> +}
> +
> static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
> struct atmel_hlcdc_plane_state *state)
> {
> @@ -470,7 +562,8 @@ static void atmel_hlcdc_plane_update_buffers(struct atmel_hlcdc_plane *plane,
> u32 sr;
> int i;
>
> - sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
> + if (!dc->desc->is_xlcdc)
> + sr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_CHSR);
>
> for (i = 0; i < state->nplanes; i++) {
> struct drm_gem_dma_object *gem = drm_fb_dma_get_gem_obj(fb, i);
> @@ -739,6 +832,20 @@ static void hlcdc_atomic_disable(struct atmel_hlcdc_plane *plane)
> atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_HLCDC_LAYER_ISR);
> }
>
> +static void xlcdc_atomic_disable(struct atmel_hlcdc_plane *plane)

Same here: *atmel*_xlcdc_atomic_disable()

> +{
> + /* Disable interrupts */
> + atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IDR,
> + 0xffffffff);
> +
> + /* Disable the layer */
> + atmel_hlcdc_layer_write_reg(&plane->layer,
> + ATMEL_XLCDC_LAYER_ENR, 0);
> +
> + /* Clear all pending interrupts */
> + atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
> +}
> +
> static void atmel_hlcdc_plane_atomic_disable(struct drm_plane *p,
> struct drm_atomic_state *state)
> {
> @@ -767,6 +874,28 @@ static void hlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
> ATMEL_HLCDC_LAYER_A2Q : ATMEL_HLCDC_LAYER_EN));
> }
>
> +static void xlcdc_atomic_update(struct atmel_hlcdc_plane *plane,
> + struct atmel_hlcdc_dc *dc)

*atmel*_xlcdc_atomic_update()

> +{
> + /* Enable the overrun interrupts. */
> + atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_IER,
> + ATMEL_XLCDC_LAYER_OVR_IRQ(0) |
> + ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
> + ATMEL_XLCDC_LAYER_OVR_IRQ(2));
> +
> + atmel_hlcdc_layer_write_reg(&plane->layer, ATMEL_XLCDC_LAYER_ENR,
> + ATMEL_XLCDC_LAYER_EN);
> +
> + /*
> + * Updating XLCDC_xxxCFGx, XLCDC_xxxFBA and XLCDC_xxxEN,
> + * (where xxx indicates each layer) requires writing one to the
> + * Update Attribute field for each layer in LCDC_ATTRE register for SAM9X7.
> + */
> + regmap_write(dc->hlcdc->regmap, ATMEL_XLCDC_ATTRE, ATMEL_XLCDC_BASE_UPDATE |
> + ATMEL_XLCDC_OVR1_UPDATE | ATMEL_XLCDC_OVR3_UPDATE |
> + ATMEL_XLCDC_HEO_UPDATE);
> +}
> +
> static void atmel_hlcdc_plane_atomic_update(struct drm_plane *p,
> struct drm_atomic_state *state)
> {
> @@ -815,6 +944,30 @@ static void hlcdc_csc_init(struct atmel_hlcdc_plane *plane,
> }
> }
>
> +static void xlcdc_csc_init(struct atmel_hlcdc_plane *plane,
> + const struct atmel_hlcdc_layer_desc *desc)

*atmel*_xlcdc_csc_init()

> +{
> + /*
> + * yuv-to-rgb-conv-factors are now defined from LCDC_HEOCFG16 to
> + * LCDC_HEOCFG21 registers in SAM9X7.
> + */
> + static const u32 xlcdc_csc_coeffs[] = {
> + 0x00000488,
> + 0x00000648,
> + 0x1EA00480,
> + 0x00001D28,
> + 0x08100480,
> + 0x00000000,
> + 0x00000007
> + };
> +
> + for (int i = 0; i < ARRAY_SIZE(xlcdc_csc_coeffs); i++) {
> + atmel_hlcdc_layer_write_cfg(&plane->layer,
> + desc->layout.csc + i,
> + xlcdc_csc_coeffs[i]);
> + }
> +}
> +
> static int atmel_hlcdc_plane_init_properties(struct atmel_hlcdc_plane *plane)
> {
> const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> @@ -865,6 +1018,23 @@ static void hlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
> desc->name);
> }
>
> +static void xlcdc_irq_dbg(struct atmel_hlcdc_plane *plane,
> + const struct atmel_hlcdc_layer_desc *desc)

*atmel*_xlcdc_irq_dbg()

> +{
> + u32 isr = atmel_hlcdc_layer_read_reg(&plane->layer, ATMEL_XLCDC_LAYER_ISR);
> +
> + /*
> + * There's not much we can do in case of overrun except informing
> + * the user. However, we are in interrupt context here, hence the
> + * use of dev_dbg().
> + */
> + if (isr &
> + (ATMEL_XLCDC_LAYER_OVR_IRQ(0) | ATMEL_XLCDC_LAYER_OVR_IRQ(1) |
> + ATMEL_XLCDC_LAYER_OVR_IRQ(2)))
> + dev_dbg(plane->base.dev->dev, "overrun on plane %s\n",
> + desc->name);
> +}
> +
> void atmel_hlcdc_plane_irq(struct atmel_hlcdc_plane *plane)
> {
> const struct atmel_hlcdc_layer_desc *desc = plane->layer.desc;
> @@ -883,6 +1053,16 @@ const struct atmel_lcdc_dc_ops atmel_hlcdc_ops = {
> .lcdc_irq_dbg = hlcdc_irq_dbg,
> };
>
> +const struct atmel_lcdc_dc_ops atmel_xlcdc_ops = {
> + .plane_setup_scaler = atmel_xlcdc_plane_setup_scaler,
> + .update_lcdc_buffers = update_xlcdc_buffers,
> + .lcdc_atomic_disable = xlcdc_atomic_disable,
> + .lcdc_update_general_settings = atmel_xlcdc_plane_update_general_settings,
> + .lcdc_atomic_update = xlcdc_atomic_update,
> + .lcdc_csc_init = xlcdc_csc_init,
> + .lcdc_irq_dbg = xlcdc_irq_dbg,
> +};
> +
> static const struct drm_plane_helper_funcs atmel_hlcdc_layer_plane_helper_funcs = {
> .atomic_check = atmel_hlcdc_plane_atomic_check,
> .atomic_update = atmel_hlcdc_plane_atomic_update,

2024-02-24 12:52:26

by claudiu beznea

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

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 <[email protected]>