| Message ID | 1448208584-6621-2-git-send-email-tjakobi@math.uni-bielefeld.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tobias, 2015? 11? 23? 01:09? Tobias Jakobi ?(?) ? ?: > First step in allowing a more generic way to setup complex > blending for the different layers. > > Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> > --- > drivers/gpu/drm/exynos/exynos_mixer.c | 84 ++++++++++++++++++++++++++++++----- > 1 file changed, 73 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c > index 7498c6e..2c1cea3 100644 > --- a/drivers/gpu/drm/exynos/exynos_mixer.c > +++ b/drivers/gpu/drm/exynos/exynos_mixer.c > @@ -63,6 +63,11 @@ struct mixer_resources { > struct clk *mout_mixer; > }; > > +struct layer_cfg { > + unsigned int index; > + unsigned int priority; > +}; > + > enum mixer_version_id { > MXR_VER_0_0_0_16, > MXR_VER_16_0_33_0, > @@ -92,6 +97,8 @@ struct mixer_context { > struct drm_device *drm_dev; > struct exynos_drm_crtc *crtc; > struct exynos_drm_plane planes[MIXER_WIN_NR]; > + const struct layer_cfg *layer_cfg; > + unsigned int num_layer; > int pipe; > unsigned long flags; > bool interlace; > @@ -110,6 +117,34 @@ struct mixer_drv_data { > bool has_sclk; > }; > > +/* > + * The default layer priorities for non-VP (video processor) > + * and VP mixer configurations. > + * > + * A higher priority means that the layer is at the top of > + * the layer stack. > + * Configurations have to be specified with its entries > + * sorted with increasing priority. > + * > + * The default config assumes the following usage scenario: > + * layer1: OSD [top] > + * layer0: main framebuffer > + * video layer: video overlay [bottom] > + * Note that the video layer is only usable when the > + * VP is available. > + */ > + > +static const struct layer_cfg nonvp_default_cfg[] = { > + { .index = 0, .priority = 1 }, /* layer0 */ > + { .index = 1, .priority = 2 }, /* layer1 */ > +}; > + > +static const struct layer_cfg vp_default_cfg[] = { > + { .index = 2, .priority = 1 }, /* video layer */ > + { .index = 0, .priority = 2 }, /* layer0 */ > + { .index = 1, .priority = 3 }, /* layer1 */ > +}; > + > static const u8 filter_y_horiz_tap8[] = { > 0, -1, -1, -1, -1, -1, -1, -1, > -1, -1, -1, -1, -1, 0, 0, 0, > @@ -268,6 +303,34 @@ static void vp_default_filter(struct mixer_resources *res) > filter_cr_horiz_tap4, sizeof(filter_cr_horiz_tap4)); > } > > +static void mixer_layer_priority(struct mixer_context *ctx) > +{ > + u32 val = 0; > + unsigned int i, priority; > + > + for (i = 0; i < ctx->num_layer; ++i) { > + priority = ctx->layer_cfg[i].priority; > + BUG_ON(priority > 15); What doesn constant, 15 mean? You need to clarify the meaning and please, use a macro instead. And it'd better to just return in this case so that the layer configuration can be set with a default value. I think it'd be enough to print out warning message. Thanks, Inki Dae > + > + switch (ctx->layer_cfg[i].index) { > + case 0: > + val |= MXR_LAYER_CFG_GRP0_VAL(priority); > + break; > + case 1: > + val |= MXR_LAYER_CFG_GRP1_VAL(priority); > + break; > + case 2: > + val |= MXR_LAYER_CFG_VP_VAL(priority); > + break; > + default: > + BUG_ON(true); > + break; > + } > + } > + > + mixer_reg_write(&ctx->mixer_res, MXR_LAYER_CFG, val); > +} > + > static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable) > { > struct mixer_resources *res = &ctx->mixer_res; > @@ -673,17 +736,7 @@ static void mixer_win_reset(struct mixer_context *ctx) > mixer_reg_writemask(res, MXR_STATUS, MXR_STATUS_16_BURST, > MXR_STATUS_BURST_MASK); > > - /* setting default layer priority: layer1 > layer0 > video > - * because typical usage scenario would be > - * layer1 - OSD > - * layer0 - framebuffer > - * video - video overlay > - */ > - val = MXR_LAYER_CFG_GRP1_VAL(3); > - val |= MXR_LAYER_CFG_GRP0_VAL(2); > - if (ctx->vp_enabled) > - val |= MXR_LAYER_CFG_VP_VAL(1); > - mixer_reg_write(res, MXR_LAYER_CFG, val); > + mixer_layer_priority(ctx); > > /* setting background color */ > mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); > @@ -1209,6 +1262,15 @@ static int mixer_probe(struct platform_device *pdev) > ctx->vp_enabled = drv->is_vp_enabled; > ctx->has_sclk = drv->has_sclk; > ctx->mxr_ver = drv->version; > + > + if (drv->is_vp_enabled) { > + ctx->layer_cfg = vp_default_cfg; > + ctx->num_layer = ARRAY_SIZE(vp_default_cfg); > + } else { > + ctx->layer_cfg = nonvp_default_cfg; > + ctx->num_layer = ARRAY_SIZE(nonvp_default_cfg); > + } > + > init_waitqueue_head(&ctx->wait_vsync_queue); > atomic_set(&ctx->wait_vsync_event, 0); > >
Hey Inki, Inki Dae wrote: > Hi Tobias, > > 2015? 11? 23? 01:09? Tobias Jakobi ?(?) ? ?: >> First step in allowing a more generic way to setup complex >> blending for the different layers. >> >> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> >> --- >> drivers/gpu/drm/exynos/exynos_mixer.c | 84 ++++++++++++++++++++++++++++++----- >> 1 file changed, 73 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c >> index 7498c6e..2c1cea3 100644 >> --- a/drivers/gpu/drm/exynos/exynos_mixer.c >> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c >> @@ -63,6 +63,11 @@ struct mixer_resources { >> struct clk *mout_mixer; >> }; >> >> +struct layer_cfg { >> + unsigned int index; >> + unsigned int priority; >> +}; >> + >> enum mixer_version_id { >> MXR_VER_0_0_0_16, >> MXR_VER_16_0_33_0, >> @@ -92,6 +97,8 @@ struct mixer_context { >> struct drm_device *drm_dev; >> struct exynos_drm_crtc *crtc; >> struct exynos_drm_plane planes[MIXER_WIN_NR]; >> + const struct layer_cfg *layer_cfg; >> + unsigned int num_layer; >> int pipe; >> unsigned long flags; >> bool interlace; >> @@ -110,6 +117,34 @@ struct mixer_drv_data { >> bool has_sclk; >> }; >> >> +/* >> + * The default layer priorities for non-VP (video processor) >> + * and VP mixer configurations. >> + * >> + * A higher priority means that the layer is at the top of >> + * the layer stack. >> + * Configurations have to be specified with its entries >> + * sorted with increasing priority. >> + * >> + * The default config assumes the following usage scenario: >> + * layer1: OSD [top] >> + * layer0: main framebuffer >> + * video layer: video overlay [bottom] >> + * Note that the video layer is only usable when the >> + * VP is available. >> + */ >> + >> +static const struct layer_cfg nonvp_default_cfg[] = { >> + { .index = 0, .priority = 1 }, /* layer0 */ >> + { .index = 1, .priority = 2 }, /* layer1 */ >> +}; >> + >> +static const struct layer_cfg vp_default_cfg[] = { >> + { .index = 2, .priority = 1 }, /* video layer */ >> + { .index = 0, .priority = 2 }, /* layer0 */ >> + { .index = 1, .priority = 3 }, /* layer1 */ >> +}; >> + >> static const u8 filter_y_horiz_tap8[] = { >> 0, -1, -1, -1, -1, -1, -1, -1, >> -1, -1, -1, -1, -1, 0, 0, 0, >> @@ -268,6 +303,34 @@ static void vp_default_filter(struct mixer_resources *res) >> filter_cr_horiz_tap4, sizeof(filter_cr_horiz_tap4)); >> } >> >> +static void mixer_layer_priority(struct mixer_context *ctx) >> +{ >> + u32 val = 0; >> + unsigned int i, priority; >> + >> + for (i = 0; i < ctx->num_layer; ++i) { >> + priority = ctx->layer_cfg[i].priority; >> + BUG_ON(priority > 15); > > What doesn constant, 15 mean? You need to clarify the meaning > and please, use a macro instead. If I read the specs correctly the layer priority is encoded with just 4-bits in the corresponding register. I'll add a define to make this more clear. > And it'd better to just return in this case so that the layer > configuration can be set with a default value. The point is that these (currently) are the default values that mixer_layer_priority() sets. The BUG_ON() is there to make sure that nobody changes {vp,nonvp}_default_cfg[] later with illegal values. With best wishes, Tobias > I think it'd be enough to print out warning message. > > Thanks, > Inki Dae > >> + >> + switch (ctx->layer_cfg[i].index) { >> + case 0: >> + val |= MXR_LAYER_CFG_GRP0_VAL(priority); >> + break; >> + case 1: >> + val |= MXR_LAYER_CFG_GRP1_VAL(priority); >> + break; >> + case 2: >> + val |= MXR_LAYER_CFG_VP_VAL(priority); >> + break; >> + default: >> + BUG_ON(true); >> + break; >> + } >> + } >> + >> + mixer_reg_write(&ctx->mixer_res, MXR_LAYER_CFG, val); >> +} >> + >> static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable) >> { >> struct mixer_resources *res = &ctx->mixer_res; >> @@ -673,17 +736,7 @@ static void mixer_win_reset(struct mixer_context *ctx) >> mixer_reg_writemask(res, MXR_STATUS, MXR_STATUS_16_BURST, >> MXR_STATUS_BURST_MASK); >> >> - /* setting default layer priority: layer1 > layer0 > video >> - * because typical usage scenario would be >> - * layer1 - OSD >> - * layer0 - framebuffer >> - * video - video overlay >> - */ >> - val = MXR_LAYER_CFG_GRP1_VAL(3); >> - val |= MXR_LAYER_CFG_GRP0_VAL(2); >> - if (ctx->vp_enabled) >> - val |= MXR_LAYER_CFG_VP_VAL(1); >> - mixer_reg_write(res, MXR_LAYER_CFG, val); >> + mixer_layer_priority(ctx); >> >> /* setting background color */ >> mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); >> @@ -1209,6 +1262,15 @@ static int mixer_probe(struct platform_device *pdev) >> ctx->vp_enabled = drv->is_vp_enabled; >> ctx->has_sclk = drv->has_sclk; >> ctx->mxr_ver = drv->version; >> + >> + if (drv->is_vp_enabled) { >> + ctx->layer_cfg = vp_default_cfg; >> + ctx->num_layer = ARRAY_SIZE(vp_default_cfg); >> + } else { >> + ctx->layer_cfg = nonvp_default_cfg; >> + ctx->num_layer = ARRAY_SIZE(nonvp_default_cfg); >> + } >> + >> init_waitqueue_head(&ctx->wait_vsync_queue); >> atomic_set(&ctx->wait_vsync_event, 0); >> >>
2015-11-24 2:44 GMT+09:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>: > Hey Inki, > > > Inki Dae wrote: >> Hi Tobias, >> >> 2015? 11? 23? 01:09? Tobias Jakobi ?(?) ? ?: >>> First step in allowing a more generic way to setup complex >>> blending for the different layers. >>> >>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> >>> --- >>> drivers/gpu/drm/exynos/exynos_mixer.c | 84 ++++++++++++++++++++++++++++++----- >>> 1 file changed, 73 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c >>> index 7498c6e..2c1cea3 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c >>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c >>> @@ -63,6 +63,11 @@ struct mixer_resources { >>> struct clk *mout_mixer; >>> }; >>> >>> +struct layer_cfg { >>> + unsigned int index; >>> + unsigned int priority; >>> +}; >>> + >>> enum mixer_version_id { >>> MXR_VER_0_0_0_16, >>> MXR_VER_16_0_33_0, >>> @@ -92,6 +97,8 @@ struct mixer_context { >>> struct drm_device *drm_dev; >>> struct exynos_drm_crtc *crtc; >>> struct exynos_drm_plane planes[MIXER_WIN_NR]; >>> + const struct layer_cfg *layer_cfg; >>> + unsigned int num_layer; >>> int pipe; >>> unsigned long flags; >>> bool interlace; >>> @@ -110,6 +117,34 @@ struct mixer_drv_data { >>> bool has_sclk; >>> }; >>> >>> +/* >>> + * The default layer priorities for non-VP (video processor) >>> + * and VP mixer configurations. >>> + * >>> + * A higher priority means that the layer is at the top of >>> + * the layer stack. >>> + * Configurations have to be specified with its entries >>> + * sorted with increasing priority. >>> + * >>> + * The default config assumes the following usage scenario: >>> + * layer1: OSD [top] >>> + * layer0: main framebuffer >>> + * video layer: video overlay [bottom] >>> + * Note that the video layer is only usable when the >>> + * VP is available. >>> + */ >>> + >>> +static const struct layer_cfg nonvp_default_cfg[] = { >>> + { .index = 0, .priority = 1 }, /* layer0 */ >>> + { .index = 1, .priority = 2 }, /* layer1 */ >>> +}; >>> + >>> +static const struct layer_cfg vp_default_cfg[] = { >>> + { .index = 2, .priority = 1 }, /* video layer */ >>> + { .index = 0, .priority = 2 }, /* layer0 */ >>> + { .index = 1, .priority = 3 }, /* layer1 */ >>> +}; >>> + >>> static const u8 filter_y_horiz_tap8[] = { >>> 0, -1, -1, -1, -1, -1, -1, -1, >>> -1, -1, -1, -1, -1, 0, 0, 0, >>> @@ -268,6 +303,34 @@ static void vp_default_filter(struct mixer_resources *res) >>> filter_cr_horiz_tap4, sizeof(filter_cr_horiz_tap4)); >>> } >>> >>> +static void mixer_layer_priority(struct mixer_context *ctx) >>> +{ >>> + u32 val = 0; >>> + unsigned int i, priority; >>> + >>> + for (i = 0; i < ctx->num_layer; ++i) { >>> + priority = ctx->layer_cfg[i].priority; >>> + BUG_ON(priority > 15); >> >> What doesn constant, 15 mean? You need to clarify the meaning >> and please, use a macro instead. > If I read the specs correctly the layer priority is encoded with just > 4-bits in the corresponding register. I'll add a define to make this > more clear. Please use a macro instead of 15. > > >> And it'd better to just return in this case so that the layer >> configuration can be set with a default value. > The point is that these (currently) are the default values that > mixer_layer_priority() sets. > The BUG_ON() is there to make sure that nobody changes > {vp,nonvp}_default_cfg[] later with illegal values. Please, know that BUG_ON is very strong macro so kernel will be halt. By any chance, if someone changed the definitions, then it'd be better to return error so that other devices could work correctly. Thanks, Inki Dae > > > With best wishes, > Tobias > > >> I think it'd be enough to print out warning message. >> >> Thanks, >> Inki Dae >> >>> + >>> + switch (ctx->layer_cfg[i].index) { >>> + case 0: >>> + val |= MXR_LAYER_CFG_GRP0_VAL(priority); >>> + break; >>> + case 1: >>> + val |= MXR_LAYER_CFG_GRP1_VAL(priority); >>> + break; >>> + case 2: >>> + val |= MXR_LAYER_CFG_VP_VAL(priority); >>> + break; >>> + default: >>> + BUG_ON(true); >>> + break; >>> + } >>> + } >>> + >>> + mixer_reg_write(&ctx->mixer_res, MXR_LAYER_CFG, val); >>> +} >>> + >>> static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable) >>> { >>> struct mixer_resources *res = &ctx->mixer_res; >>> @@ -673,17 +736,7 @@ static void mixer_win_reset(struct mixer_context *ctx) >>> mixer_reg_writemask(res, MXR_STATUS, MXR_STATUS_16_BURST, >>> MXR_STATUS_BURST_MASK); >>> >>> - /* setting default layer priority: layer1 > layer0 > video >>> - * because typical usage scenario would be >>> - * layer1 - OSD >>> - * layer0 - framebuffer >>> - * video - video overlay >>> - */ >>> - val = MXR_LAYER_CFG_GRP1_VAL(3); >>> - val |= MXR_LAYER_CFG_GRP0_VAL(2); >>> - if (ctx->vp_enabled) >>> - val |= MXR_LAYER_CFG_VP_VAL(1); >>> - mixer_reg_write(res, MXR_LAYER_CFG, val); >>> + mixer_layer_priority(ctx); >>> >>> /* setting background color */ >>> mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); >>> @@ -1209,6 +1262,15 @@ static int mixer_probe(struct platform_device *pdev) >>> ctx->vp_enabled = drv->is_vp_enabled; >>> ctx->has_sclk = drv->has_sclk; >>> ctx->mxr_ver = drv->version; >>> + >>> + if (drv->is_vp_enabled) { >>> + ctx->layer_cfg = vp_default_cfg; >>> + ctx->num_layer = ARRAY_SIZE(vp_default_cfg); >>> + } else { >>> + ctx->layer_cfg = nonvp_default_cfg; >>> + ctx->num_layer = ARRAY_SIZE(nonvp_default_cfg); >>> + } >>> + >>> init_waitqueue_head(&ctx->wait_vsync_queue); >>> atomic_set(&ctx->wait_vsync_event, 0); >>> >>> > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 7498c6e..2c1cea3 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -63,6 +63,11 @@ struct mixer_resources { struct clk *mout_mixer; }; +struct layer_cfg { + unsigned int index; + unsigned int priority; +}; + enum mixer_version_id { MXR_VER_0_0_0_16, MXR_VER_16_0_33_0, @@ -92,6 +97,8 @@ struct mixer_context { struct drm_device *drm_dev; struct exynos_drm_crtc *crtc; struct exynos_drm_plane planes[MIXER_WIN_NR]; + const struct layer_cfg *layer_cfg; + unsigned int num_layer; int pipe; unsigned long flags; bool interlace; @@ -110,6 +117,34 @@ struct mixer_drv_data { bool has_sclk; }; +/* + * The default layer priorities for non-VP (video processor) + * and VP mixer configurations. + * + * A higher priority means that the layer is at the top of + * the layer stack. + * Configurations have to be specified with its entries + * sorted with increasing priority. + * + * The default config assumes the following usage scenario: + * layer1: OSD [top] + * layer0: main framebuffer + * video layer: video overlay [bottom] + * Note that the video layer is only usable when the + * VP is available. + */ + +static const struct layer_cfg nonvp_default_cfg[] = { + { .index = 0, .priority = 1 }, /* layer0 */ + { .index = 1, .priority = 2 }, /* layer1 */ +}; + +static const struct layer_cfg vp_default_cfg[] = { + { .index = 2, .priority = 1 }, /* video layer */ + { .index = 0, .priority = 2 }, /* layer0 */ + { .index = 1, .priority = 3 }, /* layer1 */ +}; + static const u8 filter_y_horiz_tap8[] = { 0, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 0, 0, 0, @@ -268,6 +303,34 @@ static void vp_default_filter(struct mixer_resources *res) filter_cr_horiz_tap4, sizeof(filter_cr_horiz_tap4)); } +static void mixer_layer_priority(struct mixer_context *ctx) +{ + u32 val = 0; + unsigned int i, priority; + + for (i = 0; i < ctx->num_layer; ++i) { + priority = ctx->layer_cfg[i].priority; + BUG_ON(priority > 15); + + switch (ctx->layer_cfg[i].index) { + case 0: + val |= MXR_LAYER_CFG_GRP0_VAL(priority); + break; + case 1: + val |= MXR_LAYER_CFG_GRP1_VAL(priority); + break; + case 2: + val |= MXR_LAYER_CFG_VP_VAL(priority); + break; + default: + BUG_ON(true); + break; + } + } + + mixer_reg_write(&ctx->mixer_res, MXR_LAYER_CFG, val); +} + static void mixer_vsync_set_update(struct mixer_context *ctx, bool enable) { struct mixer_resources *res = &ctx->mixer_res; @@ -673,17 +736,7 @@ static void mixer_win_reset(struct mixer_context *ctx) mixer_reg_writemask(res, MXR_STATUS, MXR_STATUS_16_BURST, MXR_STATUS_BURST_MASK); - /* setting default layer priority: layer1 > layer0 > video - * because typical usage scenario would be - * layer1 - OSD - * layer0 - framebuffer - * video - video overlay - */ - val = MXR_LAYER_CFG_GRP1_VAL(3); - val |= MXR_LAYER_CFG_GRP0_VAL(2); - if (ctx->vp_enabled) - val |= MXR_LAYER_CFG_VP_VAL(1); - mixer_reg_write(res, MXR_LAYER_CFG, val); + mixer_layer_priority(ctx); /* setting background color */ mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); @@ -1209,6 +1262,15 @@ static int mixer_probe(struct platform_device *pdev) ctx->vp_enabled = drv->is_vp_enabled; ctx->has_sclk = drv->has_sclk; ctx->mxr_ver = drv->version; + + if (drv->is_vp_enabled) { + ctx->layer_cfg = vp_default_cfg; + ctx->num_layer = ARRAY_SIZE(vp_default_cfg); + } else { + ctx->layer_cfg = nonvp_default_cfg; + ctx->num_layer = ARRAY_SIZE(nonvp_default_cfg); + } + init_waitqueue_head(&ctx->wait_vsync_queue); atomic_set(&ctx->wait_vsync_event, 0);
First step in allowing a more generic way to setup complex blending for the different layers. Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> --- drivers/gpu/drm/exynos/exynos_mixer.c | 84 ++++++++++++++++++++++++++++++----- 1 file changed, 73 insertions(+), 11 deletions(-)