Re: [PATCH v2 4/4] drm/komeda: Enable Layer color management for komeda

Hi James,

On Tuesday, 13 August 2019 05:56:19 BST james qian wang (Arm Technology China) wrote:

• D71 has 3 global layer gamma table which can be used for all layers as gamma lookup table, no matter inverse or forward.
• Add komeda_color_manager/state to layer/layer_state for describe the color caps for the specific layer which will be initialized in d71_layer_init, and for D71 only layer with L_INFO_CM (color mgmt) bit can support forward gamma, and CSC.
• Update komeda-CORE code to validate and convert the plane color state to komeda_color_state.
• Enable plane color mgmt according to layer komeda_color_manager

This patch depends on:

• https://patchwork.freedesktop.org/series/30876/

Signed-off-by: James Qian Wang (Arm Technology China) james.qian.wang@arm.com

.../arm/display/komeda/d71/d71_component.c | 64 +++++++++++++++++++ .../gpu/drm/arm/display/komeda/d71/d71_dev.c | 5 +- .../gpu/drm/arm/display/komeda/d71/d71_dev.h | 2 + .../drm/arm/display/komeda/komeda_pipeline.h | 4 +- .../display/komeda/komeda_pipeline_state.c | 53 ++++++++++++++- .../gpu/drm/arm/display/komeda/komeda_plane.c | 12 ++++ .../arm/display/komeda/komeda_private_obj.c | 4 ++ 7 files changed, 139 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c index 55a8cc94808a..c9c40a36e4d2 100644 --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c @@ -189,6 +189,46 @@ static void d71_layer_update_fb(struct komeda_component *c, malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id); }

+static u32 d71_layer_update_color(struct drm_plane_state *st,

• 		  u32 __iomem *reg,
  

• 		  struct komeda_color_state *color_st,
  

• 		  u32 *mask)
  

+{

• struct komeda_coeffs_table *igamma = color_st->igamma;
• struct komeda_coeffs_table *fgamma = color_st->fgamma;
• u32 ctrl = 0, v = 0;
• if (!st->color_mgmt_changed)

• return 0;
  

• *mask |= L_IT | L_R2R | L_FT;
• if (igamma) {

• komeda_coeffs_update(igamma);
  

• ctrl |= L_IT;
  

• v = L_ITSEL(igamma->hw_id);
  

• }
• if (st->ctm) {

• u32 ctm_coeffs[KOMEDA_N_CTM_COEFFS];
  

• drm_ctm_to_coeffs(st->ctm, ctm_coeffs);
  

• malidp_write_group(reg, LAYER_RGB_RGB_COEFF0,
  

• 		   ARRAY_SIZE(ctm_coeffs),
  

• 		   ctm_coeffs);
  

• ctrl |= L_R2R; /* enable RGB2RGB conversion */
  

• }
• if (fgamma) {

• komeda_coeffs_update(fgamma);
  

• ctrl |= L_FT;
  

• v |= L_FTSEL(fgamma->hw_id);
  

• }
• malidp_write32(reg, LAYER_LT_COEFFTAB, v);
• return ctrl;

+}

static void d71_layer_disable(struct komeda_component *c) { malidp_write32_mask(c->reg, BLK_CONTROL, L_EN, 0); @@ -261,6 +301,8 @@ static void d71_layer_update(struct komeda_component *c,

malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));

• ctrl |= d71_layer_update_color(plane_st, reg, &st->color_st, &ctrl_mask);
• if (kfb->is_va) ctrl |= L_TBU_EN; malidp_write32_mask(reg, BLK_CONTROL, ctrl_mask, ctrl);

@@ -365,6 +407,12 @@ static int d71_layer_init(struct d71_dev *d71, else layer->layer_type = KOMEDA_FMT_SIMPLE_LAYER;

• layer->color_mgr.igamma_mgr = d71->glb_lt_mgr;
• if (layer_info & L_INFO_CM) {

• layer->color_mgr.has_ctm = true;
  

• layer->color_mgr.fgamma_mgr = d71->glb_lt_mgr;
  

• }
• set_range(&layer->hsize_in, 4, d71->max_line_size); set_range(&layer->vsize_in, 4, d71->max_vsize);

@@ -1140,6 +1188,21 @@ static int d71_timing_ctrlr_init(struct d71_dev *d71, return 0; }

+static void d71_gamma_update(struct komeda_coeffs_table *table) +{

• malidp_write_group(table->reg, GLB_LT_COEFF_DATA,

• 	   GLB_LT_COEFF_NUM, table->coeffs);
  

+}

+static int d71_lt_coeff_init(struct d71_dev *d71,

• 	     struct block_header *blk, u32 __iomem *reg)
  

+{

• struct komeda_coeffs_manager *mgr = d71->glb_lt_mgr;
• int hw_id = BLOCK_INFO_TYPE_ID(blk->block_info);
• return komeda_coeffs_add(mgr, hw_id, reg, d71_gamma_update);

+}

int d71_probe_block(struct d71_dev *d71, struct block_header *blk, u32 __iomem *reg) { @@ -1202,6 +1265,7 @@ int d71_probe_block(struct d71_dev *d71, break;

case D71_BLK_TYPE_GLB_LT_COEFF:

• err = d71_lt_coeff_init(d71, blk, reg);
  

  break;

  case D71_BLK_TYPE_GLB_SCL_COEFF:

diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c index d567ab7ed314..edd5d7c7f2a2 100644 --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c @@ -341,7 +341,7 @@ static int d71_enum_resources(struct komeda_dev *mdev) struct komeda_pipeline *pipe; struct block_header blk; u32 __iomem *blk_base;

• u32 i, value, offset;

• u32 i, value, offset, coeffs_size; int err;

  d71 = devm_kzalloc(mdev->dev, sizeof(*d71), GFP_KERNEL);

@@ -398,6 +398,9 @@ static int d71_enum_resources(struct komeda_dev *mdev) d71->pipes[i] = to_d71_pipeline(pipe); }

• coeffs_size = GLB_LT_COEFF_NUM * sizeof(u32);
• d71->glb_lt_mgr = komeda_coeffs_create_manager(coeffs_size);
• /* loop the register blks and probe */ i = 2; /* exclude GCU and PERIPH */ offset = D71_BLOCK_SIZE; /* skip GCU */

diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.h b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.h index 84f1878b647d..009c164a1584 100644 --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.h +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.h @@ -39,6 +39,8 @@ struct d71_dev { u32 __iomem *periph_addr;

struct d71_pipeline *pipes[D71_MAX_PIPELINE];

• /* global layer transform coefficient table manager */
  

• struct komeda_coeffs_manager *glb_lt_mgr;

};

#define to_d71_pipeline(x) container_of(x, struct d71_pipeline, base) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h index a7a84e66549d..4104c81e5032 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h @@ -10,6 +10,7 @@ #include <linux/types.h> #include <drm/drm_atomic.h> #include <drm/drm_atomic_helper.h> +#include "komeda_color_mgmt.h" #include "malidp_utils.h"

#define KOMEDA_MAX_PIPELINES 2 @@ -226,6 +227,7 @@ struct komeda_layer { struct komeda_component base; /* accepted h/v input range before rotation */ struct malidp_range hsize_in, vsize_in;

• struct komeda_color_manager color_mgr; u32 layer_type; /* RICH, SIMPLE or WB */ u32 supported_rots; /* komeda supports layer split which splits a whole image to two parts

@@ -238,7 +240,7 @@ struct komeda_layer {

struct komeda_layer_state { struct komeda_component_state base;

• /* layer specific configuration state */

• struct komeda_color_state color_st; u16 hsize, vsize; u32 rot; u16 afbc_crop_l;

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c index ea26bc9c2d00..803715c1128e 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c @@ -95,6 +95,21 @@ komeda_pipeline_get_state_and_set_crtc(struct komeda_pipeline *pipe, return st; }

+static bool komeda_component_is_new_active(struct komeda_component *c,

• 			   struct drm_atomic_state *state)
  

+{

• struct komeda_pipeline_state *ppl_old_st;
• ppl_old_st = komeda_pipeline_get_old_state(c->pipeline, state);
• if (IS_ERR(ppl_old_st))

• return true;
  

• if (has_bit(c->id, ppl_old_st->active_comps))

• return false;
  

• return true;

+}

static struct komeda_component_state * komeda_component_get_state(struct komeda_component *c, struct drm_atomic_state *state) @@ -279,6 +294,29 @@ komeda_rotate_data_flow(struct komeda_data_flow_cfg *dflow, u32 rot) } }

+static int +komeda_validate_plane_color(struct komeda_component *c,

• 	    struct komeda_color_manager *color_mgr,
  

• 	    struct komeda_color_state *color_st,
  

• 	    struct drm_plane_state *plane_st)
  

+{

• int err;
• if (komeda_component_is_new_active(c, plane_st->state))

• plane_st->color_mgmt_changed = true;
  

• if (!plane_st->color_mgmt_changed)

• return 0;
  

• err = komeda_color_validate(color_mgr, color_st,

• 		    plane_st->degamma_lut,
  

• 		    plane_st->gamma_lut);
  

• if (err)

• DRM_DEBUG_ATOMIC("%s validate color failed.\n", c->name);
  

• return err;

+}

static int komeda_layer_check_cfg(struct komeda_layer *layer, struct komeda_fb *kfb, @@ -362,6 +400,11 @@ komeda_layer_validate(struct komeda_layer *layer, st->addr[i] = komeda_fb_get_pixel_addr(kfb, dflow->in_x, dflow->in_y, i);

• err = komeda_validate_plane_color(&layer->base, &layer->color_mgr,

• 			  &st->color_st, plane_st);
  

• if (err)

• return err;
  

• err = komeda_component_validate_private(&layer->base, c_st); if (err) return err;

@@ -1177,7 +1220,7 @@ komeda_pipeline_unbound_components(struct komeda_pipeline *pipe, { struct drm_atomic_state *drm_st = new->obj.state; struct komeda_pipeline_state *old = priv_to_pipe_st(pipe->obj.state);

• struct komeda_component_state *c_st;

• struct komeda_component_state *st; struct komeda_component *c; u32 disabling_comps, id;

@@ -1188,9 +1231,13 @@ komeda_pipeline_unbound_components(struct komeda_pipeline *pipe, /* unbound all disabling component */ dp_for_each_set_bit(id, disabling_comps) { c = komeda_pipeline_get_component(pipe, id);

• c_st = komeda_component_get_state_and_set_user(c,
  

• st = komeda_component_get_state_and_set_user(c,
  	drm_st, NULL, new->crtc);
  

• WARN_ON(IS_ERR(c_st));
  

• WARN_ON(IS_ERR(st));
  

I think this needs to be: if (WARN_ON(IS_ERR(st))) continue; ...because...

• if (has_bit(id, KOMEDA_PIPELINE_LAYERS))
  

• 	komeda_color_cleanup_state(&to_layer_st(st)->color_st);
  

... this may deref an invalid `st' otherwise.

} }

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c index 98e915e325dd..69fa1dea41c9 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c @@ -11,6 +11,7 @@ #include "komeda_dev.h" #include "komeda_kms.h" #include "komeda_framebuffer.h" +#include "komeda_color_mgmt.h"

static int komeda_plane_init_data_flow(struct drm_plane_state *st, @@ -250,6 +251,7 @@ static int komeda_plane_add(struct komeda_kms_dev *kms, { struct komeda_dev *mdev = kms->base.dev_private; struct komeda_component *c = &layer->base;

• struct komeda_color_manager *color_mgr; struct komeda_plane *kplane; struct drm_plane *plane; u32 *formats, n_formats = 0;

@@ -306,6 +308,16 @@ static int komeda_plane_add(struct komeda_kms_dev *kms, if (err) goto cleanup;

• err = drm_plane_color_create_prop(plane->dev, plane);
• if (err)

• goto cleanup;
  

• color_mgr = &layer->color_mgr;
• drm_plane_enable_color_mgmt(plane,

• 	color_mgr->igamma_mgr ? KOMEDA_COLOR_LUT_SIZE : 0,
  

• 	color_mgr->has_ctm,
  

• 	color_mgr->fgamma_mgr ? KOMEDA_COLOR_LUT_SIZE : 0);
  

• err = drm_plane_create_zpos_property(plane, layer->base.id, 0, 8); if (err) goto cleanup;

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c b/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c index 914400c4af73..4c474663f554 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_private_obj.c @@ -19,12 +19,15 @@ komeda_component_state_reset(struct komeda_component_state *st) static struct drm_private_state * komeda_layer_atomic_duplicate_state(struct drm_private_obj *obj) {

• struct komeda_layer_state *old = to_layer_st(priv_to_comp_st(obj->state)); struct komeda_layer_state *st;

  st = kmemdup(obj->state, sizeof(*st), GFP_KERNEL); if (!st) return NULL;

• komeda_color_duplicate_state(&st->color_st, &old->color_st);

• komeda_component_state_reset(&st->base); __drm_atomic_helper_private_obj_duplicate_state(obj, &st->base.obj);

@@ -37,6 +40,7 @@ komeda_layer_atomic_destroy_state(struct drm_private_obj *obj, { struct komeda_layer_state *st = to_layer_st(priv_to_comp_st(state));

• komeda_color_cleanup_state(&st->color_st); kfree(st);

}

BR, Mihail