| Message ID | 1448208584-6621-5-git-send-email-tjakobi@math.uni-bielefeld.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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. > + > 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) > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@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. 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) >> -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2015-11-24 2:44 GMT+09:00 Tobias Jakobi <tjakobi@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@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Inki, my main system which I also used for development was stolen on Tuesday, so I won't be working on this series anytime soon. If anyone wants to pick it up, please go ahead. - Tobias Inki Dae wrote: > 2015-11-24 2:44 GMT+09:00 Tobias Jakobi <tjakobi@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@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 =tx->layer_state & 0xffff; >>>> + alpha_state =tx->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 =tx->layer_state >> 16; >>>> >>>> - if (is_alpha_format(ctx, cfg->index)) { >>>> + if (alpha_state & (1 << cfg->index)) { >>>> val =XR_GRP_CFG_COLOR_KEY_DISABLE; /* no blank key */ >>>> val |=XR_GRP_CFG_BLEND_PRE_MUL; >>>> val |=XR_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 =alse; >>>> + const u32 enable_state =tx->layer_state & 0xffff; >>>> >>>> for (i =; i < ctx->num_layer; ++i) { >>>> index =tx->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 =nable ? ~0 : 0; >>>> >>>> + new_layer_state =et_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 =ew_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@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/dri-devel > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; + 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); } static void mixer_run(struct mixer_context *ctx)
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@math.uni-bielefeld.de> --- drivers/gpu/drm/exynos/exynos_mixer.c | 41 +++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-)