| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20170206103233.GW3140@e110455-lin.cambridge.arm.com>
Date: Mon, 6 Feb 2017 10:32:33 +0000
From: Liviu Dudau <liviu.dudau@....com>
To: Mihail Atanassov <mihail.atanassov@....com>
Cc: dri-devel@...ts.freedesktop.org,
Brian Starkey <brian.starkey@....com>,
David Airlie <airlied@...ux.ie>, linux-kernel@...r.kernel.org,
Mali DP Maintainers <malidp@...s.arm.com>, nd@....com
Subject: Re: [PATCH v2 2/2] drm: mali-dp: enable gamma support
On Wed, Feb 01, 2017 at 02:48:50PM +0000, Mihail Atanassov wrote:
> 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@....com>
> Reviewed-by: Brian Starkey <brian.starkey@....com>
For the whole series:
Acked-by: Liviu Dudau <liviu.dudau@....com>
Thanks,
Liviu
> ---
> drivers/gpu/drm/arm/malidp_crtc.c | 132 ++++++++++++++++++++++++++++++++++++--
> 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, 199 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c
> index ebf57e6..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,21 +266,29 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc,
> }
> }
>
> + ret = malidp_crtc_atomic_check_gamma(crtc, state);
> + if (ret)
> + return ret;
> +
> 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);
> + memcpy(state->gamma_coeffs, cs->gamma_coeffs,
> + sizeof(state->gamma_coeffs));
>
> return &state->base;
> }
> @@ -201,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,
> @@ -236,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
> --
> 1.9.1
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
Powered by blists - more mailing lists