Add a custom CRTC state struct to enable storing driver-private per-CRTC state. This patch only adds the base drm_crtc_state struct.
Signed-off-by: Mihail Atanassov mihail.atanassov@arm.com --- drivers/gpu/drm/arm/malidp_crtc.c | 39 +++++++++++++++++++++++++++++++++++++-- drivers/gpu/drm/arm/malidp_drv.h | 6 ++++++ 2 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c index 08e6a71..d1cd0e7 100644 --- a/drivers/gpu/drm/arm/malidp_crtc.c +++ b/drivers/gpu/drm/arm/malidp_crtc.c @@ -160,6 +160,41 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc, return 0; }
+static struct drm_crtc_state *malidp_crtc_duplicate_state(struct drm_crtc *crtc) +{ + struct malidp_crtc_state *cs; + struct malidp_crtc_state *state; + + if (WARN_ON(!crtc->state)) + return NULL; + + cs = to_malidp_crtc_state(crtc->state); + state = kmalloc(sizeof(*state), GFP_KERNEL); + if (!state) + return NULL; + + __drm_atomic_helper_crtc_duplicate_state(crtc, &state->base); + + return &state->base; +} + +static void malidp_crtc_reset(struct drm_crtc *crtc) +{ + struct malidp_crtc_state *cs = (crtc->state) ? + to_malidp_crtc_state(crtc->state) : NULL; + + if (crtc->state) + __drm_atomic_helper_crtc_destroy_state(crtc->state); + + kfree(cs); + cs = kzalloc(sizeof(*cs), GFP_KERNEL); + if (!cs) + return; + + crtc->state = &cs->base; + crtc->state->crtc = crtc; +} + static const struct drm_crtc_helper_funcs malidp_crtc_helper_funcs = { .mode_fixup = malidp_crtc_mode_fixup, .enable = malidp_crtc_enable, @@ -171,8 +206,8 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, - .reset = drm_atomic_helper_crtc_reset, - .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, + .reset = malidp_crtc_reset, + .atomic_duplicate_state = malidp_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, };
diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h index 9fc8a2e..c7a69ae 100644 --- a/drivers/gpu/drm/arm/malidp_drv.h +++ b/drivers/gpu/drm/arm/malidp_drv.h @@ -47,6 +47,12 @@ struct malidp_plane_state { #define to_malidp_plane(x) container_of(x, struct malidp_plane, base) #define to_malidp_plane_state(x) container_of(x, struct malidp_plane_state, base)
+struct malidp_crtc_state { + struct drm_crtc_state base; +}; + +#define to_malidp_crtc_state(x) container_of(x, struct malidp_crtc_state, base) + int malidp_de_planes_init(struct drm_device *drm); void malidp_de_planes_destroy(struct drm_device *drm); int malidp_crtc_init(struct drm_device *drm);
Add gamma via the DRM GAMMA_LUT/GAMMA_LUT_SIZE CRTC properties. The expected LUT size is 4096 in order to produce as accurate a set of segments as possible.
This version uses only the green channel's gamma curve to set the hardware curve on DP550/650. For the sake of simplicity, it uses the same table of coefficients for all 3 curves on DP500.
Signed-off-by: Mihail Atanassov mihail.atanassov@arm.com --- drivers/gpu/drm/arm/malidp_crtc.c | 130 ++++++++++++++++++++++++++++++++++++-- drivers/gpu/drm/arm/malidp_drv.c | 52 +++++++++++++++ drivers/gpu/drm/arm/malidp_drv.h | 1 + drivers/gpu/drm/arm/malidp_hw.c | 3 + drivers/gpu/drm/arm/malidp_hw.h | 2 + drivers/gpu/drm/arm/malidp_regs.h | 19 ++++-- 6 files changed, 197 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c index d1cd0e7..10f79b6 100644 --- a/drivers/gpu/drm/arm/malidp_crtc.c +++ b/drivers/gpu/drm/arm/malidp_crtc.c @@ -82,6 +82,114 @@ static void malidp_crtc_disable(struct drm_crtc *crtc) clk_disable_unprepare(hwdev->pxlclk); }
+static const struct gamma_curve_segment { + u16 start; + u16 end; +} segments[MALIDP_COEFFTAB_NUM_COEFFS] = { + /* sector 0 */ + { 0, 0 }, { 1, 1 }, { 2, 2 }, { 3, 3 }, + { 4, 4 }, { 5, 5 }, { 6, 6 }, { 7, 7 }, + { 8, 8 }, { 9, 9 }, { 10, 10 }, { 11, 11 }, + { 12, 12 }, { 13, 13 }, { 14, 14 }, { 15, 15 }, + /* sector 1 */ + { 16, 19 }, { 20, 23 }, { 24, 27 }, { 28, 31 }, + /* sector 2 */ + { 32, 39 }, { 40, 47 }, { 48, 55 }, { 56, 63 }, + /* sector 3 */ + { 64, 79 }, { 80, 95 }, { 96, 111 }, { 112, 127 }, + /* sector 4 */ + { 128, 159 }, { 160, 191 }, { 192, 223 }, { 224, 255 }, + /* sector 5 */ + { 256, 319 }, { 320, 383 }, { 384, 447 }, { 448, 511 }, + /* sector 6 */ + { 512, 639 }, { 640, 767 }, { 768, 895 }, { 896, 1023 }, + { 1024, 1151 }, { 1152, 1279 }, { 1280, 1407 }, { 1408, 1535 }, + { 1536, 1663 }, { 1664, 1791 }, { 1792, 1919 }, { 1920, 2047 }, + { 2048, 2175 }, { 2176, 2303 }, { 2304, 2431 }, { 2432, 2559 }, + { 2560, 2687 }, { 2688, 2815 }, { 2816, 2943 }, { 2944, 3071 }, + { 3072, 3199 }, { 3200, 3327 }, { 3328, 3455 }, { 3456, 3583 }, + { 3584, 3711 }, { 3712, 3839 }, { 3840, 3967 }, { 3968, 4095 }, +}; + +#define DE_COEFTAB_DATA(a, b) ((((a) & 0xfff) << 16) | (((b) & 0xfff))) + +static void malidp_generate_gamma_table(struct drm_property_blob *lut_blob, + u32 coeffs[MALIDP_COEFFTAB_NUM_COEFFS]) +{ + struct drm_color_lut *lut = (struct drm_color_lut *)lut_blob->data; + int i; + + for (i = 0; i < MALIDP_COEFFTAB_NUM_COEFFS; ++i) { + u32 a, b; + u32 delta_in; + u32 out_start, out_end; + + delta_in = segments[i].end - segments[i].start; + /* DP has 12-bit internal precision for its LUTs. */ + out_start = drm_color_lut_extract(lut[segments[i].start].green, + 12); + out_end = drm_color_lut_extract(lut[segments[i].end].green, 12); + a = (delta_in == 0) ? + 0 : ((out_end - out_start) * 256) / delta_in; + b = out_start; + coeffs[i] = DE_COEFTAB_DATA(a, b); + } +} + +/* + * Check if there is a new gamma LUT and if it is of an acceptable size. Also, + * reject any LUTs that use distinct red, green, and blue curves. + */ +static int malidp_crtc_atomic_check_gamma(struct drm_crtc *crtc, + struct drm_crtc_state *state) +{ + struct malidp_crtc_state *mc = to_malidp_crtc_state(state); + struct drm_color_lut *lut; + size_t lut_size; + int i; + + if (!state->color_mgmt_changed) + return 0; + + if (!state->gamma_lut) + return 0; + + if (crtc->state->gamma_lut && (crtc->state->gamma_lut->base.id == + state->gamma_lut->base.id)) + return 0; + + if (state->gamma_lut->length % sizeof(struct drm_color_lut)) + return -EINVAL; + + lut_size = state->gamma_lut->length / sizeof(struct drm_color_lut); + if (lut_size != 4096) + return -EINVAL; + + lut = (struct drm_color_lut *)state->gamma_lut->data; + for (i = 0; i < lut_size; ++i) + if (!((lut[i].red == lut[i].green) && + (lut[i].red == lut[i].blue))) + return -EINVAL; + + if (!state->mode_changed) { + int ret; + + state->mode_changed = true; + /* + * Kerneldoc for drm_atomic_helper_check_modeset mandates that + * it be invoked when the driver sets ->mode_changed. Since + * changing the gamma LUT Doesn't depend on any external + * resources, it is safe to call it only once. + */ + ret = drm_atomic_helper_check_modeset(crtc->dev, state->state); + if (ret) + return ret; + } + + malidp_generate_gamma_table(state->gamma_lut, mc->gamma_coeffs); + return 0; +} + static int malidp_crtc_atomic_check(struct drm_crtc *crtc, struct drm_crtc_state *state) { @@ -91,6 +199,7 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc, const struct drm_plane_state *pstate; u32 rot_mem_free, rot_mem_usable; int rotated_planes = 0; + int ret;
/* * check if there is enough rotation memory available for planes @@ -157,6 +266,10 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc, } }
+ ret = malidp_crtc_atomic_check_gamma(crtc, state); + if (ret) + return ret; + return 0; }
@@ -174,6 +287,8 @@ static struct drm_crtc_state *malidp_crtc_duplicate_state(struct drm_crtc *crtc) return NULL;
__drm_atomic_helper_crtc_duplicate_state(crtc, &state->base); + memcpy(state->gamma_coeffs, cs->gamma_coeffs, + sizeof(state->gamma_coeffs));
return &state->base; } @@ -203,6 +318,7 @@ static void malidp_crtc_reset(struct drm_crtc *crtc) };
static const struct drm_crtc_funcs malidp_crtc_funcs = { + .gamma_set = drm_atomic_helper_legacy_gamma_set, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip, @@ -238,11 +354,17 @@ int malidp_crtc_init(struct drm_device *drm)
ret = drm_crtc_init_with_planes(drm, &malidp->crtc, primary, NULL, &malidp_crtc_funcs, NULL); + if (ret) + goto crtc_cleanup_planes;
- if (!ret) { - drm_crtc_helper_add(&malidp->crtc, &malidp_crtc_helper_funcs); - return 0; - } + drm_crtc_helper_add(&malidp->crtc, &malidp_crtc_helper_funcs); + drm_mode_crtc_set_gamma_size(&malidp->crtc, 4096); + /* No inverse-gamma and color adjustments yet. */ + drm_crtc_enable_color_mgmt(&malidp->crtc, + 0, + false, + 4096); + return 0;
crtc_cleanup_planes: malidp_de_planes_destroy(drm); diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c index 32f746e..a9f787d 100644 --- a/drivers/gpu/drm/arm/malidp_drv.c +++ b/drivers/gpu/drm/arm/malidp_drv.c @@ -33,6 +33,51 @@
#define MALIDP_CONF_VALID_TIMEOUT 250
+static void malidp_write_gamma_table(struct malidp_hw_device *hwdev, + u32 data[MALIDP_COEFFTAB_NUM_COEFFS]) +{ + int i; + /* Update all channels with a single gamma curve. */ + const u32 gamma_write_mask = GENMASK(18, 16); + /* + * Always write an entire table, so the address field in + * DE_COEFFTAB_ADDR is 0 and we can use the gamma_write_mask bitmask + * directly. + */ + malidp_hw_write(hwdev, gamma_write_mask, + hwdev->map.coeffs_base + MALIDP_COEF_TABLE_ADDR); + for (i = 0; i < MALIDP_COEFFTAB_NUM_COEFFS; ++i) + malidp_hw_write(hwdev, data[i], + hwdev->map.coeffs_base + + MALIDP_COEF_TABLE_DATA); +} + +static void malidp_atomic_commit_update_gamma(struct drm_crtc *crtc, + struct drm_crtc_state *old_state) +{ + struct malidp_drm *malidp = crtc_to_malidp_device(crtc); + struct malidp_hw_device *hwdev = malidp->dev; + + if (!crtc->state->color_mgmt_changed) + return; + + if (!crtc->state->gamma_lut) { + malidp_hw_clearbits(hwdev, + MALIDP_DISP_FUNC_GAM, + MALIDP_DE_DISPLAY_FUNC); + } else { + struct malidp_crtc_state *mc = + to_malidp_crtc_state(crtc->state); + + if (!old_state->gamma_lut || (crtc->state->gamma_lut->base.id != + old_state->gamma_lut->base.id)) + malidp_write_gamma_table(hwdev, mc->gamma_coeffs); + + malidp_hw_setbits(hwdev, MALIDP_DISP_FUNC_GAM, + MALIDP_DE_DISPLAY_FUNC); + } +} + /* * set the "config valid" bit and wait until the hardware acts on it */ @@ -89,8 +134,15 @@ static void malidp_atomic_commit_hw_done(struct drm_atomic_state *state) static void malidp_atomic_commit_tail(struct drm_atomic_state *state) { struct drm_device *drm = state->dev; + struct drm_crtc *crtc; + struct drm_crtc_state *old_crtc_state; + int i;
drm_atomic_helper_commit_modeset_disables(drm, state); + + for_each_crtc_in_state(state, crtc, old_crtc_state, i) + malidp_atomic_commit_update_gamma(crtc, old_crtc_state); + drm_atomic_helper_commit_modeset_enables(drm, state); drm_atomic_helper_commit_planes(drm, state, 0);
diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h index c7a69ae..374d43e 100644 --- a/drivers/gpu/drm/arm/malidp_drv.h +++ b/drivers/gpu/drm/arm/malidp_drv.h @@ -49,6 +49,7 @@ struct malidp_plane_state {
struct malidp_crtc_state { struct drm_crtc_state base; + u32 gamma_coeffs[MALIDP_COEFFTAB_NUM_COEFFS]; };
#define to_malidp_crtc_state(x) container_of(x, struct malidp_crtc_state, base) diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c index 4bdf531..5ceca17 100644 --- a/drivers/gpu/drm/arm/malidp_hw.c +++ b/drivers/gpu/drm/arm/malidp_hw.c @@ -415,6 +415,7 @@ static int malidp650_query_hw(struct malidp_hw_device *hwdev) const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = { [MALIDP_500] = { .map = { + .coeffs_base = MALIDP500_COEFFS_BASE, .se_base = MALIDP500_SE_BASE, .dc_base = MALIDP500_DC_BASE, .out_depth_base = MALIDP500_OUTPUT_DEPTH, @@ -450,6 +451,7 @@ static int malidp650_query_hw(struct malidp_hw_device *hwdev) }, [MALIDP_550] = { .map = { + .coeffs_base = MALIDP550_COEFFS_BASE, .se_base = MALIDP550_SE_BASE, .dc_base = MALIDP550_DC_BASE, .out_depth_base = MALIDP550_DE_OUTPUT_DEPTH, @@ -483,6 +485,7 @@ static int malidp650_query_hw(struct malidp_hw_device *hwdev) }, [MALIDP_650] = { .map = { + .coeffs_base = MALIDP550_COEFFS_BASE, .se_base = MALIDP550_SE_BASE, .dc_base = MALIDP550_DC_BASE, .out_depth_base = MALIDP550_DE_OUTPUT_DEPTH, diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h index 087e1202..ecff7d4 100644 --- a/drivers/gpu/drm/arm/malidp_hw.h +++ b/drivers/gpu/drm/arm/malidp_hw.h @@ -66,6 +66,8 @@ struct malidp_layer { struct malidp_hw_regmap { /* address offset of the DE register bank */ /* is always 0x0000 */ + /* address offset of the DE coefficients registers */ + const u16 coeffs_base; /* address offset of the SE registers bank */ const u16 se_base; /* address offset of the DC registers bank */ diff --git a/drivers/gpu/drm/arm/malidp_regs.h b/drivers/gpu/drm/arm/malidp_regs.h index 73fecb3..2e213c3 100644 --- a/drivers/gpu/drm/arm/malidp_regs.h +++ b/drivers/gpu/drm/arm/malidp_regs.h @@ -63,6 +63,7 @@
/* bit masks that are common between products */ #define MALIDP_CFG_VALID (1 << 0) +#define MALIDP_DISP_FUNC_GAM (1 << 0) #define MALIDP_DISP_FUNC_ILACED (1 << 8)
/* register offsets for IRQ management */ @@ -92,6 +93,12 @@ #define MALIDP_DE_H_ACTIVE(x) (((x) & 0x1fff) << 0) #define MALIDP_DE_V_ACTIVE(x) (((x) & 0x1fff) << 16)
+#define MALIDP_COEFFTAB_NUM_COEFFS 64 +/* register offsets relative to MALIDP5x0_COEFFS_BASE */ +#define MALIDP_COLOR_ADJ_COEF 0x00000 +#define MALIDP_COEF_TABLE_ADDR 0x00030 +#define MALIDP_COEF_TABLE_DATA 0x00034 + /* register offsets and bits specific to DP500 */ #define MALIDP500_DC_BASE 0x00000 #define MALIDP500_DC_CONTROL 0x0000c @@ -109,9 +116,11 @@ #define MALIDP500_BGND_COLOR 0x0003c #define MALIDP500_OUTPUT_DEPTH 0x00044 #define MALIDP500_YUV_RGB_COEF 0x00048 -#define MALIDP500_COLOR_ADJ_COEF 0x00078 -#define MALIDP500_COEF_TABLE_ADDR 0x000a8 -#define MALIDP500_COEF_TABLE_DATA 0x000ac +/* + * To match DP550/650, the start of the coeffs registers is + * at COLORADJ_COEFF0 instead of at YUV_RGB_COEF1. + */ +#define MALIDP500_COEFFS_BASE 0x00078 #define MALIDP500_DE_LV_BASE 0x00100 #define MALIDP500_DE_LV_PTR_BASE 0x00124 #define MALIDP500_DE_LG1_BASE 0x00200 @@ -136,9 +145,7 @@ #define MALIDP550_DE_DISP_SIDEBAND 0x00040 #define MALIDP550_DE_BGND_COLOR 0x00044 #define MALIDP550_DE_OUTPUT_DEPTH 0x0004c -#define MALIDP550_DE_COLOR_COEF 0x00050 -#define MALIDP550_DE_COEF_TABLE_ADDR 0x00080 -#define MALIDP550_DE_COEF_TABLE_DATA 0x00084 +#define MALIDP550_COEFFS_BASE 0x00050 #define MALIDP550_DE_LV1_BASE 0x00100 #define MALIDP550_DE_LV1_PTR_BASE 0x00124 #define MALIDP550_DE_LV2_BASE 0x00200
I'd be fine with you squashing these two together, but either way both are:
Reviewed-by: Brian Starkey brian.starkey@arm.com
(two nitpicks below, but take them or leave them.)
On Wed, Feb 01, 2017 at 10:16:57AM +0000, Mihail Atanassov wrote:
Add a custom CRTC state struct to enable storing driver-private per-CRTC state. This patch only adds the base drm_crtc_state struct.
Signed-off-by: Mihail Atanassov mihail.atanassov@arm.com
drivers/gpu/drm/arm/malidp_crtc.c | 39 +++++++++++++++++++++++++++++++++++++-- drivers/gpu/drm/arm/malidp_drv.h | 6 ++++++ 2 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c index 08e6a71..d1cd0e7 100644 --- a/drivers/gpu/drm/arm/malidp_crtc.c +++ b/drivers/gpu/drm/arm/malidp_crtc.c @@ -160,6 +160,41 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc, return 0; }
+static struct drm_crtc_state *malidp_crtc_duplicate_state(struct drm_crtc *crtc) +{
- struct malidp_crtc_state *cs;
- struct malidp_crtc_state *state;
- if (WARN_ON(!crtc->state))
return NULL;- cs = to_malidp_crtc_state(crtc->state);
This is unused in this commit.
- state = kmalloc(sizeof(*state), GFP_KERNEL);
- if (!state)
return NULL;- __drm_atomic_helper_crtc_duplicate_state(crtc, &state->base);
- return &state->base;
+}
+static void malidp_crtc_reset(struct drm_crtc *crtc) +{
- struct malidp_crtc_state *cs = (crtc->state) ?
to_malidp_crtc_state(crtc->state) : NULL;- if (crtc->state)
__drm_atomic_helper_crtc_destroy_state(crtc->state);
I guess the compiler would do it anyway, but you could put the cs assignment inside this 'if' instead of checking crtc->state twice.
- kfree(cs);
- cs = kzalloc(sizeof(*cs), GFP_KERNEL);
- if (!cs)
return;- crtc->state = &cs->base;
- crtc->state->crtc = crtc;
+}
static const struct drm_crtc_helper_funcs malidp_crtc_helper_funcs = { .mode_fixup = malidp_crtc_mode_fixup, .enable = malidp_crtc_enable, @@ -171,8 +206,8 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc, .destroy = drm_crtc_cleanup, .set_config = drm_atomic_helper_set_config, .page_flip = drm_atomic_helper_page_flip,
- .reset = drm_atomic_helper_crtc_reset,
- .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
- .reset = malidp_crtc_reset,
- .atomic_duplicate_state = malidp_crtc_duplicate_state, .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
};
diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h index 9fc8a2e..c7a69ae 100644 --- a/drivers/gpu/drm/arm/malidp_drv.h +++ b/drivers/gpu/drm/arm/malidp_drv.h @@ -47,6 +47,12 @@ struct malidp_plane_state { #define to_malidp_plane(x) container_of(x, struct malidp_plane, base) #define to_malidp_plane_state(x) container_of(x, struct malidp_plane_state, base)
+struct malidp_crtc_state {
- struct drm_crtc_state base;
+};
+#define to_malidp_crtc_state(x) container_of(x, struct malidp_crtc_state, base)
int malidp_de_planes_init(struct drm_device *drm); void malidp_de_planes_destroy(struct drm_device *drm); int malidp_crtc_init(struct drm_device *drm); -- 1.9.1
dri-devel@lists.freedesktop.org
-
Brian Starkey -
Mihail Atanassov