Hi,
On 20/05/16 09:35, Jyri Sarha wrote:
> Add gamma table support to DSS dispc.
>
> DSS driver initializes the default gamma table at component bind time
> and holds a copy of all gamma tables in it's internal data structure.
"its"
> Each call to dispc_mgr_set_gamma() updates the internal table and
> triggers write the HW, if it is enabled. The tables are restored to HW
"... to the HW"?
> in PM resume callback. The tables at DSS side matches the HW tables in
> size and in number of significant bits per color component. The
Maybe "the tables in RAM match the tables in HW".
> dispc_mgr_set_gamma() converts the size of any given table to match
> the table size in HW.
>
> dispc_mgr_gamma_size() gives HW gamma table size for the channel
> and returns 0 if gamma table is not supported by HW or DSS driver.
>
> Signed-off-by: Jyri Sarha <jsarha at ti.com>
> ---
> drivers/gpu/drm/omapdrm/dss/dispc.c | 189
> ++++++++++++++++++++++++++---
> drivers/gpu/drm/omapdrm/dss/dispc.h | 5 +
> drivers/gpu/drm/omapdrm/dss/dss_features.c | 2 +
> drivers/gpu/drm/omapdrm/dss/dss_features.h | 1 +
> drivers/gpu/drm/omapdrm/dss/hdmi4.c | 3 -
> drivers/gpu/drm/omapdrm/dss/hdmi5.c | 3 -
> drivers/gpu/drm/omapdrm/dss/omapdss.h | 5 +
> 7 files changed, 186 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c
> b/drivers/gpu/drm/omapdrm/dss/dispc.c
> index f83608b..87dde05 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -116,6 +116,7 @@ struct dispc_features {
> };
>
> #define DISPC_MAX_NR_FIFOS 5
> +#define DISPC_MAX_CHANNEL_GAMMA 4
>
> static struct {
> struct platform_device *pdev;
> @@ -135,6 +136,8 @@ static struct {
> bool ctx_valid;
> u32 ctx[DISPC_SZ_REGS / sizeof(u32)];
>
> + u32 *gamma_table[DISPC_MAX_CHANNEL_GAMMA];
> +
> const struct dispc_features *feat;
>
> bool is_enabled;
> @@ -178,11 +181,19 @@ struct dispc_reg_field {
> u8 low;
> };
>
> +struct dispc_gamma_desc {
> + u32 len;
> + u32 bits;
> + u16 reg;
> + bool has_index;
> +};
> +
> static const struct {
> const char *name;
> u32 vsync_irq;
> u32 framedone_irq;
> u32 sync_lost_irq;
> + struct dispc_gamma_desc gamma;
> struct dispc_reg_field reg_desc[DISPC_MGR_FLD_NUM];
> } mgr_desc[] = {
> [OMAP_DSS_CHANNEL_LCD] = {
> @@ -190,6 +201,12 @@ static const struct {
> .vsync_irq = DISPC_IRQ_VSYNC,
> .framedone_irq = DISPC_IRQ_FRAMEDONE,
> .sync_lost_irq = DISPC_IRQ_SYNC_LOST,
> + .gamma = {
> + .len = 256,
> + .bits = 8,
> + .reg = DISPC_GAMMA_TABLE0,
> + .has_index = true,
> + },
> .reg_desc = {
> [DISPC_MGR_FLD_ENABLE] = { DISPC_CONTROL, 0,
> 0 },
> [DISPC_MGR_FLD_STNTFT] = { DISPC_CONTROL, 3,
> 3 },
> @@ -207,6 +224,12 @@ static const struct {
> .vsync_irq = DISPC_IRQ_EVSYNC_ODD | DISPC_IRQ_EVSYNC_EVEN,
> .framedone_irq = DISPC_IRQ_FRAMEDONETV,
> .sync_lost_irq = DISPC_IRQ_SYNC_LOST_DIGIT,
> + .gamma = {
> + .len = 1024,
> + .bits = 10,
> + .reg = DISPC_GAMMA_TABLE2,
> + .has_index = false,
> + },
> .reg_desc = {
> [DISPC_MGR_FLD_ENABLE] = { DISPC_CONTROL, 1,
> 1 },
> [DISPC_MGR_FLD_STNTFT] = { },
> @@ -224,6 +247,12 @@ static const struct {
> .vsync_irq = DISPC_IRQ_VSYNC2,
> .framedone_irq = DISPC_IRQ_FRAMEDONE2,
> .sync_lost_irq = DISPC_IRQ_SYNC_LOST2,
> + .gamma = {
> + .len = 256,
> + .bits = 8,
> + .reg = DISPC_GAMMA_TABLE1,
> + .has_index = true,
> + },
> .reg_desc = {
> [DISPC_MGR_FLD_ENABLE] = { DISPC_CONTROL2, 0,
> 0 },
> [DISPC_MGR_FLD_STNTFT] = { DISPC_CONTROL2, 3,
> 3 },
> @@ -241,6 +270,12 @@ static const struct {
> .vsync_irq = DISPC_IRQ_VSYNC3,
> .framedone_irq = DISPC_IRQ_FRAMEDONE3,
> .sync_lost_irq = DISPC_IRQ_SYNC_LOST3,
> + .gamma = {
> + .len = 256,
> + .bits = 8,
> + .reg = DISPC_GAMMA_TABLE3,
> + .has_index = true,
> + },
> .reg_desc = {
> [DISPC_MGR_FLD_ENABLE] = { DISPC_CONTROL3, 0,
> 0 },
> [DISPC_MGR_FLD_STNTFT] = { DISPC_CONTROL3, 3,
> 3 },
> @@ -1084,20 +1119,6 @@ static u32 dispc_ovl_get_burst_size(enum omap_plane
> plane)
> return unit * 8;
> }
>
> -void dispc_enable_gamma_table(bool enable)
> -{
> - /*
> - * This is partially implemented to support only disabling of
> - * the gamma table.
> - */
> - if (enable) {
> - DSSWARN("Gamma table enabling for TV not yet supported");
> - return;
> - }
> -
> - REG_FLD_MOD(DISPC_CONFIG, enable, 9, 9);
> -}
> -
> static void dispc_mgr_enable_cpr(enum omap_channel channel, bool enable)
> {
> if (channel == OMAP_DSS_CHANNEL_DIGIT)
> @@ -3814,6 +3835,128 @@ void dispc_disable_sidle(void)
> REG_FLD_MOD(DISPC_SYSCONFIG, 1, 4, 3); /* SIDLEMODE: no idle */
> }
>
> +u32 dispc_mgr_gamma_size(enum omap_channel channel)
> +{
> + const struct dispc_gamma_desc *gdesc = &mgr_desc[channel].gamma;
> +
> + if (!dss_has_feature(FEAT_GAMMA_TABLE))
> + return 0;
> +
This should probably check the availability of LCD2/LCD3?
> + return gdesc->len;
> +}
> +EXPORT_SYMBOL(dispc_mgr_gamma_size);
> +
> +static void dispc_mgr_write_gamma_table(enum omap_channel channel)
> +{
> + const struct dispc_gamma_desc *gdesc = &mgr_desc[channel].gamma;
> + u32 *table = dispc.gamma_table[channel];
> + unsigned int i;
> +
> + DSSDBG("%s: channel %d\n", __func__, channel);
> +
> + for (i = 0; i < gdesc->len; ++i) {
> + u32 v = table[i];
> +
> + if (gdesc->has_index)
> + v |= i << 24;
> + else if (i == 0)
> + v |= 1 << 31;
> +
> + dispc_write_reg(gdesc->reg, v);
> + }
> +}
> +
> +static void dispc_restore_gamma_tables(void)
> +{
> + DSSDBG("%s()\n", __func__);
> +
> + if (!dss_has_feature(FEAT_GAMMA_TABLE))
> + return;
> +
> + dispc_mgr_write_gamma_table(OMAP_DSS_CHANNEL_LCD);
> +
> + dispc_mgr_write_gamma_table(OMAP_DSS_CHANNEL_DIGIT);
> +
> + if (dss_has_feature(FEAT_MGR_LCD2))
> + dispc_mgr_write_gamma_table(OMAP_DSS_CHANNEL_LCD2);
> +
> + if (dss_has_feature(FEAT_MGR_LCD3))
> + dispc_mgr_write_gamma_table(OMAP_DSS_CHANNEL_LCD3);
> +}
> +
> +void dispc_mgr_set_gamma(enum omap_channel channel,
> + const struct drm_color_lut *lut,
> + unsigned int length)
> +{
> + const struct dispc_gamma_desc *gdesc = &mgr_desc[channel].gamma;
> + u32 *table = dispc.gamma_table[channel];
> + int i;
unsigned.
> +
> + DSSDBG("%s: channel %d, lut len %u, hw len %u\n", __func__,
> + channel, length, gdesc->len);
> +
> + if (!dss_has_feature(FEAT_GAMMA_TABLE))
> + return;
> +
> + for (i = 0; i < length; ++i) {
> + int first = i * (gdesc->len - 1) / (length - 1);
> + int last = (i + 1) * (gdesc->len - 1) / (length - 1);
This might be slightly more readable if you hade src_len and dst_len
local variables. Or maybe it's clear enough, it's a short function.
> + int w = last - first;
unsigned.
> + u16 r, g, b;
> + int j;
unsigned.
> +
> + for (j = 0; j <= w; j++) {
> + r = (lut[i].red * (w - j) + lut[i+1].red * j) / w;
> + g = (lut[i].green * (w - j) + lut[i+1].green * j) / w;
> + b = (lut[i].blue * (w - j) + lut[i+1].blue * j) / w;
> +
> + r >>= (16 - gdesc->bits);
> + g >>= (16 - gdesc->bits);
> + b >>= (16 - gdesc->bits);
I don't think the parenthesis do anything here.
> +
> + table[first + j] = (r << (gdesc->bits * 2)) |
> + (g << gdesc->bits) | b;
> + }
> + }
> +
> + if (dispc.is_enabled)
> + dispc_mgr_write_gamma_table(channel);
> +}
> +EXPORT_SYMBOL(dispc_mgr_set_gamma);
> +
> +static int dispc_init_gamma_tables(void)
> +{
> + int channel;
> +
> + if (!dss_has_feature(FEAT_GAMMA_TABLE))
> + return;
> +
> + for (channel = 0; channel < ARRAY_SIZE(dispc.gamma_table); channel++) {
> + const struct dispc_gamma_desc *gdesc = &mgr_desc[channel].gamma;
> + u32 *table;
> + int i;
unsigned.
> +
> + if (channel == OMAP_DSS_CHANNEL_LCD2 &&
> + !dss_has_feature(FEAT_MGR_LCD2))
> + continue;
> +
> + if (channel == OMAP_DSS_CHANNEL_LCD3 &&
> + !dss_has_feature(FEAT_MGR_LCD3))
> + continue;
> +
> + table = devm_kmalloc_array(&dispc.pdev->dev, gdesc->len,
> + sizeof(table[i]), GFP_KERNEL);
Wouldn't allocating gdesc->len * sizeof(u32) be much more
understandable? 'i' is even uninitialized at this point, making the code
quite suspicious.
> + if (!table)
> + return -ENOMEM;
> +
> + for (i = 0; i < gdesc->len; ++i)
> + table[i] =
> + (i << 2*gdesc->bits) | (i << gdesc->bits) | i;
Add an empty line here.
Add spaces around '*'.
And instead of splitting the line above, perhaps assign the value to u32
local, and assign that to table[i].
> + dispc.gamma_table[channel] = table;
> + }
> + return 0;
> +}
> +
> static void _omap_dispc_initial_config(void)
> {
> u32 l;
> @@ -3829,8 +3972,16 @@ static void _omap_dispc_initial_config(void)
> dispc.core_clk_rate = dispc_fclk_rate();
> }
>
> - /* FUNCGATED */
> - if (dss_has_feature(FEAT_FUNCGATED))
> + /* Use gamma table */
Use gamma table for what, instead of what?
> + if (dss_has_feature(FEAT_GAMMA_TABLE))
> + REG_FLD_MOD(DISPC_CONFIG, 1, 3, 3);
> +
> + /* For older DSS versions (FEAT_FUNCGATED) this enables
> + * func-clock auto-gating. For newer versions
> + * (FEAT_GAMMA_TABLE) this enables tv-out gamma tables.
> + */
> + if (dss_has_feature(FEAT_FUNCGATED) ||
> + dss_has_feature(FEAT_GAMMA_TABLE))
> REG_FLD_MOD(DISPC_CONFIG, 1, 9, 9);
>
> dispc_setup_color_conv_coef();
> @@ -4100,6 +4251,10 @@ static int dispc_bind(struct device *dev, struct
> device *master, void *data)
> }
> }
>
> + r = dispc_init_gamma_tables();
> + if (r)
> + return r;
> +
> pm_runtime_enable(&pdev->dev);
>
> r = dispc_runtime_get();
> @@ -4170,6 +4325,8 @@ static int dispc_runtime_resume(struct device *dev)
> _omap_dispc_initial_config();
>
> dispc_restore_context();
> +
> + dispc_restore_gamma_tables();
> }
>
> dispc.is_enabled = true;
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.h
> b/drivers/gpu/drm/omapdrm/dss/dispc.h
> index 4837442..bc1d812 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.h
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.h
> @@ -42,6 +42,11 @@
> #define DISPC_MSTANDBY_CTRL 0x0858
> #define DISPC_GLOBAL_MFLAG_ATTRIBUTE 0x085C
>
> +#define DISPC_GAMMA_TABLE0 0x0630
> +#define DISPC_GAMMA_TABLE1 0x0634
> +#define DISPC_GAMMA_TABLE2 0x0638
> +#define DISPC_GAMMA_TABLE3 0x0850
> +
> /* DISPC overlay registers */
> #define DISPC_OVL_BA0(n) (DISPC_OVL_BASE(n) + \
> DISPC_BA0_OFFSET(n))
> diff --git a/drivers/gpu/drm/omapdrm/dss/dss_features.c
> b/drivers/gpu/drm/omapdrm/dss/dss_features.c
> index c886a29..f77b1c5 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dss_features.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dss_features.c
> @@ -593,6 +593,7 @@ static const enum dss_feat_id omap4_dss_feat_list[] = {
> FEAT_ALPHA_FREE_ZORDER,
> FEAT_FIFO_MERGE,
> FEAT_BURST_2D,
> + FEAT_GAMMA_TABLE,
> };
The dss_features is deprecated. New features should be added to the
respective driver. In this case, dispc.c. See struct dispc_features.
Tomi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160520/c065ae3d/attachment-0001.sig>
