[PATCH v2 4/5] drm/exynos: mixer: do blending setup in mixer_cfg_layer()

[Inki Dae]

2015-11-24 2:44 GMT+09:00 Tobias Jakobi <tjakobi at math.uni-bielefeld.de>:
> Hey Inki,
>
>
> Inki Dae wrote:
>>
>>
>> 2015년 11월 23일 01:09에 Tobias Jakobi 이(가) 쓴 글:
>>> This updates the blending setup when the layer configuration
>>> changes (triggered by mixer_win_{commit,disable}).
>>>
>>> To avoid unnecesary reconfigurations we cache the layer
>>> state in the mixer context.
>>>
>>> Extra care has to be taken for the layer that is currently
>>> being enabled/disabled.
>>>
>>> Signed-off-by: Tobias Jakobi <tjakobi at math.uni-bielefeld.de>
>>> ---
>>>  drivers/gpu/drm/exynos/exynos_mixer.c | 41 +++++++++++++++++++++++++++++++++--
>>>  1 file changed, 39 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
>>> index ec9659e..1c24fb5 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
>>> @@ -99,6 +99,7 @@ struct mixer_context {
>>>      struct exynos_drm_plane planes[MIXER_WIN_NR];
>>>      const struct layer_cfg  *layer_cfg;
>>>      unsigned int            num_layer;
>>> +    u32                     layer_state;
>>>      int                     pipe;
>>>      unsigned long           flags;
>>>      bool                    interlace;
>>> @@ -189,6 +190,27 @@ static inline bool is_alpha_format(const struct mixer_context* ctx, unsigned int
>>>      }
>>>  }
>>>
>>> +static inline u32 get_layer_state(const struct mixer_context *ctx,
>>> +    unsigned int win, bool enable)
>>> +{
>>> +    u32 enable_state, alpha_state;
>>> +
>>> +    enable_state = ctx->layer_state & 0xffff;
>>> +    alpha_state = ctx->layer_state >> 16;
>>> +
>>> +    if (enable)
>>> +            enable_state |= (1 << win);
>>> +    else
>>> +            enable_state &= ~(1 << win);
>>> +
>>> +    if (enable && is_alpha_format(ctx, win))
>>> +            alpha_state |= (1 << win);
>>> +    else
>>> +            alpha_state &= ~(1 << win);
>>> +
>>> +    return ((alpha_state << 16) | enable_state);
>>> +}
>>> +
>>>  static inline u32 vp_reg_read(struct mixer_resources *res, u32 reg_id)
>>>  {
>>>      return readl(res->vp_regs + reg_id);
>>> @@ -370,8 +392,9 @@ static void mixer_general_layer(struct mixer_context *ctx,
>>>  {
>>>      u32 val;
>>>      struct mixer_resources *res = &ctx->mixer_res;
>>> +    const u32 alpha_state = ctx->layer_state >> 16;
>>>
>>> -    if (is_alpha_format(ctx, cfg->index)) {
>>> +    if (alpha_state & (1 << cfg->index)) {
>>>              val  = MXR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */
>>>              val |= MXR_GRP_CFG_BLEND_PRE_MUL;
>>>              val |= MXR_GRP_CFG_PIXEL_BLEND_EN; /* blending based on pixel alpha */
>>> @@ -397,10 +420,11 @@ static void mixer_general_layer(struct mixer_context *ctx,
>>>      }
>>>  }
>>>
>>> -static void mixer_layer_blending(struct mixer_context *ctx, unsigned int enable_state)
>>> +static void mixer_layer_blending(struct mixer_context *ctx)
>>>  {
>>>      unsigned int i, index;
>>>      bool bottom_layer = false;
>>> +    const u32 enable_state = ctx->layer_state & 0xffff;
>>>
>>>      for (i = 0; i < ctx->num_layer; ++i) {
>>>              index = ctx->layer_cfg[i].index;
>>> @@ -503,8 +527,19 @@ static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win,
>>>                              bool enable)
>>>  {
>>>      struct mixer_resources *res = &ctx->mixer_res;
>>> +    u32 new_layer_state;
>>>      u32 val = enable ? ~0 : 0;
>>>
>>> +    new_layer_state = get_layer_state(ctx, win, enable);
>>> +    if (new_layer_state == ctx->layer_state)
>>> +            return;
>>> +
>>> +    /*
>>> +     * Update the layer state so that mixer_layer_blending()
>>> +     * below can use it.
>>> +     */
>>> +    ctx->layer_state = new_layer_state;
>>
>> It may be trivial but I think it'd be better to move above line to most bottom of this function.
> Sure, I can do that, but I would rather know what's the rationale here.

Very simple. Above line sets layer_state to new_layer_state logically
but physically not.
So it'd be reasonable to update layer state after physically setting
the mixer layer.

Thanks,
Inki Dae

>
>
> With best wishes,
> Tobias
>
>>> +
>>>      switch (win) {
>>>      case 0:
>>>              mixer_reg_writemask(res, MXR_CFG, val, MXR_CFG_GRP0_ENABLE);
>>> @@ -520,6 +555,8 @@ static void mixer_cfg_layer(struct mixer_context *ctx, unsigned int win,
>>>              }
>>>              break;
>>>      }
>>> +
>>> +    mixer_layer_blending(ctx);
>>
>> Here.
>>
>> Thanks,
>> Inki Dae
>>
>>>  }
>>>
>>>  static void mixer_run(struct mixer_context *ctx)
>>>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel