Changes in v11:
- As per previous conversation with CK:
- Definitions for default LUT bits were removed
- Specifying number of bits per LUT component was merged into
the commit adding support for 12-bit LUT
- MT8195 support was moved to a new commit
- Other spare changes
- Added missing AAL_GAMMA_LUT_EN bit setting to mtk_aal_gamma_set()
- Rebased over next-20231012
Changes in v10:
- Moved snippet from patch [7/15] to patch [6/15] as that was
intended to be there instead; fixes build issue for patch [6/15]
as pointed out by the kernel text robot (oops, sorry!)
Changes in v9:
- As per previous conversation with CK Hu, added a commit that
de-commonizes the gamma setting function that was used in
both DISP_AAL and DISP_GAMMA, now each of them have their
own .gamma_set() callback (mtk_disp_gamma_set_common() has
been removed).
- Added a change to use bitfield macros in mtk_disp_aal.c
- Added a change to compress of_device_id entries in mtk_disp_aal.c
- Tested again on MT6795, MT8173, MT8186, MT8192, MT8195
Changes in v8:
- Changed lut_size to be a mtk_disp_gamma_set_common() function
parameter to pass lut size from AAL
Changes in v7:
- Added check for NULL dev for AAL-gamma case
- Added get_lut_size callback for AAL-gamma
- Added comment to clarify SoC 10/12 bits support and old vs new
register layout as suggested by Alexandre M.
Changes in v6:
- Fixed smatch warning in patch 11/11, ref.:
https://lore.kernel.org/all/[email protected]/
Changes in v5:
- Removed incorrect comment on default LUT size and bits
- Removed useless check for num_lut_banks
- Added comment about CMDQ implementation on patch 5
- Evaluated passing lut size/bits from AAL, idea discarded as
the implementation would be rather tricky while bringing no
benefits.
Changes in v4:
- Fixed assignment typo appeared in v3
Changes in v3:
- Fixed issues due to variables renaming during cleanup (oops)
- This is actually the right series, since v2 was taken from the
wrong kernel tree.... :-)
Changes in v2:
- Added explicit inclusion of linux/bitfield.h in patch [06/11]
This series adds support for GAMMA IP requiring and/or supporting
a 12-bits LUT using a slightly different register layout and programming
sequence for multiple LUT banks: this IP version is currently found
on a number of SoCs, not only including the Chromebook/IoT oriented
Kompanio 1200/1380 MT8195/MT8195T, but also Smartphone chips such as
the Dimensity 9200 (MT6985) and others.
This series was tested on MT8195, MT8192, MT8173, MT6795:
* MT6795, MT8192, MT8173: No regression, works fine.
* MT8195: Color correction is finally working!
AngeloGioacchino Del Regno (15):
drm/mediatek: gamma: Reduce indentation in mtk_gamma_set_common()
drm/mediatek: gamma: Support SoC specific LUT size
drm/mediatek: gamma: Improve and simplify HW LUT calculation
drm/mediatek: gamma: Enable the Gamma LUT table only after programming
drm/mediatek: gamma: Use bitfield macros
drm/mediatek: aal: Use bitfield macros
drm/mediatek: De-commonize disp_aal/disp_gamma gamma_set functions
drm/mediatek: gamma: Support multi-bank gamma LUT
drm/mediatek: gamma: Add support for 12-bit LUT
drm/mediatek: gamma: Add support for MT8195
drm/mediatek: gamma: Make sure relay mode is disabled
drm/mediatek: gamma: Program gamma LUT type for descending or rising
drm/mediatek: aal: Add kerneldoc for struct mtk_disp_aal
drm/mediatek: gamma: Add kerneldoc for struct mtk_disp_gamma
drm/mediatek: aal: Compress of_device_id entries and add sentinel
Jason-JH.Lin (1):
drm/mediatek: gamma: Adjust mtk_drm_gamma_set_common parameters
drivers/gpu/drm/mediatek/mtk_disp_aal.c | 86 ++++++++-
drivers/gpu/drm/mediatek/mtk_disp_drv.h | 3 +-
drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 203 ++++++++++++++++----
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 8 +-
drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 1 -
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 2 +
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 9 +
7 files changed, 260 insertions(+), 52 deletions(-)
--
2.42.0
Invert the check for state->gamma_lut and move it at the beginning
of the function to reduce indentation: this prepares the code for
keeping readability on later additions.
This commit brings no functional changes.
Reviewed-by: Jason-JH.Lin <[email protected]>
Reviewed-by: Alexandre Mergnat <[email protected]>
Reviewed-by: CK Hu <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 45 ++++++++++++-----------
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 68e2565b88a5..63840e25416b 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -64,6 +64,10 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
u32 word;
u32 diff[3] = {0};
+ /* If there's no gamma lut there's nothing to do here. */
+ if (!state->gamma_lut)
+ return;
+
/* If we're called from AAL, dev is NULL */
gamma = dev ? dev_get_drvdata(dev) : NULL;
@@ -72,29 +76,26 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
else
lut_diff = false;
- if (state->gamma_lut) {
- reg = readl(regs + DISP_GAMMA_CFG);
- reg = reg | GAMMA_LUT_EN;
- writel(reg, regs + DISP_GAMMA_CFG);
- lut_base = regs + DISP_GAMMA_LUT;
- lut = (struct drm_color_lut *)state->gamma_lut->data;
- for (i = 0; i < MTK_LUT_SIZE; i++) {
-
- if (!lut_diff || (i % 2 == 0)) {
- word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
- (((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
- ((lut[i].blue >> 6) & LUT_10BIT_MASK);
- } else {
- diff[0] = (lut[i].red >> 6) - (lut[i - 1].red >> 6);
- diff[1] = (lut[i].green >> 6) - (lut[i - 1].green >> 6);
- diff[2] = (lut[i].blue >> 6) - (lut[i - 1].blue >> 6);
-
- word = ((diff[0] & LUT_10BIT_MASK) << 20) +
- ((diff[1] & LUT_10BIT_MASK) << 10) +
- (diff[2] & LUT_10BIT_MASK);
- }
- writel(word, (lut_base + i * 4));
+ reg = readl(regs + DISP_GAMMA_CFG);
+ reg = reg | GAMMA_LUT_EN;
+ writel(reg, regs + DISP_GAMMA_CFG);
+ lut_base = regs + DISP_GAMMA_LUT;
+ lut = (struct drm_color_lut *)state->gamma_lut->data;
+ for (i = 0; i < MTK_LUT_SIZE; i++) {
+ if (!lut_diff || (i % 2 == 0)) {
+ word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
+ (((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
+ ((lut[i].blue >> 6) & LUT_10BIT_MASK);
+ } else {
+ diff[0] = (lut[i].red >> 6) - (lut[i - 1].red >> 6);
+ diff[1] = (lut[i].green >> 6) - (lut[i - 1].green >> 6);
+ diff[2] = (lut[i].blue >> 6) - (lut[i - 1].blue >> 6);
+
+ word = ((diff[0] & LUT_10BIT_MASK) << 20) +
+ ((diff[1] & LUT_10BIT_MASK) << 10) +
+ (diff[2] & LUT_10BIT_MASK);
}
+ writel(word, (lut_base + i * 4));
}
}
--
2.42.0
Newer SoCs support a bigger Gamma LUT table: wire up a callback
to retrieve the correct LUT size for each different Gamma IP.
Co-developed-by: Jason-JH.Lin <[email protected]>
Signed-off-by: Jason-JH.Lin <[email protected]>
[Angelo: Rewritten commit message/description + porting]
Reviewed-by: Jason-JH.Lin <[email protected]>
Reviewed-by: Alexandre Mergnat <[email protected]>
Reviewed-by: CK Hu <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_aal.c | 17 +++++++++++++++-
drivers/gpu/drm/mediatek/mtk_disp_drv.h | 2 ++
drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 22 ++++++++++++++++++---
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 8 ++++++--
drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 1 -
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 2 ++
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 9 +++++++++
7 files changed, 54 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index 662c5d03ee43..dc26ddce0c6e 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -19,7 +19,7 @@
#define AAL_EN BIT(0)
#define DISP_AAL_SIZE 0x0030
#define DISP_AAL_OUTPUT_SIZE 0x04d8
-
+#define DISP_AAL_LUT_SIZE 512
struct mtk_disp_aal_data {
bool has_gamma;
@@ -56,6 +56,21 @@ void mtk_aal_config(struct device *dev, unsigned int w,
mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, DISP_AAL_OUTPUT_SIZE);
}
+/**
+ * mtk_aal_gamma_get_lut_size() - Get gamma LUT size for AAL
+ * @dev: Pointer to struct device
+ *
+ * Return: 0 if gamma control not supported in AAL or gamma LUT size
+ */
+unsigned int mtk_aal_gamma_get_lut_size(struct device *dev)
+{
+ struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+
+ if (aal->data && aal->data->has_gamma)
+ return DISP_AAL_LUT_SIZE;
+ return 0;
+}
+
void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
{
struct mtk_disp_aal *aal = dev_get_drvdata(dev);
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
index 75045932353e..ca377265e5eb 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -17,6 +17,7 @@ void mtk_aal_clk_disable(struct device *dev);
void mtk_aal_config(struct device *dev, unsigned int w,
unsigned int h, unsigned int vrefresh,
unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
+unsigned int mtk_aal_gamma_get_lut_size(struct device *dev);
void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state);
void mtk_aal_start(struct device *dev);
void mtk_aal_stop(struct device *dev);
@@ -53,6 +54,7 @@ void mtk_gamma_clk_disable(struct device *dev);
void mtk_gamma_config(struct device *dev, unsigned int w,
unsigned int h, unsigned int vrefresh,
unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
+unsigned int mtk_gamma_get_lut_size(struct device *dev);
void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state);
void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crtc_state *state);
void mtk_gamma_start(struct device *dev);
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 63840e25416b..bb237523d4b7 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -28,6 +28,7 @@
struct mtk_disp_gamma_data {
bool has_dither;
bool lut_diff;
+ u16 lut_size;
};
/*
@@ -54,6 +55,15 @@ void mtk_gamma_clk_disable(struct device *dev)
clk_disable_unprepare(gamma->clk);
}
+unsigned int mtk_gamma_get_lut_size(struct device *dev)
+{
+ struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
+
+ if (gamma && gamma->data)
+ return gamma->data->lut_size;
+ return 0;
+}
+
void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crtc_state *state)
{
struct mtk_disp_gamma *gamma;
@@ -61,6 +71,7 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
struct drm_color_lut *lut;
void __iomem *lut_base;
bool lut_diff;
+ u16 lut_size;
u32 word;
u32 diff[3] = {0};
@@ -71,17 +82,20 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
/* If we're called from AAL, dev is NULL */
gamma = dev ? dev_get_drvdata(dev) : NULL;
- if (gamma && gamma->data)
+ if (gamma && gamma->data) {
lut_diff = gamma->data->lut_diff;
- else
+ lut_size = gamma->data->lut_size;
+ } else {
lut_diff = false;
+ lut_size = 512;
+ }
reg = readl(regs + DISP_GAMMA_CFG);
reg = reg | GAMMA_LUT_EN;
writel(reg, regs + DISP_GAMMA_CFG);
lut_base = regs + DISP_GAMMA_LUT;
lut = (struct drm_color_lut *)state->gamma_lut->data;
- for (i = 0; i < MTK_LUT_SIZE; i++) {
+ for (i = 0; i < lut_size; i++) {
if (!lut_diff || (i % 2 == 0)) {
word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
(((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
@@ -192,10 +206,12 @@ static void mtk_disp_gamma_remove(struct platform_device *pdev)
static const struct mtk_disp_gamma_data mt8173_gamma_driver_data = {
.has_dither = true,
+ .lut_size = 512,
};
static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
.lut_diff = true,
+ .lut_size = 512,
};
static const struct of_device_id mtk_disp_gamma_driver_dt_match[] = {
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 45d05b6b7071..18da16e5626b 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -943,8 +943,12 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
mtk_crtc->ddp_comp[i] = comp;
if (comp->funcs) {
- if (comp->funcs->gamma_set)
- gamma_lut_size = MTK_LUT_SIZE;
+ if (comp->funcs->gamma_set && comp->funcs->gamma_get_lut_size) {
+ unsigned int lut_sz = mtk_ddp_gamma_get_lut_size(comp);
+
+ if (lut_sz)
+ gamma_lut_size = lut_sz;
+ }
if (comp->funcs->ctm_set)
has_ctm = true;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
index 3e9046993d09..b2e50292e57d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
@@ -10,7 +10,6 @@
#include "mtk_drm_ddp_comp.h"
#include "mtk_drm_plane.h"
-#define MTK_LUT_SIZE 512
#define MTK_MAX_BPC 10
#define MTK_MIN_BPC 3
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 5d392ce96a14..51f802be8440 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -263,6 +263,7 @@ static void mtk_ufoe_start(struct device *dev)
static const struct mtk_ddp_comp_funcs ddp_aal = {
.clk_enable = mtk_aal_clk_enable,
.clk_disable = mtk_aal_clk_disable,
+ .gamma_get_lut_size = mtk_aal_gamma_get_lut_size,
.gamma_set = mtk_aal_gamma_set,
.config = mtk_aal_config,
.start = mtk_aal_start,
@@ -314,6 +315,7 @@ static const struct mtk_ddp_comp_funcs ddp_dsi = {
static const struct mtk_ddp_comp_funcs ddp_gamma = {
.clk_enable = mtk_gamma_clk_enable,
.clk_disable = mtk_gamma_clk_disable,
+ .gamma_get_lut_size = mtk_gamma_get_lut_size,
.gamma_set = mtk_gamma_set,
.config = mtk_gamma_config,
.start = mtk_gamma_start,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
index febcaeef16a1..c1355960e195 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
@@ -67,6 +67,7 @@ struct mtk_ddp_comp_funcs {
void (*layer_config)(struct device *dev, unsigned int idx,
struct mtk_plane_state *state,
struct cmdq_pkt *cmdq_pkt);
+ unsigned int (*gamma_get_lut_size)(struct device *dev);
void (*gamma_set)(struct device *dev,
struct drm_crtc_state *state);
void (*bgclr_in_on)(struct device *dev);
@@ -186,6 +187,14 @@ static inline void mtk_ddp_comp_layer_config(struct mtk_ddp_comp *comp,
comp->funcs->layer_config(comp->dev, idx, state, cmdq_pkt);
}
+static inline unsigned int mtk_ddp_gamma_get_lut_size(struct mtk_ddp_comp *comp)
+{
+ if (comp->funcs && comp->funcs->gamma_get_lut_size)
+ return comp->funcs->gamma_get_lut_size(comp->dev);
+
+ return 0;
+}
+
static inline void mtk_ddp_gamma_set(struct mtk_ddp_comp *comp,
struct drm_crtc_state *state)
{
--
2.42.0
Make the code more robust and improve readability by using bitfield
macros instead of open coding bit operations.
While at it, also add a definition for LUT_BITS_DEFAULT.
Reviewed-by: Jason-JH.Lin <[email protected]>
Reviewed-by: Alexandre Mergnat <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 28 +++++++++++++++--------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index d35eaf6dbc2d..81c04518a5eb 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -3,6 +3,7 @@
* Copyright (c) 2021 MediaTek Inc.
*/
+#include <linux/bitfield.h>
#include <linux/clk.h>
#include <linux/component.h>
#include <linux/module.h>
@@ -21,8 +22,14 @@
#define GAMMA_LUT_EN BIT(1)
#define GAMMA_DITHERING BIT(2)
#define DISP_GAMMA_SIZE 0x0030
+#define DISP_GAMMA_SIZE_HSIZE GENMASK(28, 16)
+#define DISP_GAMMA_SIZE_VSIZE GENMASK(12, 0)
#define DISP_GAMMA_LUT 0x0700
+#define DISP_GAMMA_LUT_10BIT_R GENMASK(29, 20)
+#define DISP_GAMMA_LUT_10BIT_G GENMASK(19, 10)
+#define DISP_GAMMA_LUT_10BIT_B GENMASK(9, 0)
+
struct mtk_disp_gamma_data {
bool has_dither;
bool lut_diff;
@@ -97,9 +104,9 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
hwlut.blue = drm_color_lut_extract(lut[i].blue, 10);
if (!lut_diff || (i % 2 == 0)) {
- word = hwlut.red << 20 +
- hwlut.green << 10 +
- hwlut.red;
+ word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, hwlut.red);
+ word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, hwlut.green);
+ word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, hwlut.blue);
} else {
diff.red = lut[i].red - lut[i - 1].red;
diff.red = drm_color_lut_extract(diff.red, 10);
@@ -110,9 +117,9 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
diff.blue = lut[i].blue - lut[i - 1].blue;
diff.blue = drm_color_lut_extract(diff.blue, 10);
- word = diff.blue << 20 +
- diff.green << 10 +
- diff.red;
+ word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, diff.red);
+ word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, diff.green);
+ word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, diff.blue);
}
writel(word, (lut_base + i * 4));
}
@@ -120,7 +127,7 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
cfg_val = readl(regs + DISP_GAMMA_CFG);
/* Enable the gamma table */
- cfg_val |= GAMMA_LUT_EN;
+ cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
writel(cfg_val, regs + DISP_GAMMA_CFG);
}
@@ -137,9 +144,12 @@ void mtk_gamma_config(struct device *dev, unsigned int w,
unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
{
struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
+ u32 sz;
+
+ sz = FIELD_PREP(DISP_GAMMA_SIZE_HSIZE, w);
+ sz |= FIELD_PREP(DISP_GAMMA_SIZE_VSIZE, h);
- mtk_ddp_write(cmdq_pkt, h << 16 | w, &gamma->cmdq_reg, gamma->regs,
- DISP_GAMMA_SIZE);
+ mtk_ddp_write(cmdq_pkt, sz, &gamma->cmdq_reg, gamma->regs, DISP_GAMMA_SIZE);
if (gamma->data && gamma->data->has_dither)
mtk_dither_set_common(gamma->regs, &gamma->cmdq_reg, bpc,
DISP_GAMMA_CFG, GAMMA_DITHERING, cmdq_pkt);
--
2.42.0
Make the code more robust and improve readability by using bitfield
macros instead of open coding bit operations.
Reviewed-by: CK Hu <[email protected]>
Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_aal.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index dc26ddce0c6e..05f9be23fa47 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -18,6 +18,8 @@
#define DISP_AAL_EN 0x0000
#define AAL_EN BIT(0)
#define DISP_AAL_SIZE 0x0030
+#define DISP_AAL_SIZE_HSIZE GENMASK(28, 16)
+#define DISP_AAL_SIZE_VSIZE GENMASK(12, 0)
#define DISP_AAL_OUTPUT_SIZE 0x04d8
#define DISP_AAL_LUT_SIZE 512
@@ -51,9 +53,13 @@ void mtk_aal_config(struct device *dev, unsigned int w,
unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
{
struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+ u32 sz;
- mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, DISP_AAL_SIZE);
- mtk_ddp_write(cmdq_pkt, w << 16 | h, &aal->cmdq_reg, aal->regs, DISP_AAL_OUTPUT_SIZE);
+ sz = FIELD_PREP(DISP_AAL_SIZE_HSIZE, w);
+ sz |= FIELD_PREP(DISP_AAL_SIZE_VSIZE, h);
+
+ mtk_ddp_write(cmdq_pkt, sz, &aal->cmdq_reg, aal->regs, DISP_AAL_SIZE);
+ mtk_ddp_write(cmdq_pkt, sz, &aal->cmdq_reg, aal->regs, DISP_AAL_OUTPUT_SIZE);
}
/**
--
2.42.0
Newer Gamma IP have got multiple LUT banks: support specifying the
size of the LUT banks and handle bank-switching before programming
the LUT in mtk_gamma_set_common() in preparation for adding support
for MT8195 and newer SoCs.
Suggested-by: Jason-JH.Lin <[email protected]>
[Angelo: Refactored original commit]
Reviewed-by: Alexandre Mergnat <[email protected]>
Reviewed-by: CK Hu <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 70 ++++++++++++++---------
1 file changed, 44 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 0929f8830d6d..911468984ad5 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -24,6 +24,8 @@
#define DISP_GAMMA_SIZE 0x0030
#define DISP_GAMMA_SIZE_HSIZE GENMASK(28, 16)
#define DISP_GAMMA_SIZE_VSIZE GENMASK(12, 0)
+#define DISP_GAMMA_BANK 0x0100
+#define DISP_GAMMA_BANK_BANK GENMASK(1, 0)
#define DISP_GAMMA_LUT 0x0700
#define DISP_GAMMA_LUT_10BIT_R GENMASK(29, 20)
@@ -33,6 +35,7 @@
struct mtk_disp_gamma_data {
bool has_dither;
bool lut_diff;
+ u16 lut_bank_size;
u16 lut_size;
};
@@ -75,40 +78,53 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
unsigned int i;
struct drm_color_lut *lut;
void __iomem *lut_base;
- u32 cfg_val, word;
+ u32 cfg_val, lbank_val, word;
+ int cur_bank, num_lut_banks;
/* If there's no gamma lut there's nothing to do here. */
if (!state->gamma_lut)
return;
+ num_lut_banks = gamma->data->lut_size / gamma->data->lut_bank_size;
lut_base = gamma->regs + DISP_GAMMA_LUT;
lut = (struct drm_color_lut *)state->gamma_lut->data;
- for (i = 0; i < gamma->data->lut_size; i++) {
- struct drm_color_lut diff, hwlut;
-
- hwlut.red = drm_color_lut_extract(lut[i].red, 10);
- hwlut.green = drm_color_lut_extract(lut[i].green, 10);
- hwlut.blue = drm_color_lut_extract(lut[i].blue, 10);
-
- if (!gamma->data->lut_diff || (i % 2 == 0)) {
- word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, hwlut.red);
- word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, hwlut.green);
- word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, hwlut.blue);
- } else {
- diff.red = lut[i].red - lut[i - 1].red;
- diff.red = drm_color_lut_extract(diff.red, 10);
-
- diff.green = lut[i].green - lut[i - 1].green;
- diff.green = drm_color_lut_extract(diff.green, 10);
-
- diff.blue = lut[i].blue - lut[i - 1].blue;
- diff.blue = drm_color_lut_extract(diff.blue, 10);
-
- word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, diff.red);
- word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, diff.green);
- word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, diff.blue);
+
+ for (cur_bank = 0; cur_bank < num_lut_banks; cur_bank++) {
+
+ /* Switch gamma bank and set data mode before writing LUT */
+ if (num_lut_banks > 1) {
+ lbank_val = FIELD_PREP(DISP_GAMMA_BANK_BANK, cur_bank);
+ writel(lbank_val, gamma->regs + DISP_GAMMA_BANK);
+ }
+
+ for (i = 0; i < gamma->data->lut_bank_size; i++) {
+ int n = cur_bank * gamma->data->lut_bank_size + i;
+ struct drm_color_lut diff, hwlut;
+
+ hwlut.red = drm_color_lut_extract(lut[n].red, 10);
+ hwlut.green = drm_color_lut_extract(lut[n].green, 10);
+ hwlut.blue = drm_color_lut_extract(lut[n].blue, 10);
+
+ if (!gamma->data->lut_diff || (i % 2 == 0)) {
+ word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, hwlut.red);
+ word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, hwlut.green);
+ word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, hwlut.blue);
+ } else {
+ diff.red = lut[n].red - lut[n - 1].red;
+ diff.red = drm_color_lut_extract(diff.red, 10);
+
+ diff.green = lut[n].green - lut[n - 1].green;
+ diff.green = drm_color_lut_extract(diff.green, 10);
+
+ diff.blue = lut[n].blue - lut[n - 1].blue;
+ diff.blue = drm_color_lut_extract(diff.blue, 10);
+
+ word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, diff.red);
+ word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, diff.green);
+ word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, diff.blue);
+ }
+ writel(word, lut_base + i * 4);
}
- writel(word, (lut_base + i * 4));
}
cfg_val = readl(gamma->regs + DISP_GAMMA_CFG);
@@ -208,10 +224,12 @@ static void mtk_disp_gamma_remove(struct platform_device *pdev)
static const struct mtk_disp_gamma_data mt8173_gamma_driver_data = {
.has_dither = true,
+ .lut_bank_size = 512,
.lut_size = 512,
};
static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
+ .lut_bank_size = 512,
.lut_diff = true,
.lut_size = 512,
};
--
2.42.0
Now that this driver supports 12-bit LUTs, we can add support for the
DISP_GAMMA found on the MT8195 SoC: add its driver data and compatible.
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 6305cd95e6d4..bcc33aeca885 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -279,11 +279,20 @@ static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
.lut_size = 512,
};
+static const struct mtk_disp_gamma_data mt8195_gamma_driver_data = {
+ .lut_bank_size = 256,
+ .lut_bits = 12,
+ .lut_diff = true,
+ .lut_size = 1024,
+};
+
static const struct of_device_id mtk_disp_gamma_driver_dt_match[] = {
{ .compatible = "mediatek,mt8173-disp-gamma",
.data = &mt8173_gamma_driver_data},
{ .compatible = "mediatek,mt8183-disp-gamma",
.data = &mt8183_gamma_driver_data},
+ { .compatible = "mediatek,mt8195-disp-gamma",
+ .data = &mt8195_gamma_driver_data},
{},
};
MODULE_DEVICE_TABLE(of, mtk_disp_gamma_driver_dt_match);
--
2.42.0
In preparation for adding a 12-bits gamma support for the DISP_GAMMA
IP, remove the mtk_gamma_set_common() function and move the relevant
bits in mtk_gamma_set() for DISP_GAMMA and mtk_aal_gamma_set() for
DISP_AAL: since the latter has no more support for gamma manipulation
(being moved to a different IP) in newer revisions, those functions
are about to diverge and it makes no sense to keep a common one (with
all the complications of passing common data and making exclusions
for device driver data) for just a few bits.
This commit brings no functional changes.
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_aal.c | 41 +++++++++++++++++++++--
drivers/gpu/drm/mediatek/mtk_disp_drv.h | 1 -
drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 34 ++++---------------
3 files changed, 46 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index 05f9be23fa47..a618be9b3dba 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -17,10 +17,17 @@
#define DISP_AAL_EN 0x0000
#define AAL_EN BIT(0)
+#define DISP_AAL_CFG 0x0020
+#define AAL_GAMMA_LUT_EN BIT(1)
#define DISP_AAL_SIZE 0x0030
#define DISP_AAL_SIZE_HSIZE GENMASK(28, 16)
#define DISP_AAL_SIZE_VSIZE GENMASK(12, 0)
#define DISP_AAL_OUTPUT_SIZE 0x04d8
+#define DISP_AAL_GAMMA_LUT 0x0700
+#define DISP_AAL_GAMMA_LUT_R GENMASK(29, 20)
+#define DISP_AAL_GAMMA_LUT_G GENMASK(19, 10)
+#define DISP_AAL_GAMMA_LUT_B GENMASK(9, 0)
+#define DISP_AAL_LUT_BITS 10
#define DISP_AAL_LUT_SIZE 512
struct mtk_disp_aal_data {
@@ -80,9 +87,39 @@ unsigned int mtk_aal_gamma_get_lut_size(struct device *dev)
void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
{
struct mtk_disp_aal *aal = dev_get_drvdata(dev);
+ struct drm_color_lut *lut;
+ unsigned int i;
+ u32 cfg_val;
+
+ /* If gamma is not supported in AAL, go out immediately */
+ if (!(aal->data && aal->data->has_gamma))
+ return;
+
+ /* Also, if there's no gamma lut there's nothing to do here. */
+ if (!state->gamma_lut)
+ return;
+
+ lut = (struct drm_color_lut *)state->gamma_lut->data;
+ for (i = 0; i < DISP_AAL_LUT_SIZE; i++) {
+ struct drm_color_lut hwlut = {
+ .red = drm_color_lut_extract(lut[i].red, DISP_AAL_LUT_BITS),
+ .green = drm_color_lut_extract(lut[i].green, DISP_AAL_LUT_BITS),
+ .blue = drm_color_lut_extract(lut[i].blue, DISP_AAL_LUT_BITS)
+ };
+ u32 word;
+
+ word = FIELD_PREP(DISP_AAL_GAMMA_LUT_R, hwlut.red);
+ word |= FIELD_PREP(DISP_AAL_GAMMA_LUT_G, hwlut.green);
+ word |= FIELD_PREP(DISP_AAL_GAMMA_LUT_B, hwlut.blue);
+ writel(word, aal->regs + DISP_AAL_GAMMA_LUT + i * 4);
+ }
- if (aal->data && aal->data->has_gamma)
- mtk_gamma_set_common(NULL, aal->regs, state);
+ cfg_val = readl(aal->regs + DISP_AAL_CFG);
+
+ /* Enable the gamma table */
+ cfg_val |= FIELD_PREP(AAL_GAMMA_LUT_EN, 1);
+
+ writel(cfg_val, aal->regs + DISP_AAL_CFG);
}
void mtk_aal_start(struct device *dev)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
index ca377265e5eb..54d3712e2afd 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -56,7 +56,6 @@ void mtk_gamma_config(struct device *dev, unsigned int w,
unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
unsigned int mtk_gamma_get_lut_size(struct device *dev);
void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state);
-void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crtc_state *state);
void mtk_gamma_start(struct device *dev);
void mtk_gamma_stop(struct device *dev);
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 81c04518a5eb..0929f8830d6d 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -69,41 +69,28 @@ unsigned int mtk_gamma_get_lut_size(struct device *dev)
return 0;
}
-void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crtc_state *state)
+void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
{
- struct mtk_disp_gamma *gamma;
+ struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
unsigned int i;
struct drm_color_lut *lut;
void __iomem *lut_base;
- bool lut_diff;
- u16 lut_size;
u32 cfg_val, word;
/* If there's no gamma lut there's nothing to do here. */
if (!state->gamma_lut)
return;
- /* If we're called from AAL, dev is NULL */
- gamma = dev ? dev_get_drvdata(dev) : NULL;
-
- if (gamma && gamma->data) {
- lut_diff = gamma->data->lut_diff;
- lut_size = gamma->data->lut_size;
- } else {
- lut_diff = false;
- lut_size = 512;
- }
-
- lut_base = regs + DISP_GAMMA_LUT;
+ lut_base = gamma->regs + DISP_GAMMA_LUT;
lut = (struct drm_color_lut *)state->gamma_lut->data;
- for (i = 0; i < lut_size; i++) {
+ for (i = 0; i < gamma->data->lut_size; i++) {
struct drm_color_lut diff, hwlut;
hwlut.red = drm_color_lut_extract(lut[i].red, 10);
hwlut.green = drm_color_lut_extract(lut[i].green, 10);
hwlut.blue = drm_color_lut_extract(lut[i].blue, 10);
- if (!lut_diff || (i % 2 == 0)) {
+ if (!gamma->data->lut_diff || (i % 2 == 0)) {
word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, hwlut.red);
word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, hwlut.green);
word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, hwlut.blue);
@@ -124,19 +111,12 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
writel(word, (lut_base + i * 4));
}
- cfg_val = readl(regs + DISP_GAMMA_CFG);
+ cfg_val = readl(gamma->regs + DISP_GAMMA_CFG);
/* Enable the gamma table */
cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
- writel(cfg_val, regs + DISP_GAMMA_CFG);
-}
-
-void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
-{
- struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
-
- mtk_gamma_set_common(dev, gamma->regs, state);
+ writel(cfg_val, gamma->regs + DISP_GAMMA_CFG);
}
void mtk_gamma_config(struct device *dev, unsigned int w,
--
2.42.0
Disable relay mode at the end of LUT programming to make sure that the
processed image goes through in both DISP_GAMMA and DISP_AAL for gamma
setting.
Reviewed-by: Jason-JH.Lin <[email protected]>
Reviewed-by: Alexandre Mergnat <[email protected]>
Reviewed-by: CK Hu <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_aal.c | 4 ++++
drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index a618be9b3dba..15f91cea9f20 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -18,6 +18,7 @@
#define DISP_AAL_EN 0x0000
#define AAL_EN BIT(0)
#define DISP_AAL_CFG 0x0020
+#define AAL_RELAY_MODE BIT(0)
#define AAL_GAMMA_LUT_EN BIT(1)
#define DISP_AAL_SIZE 0x0030
#define DISP_AAL_SIZE_HSIZE GENMASK(28, 16)
@@ -119,6 +120,9 @@ void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state *state)
/* Enable the gamma table */
cfg_val |= FIELD_PREP(AAL_GAMMA_LUT_EN, 1);
+ /* Disable RELAY mode to pass the processed image */
+ cfg_val &= ~AAL_RELAY_MODE;
+
writel(cfg_val, aal->regs + DISP_AAL_CFG);
}
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index bcc33aeca885..6746033615db 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -19,6 +19,7 @@
#define DISP_GAMMA_EN 0x0000
#define GAMMA_EN BIT(0)
#define DISP_GAMMA_CFG 0x0020
+#define GAMMA_RELAY_MODE BIT(0)
#define GAMMA_LUT_EN BIT(1)
#define GAMMA_DITHERING BIT(2)
#define DISP_GAMMA_SIZE 0x0030
@@ -175,6 +176,9 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
/* Enable the gamma table */
cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
+ /* Disable RELAY mode to pass the processed image */
+ cfg_val &= ~GAMMA_RELAY_MODE;
+
writel(cfg_val, gamma->regs + DISP_GAMMA_CFG);
}
--
2.42.0
New SoCs, like MT8195, not only may support bigger lookup tables, but
have got a different register layout to support bigger precision:
support specifying the number of `lut_bits` for each SoC and use it
in mtk_gamma_set_common() to perform the right calculations and add
support for 12-bit gamma lookup tables.
While at it, also reorder the variables in mtk_gamma_set_common()
and rename `lut_base` to `lut0_base` to improve readability.
Reviewed-by: Jason-JH.Lin <[email protected]>
Reviewed-by: Alexandre Mergnat <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 83 +++++++++++++++++------
1 file changed, 64 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 911468984ad5..6305cd95e6d4 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -26,17 +26,26 @@
#define DISP_GAMMA_SIZE_VSIZE GENMASK(12, 0)
#define DISP_GAMMA_BANK 0x0100
#define DISP_GAMMA_BANK_BANK GENMASK(1, 0)
+#define DISP_GAMMA_BANK_DATA_MODE BIT(2)
#define DISP_GAMMA_LUT 0x0700
+#define DISP_GAMMA_LUT1 0x0b00
+/* For 10 bit LUT layout, R/G/B are in the same register */
#define DISP_GAMMA_LUT_10BIT_R GENMASK(29, 20)
#define DISP_GAMMA_LUT_10BIT_G GENMASK(19, 10)
#define DISP_GAMMA_LUT_10BIT_B GENMASK(9, 0)
+/* For 12 bit LUT layout, R/G are in LUT, B is in LUT1 */
+#define DISP_GAMMA_LUT_12BIT_R GENMASK(11, 0)
+#define DISP_GAMMA_LUT_12BIT_G GENMASK(23, 12)
+#define DISP_GAMMA_LUT_12BIT_B GENMASK(11, 0)
+
struct mtk_disp_gamma_data {
bool has_dither;
bool lut_diff;
u16 lut_bank_size;
u16 lut_size;
+ u8 lut_bits;
};
/*
@@ -72,28 +81,48 @@ unsigned int mtk_gamma_get_lut_size(struct device *dev)
return 0;
}
+/*
+ * SoCs supporting 12-bits LUTs are using a new register layout that does
+ * always support (by HW) both 12-bits and 10-bits LUT but, on those, we
+ * ignore the support for 10-bits in this driver and always use 12-bits.
+ *
+ * Summarizing:
+ * - SoC HW support 9/10-bits LUT only
+ * - Old register layout
+ * - 10-bits LUT supported
+ * - 9-bits LUT not supported
+ * - SoC HW support both 10/12bits LUT
+ * - New register layout
+ * - 12-bits LUT supported
+ * - 10-its LUT not supported
+ */
void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
{
struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
- unsigned int i;
- struct drm_color_lut *lut;
- void __iomem *lut_base;
- u32 cfg_val, lbank_val, word;
+ void __iomem *lut0_base = gamma->regs + DISP_GAMMA_LUT;
+ void __iomem *lut1_base = gamma->regs + DISP_GAMMA_LUT1;
+ u32 cfg_val, data_mode, lbank_val, word[2];
+ u8 lut_bits = gamma->data->lut_bits;
int cur_bank, num_lut_banks;
+ struct drm_color_lut *lut;
+ unsigned int i;
/* If there's no gamma lut there's nothing to do here. */
if (!state->gamma_lut)
return;
num_lut_banks = gamma->data->lut_size / gamma->data->lut_bank_size;
- lut_base = gamma->regs + DISP_GAMMA_LUT;
lut = (struct drm_color_lut *)state->gamma_lut->data;
+ /* Switch to 12 bits data mode if supported */
+ data_mode = FIELD_PREP(DISP_GAMMA_BANK_DATA_MODE, !!(lut_bits == 12));
+
for (cur_bank = 0; cur_bank < num_lut_banks; cur_bank++) {
/* Switch gamma bank and set data mode before writing LUT */
if (num_lut_banks > 1) {
lbank_val = FIELD_PREP(DISP_GAMMA_BANK_BANK, cur_bank);
+ lbank_val |= data_mode;
writel(lbank_val, gamma->regs + DISP_GAMMA_BANK);
}
@@ -101,29 +130,43 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
int n = cur_bank * gamma->data->lut_bank_size + i;
struct drm_color_lut diff, hwlut;
- hwlut.red = drm_color_lut_extract(lut[n].red, 10);
- hwlut.green = drm_color_lut_extract(lut[n].green, 10);
- hwlut.blue = drm_color_lut_extract(lut[n].blue, 10);
+ hwlut.red = drm_color_lut_extract(lut[n].red, lut_bits);
+ hwlut.green = drm_color_lut_extract(lut[n].green, lut_bits);
+ hwlut.blue = drm_color_lut_extract(lut[n].blue, lut_bits);
if (!gamma->data->lut_diff || (i % 2 == 0)) {
- word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, hwlut.red);
- word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, hwlut.green);
- word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, hwlut.blue);
+ if (lut_bits == 12) {
+ word[0] = FIELD_PREP(DISP_GAMMA_LUT_12BIT_R, hwlut.red);
+ word[0] |= FIELD_PREP(DISP_GAMMA_LUT_12BIT_G, hwlut.green);
+ word[1] = FIELD_PREP(DISP_GAMMA_LUT_12BIT_B, hwlut.blue);
+ } else {
+ word[0] = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, hwlut.red);
+ word[0] |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, hwlut.green);
+ word[0] |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, hwlut.blue);
+ }
} else {
diff.red = lut[n].red - lut[n - 1].red;
- diff.red = drm_color_lut_extract(diff.red, 10);
+ diff.red = drm_color_lut_extract(diff.red, lut_bits);
diff.green = lut[n].green - lut[n - 1].green;
- diff.green = drm_color_lut_extract(diff.green, 10);
+ diff.green = drm_color_lut_extract(diff.green, lut_bits);
diff.blue = lut[n].blue - lut[n - 1].blue;
- diff.blue = drm_color_lut_extract(diff.blue, 10);
-
- word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, diff.red);
- word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, diff.green);
- word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, diff.blue);
+ diff.blue = drm_color_lut_extract(diff.blue, lut_bits);
+
+ if (lut_bits == 12) {
+ word[0] = FIELD_PREP(DISP_GAMMA_LUT_12BIT_R, diff.red);
+ word[0] |= FIELD_PREP(DISP_GAMMA_LUT_12BIT_G, diff.green);
+ word[1] = FIELD_PREP(DISP_GAMMA_LUT_12BIT_B, diff.blue);
+ } else {
+ word[0] = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, diff.red);
+ word[0] |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, diff.green);
+ word[0] |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, diff.blue);
+ }
}
- writel(word, lut_base + i * 4);
+ writel(word[0], lut0_base + i * 4);
+ if (lut_bits == 12)
+ writel(word[1], lut1_base + i * 4);
}
}
@@ -225,11 +268,13 @@ static void mtk_disp_gamma_remove(struct platform_device *pdev)
static const struct mtk_disp_gamma_data mt8173_gamma_driver_data = {
.has_dither = true,
.lut_bank_size = 512,
+ .lut_bits = 10,
.lut_size = 512,
};
static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
.lut_bank_size = 512,
+ .lut_bits = 10,
.lut_diff = true,
.lut_size = 512,
};
--
2.42.0
All of the SoCs that don't have dithering control in the gamma IP
have got a GAMMA_LUT_TYPE bit that tells to the IP if the LUT is
"descending" (bit set) or "rising" (bit cleared): make sure to set
it correctly after programming the LUT.
Reviewed-by: Jason-JH.Lin <[email protected]>
Reviewed-by: Alexandre Mergnat <[email protected]>
Reviewed-by: CK Hu <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 6746033615db..0f116c0e51b5 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -22,6 +22,7 @@
#define GAMMA_RELAY_MODE BIT(0)
#define GAMMA_LUT_EN BIT(1)
#define GAMMA_DITHERING BIT(2)
+#define GAMMA_LUT_TYPE BIT(2)
#define DISP_GAMMA_SIZE 0x0030
#define DISP_GAMMA_SIZE_HSIZE GENMASK(28, 16)
#define DISP_GAMMA_SIZE_VSIZE GENMASK(12, 0)
@@ -82,6 +83,17 @@ unsigned int mtk_gamma_get_lut_size(struct device *dev)
return 0;
}
+static bool mtk_gamma_lut_is_descending(struct drm_color_lut *lut, u32 lut_size)
+{
+ u64 first, last;
+ int last_entry = lut_size - 1;
+
+ first = lut[0].red + lut[0].green + lut[0].blue;
+ last = lut[last_entry].red + lut[last_entry].green + lut[last_entry].blue;
+
+ return !!(first > last);
+}
+
/*
* SoCs supporting 12-bits LUTs are using a new register layout that does
* always support (by HW) both 12-bits and 10-bits LUT but, on those, we
@@ -173,6 +185,14 @@ void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
cfg_val = readl(gamma->regs + DISP_GAMMA_CFG);
+ if (!gamma->data->has_dither) {
+ /* Descending or Rising LUT */
+ if (mtk_gamma_lut_is_descending(lut, gamma->data->lut_size - 1))
+ cfg_val |= FIELD_PREP(GAMMA_LUT_TYPE, 1);
+ else
+ cfg_val &= ~GAMMA_LUT_TYPE;
+ }
+
/* Enable the gamma table */
cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
--
2.42.0
Move the write to DISP_GAMMA_CFG to enable the Gamma LUT to after
programming the actual table to avoid potential visual glitches during
table modification.
Note:
GAMMA should get enabled in between vblanks, but this requires many
efforts in order to make this happen, as that requires migrating all
of the writes to make use of CMDQ instead of cpu writes and that's
not trivial. For this reason, this patch only moves the LUT enable.
The CMDQ rework will come at a later time.
Reviewed-by: Jason-JH.Lin <[email protected]>
Reviewed-by: Alexandre Mergnat <[email protected]>
Reviewed-by: CK Hu <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 8506b9a0a811..d35eaf6dbc2d 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -65,12 +65,12 @@ unsigned int mtk_gamma_get_lut_size(struct device *dev)
void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crtc_state *state)
{
struct mtk_disp_gamma *gamma;
- unsigned int i, reg;
+ unsigned int i;
struct drm_color_lut *lut;
void __iomem *lut_base;
bool lut_diff;
u16 lut_size;
- u32 word;
+ u32 cfg_val, word;
/* If there's no gamma lut there's nothing to do here. */
if (!state->gamma_lut)
@@ -87,9 +87,6 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
lut_size = 512;
}
- reg = readl(regs + DISP_GAMMA_CFG);
- reg = reg | GAMMA_LUT_EN;
- writel(reg, regs + DISP_GAMMA_CFG);
lut_base = regs + DISP_GAMMA_LUT;
lut = (struct drm_color_lut *)state->gamma_lut->data;
for (i = 0; i < lut_size; i++) {
@@ -119,6 +116,13 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
}
writel(word, (lut_base + i * 4));
}
+
+ cfg_val = readl(regs + DISP_GAMMA_CFG);
+
+ /* Enable the gamma table */
+ cfg_val |= GAMMA_LUT_EN;
+
+ writel(cfg_val, regs + DISP_GAMMA_CFG);
}
void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
--
2.42.0
Compress the entry for mediatek,mt8173-disp-aal, as it fits in one
line, and fix the style; while at it, also add the usual sentinel
comment to the last entry.
This commit brings no functional changes.
Reviewed-by: CK Hu <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_aal.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index 7b3e1c275056..677e7d378e7a 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -209,10 +209,9 @@ static const struct mtk_disp_aal_data mt8173_aal_driver_data = {
};
static const struct of_device_id mtk_disp_aal_driver_dt_match[] = {
- { .compatible = "mediatek,mt8173-disp-aal",
- .data = &mt8173_aal_driver_data},
- { .compatible = "mediatek,mt8183-disp-aal"},
- {},
+ { .compatible = "mediatek,mt8173-disp-aal", .data = &mt8173_aal_driver_data },
+ { .compatible = "mediatek,mt8183-disp-aal" },
+ { /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, mtk_disp_aal_driver_dt_match);
--
2.42.0
The kerneldoc for struct mtk_disp_aal is missing: write one and
document this structure.
Reviewed-by: CK Hu <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_aal.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
index 15f91cea9f20..7b3e1c275056 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
@@ -35,6 +35,13 @@ struct mtk_disp_aal_data {
bool has_gamma;
};
+ /**
+ * struct mtk_disp_aal - Display Adaptive Ambient Light driver structure
+ * @clk: clock for DISP_AAL controller
+ * @regs: MMIO registers base
+ * @cmdq_reg: CMDQ Client register
+ * @data: platform specific data for DISP_AAL
+ */
struct mtk_disp_aal {
struct clk *clk;
void __iomem *regs;
--
2.42.0
The mtk_disp_gamma structure was completely undocumented: add some
kerneldoc documentation to it.
Reviewed-by: CK Hu <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index 0f116c0e51b5..52c752bc5f41 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -50,8 +50,12 @@ struct mtk_disp_gamma_data {
u8 lut_bits;
};
-/*
- * struct mtk_disp_gamma - DISP_GAMMA driver structure
+/**
+ * struct mtk_disp_gamma - Display Gamma driver structure
+ * @clk: clock for DISP_GAMMA block
+ * @regs: MMIO registers base
+ * @cmdq_reg: CMDQ Client register
+ * @data: platform data for DISP_GAMMA
*/
struct mtk_disp_gamma {
struct clk *clk;
--
2.42.0
Use drm_color_lut_extract() to avoid open-coding the bits reduction
calculations for each color channel and use a struct drm_color_lut
to temporarily store the information instead of an array of u32.
Also, slightly improve the precision of the HW LUT calculation in the
LUT DIFF case by performing the subtractions on the 16-bits values and
doing the 10 bits conversion later.
Reviewed-by: Jason-JH.Lin <[email protected]>
Reviewed-by: Alexandre Mergnat <[email protected]>
Reviewed-by: CK Hu <[email protected]>
Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 32 ++++++++++++++---------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
index bb237523d4b7..8506b9a0a811 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
@@ -23,8 +23,6 @@
#define DISP_GAMMA_SIZE 0x0030
#define DISP_GAMMA_LUT 0x0700
-#define LUT_10BIT_MASK 0x03ff
-
struct mtk_disp_gamma_data {
bool has_dither;
bool lut_diff;
@@ -73,7 +71,6 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
bool lut_diff;
u16 lut_size;
u32 word;
- u32 diff[3] = {0};
/* If there's no gamma lut there's nothing to do here. */
if (!state->gamma_lut)
@@ -96,18 +93,29 @@ void mtk_gamma_set_common(struct device *dev, void __iomem *regs, struct drm_crt
lut_base = regs + DISP_GAMMA_LUT;
lut = (struct drm_color_lut *)state->gamma_lut->data;
for (i = 0; i < lut_size; i++) {
+ struct drm_color_lut diff, hwlut;
+
+ hwlut.red = drm_color_lut_extract(lut[i].red, 10);
+ hwlut.green = drm_color_lut_extract(lut[i].green, 10);
+ hwlut.blue = drm_color_lut_extract(lut[i].blue, 10);
+
if (!lut_diff || (i % 2 == 0)) {
- word = (((lut[i].red >> 6) & LUT_10BIT_MASK) << 20) +
- (((lut[i].green >> 6) & LUT_10BIT_MASK) << 10) +
- ((lut[i].blue >> 6) & LUT_10BIT_MASK);
+ word = hwlut.red << 20 +
+ hwlut.green << 10 +
+ hwlut.red;
} else {
- diff[0] = (lut[i].red >> 6) - (lut[i - 1].red >> 6);
- diff[1] = (lut[i].green >> 6) - (lut[i - 1].green >> 6);
- diff[2] = (lut[i].blue >> 6) - (lut[i - 1].blue >> 6);
+ diff.red = lut[i].red - lut[i - 1].red;
+ diff.red = drm_color_lut_extract(diff.red, 10);
+
+ diff.green = lut[i].green - lut[i - 1].green;
+ diff.green = drm_color_lut_extract(diff.green, 10);
+
+ diff.blue = lut[i].blue - lut[i - 1].blue;
+ diff.blue = drm_color_lut_extract(diff.blue, 10);
- word = ((diff[0] & LUT_10BIT_MASK) << 20) +
- ((diff[1] & LUT_10BIT_MASK) << 10) +
- (diff[2] & LUT_10BIT_MASK);
+ word = diff.blue << 20 +
+ diff.green << 10 +
+ diff.red;
}
writel(word, (lut_base + i * 4));
}
--
2.42.0
On Thu, Oct 12, 2023 at 11:57:28AM +0200, AngeloGioacchino Del Regno wrote:
> In preparation for adding a 12-bits gamma support for the DISP_GAMMA
> IP, remove the mtk_gamma_set_common() function and move the relevant
> bits in mtk_gamma_set() for DISP_GAMMA and mtk_aal_gamma_set() for
> DISP_AAL: since the latter has no more support for gamma manipulation
> (being moved to a different IP) in newer revisions, those functions
> are about to diverge and it makes no sense to keep a common one (with
> all the complications of passing common data and making exclusions
> for device driver data) for just a few bits.
>
> This commit brings no functional changes.
>
> Signed-off-by: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: N?colas F. R. A. Prado <[email protected]>
Thanks,
N?colas
Hi, Angelo:
On Thu, 2023-10-12 at 11:57 +0200, AngeloGioacchino Del Regno wrote:
> Make the code more robust and improve readability by using bitfield
> macros instead of open coding bit operations.
> While at it, also add a definition for LUT_BITS_DEFAULT.
When I apply, I would remove the description of LUT_BITS_DEFAULT.
Reviewed-by: CK Hu <[email protected]>
>
> Reviewed-by: Jason-JH.Lin <[email protected]>
> Reviewed-by: Alexandre Mergnat <[email protected]>
> Signed-off-by: AngeloGioacchino Del Regno <
> [email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 28 +++++++++++++++----
> ----
> 1 file changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index d35eaf6dbc2d..81c04518a5eb 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -3,6 +3,7 @@
> * Copyright (c) 2021 MediaTek Inc.
> */
>
> +#include <linux/bitfield.h>
> #include <linux/clk.h>
> #include <linux/component.h>
> #include <linux/module.h>
> @@ -21,8 +22,14 @@
> #define GAMMA_LUT_EN BIT(1)
> #define GAMMA_DITHERING BIT(2)
> #define DISP_GAMMA_SIZE 0x0030
> +#define DISP_GAMMA_SIZE_HSIZE GENMASK
> (28, 16)
> +#define DISP_GAMMA_SIZE_VSIZE GENMASK
> (12, 0)
> #define DISP_GAMMA_LUT 0x0700
>
> +#define DISP_GAMMA_LUT_10BIT_R GENMASK(29, 20)
> +#define DISP_GAMMA_LUT_10BIT_G GENMASK(19, 10)
> +#define DISP_GAMMA_LUT_10BIT_B GENMASK(9, 0)
> +
> struct mtk_disp_gamma_data {
> bool has_dither;
> bool lut_diff;
> @@ -97,9 +104,9 @@ void mtk_gamma_set_common(struct device *dev, void
> __iomem *regs, struct drm_crt
> hwlut.blue = drm_color_lut_extract(lut[i].blue, 10);
>
> if (!lut_diff || (i % 2 == 0)) {
> - word = hwlut.red << 20 +
> - hwlut.green << 10 +
> - hwlut.red;
> + word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R,
> hwlut.red);
> + word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G,
> hwlut.green);
> + word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_B,
> hwlut.blue);
> } else {
> diff.red = lut[i].red - lut[i - 1].red;
> diff.red = drm_color_lut_extract(diff.red, 10);
> @@ -110,9 +117,9 @@ void mtk_gamma_set_common(struct device *dev,
> void __iomem *regs, struct drm_crt
> diff.blue = lut[i].blue - lut[i - 1].blue;
> diff.blue = drm_color_lut_extract(diff.blue,
> 10);
>
> - word = diff.blue << 20 +
> - diff.green << 10 +
> - diff.red;
> + word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R,
> diff.red);
> + word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G,
> diff.green);
> + word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_B,
> diff.blue);
> }
> writel(word, (lut_base + i * 4));
> }
> @@ -120,7 +127,7 @@ void mtk_gamma_set_common(struct device *dev,
> void __iomem *regs, struct drm_crt
> cfg_val = readl(regs + DISP_GAMMA_CFG);
>
> /* Enable the gamma table */
> - cfg_val |= GAMMA_LUT_EN;
> + cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
>
> writel(cfg_val, regs + DISP_GAMMA_CFG);
> }
> @@ -137,9 +144,12 @@ void mtk_gamma_config(struct device *dev,
> unsigned int w,
> unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
> {
> struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> + u32 sz;
> +
> + sz = FIELD_PREP(DISP_GAMMA_SIZE_HSIZE, w);
> + sz |= FIELD_PREP(DISP_GAMMA_SIZE_VSIZE, h);
>
> - mtk_ddp_write(cmdq_pkt, h << 16 | w, &gamma->cmdq_reg, gamma-
> >regs,
> - DISP_GAMMA_SIZE);
> + mtk_ddp_write(cmdq_pkt, sz, &gamma->cmdq_reg, gamma->regs,
> DISP_GAMMA_SIZE);
> if (gamma->data && gamma->data->has_dither)
> mtk_dither_set_common(gamma->regs, &gamma->cmdq_reg,
> bpc,
> DISP_GAMMA_CFG, GAMMA_DITHERING,
> cmdq_pkt);
Hi, Angelo:
On Thu, 2023-10-12 at 11:57 +0200, AngeloGioacchino Del Regno wrote:
> In preparation for adding a 12-bits gamma support for the DISP_GAMMA
> IP, remove the mtk_gamma_set_common() function and move the relevant
> bits in mtk_gamma_set() for DISP_GAMMA and mtk_aal_gamma_set() for
> DISP_AAL: since the latter has no more support for gamma manipulation
> (being moved to a different IP) in newer revisions, those functions
> are about to diverge and it makes no sense to keep a common one (with
> all the complications of passing common data and making exclusions
> for device driver data) for just a few bits.
>
> This commit brings no functional changes.
Reviewed-by: CK Hu <[email protected]>
>
> Signed-off-by: AngeloGioacchino Del Regno <
> [email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_aal.c | 41
> +++++++++++++++++++++--
> drivers/gpu/drm/mediatek/mtk_disp_drv.h | 1 -
> drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 34 ++++---------------
> 3 files changed, 46 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index 05f9be23fa47..a618be9b3dba 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -17,10 +17,17 @@
>
> #define DISP_AAL_EN 0x0000
> #define AAL_EN BIT(0)
> +#define DISP_AAL_CFG 0x0020
> +#define AAL_GAMMA_LUT_EN BIT(1)
> #define DISP_AAL_SIZE 0x0030
> #define DISP_AAL_SIZE_HSIZE GENMASK(28, 16)
> #define DISP_AAL_SIZE_VSIZE GENMASK(12, 0)
> #define DISP_AAL_OUTPUT_SIZE 0x04d8
> +#define DISP_AAL_GAMMA_LUT 0x0700
> +#define DISP_AAL_GAMMA_LUT_R GENMASK(29, 20)
> +#define DISP_AAL_GAMMA_LUT_G GENMASK(19, 10)
> +#define DISP_AAL_GAMMA_LUT_B GENMASK(9, 0)
> +#define DISP_AAL_LUT_BITS 10
> #define DISP_AAL_LUT_SIZE 512
>
> struct mtk_disp_aal_data {
> @@ -80,9 +87,39 @@ unsigned int mtk_aal_gamma_get_lut_size(struct
> device *dev)
> void mtk_aal_gamma_set(struct device *dev, struct drm_crtc_state
> *state)
> {
> struct mtk_disp_aal *aal = dev_get_drvdata(dev);
> + struct drm_color_lut *lut;
> + unsigned int i;
> + u32 cfg_val;
> +
> + /* If gamma is not supported in AAL, go out immediately */
> + if (!(aal->data && aal->data->has_gamma))
> + return;
> +
> + /* Also, if there's no gamma lut there's nothing to do here. */
> + if (!state->gamma_lut)
> + return;
> +
> + lut = (struct drm_color_lut *)state->gamma_lut->data;
> + for (i = 0; i < DISP_AAL_LUT_SIZE; i++) {
> + struct drm_color_lut hwlut = {
> + .red = drm_color_lut_extract(lut[i].red,
> DISP_AAL_LUT_BITS),
> + .green = drm_color_lut_extract(lut[i].green,
> DISP_AAL_LUT_BITS),
> + .blue = drm_color_lut_extract(lut[i].blue,
> DISP_AAL_LUT_BITS)
> + };
> + u32 word;
> +
> + word = FIELD_PREP(DISP_AAL_GAMMA_LUT_R, hwlut.red);
> + word |= FIELD_PREP(DISP_AAL_GAMMA_LUT_G, hwlut.green);
> + word |= FIELD_PREP(DISP_AAL_GAMMA_LUT_B, hwlut.blue);
> + writel(word, aal->regs + DISP_AAL_GAMMA_LUT + i * 4);
> + }
>
> - if (aal->data && aal->data->has_gamma)
> - mtk_gamma_set_common(NULL, aal->regs, state);
> + cfg_val = readl(aal->regs + DISP_AAL_CFG);
> +
> + /* Enable the gamma table */
> + cfg_val |= FIELD_PREP(AAL_GAMMA_LUT_EN, 1);
> +
> + writel(cfg_val, aal->regs + DISP_AAL_CFG);
> }
>
> void mtk_aal_start(struct device *dev)
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> index ca377265e5eb..54d3712e2afd 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> @@ -56,7 +56,6 @@ void mtk_gamma_config(struct device *dev, unsigned
> int w,
> unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> unsigned int mtk_gamma_get_lut_size(struct device *dev);
> void mtk_gamma_set(struct device *dev, struct drm_crtc_state
> *state);
> -void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
> struct drm_crtc_state *state);
> void mtk_gamma_start(struct device *dev);
> void mtk_gamma_stop(struct device *dev);
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 81c04518a5eb..0929f8830d6d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -69,41 +69,28 @@ unsigned int mtk_gamma_get_lut_size(struct device
> *dev)
> return 0;
> }
>
> -void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
> struct drm_crtc_state *state)
> +void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
> {
> - struct mtk_disp_gamma *gamma;
> + struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> unsigned int i;
> struct drm_color_lut *lut;
> void __iomem *lut_base;
> - bool lut_diff;
> - u16 lut_size;
> u32 cfg_val, word;
>
> /* If there's no gamma lut there's nothing to do here. */
> if (!state->gamma_lut)
> return;
>
> - /* If we're called from AAL, dev is NULL */
> - gamma = dev ? dev_get_drvdata(dev) : NULL;
> -
> - if (gamma && gamma->data) {
> - lut_diff = gamma->data->lut_diff;
> - lut_size = gamma->data->lut_size;
> - } else {
> - lut_diff = false;
> - lut_size = 512;
> - }
> -
> - lut_base = regs + DISP_GAMMA_LUT;
> + lut_base = gamma->regs + DISP_GAMMA_LUT;
> lut = (struct drm_color_lut *)state->gamma_lut->data;
> - for (i = 0; i < lut_size; i++) {
> + for (i = 0; i < gamma->data->lut_size; i++) {
> struct drm_color_lut diff, hwlut;
>
> hwlut.red = drm_color_lut_extract(lut[i].red, 10);
> hwlut.green = drm_color_lut_extract(lut[i].green, 10);
> hwlut.blue = drm_color_lut_extract(lut[i].blue, 10);
>
> - if (!lut_diff || (i % 2 == 0)) {
> + if (!gamma->data->lut_diff || (i % 2 == 0)) {
> word = FIELD_PREP(DISP_GAMMA_LUT_10BIT_R,
> hwlut.red);
> word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_G,
> hwlut.green);
> word |= FIELD_PREP(DISP_GAMMA_LUT_10BIT_B,
> hwlut.blue);
> @@ -124,19 +111,12 @@ void mtk_gamma_set_common(struct device *dev,
> void __iomem *regs, struct drm_crt
> writel(word, (lut_base + i * 4));
> }
>
> - cfg_val = readl(regs + DISP_GAMMA_CFG);
> + cfg_val = readl(gamma->regs + DISP_GAMMA_CFG);
>
> /* Enable the gamma table */
> cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
>
> - writel(cfg_val, regs + DISP_GAMMA_CFG);
> -}
> -
> -void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
> -{
> - struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> -
> - mtk_gamma_set_common(dev, gamma->regs, state);
> + writel(cfg_val, gamma->regs + DISP_GAMMA_CFG);
> }
>
> void mtk_gamma_config(struct device *dev, unsigned int w,
Hi, Angelo:
On Thu, 2023-10-12 at 11:57 +0200, AngeloGioacchino Del Regno wrote:
> New SoCs, like MT8195, not only may support bigger lookup tables, but
> have got a different register layout to support bigger precision:
> support specifying the number of `lut_bits` for each SoC and use it
> in mtk_gamma_set_common() to perform the right calculations and add
> support for 12-bit gamma lookup tables.
>
> While at it, also reorder the variables in mtk_gamma_set_common()
> and rename `lut_base` to `lut0_base` to improve readability.
Reviewed-by: CK Hu <[email protected]>
>
> Reviewed-by: Jason-JH.Lin <[email protected]>
> Reviewed-by: Alexandre Mergnat <[email protected]>
> Signed-off-by: AngeloGioacchino Del Regno <
> [email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 83 +++++++++++++++++--
> ----
> 1 file changed, 64 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 911468984ad5..6305cd95e6d4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -26,17 +26,26 @@
> #define DISP_GAMMA_SIZE_VSIZE GENMASK
> (12, 0)
> #define DISP_GAMMA_BANK 0x0100
> #define DISP_GAMMA_BANK_BANK GENMASK(1, 0)
> +#define DISP_GAMMA_BANK_DATA_MODE BIT(2)
> #define DISP_GAMMA_LUT 0x0700
> +#define DISP_GAMMA_LUT1 0x0b00
>
> +/* For 10 bit LUT layout, R/G/B are in the same register */
> #define DISP_GAMMA_LUT_10BIT_R GENMASK(29, 20)
> #define DISP_GAMMA_LUT_10BIT_G GENMASK(19, 10)
> #define DISP_GAMMA_LUT_10BIT_B GENMASK(9, 0)
>
> +/* For 12 bit LUT layout, R/G are in LUT, B is in LUT1 */
> +#define DISP_GAMMA_LUT_12BIT_R GENMASK(11, 0)
> +#define DISP_GAMMA_LUT_12BIT_G GENMASK(23, 12)
> +#define DISP_GAMMA_LUT_12BIT_B GENMASK(11, 0)
> +
> struct mtk_disp_gamma_data {
> bool has_dither;
> bool lut_diff;
> u16 lut_bank_size;
> u16 lut_size;
> + u8 lut_bits;
> };
>
> /*
> @@ -72,28 +81,48 @@ unsigned int mtk_gamma_get_lut_size(struct device
> *dev)
> return 0;
> }
>
> +/*
> + * SoCs supporting 12-bits LUTs are using a new register layout that
> does
> + * always support (by HW) both 12-bits and 10-bits LUT but, on
> those, we
> + * ignore the support for 10-bits in this driver and always use 12-
> bits.
> + *
> + * Summarizing:
> + * - SoC HW support 9/10-bits LUT only
> + * - Old register layout
> + * - 10-bits LUT supported
> + * - 9-bits LUT not supported
> + * - SoC HW support both 10/12bits LUT
> + * - New register layout
> + * - 12-bits LUT supported
> + * - 10-its LUT not supported
> + */
> void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
> {
> struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
> - unsigned int i;
> - struct drm_color_lut *lut;
> - void __iomem *lut_base;
> - u32 cfg_val, lbank_val, word;
> + void __iomem *lut0_base = gamma->regs + DISP_GAMMA_LUT;
> + void __iomem *lut1_base = gamma->regs + DISP_GAMMA_LUT1;
> + u32 cfg_val, data_mode, lbank_val, word[2];
> + u8 lut_bits = gamma->data->lut_bits;
> int cur_bank, num_lut_banks;
> + struct drm_color_lut *lut;
> + unsigned int i;
>
> /* If there's no gamma lut there's nothing to do here. */
> if (!state->gamma_lut)
> return;
>
> num_lut_banks = gamma->data->lut_size / gamma->data-
> >lut_bank_size;
> - lut_base = gamma->regs + DISP_GAMMA_LUT;
> lut = (struct drm_color_lut *)state->gamma_lut->data;
>
> + /* Switch to 12 bits data mode if supported */
> + data_mode = FIELD_PREP(DISP_GAMMA_BANK_DATA_MODE, !!(lut_bits
> == 12));
> +
> for (cur_bank = 0; cur_bank < num_lut_banks; cur_bank++) {
>
> /* Switch gamma bank and set data mode before writing
> LUT */
> if (num_lut_banks > 1) {
> lbank_val = FIELD_PREP(DISP_GAMMA_BANK_BANK,
> cur_bank);
> + lbank_val |= data_mode;
> writel(lbank_val, gamma->regs +
> DISP_GAMMA_BANK);
> }
>
> @@ -101,29 +130,43 @@ void mtk_gamma_set(struct device *dev, struct
> drm_crtc_state *state)
> int n = cur_bank * gamma->data->lut_bank_size +
> i;
> struct drm_color_lut diff, hwlut;
>
> - hwlut.red = drm_color_lut_extract(lut[n].red,
> 10);
> - hwlut.green =
> drm_color_lut_extract(lut[n].green, 10);
> - hwlut.blue = drm_color_lut_extract(lut[n].blue,
> 10);
> + hwlut.red = drm_color_lut_extract(lut[n].red,
> lut_bits);
> + hwlut.green =
> drm_color_lut_extract(lut[n].green, lut_bits);
> + hwlut.blue = drm_color_lut_extract(lut[n].blue,
> lut_bits);
>
> if (!gamma->data->lut_diff || (i % 2 == 0)) {
> - word =
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, hwlut.red);
> - word |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, hwlut.green);
> - word |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, hwlut.blue);
> + if (lut_bits == 12) {
> + word[0] =
> FIELD_PREP(DISP_GAMMA_LUT_12BIT_R, hwlut.red);
> + word[0] |=
> FIELD_PREP(DISP_GAMMA_LUT_12BIT_G, hwlut.green);
> + word[1] =
> FIELD_PREP(DISP_GAMMA_LUT_12BIT_B, hwlut.blue);
> + } else {
> + word[0] =
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, hwlut.red);
> + word[0] |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, hwlut.green);
> + word[0] |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, hwlut.blue);
> + }
> } else {
> diff.red = lut[n].red - lut[n - 1].red;
> - diff.red =
> drm_color_lut_extract(diff.red, 10);
> + diff.red =
> drm_color_lut_extract(diff.red, lut_bits);
>
> diff.green = lut[n].green - lut[n -
> 1].green;
> - diff.green =
> drm_color_lut_extract(diff.green, 10);
> + diff.green =
> drm_color_lut_extract(diff.green, lut_bits);
>
> diff.blue = lut[n].blue - lut[n -
> 1].blue;
> - diff.blue =
> drm_color_lut_extract(diff.blue, 10);
> -
> - word =
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, diff.red);
> - word |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, diff.green);
> - word |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, diff.blue);
> + diff.blue =
> drm_color_lut_extract(diff.blue, lut_bits);
> +
> + if (lut_bits == 12) {
> + word[0] =
> FIELD_PREP(DISP_GAMMA_LUT_12BIT_R, diff.red);
> + word[0] |=
> FIELD_PREP(DISP_GAMMA_LUT_12BIT_G, diff.green);
> + word[1] =
> FIELD_PREP(DISP_GAMMA_LUT_12BIT_B, diff.blue);
> + } else {
> + word[0] =
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_R, diff.red);
> + word[0] |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_G, diff.green);
> + word[0] |=
> FIELD_PREP(DISP_GAMMA_LUT_10BIT_B, diff.blue);
> + }
> }
> - writel(word, lut_base + i * 4);
> + writel(word[0], lut0_base + i * 4);
> + if (lut_bits == 12)
> + writel(word[1], lut1_base + i * 4);
> }
> }
>
> @@ -225,11 +268,13 @@ static void mtk_disp_gamma_remove(struct
> platform_device *pdev)
> static const struct mtk_disp_gamma_data mt8173_gamma_driver_data = {
> .has_dither = true,
> .lut_bank_size = 512,
> + .lut_bits = 10,
> .lut_size = 512,
> };
>
> static const struct mtk_disp_gamma_data mt8183_gamma_driver_data = {
> .lut_bank_size = 512,
> + .lut_bits = 10,
> .lut_diff = true,
> .lut_size = 512,
> };
Hi, Angelo:
On Thu, 2023-10-12 at 11:57 +0200, AngeloGioacchino Del Regno wrote:
> Now that this driver supports 12-bit LUTs, we can add support for the
> DISP_GAMMA found on the MT8195 SoC: add its driver data and
> compatible.
Reviewed-by: CK Hu <[email protected]>
>
> Signed-off-by: AngeloGioacchino Del Regno <
> [email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index 6305cd95e6d4..bcc33aeca885 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -279,11 +279,20 @@ static const struct mtk_disp_gamma_data
> mt8183_gamma_driver_data = {
> .lut_size = 512,
> };
>
> +static const struct mtk_disp_gamma_data mt8195_gamma_driver_data = {
> + .lut_bank_size = 256,
> + .lut_bits = 12,
> + .lut_diff = true,
> + .lut_size = 1024,
> +};
> +
> static const struct of_device_id mtk_disp_gamma_driver_dt_match[] =
> {
> { .compatible = "mediatek,mt8173-disp-gamma",
> .data = &mt8173_gamma_driver_data},
> { .compatible = "mediatek,mt8183-disp-gamma",
> .data = &mt8183_gamma_driver_data},
> + { .compatible = "mediatek,mt8195-disp-gamma",
> + .data = &mt8195_gamma_driver_data},
> {},
> };
> MODULE_DEVICE_TABLE(of, mtk_disp_gamma_driver_dt_match);
Il 13/10/23 06:00, CK Hu (胡俊光) ha scritto:
> Hi, Angelo:
>
> On Thu, 2023-10-12 at 11:57 +0200, AngeloGioacchino Del Regno wrote:
>> Make the code more robust and improve readability by using bitfield
>> macros instead of open coding bit operations.
>> While at it, also add a definition for LUT_BITS_DEFAULT.
>
> When I apply, I would remove the description of LUT_BITS_DEFAULT.
>
> Reviewed-by: CK Hu <[email protected]>
Sorry for forgetting about removing that from the commit description after
removing it from the code.
Thanks for removing it while applying, that simplifies my workflow.
Cheers,
Angelo
Il 13/10/23 06:07, CK Hu (胡俊光) ha scritto:
> Hi, Angelo:
>
> On Thu, 2023-10-12 at 11:57 +0200, AngeloGioacchino Del Regno wrote:
>> In preparation for adding a 12-bits gamma support for the DISP_GAMMA
>> IP, remove the mtk_gamma_set_common() function and move the relevant
>> bits in mtk_gamma_set() for DISP_GAMMA and mtk_aal_gamma_set() for
>> DISP_AAL: since the latter has no more support for gamma manipulation
>> (being moved to a different IP) in newer revisions, those functions
>> are about to diverge and it makes no sense to keep a common one (with
>> all the complications of passing common data and making exclusions
>> for device driver data) for just a few bits.
>>
>> This commit brings no functional changes.
>
> Reviewed-by: CK Hu <[email protected]>
>
>>
>> Signed-off-by: AngeloGioacchino Del Regno <
>> [email protected]>
..snip..
>> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> index 81c04518a5eb..0929f8830d6d 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
>> @@ -69,41 +69,28 @@ unsigned int mtk_gamma_get_lut_size(struct device
>> *dev)
>> return 0;
>> }
>>
>> -void mtk_gamma_set_common(struct device *dev, void __iomem *regs,
>> struct drm_crtc_state *state)
>> +void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>> {
..snip..
>>
>> - cfg_val = readl(regs + DISP_GAMMA_CFG);
>> + cfg_val = readl(gamma->regs + DISP_GAMMA_CFG);
>>
>> /* Enable the gamma table */
>> cfg_val |= FIELD_PREP(GAMMA_LUT_EN, 1);
>>
>> - writel(cfg_val, regs + DISP_GAMMA_CFG);
>> -}
>> -
>> -void mtk_gamma_set(struct device *dev, struct drm_crtc_state *state)
>> -{
>> - struct mtk_disp_gamma *gamma = dev_get_drvdata(dev);
>> -
>> - mtk_gamma_set_common(dev, gamma->regs, state);
>> + writel(cfg_val, gamma->regs + DISP_GAMMA_CFG);
Hello CK,
something went wrong while you applied this patch: this writel became a readl,
breaking mtk_disp_gamma entirely. Please check:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/mediatek/mtk_disp_gamma.c?h=next-20231025&id=a6b39cd248f3321dbf066f95f95a9841f891229e
Thanks,
Angelo