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) 
> <[email protected]>
> ---
>  .../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



Reply via email to