| Message ID | 1489155714-13066-1-git-send-email-tjakobi@math.uni-bielefeld.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Gentle ping. - Tobias Tobias Jakobi wrote: > The output stage of the mixer uses YCbCr for the internal > computations, which is the reason that some registers take > YCbCr related data as input. In particular this applies > to MXR_BG_COLOR{0,1,2} and MXR_CM_COEFF_{Y,CB,CR}. > > Document the formatting of the data which we write to > these registers. > > While at it, unify wording of comments in the register header. > > Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> > Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> > --- > Changes in v2: > - use floating point values as input for the macros, as > suggested by Andrzej > - the floating point values have been tuned to exactly match > the values that are currently used > > Changes in v3: > - use only three digit values (pointed out by Andrzej) > > drivers/gpu/drm/exynos/exynos_mixer.c | 33 +++++++++++++++++++++++++-------- > drivers/gpu/drm/exynos/regs-mixer.h | 7 +++++-- > 2 files changed, 30 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c > index 41d0c36..9648dd5 100644 > --- a/drivers/gpu/drm/exynos/exynos_mixer.c > +++ b/drivers/gpu/drm/exynos/exynos_mixer.c > @@ -45,6 +45,22 @@ > #define MIXER_WIN_NR 3 > #define VP_DEFAULT_WIN 2 > > +/* > + * Mixer color space conversion coefficient triplet. > + * Used for CSC from RGB to YCbCr. > + * Each coefficient is a 10-bit fixed point number with > + * sign and no integer part, i.e. > + * [0:8] = fractional part (representing a value y = x / 2^9) > + * [9] = sign > + * Negative values are encoded with two's complement. > + */ > +#define MXR_CSC_C(x) ((int)((x) * 512.0) & 0x3ff) > +#define MXR_CSC_CT(a0, a1, a2) \ > + ((MXR_CSC_C(a0) << 20) | (MXR_CSC_C(a1) << 10) | (MXR_CSC_C(a2) << 0)) > + > +/* YCbCr value, used for mixer background color configuration. */ > +#define MXR_YCBCR_VAL(y, cb, cr) (((y) << 16) | ((cb) << 8) | ((cr) << 0)) > + > /* The pixelformats that are natively supported by the mixer. */ > #define MXR_FORMAT_RGB565 4 > #define MXR_FORMAT_ARGB1555 5 > @@ -391,13 +407,14 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height) > case 1080: > default: > val = MXR_CFG_RGB709_16_235; > + /* Configure the BT.709 CSC matrix for full range RGB. */ > mixer_reg_write(res, MXR_CM_COEFF_Y, > - (1 << 30) | (94 << 20) | (314 << 10) | > - (32 << 0)); > + MXR_CSC_CT( 0.184, 0.614, 0.063) | > + MXR_CM_COEFF_RGB_FULL); > mixer_reg_write(res, MXR_CM_COEFF_CB, > - (972 << 20) | (851 << 10) | (225 << 0)); > + MXR_CSC_CT(-0.102, -0.338, 0.440)); > mixer_reg_write(res, MXR_CM_COEFF_CR, > - (225 << 20) | (820 << 10) | (1004 << 0)); > + MXR_CSC_CT( 0.440, -0.399, -0.040)); > break; > } > > @@ -715,10 +732,10 @@ static void mixer_win_reset(struct mixer_context *ctx) > /* reset default layer priority */ > mixer_reg_write(res, MXR_LAYER_CFG, 0); > > - /* setting background color */ > - mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); > - mixer_reg_write(res, MXR_BG_COLOR1, 0x008080); > - mixer_reg_write(res, MXR_BG_COLOR2, 0x008080); > + /* set all background colors to RGB (0,0,0) */ > + mixer_reg_write(res, MXR_BG_COLOR0, MXR_YCBCR_VAL(0, 128, 128)); > + mixer_reg_write(res, MXR_BG_COLOR1, MXR_YCBCR_VAL(0, 128, 128)); > + mixer_reg_write(res, MXR_BG_COLOR2, MXR_YCBCR_VAL(0, 128, 128)); > > if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) { > /* configuration of Video Processor Registers */ > diff --git a/drivers/gpu/drm/exynos/regs-mixer.h b/drivers/gpu/drm/exynos/regs-mixer.h > index 7f22df5..c311f57 100644 > --- a/drivers/gpu/drm/exynos/regs-mixer.h > +++ b/drivers/gpu/drm/exynos/regs-mixer.h > @@ -140,11 +140,11 @@ > #define MXR_INT_EN_VSYNC (1 << 11) > #define MXR_INT_EN_ALL (0x0f << 8) > > -/* bit for MXR_INT_STATUS */ > +/* bits for MXR_INT_STATUS */ > #define MXR_INT_CLEAR_VSYNC (1 << 11) > #define MXR_INT_STATUS_VSYNC (1 << 0) > > -/* bit for MXR_LAYER_CFG */ > +/* bits for MXR_LAYER_CFG */ > #define MXR_LAYER_CFG_GRP1_VAL(x) MXR_MASK_VAL(x, 11, 8) > #define MXR_LAYER_CFG_GRP1_MASK MXR_LAYER_CFG_GRP1_VAL(~0) > #define MXR_LAYER_CFG_GRP0_VAL(x) MXR_MASK_VAL(x, 7, 4) > @@ -152,5 +152,8 @@ > #define MXR_LAYER_CFG_VP_VAL(x) MXR_MASK_VAL(x, 3, 0) > #define MXR_LAYER_CFG_VP_MASK MXR_LAYER_CFG_VP_VAL(~0) > > +/* bits for MXR_CM_COEFF_Y */ > +#define MXR_CM_COEFF_RGB_FULL (1 << 30) > + > #endif /* SAMSUNG_REGS_MIXER_H */ > >
Hello Daniel, same question here. Patch doesn't introduce any functional changes (just adds code documentation), so can you merge it through drm-misc? With best wishes, Tobias Tobias Jakobi wrote: > The output stage of the mixer uses YCbCr for the internal > computations, which is the reason that some registers take > YCbCr related data as input. In particular this applies > to MXR_BG_COLOR{0,1,2} and MXR_CM_COEFF_{Y,CB,CR}. > > Document the formatting of the data which we write to > these registers. > > While at it, unify wording of comments in the register header. > > Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> > Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> > --- > Changes in v2: > - use floating point values as input for the macros, as > suggested by Andrzej > - the floating point values have been tuned to exactly match > the values that are currently used > > Changes in v3: > - use only three digit values (pointed out by Andrzej) > > drivers/gpu/drm/exynos/exynos_mixer.c | 33 +++++++++++++++++++++++++-------- > drivers/gpu/drm/exynos/regs-mixer.h | 7 +++++-- > 2 files changed, 30 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c > index 41d0c36..9648dd5 100644 > --- a/drivers/gpu/drm/exynos/exynos_mixer.c > +++ b/drivers/gpu/drm/exynos/exynos_mixer.c > @@ -45,6 +45,22 @@ > #define MIXER_WIN_NR 3 > #define VP_DEFAULT_WIN 2 > > +/* > + * Mixer color space conversion coefficient triplet. > + * Used for CSC from RGB to YCbCr. > + * Each coefficient is a 10-bit fixed point number with > + * sign and no integer part, i.e. > + * [0:8] = fractional part (representing a value y = x / 2^9) > + * [9] = sign > + * Negative values are encoded with two's complement. > + */ > +#define MXR_CSC_C(x) ((int)((x) * 512.0) & 0x3ff) > +#define MXR_CSC_CT(a0, a1, a2) \ > + ((MXR_CSC_C(a0) << 20) | (MXR_CSC_C(a1) << 10) | (MXR_CSC_C(a2) << 0)) > + > +/* YCbCr value, used for mixer background color configuration. */ > +#define MXR_YCBCR_VAL(y, cb, cr) (((y) << 16) | ((cb) << 8) | ((cr) << 0)) > + > /* The pixelformats that are natively supported by the mixer. */ > #define MXR_FORMAT_RGB565 4 > #define MXR_FORMAT_ARGB1555 5 > @@ -391,13 +407,14 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height) > case 1080: > default: > val = MXR_CFG_RGB709_16_235; > + /* Configure the BT.709 CSC matrix for full range RGB. */ > mixer_reg_write(res, MXR_CM_COEFF_Y, > - (1 << 30) | (94 << 20) | (314 << 10) | > - (32 << 0)); > + MXR_CSC_CT( 0.184, 0.614, 0.063) | > + MXR_CM_COEFF_RGB_FULL); > mixer_reg_write(res, MXR_CM_COEFF_CB, > - (972 << 20) | (851 << 10) | (225 << 0)); > + MXR_CSC_CT(-0.102, -0.338, 0.440)); > mixer_reg_write(res, MXR_CM_COEFF_CR, > - (225 << 20) | (820 << 10) | (1004 << 0)); > + MXR_CSC_CT( 0.440, -0.399, -0.040)); > break; > } > > @@ -715,10 +732,10 @@ static void mixer_win_reset(struct mixer_context *ctx) > /* reset default layer priority */ > mixer_reg_write(res, MXR_LAYER_CFG, 0); > > - /* setting background color */ > - mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); > - mixer_reg_write(res, MXR_BG_COLOR1, 0x008080); > - mixer_reg_write(res, MXR_BG_COLOR2, 0x008080); > + /* set all background colors to RGB (0,0,0) */ > + mixer_reg_write(res, MXR_BG_COLOR0, MXR_YCBCR_VAL(0, 128, 128)); > + mixer_reg_write(res, MXR_BG_COLOR1, MXR_YCBCR_VAL(0, 128, 128)); > + mixer_reg_write(res, MXR_BG_COLOR2, MXR_YCBCR_VAL(0, 128, 128)); > > if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) { > /* configuration of Video Processor Registers */ > diff --git a/drivers/gpu/drm/exynos/regs-mixer.h b/drivers/gpu/drm/exynos/regs-mixer.h > index 7f22df5..c311f57 100644 > --- a/drivers/gpu/drm/exynos/regs-mixer.h > +++ b/drivers/gpu/drm/exynos/regs-mixer.h > @@ -140,11 +140,11 @@ > #define MXR_INT_EN_VSYNC (1 << 11) > #define MXR_INT_EN_ALL (0x0f << 8) > > -/* bit for MXR_INT_STATUS */ > +/* bits for MXR_INT_STATUS */ > #define MXR_INT_CLEAR_VSYNC (1 << 11) > #define MXR_INT_STATUS_VSYNC (1 << 0) > > -/* bit for MXR_LAYER_CFG */ > +/* bits for MXR_LAYER_CFG */ > #define MXR_LAYER_CFG_GRP1_VAL(x) MXR_MASK_VAL(x, 11, 8) > #define MXR_LAYER_CFG_GRP1_MASK MXR_LAYER_CFG_GRP1_VAL(~0) > #define MXR_LAYER_CFG_GRP0_VAL(x) MXR_MASK_VAL(x, 7, 4) > @@ -152,5 +152,8 @@ > #define MXR_LAYER_CFG_VP_VAL(x) MXR_MASK_VAL(x, 3, 0) > #define MXR_LAYER_CFG_VP_MASK MXR_LAYER_CFG_VP_VAL(~0) > > +/* bits for MXR_CM_COEFF_Y */ > +#define MXR_CM_COEFF_RGB_FULL (1 << 30) > + > #endif /* SAMSUNG_REGS_MIXER_H */ > >
2017-03-29 20:56 GMT+09:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>: > Hello Daniel, > > same question here. Patch doesn't introduce any functional changes (just > adds code documentation), so can you merge it through drm-misc? > Sorry for late. Confirmed just now. I will check it on next Monday. Thanks, Inki Dae > With best wishes, > Tobias > > > Tobias Jakobi wrote: >> The output stage of the mixer uses YCbCr for the internal >> computations, which is the reason that some registers take >> YCbCr related data as input. In particular this applies >> to MXR_BG_COLOR{0,1,2} and MXR_CM_COEFF_{Y,CB,CR}. >> >> Document the formatting of the data which we write to >> these registers. >> >> While at it, unify wording of comments in the register header. >> >> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> >> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> >> --- >> Changes in v2: >> - use floating point values as input for the macros, as >> suggested by Andrzej >> - the floating point values have been tuned to exactly match >> the values that are currently used >> >> Changes in v3: >> - use only three digit values (pointed out by Andrzej) >> >> drivers/gpu/drm/exynos/exynos_mixer.c | 33 +++++++++++++++++++++++++-------- >> drivers/gpu/drm/exynos/regs-mixer.h | 7 +++++-- >> 2 files changed, 30 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c >> index 41d0c36..9648dd5 100644 >> --- a/drivers/gpu/drm/exynos/exynos_mixer.c >> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c >> @@ -45,6 +45,22 @@ >> #define MIXER_WIN_NR 3 >> #define VP_DEFAULT_WIN 2 >> >> +/* >> + * Mixer color space conversion coefficient triplet. >> + * Used for CSC from RGB to YCbCr. >> + * Each coefficient is a 10-bit fixed point number with >> + * sign and no integer part, i.e. >> + * [0:8] = fractional part (representing a value y = x / 2^9) >> + * [9] = sign >> + * Negative values are encoded with two's complement. >> + */ >> +#define MXR_CSC_C(x) ((int)((x) * 512.0) & 0x3ff) >> +#define MXR_CSC_CT(a0, a1, a2) \ >> + ((MXR_CSC_C(a0) << 20) | (MXR_CSC_C(a1) << 10) | (MXR_CSC_C(a2) << 0)) >> + >> +/* YCbCr value, used for mixer background color configuration. */ >> +#define MXR_YCBCR_VAL(y, cb, cr) (((y) << 16) | ((cb) << 8) | ((cr) << 0)) >> + >> /* The pixelformats that are natively supported by the mixer. */ >> #define MXR_FORMAT_RGB565 4 >> #define MXR_FORMAT_ARGB1555 5 >> @@ -391,13 +407,14 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height) >> case 1080: >> default: >> val = MXR_CFG_RGB709_16_235; >> + /* Configure the BT.709 CSC matrix for full range RGB. */ >> mixer_reg_write(res, MXR_CM_COEFF_Y, >> - (1 << 30) | (94 << 20) | (314 << 10) | >> - (32 << 0)); >> + MXR_CSC_CT( 0.184, 0.614, 0.063) | >> + MXR_CM_COEFF_RGB_FULL); >> mixer_reg_write(res, MXR_CM_COEFF_CB, >> - (972 << 20) | (851 << 10) | (225 << 0)); >> + MXR_CSC_CT(-0.102, -0.338, 0.440)); >> mixer_reg_write(res, MXR_CM_COEFF_CR, >> - (225 << 20) | (820 << 10) | (1004 << 0)); >> + MXR_CSC_CT( 0.440, -0.399, -0.040)); >> break; >> } >> >> @@ -715,10 +732,10 @@ static void mixer_win_reset(struct mixer_context *ctx) >> /* reset default layer priority */ >> mixer_reg_write(res, MXR_LAYER_CFG, 0); >> >> - /* setting background color */ >> - mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); >> - mixer_reg_write(res, MXR_BG_COLOR1, 0x008080); >> - mixer_reg_write(res, MXR_BG_COLOR2, 0x008080); >> + /* set all background colors to RGB (0,0,0) */ >> + mixer_reg_write(res, MXR_BG_COLOR0, MXR_YCBCR_VAL(0, 128, 128)); >> + mixer_reg_write(res, MXR_BG_COLOR1, MXR_YCBCR_VAL(0, 128, 128)); >> + mixer_reg_write(res, MXR_BG_COLOR2, MXR_YCBCR_VAL(0, 128, 128)); >> >> if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) { >> /* configuration of Video Processor Registers */ >> diff --git a/drivers/gpu/drm/exynos/regs-mixer.h b/drivers/gpu/drm/exynos/regs-mixer.h >> index 7f22df5..c311f57 100644 >> --- a/drivers/gpu/drm/exynos/regs-mixer.h >> +++ b/drivers/gpu/drm/exynos/regs-mixer.h >> @@ -140,11 +140,11 @@ >> #define MXR_INT_EN_VSYNC (1 << 11) >> #define MXR_INT_EN_ALL (0x0f << 8) >> >> -/* bit for MXR_INT_STATUS */ >> +/* bits for MXR_INT_STATUS */ >> #define MXR_INT_CLEAR_VSYNC (1 << 11) >> #define MXR_INT_STATUS_VSYNC (1 << 0) >> >> -/* bit for MXR_LAYER_CFG */ >> +/* bits for MXR_LAYER_CFG */ >> #define MXR_LAYER_CFG_GRP1_VAL(x) MXR_MASK_VAL(x, 11, 8) >> #define MXR_LAYER_CFG_GRP1_MASK MXR_LAYER_CFG_GRP1_VAL(~0) >> #define MXR_LAYER_CFG_GRP0_VAL(x) MXR_MASK_VAL(x, 7, 4) >> @@ -152,5 +152,8 @@ >> #define MXR_LAYER_CFG_VP_VAL(x) MXR_MASK_VAL(x, 3, 0) >> #define MXR_LAYER_CFG_VP_MASK MXR_LAYER_CFG_VP_VAL(~0) >> >> +/* bits for MXR_CM_COEFF_Y */ >> +#define MXR_CM_COEFF_RGB_FULL (1 << 30) >> + >> #endif /* SAMSUNG_REGS_MIXER_H */ >> >> > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Inki Dae wrote: > 2017-03-29 20:56 GMT+09:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>: >> Hello Daniel, >> >> same question here. Patch doesn't introduce any functional changes (just >> adds code documentation), so can you merge it through drm-misc? >> > > Sorry for late. Confirmed just now. I will check it on next Monday. You have added the wrong patch (v2 instead of v3). - Tobias > Thanks, > Inki Dae > >> With best wishes, >> Tobias >> >> >> Tobias Jakobi wrote: >>> The output stage of the mixer uses YCbCr for the internal >>> computations, which is the reason that some registers take >>> YCbCr related data as input. In particular this applies >>> to MXR_BG_COLOR{0,1,2} and MXR_CM_COEFF_{Y,CB,CR}. >>> >>> Document the formatting of the data which we write to >>> these registers. >>> >>> While at it, unify wording of comments in the register header. >>> >>> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> >>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> >>> --- >>> Changes in v2: >>> - use floating point values as input for the macros, as >>> suggested by Andrzej >>> - the floating point values have been tuned to exactly match >>> the values that are currently used >>> >>> Changes in v3: >>> - use only three digit values (pointed out by Andrzej) >>> >>> drivers/gpu/drm/exynos/exynos_mixer.c | 33 +++++++++++++++++++++++++-------- >>> drivers/gpu/drm/exynos/regs-mixer.h | 7 +++++-- >>> 2 files changed, 30 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c >>> index 41d0c36..9648dd5 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c >>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c >>> @@ -45,6 +45,22 @@ >>> #define MIXER_WIN_NR 3 >>> #define VP_DEFAULT_WIN 2 >>> >>> +/* >>> + * Mixer color space conversion coefficient triplet. >>> + * Used for CSC from RGB to YCbCr. >>> + * Each coefficient is a 10-bit fixed point number with >>> + * sign and no integer part, i.e. >>> + * [0:8] = fractional part (representing a value y = x / 2^9) >>> + * [9] = sign >>> + * Negative values are encoded with two's complement. >>> + */ >>> +#define MXR_CSC_C(x) ((int)((x) * 512.0) & 0x3ff) >>> +#define MXR_CSC_CT(a0, a1, a2) \ >>> + ((MXR_CSC_C(a0) << 20) | (MXR_CSC_C(a1) << 10) | (MXR_CSC_C(a2) << 0)) >>> + >>> +/* YCbCr value, used for mixer background color configuration. */ >>> +#define MXR_YCBCR_VAL(y, cb, cr) (((y) << 16) | ((cb) << 8) | ((cr) << 0)) >>> + >>> /* The pixelformats that are natively supported by the mixer. */ >>> #define MXR_FORMAT_RGB565 4 >>> #define MXR_FORMAT_ARGB1555 5 >>> @@ -391,13 +407,14 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height) >>> case 1080: >>> default: >>> val = MXR_CFG_RGB709_16_235; >>> + /* Configure the BT.709 CSC matrix for full range RGB. */ >>> mixer_reg_write(res, MXR_CM_COEFF_Y, >>> - (1 << 30) | (94 << 20) | (314 << 10) | >>> - (32 << 0)); >>> + MXR_CSC_CT( 0.184, 0.614, 0.063) | >>> + MXR_CM_COEFF_RGB_FULL); >>> mixer_reg_write(res, MXR_CM_COEFF_CB, >>> - (972 << 20) | (851 << 10) | (225 << 0)); >>> + MXR_CSC_CT(-0.102, -0.338, 0.440)); >>> mixer_reg_write(res, MXR_CM_COEFF_CR, >>> - (225 << 20) | (820 << 10) | (1004 << 0)); >>> + MXR_CSC_CT( 0.440, -0.399, -0.040)); >>> break; >>> } >>> >>> @@ -715,10 +732,10 @@ static void mixer_win_reset(struct mixer_context *ctx) >>> /* reset default layer priority */ >>> mixer_reg_write(res, MXR_LAYER_CFG, 0); >>> >>> - /* setting background color */ >>> - mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); >>> - mixer_reg_write(res, MXR_BG_COLOR1, 0x008080); >>> - mixer_reg_write(res, MXR_BG_COLOR2, 0x008080); >>> + /* set all background colors to RGB (0,0,0) */ >>> + mixer_reg_write(res, MXR_BG_COLOR0, MXR_YCBCR_VAL(0, 128, 128)); >>> + mixer_reg_write(res, MXR_BG_COLOR1, MXR_YCBCR_VAL(0, 128, 128)); >>> + mixer_reg_write(res, MXR_BG_COLOR2, MXR_YCBCR_VAL(0, 128, 128)); >>> >>> if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) { >>> /* configuration of Video Processor Registers */ >>> diff --git a/drivers/gpu/drm/exynos/regs-mixer.h b/drivers/gpu/drm/exynos/regs-mixer.h >>> index 7f22df5..c311f57 100644 >>> --- a/drivers/gpu/drm/exynos/regs-mixer.h >>> +++ b/drivers/gpu/drm/exynos/regs-mixer.h >>> @@ -140,11 +140,11 @@ >>> #define MXR_INT_EN_VSYNC (1 << 11) >>> #define MXR_INT_EN_ALL (0x0f << 8) >>> >>> -/* bit for MXR_INT_STATUS */ >>> +/* bits for MXR_INT_STATUS */ >>> #define MXR_INT_CLEAR_VSYNC (1 << 11) >>> #define MXR_INT_STATUS_VSYNC (1 << 0) >>> >>> -/* bit for MXR_LAYER_CFG */ >>> +/* bits for MXR_LAYER_CFG */ >>> #define MXR_LAYER_CFG_GRP1_VAL(x) MXR_MASK_VAL(x, 11, 8) >>> #define MXR_LAYER_CFG_GRP1_MASK MXR_LAYER_CFG_GRP1_VAL(~0) >>> #define MXR_LAYER_CFG_GRP0_VAL(x) MXR_MASK_VAL(x, 7, 4) >>> @@ -152,5 +152,8 @@ >>> #define MXR_LAYER_CFG_VP_VAL(x) MXR_MASK_VAL(x, 3, 0) >>> #define MXR_LAYER_CFG_VP_MASK MXR_LAYER_CFG_VP_VAL(~0) >>> >>> +/* bits for MXR_CM_COEFF_Y */ >>> +#define MXR_CM_COEFF_RGB_FULL (1 << 30) >>> + >>> #endif /* SAMSUNG_REGS_MIXER_H */ >>> >>> >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://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 >
2017년 04월 10일 19:27에 Tobias Jakobi 이(가) 쓴 글: > Inki Dae wrote: >> 2017-03-29 20:56 GMT+09:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>: >>> Hello Daniel, >>> >>> same question here. Patch doesn't introduce any functional changes (just >>> adds code documentation), so can you merge it through drm-misc? >>> >> >> Sorry for late. Confirmed just now. I will check it on next Monday. > You have added the wrong patch (v2 instead of v3). Tobias, you had posted this patch with other one like below, [PATCH v2 1/2] drm/exynos: mixer: simplify mixer_cfg_rgb_fmt() [PATCH v2 2/2] drm/exynos: mixer: document YCbCr magic numbers And than you have posted it without any comment again, v3, [PATCH v3] drm/exynos: mixer: document YCbCr magic numbers Please use --cover-letter option to make patch series and keep the patch set to be consistent with previous version. I can merge the new one - really trivial - but I think it'd be better you to resend this patch series for other people. This patch set has been removed from my tree. Please resend (not v4) this patch set like below, [PATCH v3 0/2] drm/exynos: clean up mixer driver?? [PATCH v3 1/2] drm/exynos: mixer: simplify mixer_cfg_rgb_fmt() [PATCH v3 2/2] drm/exynos: mixer: document YCbCr magic numbers Thanks, Inki Dae > > - Tobias > > >> Thanks, >> Inki Dae >> >>> With best wishes, >>> Tobias >>> >>> >>> Tobias Jakobi wrote: >>>> The output stage of the mixer uses YCbCr for the internal >>>> computations, which is the reason that some registers take >>>> YCbCr related data as input. In particular this applies >>>> to MXR_BG_COLOR{0,1,2} and MXR_CM_COEFF_{Y,CB,CR}. >>>> >>>> Document the formatting of the data which we write to >>>> these registers. >>>> >>>> While at it, unify wording of comments in the register header. >>>> >>>> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> >>>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> >>>> --- >>>> Changes in v2: >>>> - use floating point values as input for the macros, as >>>> suggested by Andrzej >>>> - the floating point values have been tuned to exactly match >>>> the values that are currently used >>>> >>>> Changes in v3: >>>> - use only three digit values (pointed out by Andrzej) >>>> >>>> drivers/gpu/drm/exynos/exynos_mixer.c | 33 +++++++++++++++++++++++++-------- >>>> drivers/gpu/drm/exynos/regs-mixer.h | 7 +++++-- >>>> 2 files changed, 30 insertions(+), 10 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c >>>> index 41d0c36..9648dd5 100644 >>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c >>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c >>>> @@ -45,6 +45,22 @@ >>>> #define MIXER_WIN_NR 3 >>>> #define VP_DEFAULT_WIN 2 >>>> >>>> +/* >>>> + * Mixer color space conversion coefficient triplet. >>>> + * Used for CSC from RGB to YCbCr. >>>> + * Each coefficient is a 10-bit fixed point number with >>>> + * sign and no integer part, i.e. >>>> + * [0:8] = fractional part (representing a value y = x / 2^9) >>>> + * [9] = sign >>>> + * Negative values are encoded with two's complement. >>>> + */ >>>> +#define MXR_CSC_C(x) ((int)((x) * 512.0) & 0x3ff) >>>> +#define MXR_CSC_CT(a0, a1, a2) \ >>>> + ((MXR_CSC_C(a0) << 20) | (MXR_CSC_C(a1) << 10) | (MXR_CSC_C(a2) << 0)) >>>> + >>>> +/* YCbCr value, used for mixer background color configuration. */ >>>> +#define MXR_YCBCR_VAL(y, cb, cr) (((y) << 16) | ((cb) << 8) | ((cr) << 0)) >>>> + >>>> /* The pixelformats that are natively supported by the mixer. */ >>>> #define MXR_FORMAT_RGB565 4 >>>> #define MXR_FORMAT_ARGB1555 5 >>>> @@ -391,13 +407,14 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height) >>>> case 1080: >>>> default: >>>> val = MXR_CFG_RGB709_16_235; >>>> + /* Configure the BT.709 CSC matrix for full range RGB. */ >>>> mixer_reg_write(res, MXR_CM_COEFF_Y, >>>> - (1 << 30) | (94 << 20) | (314 << 10) | >>>> - (32 << 0)); >>>> + MXR_CSC_CT( 0.184, 0.614, 0.063) | >>>> + MXR_CM_COEFF_RGB_FULL); >>>> mixer_reg_write(res, MXR_CM_COEFF_CB, >>>> - (972 << 20) | (851 << 10) | (225 << 0)); >>>> + MXR_CSC_CT(-0.102, -0.338, 0.440)); >>>> mixer_reg_write(res, MXR_CM_COEFF_CR, >>>> - (225 << 20) | (820 << 10) | (1004 << 0)); >>>> + MXR_CSC_CT( 0.440, -0.399, -0.040)); >>>> break; >>>> } >>>> >>>> @@ -715,10 +732,10 @@ static void mixer_win_reset(struct mixer_context *ctx) >>>> /* reset default layer priority */ >>>> mixer_reg_write(res, MXR_LAYER_CFG, 0); >>>> >>>> - /* setting background color */ >>>> - mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); >>>> - mixer_reg_write(res, MXR_BG_COLOR1, 0x008080); >>>> - mixer_reg_write(res, MXR_BG_COLOR2, 0x008080); >>>> + /* set all background colors to RGB (0,0,0) */ >>>> + mixer_reg_write(res, MXR_BG_COLOR0, MXR_YCBCR_VAL(0, 128, 128)); >>>> + mixer_reg_write(res, MXR_BG_COLOR1, MXR_YCBCR_VAL(0, 128, 128)); >>>> + mixer_reg_write(res, MXR_BG_COLOR2, MXR_YCBCR_VAL(0, 128, 128)); >>>> >>>> if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) { >>>> /* configuration of Video Processor Registers */ >>>> diff --git a/drivers/gpu/drm/exynos/regs-mixer.h b/drivers/gpu/drm/exynos/regs-mixer.h >>>> index 7f22df5..c311f57 100644 >>>> --- a/drivers/gpu/drm/exynos/regs-mixer.h >>>> +++ b/drivers/gpu/drm/exynos/regs-mixer.h >>>> @@ -140,11 +140,11 @@ >>>> #define MXR_INT_EN_VSYNC (1 << 11) >>>> #define MXR_INT_EN_ALL (0x0f << 8) >>>> >>>> -/* bit for MXR_INT_STATUS */ >>>> +/* bits for MXR_INT_STATUS */ >>>> #define MXR_INT_CLEAR_VSYNC (1 << 11) >>>> #define MXR_INT_STATUS_VSYNC (1 << 0) >>>> >>>> -/* bit for MXR_LAYER_CFG */ >>>> +/* bits for MXR_LAYER_CFG */ >>>> #define MXR_LAYER_CFG_GRP1_VAL(x) MXR_MASK_VAL(x, 11, 8) >>>> #define MXR_LAYER_CFG_GRP1_MASK MXR_LAYER_CFG_GRP1_VAL(~0) >>>> #define MXR_LAYER_CFG_GRP0_VAL(x) MXR_MASK_VAL(x, 7, 4) >>>> @@ -152,5 +152,8 @@ >>>> #define MXR_LAYER_CFG_VP_VAL(x) MXR_MASK_VAL(x, 3, 0) >>>> #define MXR_LAYER_CFG_VP_MASK MXR_LAYER_CFG_VP_VAL(~0) >>>> >>>> +/* bits for MXR_CM_COEFF_Y */ >>>> +#define MXR_CM_COEFF_RGB_FULL (1 << 30) >>>> + >>>> #endif /* SAMSUNG_REGS_MIXER_H */ >>>> >>>> >>> >>> _______________________________________________ >>> dri-devel mailing list >>> dri-devel@lists.freedesktop.org >>> https://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 > > >
Inki Dae wrote: > > > 2017년 04월 10일 19:27에 Tobias Jakobi 이(가) 쓴 글: >> Inki Dae wrote: >>> 2017-03-29 20:56 GMT+09:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>: >>>> Hello Daniel, >>>> >>>> same question here. Patch doesn't introduce any functional changes (just >>>> adds code documentation), so can you merge it through drm-misc? >>>> >>> >>> Sorry for late. Confirmed just now. I will check it on next Monday. >> You have added the wrong patch (v2 instead of v3). > > Tobias, > > you had posted this patch with other one like below, > [PATCH v2 1/2] drm/exynos: mixer: simplify mixer_cfg_rgb_fmt() > [PATCH v2 2/2] drm/exynos: mixer: document YCbCr magic numbers > > And than you have posted it without any comment again, v3, > [PATCH v3] drm/exynos: mixer: document YCbCr magic numbers > > Please use --cover-letter option to make patch series and keep the patch set to be consistent with previous version. This is not necessary. There were only changes to 'drm/exynos: mixer: document YCbCr magic numbers' and we're talking about _two_ patches here (which can be applied independently). > > I can merge the new one - really trivial - but I think it'd be better you to resend this patch series for other people. > > This patch set has been removed from my tree. Please resend (not v4) this patch set like below, > [PATCH v3 0/2] drm/exynos: clean up mixer driver?? > [PATCH v3 1/2] drm/exynos: mixer: simplify mixer_cfg_rgb_fmt() > [PATCH v3 2/2] drm/exynos: mixer: document YCbCr magic numbers Not necessary, just pick up the two patches which for which I already pinged you. For you convenience, here are the two on dri-devel patchwork: https://patchwork.kernel.org/patch/9617085/ https://patchwork.kernel.org/patch/9617493/ - Tobias > > Thanks, > Inki Dae > >> >> - Tobias >> >> >>> Thanks, >>> Inki Dae >>> >>>> With best wishes, >>>> Tobias >>>> >>>> >>>> Tobias Jakobi wrote: >>>>> The output stage of the mixer uses YCbCr for the internal >>>>> computations, which is the reason that some registers take >>>>> YCbCr related data as input. In particular this applies >>>>> to MXR_BG_COLOR{0,1,2} and MXR_CM_COEFF_{Y,CB,CR}. >>>>> >>>>> Document the formatting of the data which we write to >>>>> these registers. >>>>> >>>>> While at it, unify wording of comments in the register header. >>>>> >>>>> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> >>>>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> >>>>> --- >>>>> Changes in v2: >>>>> - use floating point values as input for the macros, as >>>>> suggested by Andrzej >>>>> - the floating point values have been tuned to exactly match >>>>> the values that are currently used >>>>> >>>>> Changes in v3: >>>>> - use only three digit values (pointed out by Andrzej) >>>>> >>>>> drivers/gpu/drm/exynos/exynos_mixer.c | 33 +++++++++++++++++++++++++-------- >>>>> drivers/gpu/drm/exynos/regs-mixer.h | 7 +++++-- >>>>> 2 files changed, 30 insertions(+), 10 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c >>>>> index 41d0c36..9648dd5 100644 >>>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c >>>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c >>>>> @@ -45,6 +45,22 @@ >>>>> #define MIXER_WIN_NR 3 >>>>> #define VP_DEFAULT_WIN 2 >>>>> >>>>> +/* >>>>> + * Mixer color space conversion coefficient triplet. >>>>> + * Used for CSC from RGB to YCbCr. >>>>> + * Each coefficient is a 10-bit fixed point number with >>>>> + * sign and no integer part, i.e. >>>>> + * [0:8] = fractional part (representing a value y = x / 2^9) >>>>> + * [9] = sign >>>>> + * Negative values are encoded with two's complement. >>>>> + */ >>>>> +#define MXR_CSC_C(x) ((int)((x) * 512.0) & 0x3ff) >>>>> +#define MXR_CSC_CT(a0, a1, a2) \ >>>>> + ((MXR_CSC_C(a0) << 20) | (MXR_CSC_C(a1) << 10) | (MXR_CSC_C(a2) << 0)) >>>>> + >>>>> +/* YCbCr value, used for mixer background color configuration. */ >>>>> +#define MXR_YCBCR_VAL(y, cb, cr) (((y) << 16) | ((cb) << 8) | ((cr) << 0)) >>>>> + >>>>> /* The pixelformats that are natively supported by the mixer. */ >>>>> #define MXR_FORMAT_RGB565 4 >>>>> #define MXR_FORMAT_ARGB1555 5 >>>>> @@ -391,13 +407,14 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height) >>>>> case 1080: >>>>> default: >>>>> val = MXR_CFG_RGB709_16_235; >>>>> + /* Configure the BT.709 CSC matrix for full range RGB. */ >>>>> mixer_reg_write(res, MXR_CM_COEFF_Y, >>>>> - (1 << 30) | (94 << 20) | (314 << 10) | >>>>> - (32 << 0)); >>>>> + MXR_CSC_CT( 0.184, 0.614, 0.063) | >>>>> + MXR_CM_COEFF_RGB_FULL); >>>>> mixer_reg_write(res, MXR_CM_COEFF_CB, >>>>> - (972 << 20) | (851 << 10) | (225 << 0)); >>>>> + MXR_CSC_CT(-0.102, -0.338, 0.440)); >>>>> mixer_reg_write(res, MXR_CM_COEFF_CR, >>>>> - (225 << 20) | (820 << 10) | (1004 << 0)); >>>>> + MXR_CSC_CT( 0.440, -0.399, -0.040)); >>>>> break; >>>>> } >>>>> >>>>> @@ -715,10 +732,10 @@ static void mixer_win_reset(struct mixer_context *ctx) >>>>> /* reset default layer priority */ >>>>> mixer_reg_write(res, MXR_LAYER_CFG, 0); >>>>> >>>>> - /* setting background color */ >>>>> - mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); >>>>> - mixer_reg_write(res, MXR_BG_COLOR1, 0x008080); >>>>> - mixer_reg_write(res, MXR_BG_COLOR2, 0x008080); >>>>> + /* set all background colors to RGB (0,0,0) */ >>>>> + mixer_reg_write(res, MXR_BG_COLOR0, MXR_YCBCR_VAL(0, 128, 128)); >>>>> + mixer_reg_write(res, MXR_BG_COLOR1, MXR_YCBCR_VAL(0, 128, 128)); >>>>> + mixer_reg_write(res, MXR_BG_COLOR2, MXR_YCBCR_VAL(0, 128, 128)); >>>>> >>>>> if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) { >>>>> /* configuration of Video Processor Registers */ >>>>> diff --git a/drivers/gpu/drm/exynos/regs-mixer.h b/drivers/gpu/drm/exynos/regs-mixer.h >>>>> index 7f22df5..c311f57 100644 >>>>> --- a/drivers/gpu/drm/exynos/regs-mixer.h >>>>> +++ b/drivers/gpu/drm/exynos/regs-mixer.h >>>>> @@ -140,11 +140,11 @@ >>>>> #define MXR_INT_EN_VSYNC (1 << 11) >>>>> #define MXR_INT_EN_ALL (0x0f << 8) >>>>> >>>>> -/* bit for MXR_INT_STATUS */ >>>>> +/* bits for MXR_INT_STATUS */ >>>>> #define MXR_INT_CLEAR_VSYNC (1 << 11) >>>>> #define MXR_INT_STATUS_VSYNC (1 << 0) >>>>> >>>>> -/* bit for MXR_LAYER_CFG */ >>>>> +/* bits for MXR_LAYER_CFG */ >>>>> #define MXR_LAYER_CFG_GRP1_VAL(x) MXR_MASK_VAL(x, 11, 8) >>>>> #define MXR_LAYER_CFG_GRP1_MASK MXR_LAYER_CFG_GRP1_VAL(~0) >>>>> #define MXR_LAYER_CFG_GRP0_VAL(x) MXR_MASK_VAL(x, 7, 4) >>>>> @@ -152,5 +152,8 @@ >>>>> #define MXR_LAYER_CFG_VP_VAL(x) MXR_MASK_VAL(x, 3, 0) >>>>> #define MXR_LAYER_CFG_VP_MASK MXR_LAYER_CFG_VP_VAL(~0) >>>>> >>>>> +/* bits for MXR_CM_COEFF_Y */ >>>>> +#define MXR_CM_COEFF_RGB_FULL (1 << 30) >>>>> + >>>>> #endif /* SAMSUNG_REGS_MIXER_H */ >>>>> >>>>> >>>> >>>> _______________________________________________ >>>> dri-devel mailing list >>>> dri-devel@lists.freedesktop.org >>>> https://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 >> >> >> > -- > 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 >
2017년 04월 11일 17:01에 Tobias Jakobi 이(가) 쓴 글: > Inki Dae wrote: >> >> >> 2017년 04월 10일 19:27에 Tobias Jakobi 이(가) 쓴 글: >>> Inki Dae wrote: >>>> 2017-03-29 20:56 GMT+09:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>: >>>>> Hello Daniel, >>>>> >>>>> same question here. Patch doesn't introduce any functional changes (just >>>>> adds code documentation), so can you merge it through drm-misc? >>>>> >>>> >>>> Sorry for late. Confirmed just now. I will check it on next Monday. >>> You have added the wrong patch (v2 instead of v3). >> >> Tobias, >> >> you had posted this patch with other one like below, >> [PATCH v2 1/2] drm/exynos: mixer: simplify mixer_cfg_rgb_fmt() >> [PATCH v2 2/2] drm/exynos: mixer: document YCbCr magic numbers >> >> And than you have posted it without any comment again, v3, >> [PATCH v3] drm/exynos: mixer: document YCbCr magic numbers >> >> Please use --cover-letter option to make patch series and keep the patch set to be consistent with previous version. > This is not necessary. There were only changes to 'drm/exynos: mixer: > document YCbCr magic numbers' and we're talking about _two_ patches here > (which can be applied independently). Tobias, why did you post the patch set 1/2 and 2/2 before? If these patches can be applied independently - you found out later - then you need to clarify why you posted them separately. Thanks, Inki Dae > > >> >> I can merge the new one - really trivial - but I think it'd be better you to resend this patch series for other people. >> >> This patch set has been removed from my tree. Please resend (not v4) this patch set like below, >> [PATCH v3 0/2] drm/exynos: clean up mixer driver?? >> [PATCH v3 1/2] drm/exynos: mixer: simplify mixer_cfg_rgb_fmt() >> [PATCH v3 2/2] drm/exynos: mixer: document YCbCr magic numbers > Not necessary, just pick up the two patches which for which I already > pinged you. For you convenience, here are the two on dri-devel patchwork: > https://patchwork.kernel.org/patch/9617085/ > https://patchwork.kernel.org/patch/9617493/ > > - Tobias > > > >> >> Thanks, >> Inki Dae >> >>> >>> - Tobias >>> >>> >>>> Thanks, >>>> Inki Dae >>>> >>>>> With best wishes, >>>>> Tobias >>>>> >>>>> >>>>> Tobias Jakobi wrote: >>>>>> The output stage of the mixer uses YCbCr for the internal >>>>>> computations, which is the reason that some registers take >>>>>> YCbCr related data as input. In particular this applies >>>>>> to MXR_BG_COLOR{0,1,2} and MXR_CM_COEFF_{Y,CB,CR}. >>>>>> >>>>>> Document the formatting of the data which we write to >>>>>> these registers. >>>>>> >>>>>> While at it, unify wording of comments in the register header. >>>>>> >>>>>> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> >>>>>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> >>>>>> --- >>>>>> Changes in v2: >>>>>> - use floating point values as input for the macros, as >>>>>> suggested by Andrzej >>>>>> - the floating point values have been tuned to exactly match >>>>>> the values that are currently used >>>>>> >>>>>> Changes in v3: >>>>>> - use only three digit values (pointed out by Andrzej) >>>>>> >>>>>> drivers/gpu/drm/exynos/exynos_mixer.c | 33 +++++++++++++++++++++++++-------- >>>>>> drivers/gpu/drm/exynos/regs-mixer.h | 7 +++++-- >>>>>> 2 files changed, 30 insertions(+), 10 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c >>>>>> index 41d0c36..9648dd5 100644 >>>>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c >>>>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c >>>>>> @@ -45,6 +45,22 @@ >>>>>> #define MIXER_WIN_NR 3 >>>>>> #define VP_DEFAULT_WIN 2 >>>>>> >>>>>> +/* >>>>>> + * Mixer color space conversion coefficient triplet. >>>>>> + * Used for CSC from RGB to YCbCr. >>>>>> + * Each coefficient is a 10-bit fixed point number with >>>>>> + * sign and no integer part, i.e. >>>>>> + * [0:8] = fractional part (representing a value y = x / 2^9) >>>>>> + * [9] = sign >>>>>> + * Negative values are encoded with two's complement. >>>>>> + */ >>>>>> +#define MXR_CSC_C(x) ((int)((x) * 512.0) & 0x3ff) >>>>>> +#define MXR_CSC_CT(a0, a1, a2) \ >>>>>> + ((MXR_CSC_C(a0) << 20) | (MXR_CSC_C(a1) << 10) | (MXR_CSC_C(a2) << 0)) >>>>>> + >>>>>> +/* YCbCr value, used for mixer background color configuration. */ >>>>>> +#define MXR_YCBCR_VAL(y, cb, cr) (((y) << 16) | ((cb) << 8) | ((cr) << 0)) >>>>>> + >>>>>> /* The pixelformats that are natively supported by the mixer. */ >>>>>> #define MXR_FORMAT_RGB565 4 >>>>>> #define MXR_FORMAT_ARGB1555 5 >>>>>> @@ -391,13 +407,14 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height) >>>>>> case 1080: >>>>>> default: >>>>>> val = MXR_CFG_RGB709_16_235; >>>>>> + /* Configure the BT.709 CSC matrix for full range RGB. */ >>>>>> mixer_reg_write(res, MXR_CM_COEFF_Y, >>>>>> - (1 << 30) | (94 << 20) | (314 << 10) | >>>>>> - (32 << 0)); >>>>>> + MXR_CSC_CT( 0.184, 0.614, 0.063) | >>>>>> + MXR_CM_COEFF_RGB_FULL); >>>>>> mixer_reg_write(res, MXR_CM_COEFF_CB, >>>>>> - (972 << 20) | (851 << 10) | (225 << 0)); >>>>>> + MXR_CSC_CT(-0.102, -0.338, 0.440)); >>>>>> mixer_reg_write(res, MXR_CM_COEFF_CR, >>>>>> - (225 << 20) | (820 << 10) | (1004 << 0)); >>>>>> + MXR_CSC_CT( 0.440, -0.399, -0.040)); >>>>>> break; >>>>>> } >>>>>> >>>>>> @@ -715,10 +732,10 @@ static void mixer_win_reset(struct mixer_context *ctx) >>>>>> /* reset default layer priority */ >>>>>> mixer_reg_write(res, MXR_LAYER_CFG, 0); >>>>>> >>>>>> - /* setting background color */ >>>>>> - mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); >>>>>> - mixer_reg_write(res, MXR_BG_COLOR1, 0x008080); >>>>>> - mixer_reg_write(res, MXR_BG_COLOR2, 0x008080); >>>>>> + /* set all background colors to RGB (0,0,0) */ >>>>>> + mixer_reg_write(res, MXR_BG_COLOR0, MXR_YCBCR_VAL(0, 128, 128)); >>>>>> + mixer_reg_write(res, MXR_BG_COLOR1, MXR_YCBCR_VAL(0, 128, 128)); >>>>>> + mixer_reg_write(res, MXR_BG_COLOR2, MXR_YCBCR_VAL(0, 128, 128)); >>>>>> >>>>>> if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) { >>>>>> /* configuration of Video Processor Registers */ >>>>>> diff --git a/drivers/gpu/drm/exynos/regs-mixer.h b/drivers/gpu/drm/exynos/regs-mixer.h >>>>>> index 7f22df5..c311f57 100644 >>>>>> --- a/drivers/gpu/drm/exynos/regs-mixer.h >>>>>> +++ b/drivers/gpu/drm/exynos/regs-mixer.h >>>>>> @@ -140,11 +140,11 @@ >>>>>> #define MXR_INT_EN_VSYNC (1 << 11) >>>>>> #define MXR_INT_EN_ALL (0x0f << 8) >>>>>> >>>>>> -/* bit for MXR_INT_STATUS */ >>>>>> +/* bits for MXR_INT_STATUS */ >>>>>> #define MXR_INT_CLEAR_VSYNC (1 << 11) >>>>>> #define MXR_INT_STATUS_VSYNC (1 << 0) >>>>>> >>>>>> -/* bit for MXR_LAYER_CFG */ >>>>>> +/* bits for MXR_LAYER_CFG */ >>>>>> #define MXR_LAYER_CFG_GRP1_VAL(x) MXR_MASK_VAL(x, 11, 8) >>>>>> #define MXR_LAYER_CFG_GRP1_MASK MXR_LAYER_CFG_GRP1_VAL(~0) >>>>>> #define MXR_LAYER_CFG_GRP0_VAL(x) MXR_MASK_VAL(x, 7, 4) >>>>>> @@ -152,5 +152,8 @@ >>>>>> #define MXR_LAYER_CFG_VP_VAL(x) MXR_MASK_VAL(x, 3, 0) >>>>>> #define MXR_LAYER_CFG_VP_MASK MXR_LAYER_CFG_VP_VAL(~0) >>>>>> >>>>>> +/* bits for MXR_CM_COEFF_Y */ >>>>>> +#define MXR_CM_COEFF_RGB_FULL (1 << 30) >>>>>> + >>>>>> #endif /* SAMSUNG_REGS_MIXER_H */ >>>>>> >>>>>> >>>>> >>>>> _______________________________________________ >>>>> dri-devel mailing list >>>>> dri-devel@lists.freedesktop.org >>>>> https://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 >>> >>> >>> >> -- >> 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, Inki Dae wrote: <snip> > Tobias, why did you post the patch set 1/2 and 2/2 before? > If these patches can be applied independently - you found out later - then you need to clarify why you posted them separately. after I posted v1 of the patchset, Andrzej responded with a Reviewed-By for patch 1/2 and suggestions for patch 2/2. I then integrated the suggestions and added the Reviewed-By tag, hence changing both patch 1/2 and 2/2. I then posted v2 of the patchset, because _both_ patches changed. Andrzej then pointed out that the values used in patch 2/2 could be further simplied. I then posted a v3 of patch 2/2, because only this one patch changed. When I pinged you concerning the patches, I specifically did this with v3 of 2/2, and v2 of 1/2. It was perfectly clear which patches I wanted you to pick up. I can understand when maintainers of high-volume subsystems sometimes get patch versions mixed up. But drm-exynos only gets very few patches each cycle. If you have problems dealing with your maintainer duties in a timely manner, I would suggest to contact Daniel Vetter so that exynos is added to drm-misc. I personally think exynos would benefit from such a move. With best wishes, Tobias > > Thanks, > Inki Dae > >> >> >>> >>> I can merge the new one - really trivial - but I think it'd be better you to resend this patch series for other people. >>> >>> This patch set has been removed from my tree. Please resend (not v4) this patch set like below, >>> [PATCH v3 0/2] drm/exynos: clean up mixer driver?? >>> [PATCH v3 1/2] drm/exynos: mixer: simplify mixer_cfg_rgb_fmt() >>> [PATCH v3 2/2] drm/exynos: mixer: document YCbCr magic numbers >> Not necessary, just pick up the two patches which for which I already >> pinged you. For you convenience, here are the two on dri-devel patchwork: >> https://patchwork.kernel.org/patch/9617085/ >> https://patchwork.kernel.org/patch/9617493/ >> >> - Tobias >> >> >> >>> >>> Thanks, >>> Inki Dae >>> >>>> >>>> - Tobias >>>> >>>> >>>>> Thanks, >>>>> Inki Dae >>>>> >>>>>> With best wishes, >>>>>> Tobias >>>>>> >>>>>> >>>>>> Tobias Jakobi wrote: >>>>>>> The output stage of the mixer uses YCbCr for the internal >>>>>>> computations, which is the reason that some registers take >>>>>>> YCbCr related data as input. In particular this applies >>>>>>> to MXR_BG_COLOR{0,1,2} and MXR_CM_COEFF_{Y,CB,CR}. >>>>>>> >>>>>>> Document the formatting of the data which we write to >>>>>>> these registers. >>>>>>> >>>>>>> While at it, unify wording of comments in the register header. >>>>>>> >>>>>>> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> >>>>>>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> >>>>>>> --- >>>>>>> Changes in v2: >>>>>>> - use floating point values as input for the macros, as >>>>>>> suggested by Andrzej >>>>>>> - the floating point values have been tuned to exactly match >>>>>>> the values that are currently used >>>>>>> >>>>>>> Changes in v3: >>>>>>> - use only three digit values (pointed out by Andrzej) >>>>>>> >>>>>>> drivers/gpu/drm/exynos/exynos_mixer.c | 33 +++++++++++++++++++++++++-------- >>>>>>> drivers/gpu/drm/exynos/regs-mixer.h | 7 +++++-- >>>>>>> 2 files changed, 30 insertions(+), 10 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c >>>>>>> index 41d0c36..9648dd5 100644 >>>>>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c >>>>>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c >>>>>>> @@ -45,6 +45,22 @@ >>>>>>> #define MIXER_WIN_NR 3 >>>>>>> #define VP_DEFAULT_WIN 2 >>>>>>> >>>>>>> +/* >>>>>>> + * Mixer color space conversion coefficient triplet. >>>>>>> + * Used for CSC from RGB to YCbCr. >>>>>>> + * Each coefficient is a 10-bit fixed point number with >>>>>>> + * sign and no integer part, i.e. >>>>>>> + * [0:8] = fractional part (representing a value y = x / 2^9) >>>>>>> + * [9] = sign >>>>>>> + * Negative values are encoded with two's complement. >>>>>>> + */ >>>>>>> +#define MXR_CSC_C(x) ((int)((x) * 512.0) & 0x3ff) >>>>>>> +#define MXR_CSC_CT(a0, a1, a2) \ >>>>>>> + ((MXR_CSC_C(a0) << 20) | (MXR_CSC_C(a1) << 10) | (MXR_CSC_C(a2) << 0)) >>>>>>> + >>>>>>> +/* YCbCr value, used for mixer background color configuration. */ >>>>>>> +#define MXR_YCBCR_VAL(y, cb, cr) (((y) << 16) | ((cb) << 8) | ((cr) << 0)) >>>>>>> + >>>>>>> /* The pixelformats that are natively supported by the mixer. */ >>>>>>> #define MXR_FORMAT_RGB565 4 >>>>>>> #define MXR_FORMAT_ARGB1555 5 >>>>>>> @@ -391,13 +407,14 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height) >>>>>>> case 1080: >>>>>>> default: >>>>>>> val = MXR_CFG_RGB709_16_235; >>>>>>> + /* Configure the BT.709 CSC matrix for full range RGB. */ >>>>>>> mixer_reg_write(res, MXR_CM_COEFF_Y, >>>>>>> - (1 << 30) | (94 << 20) | (314 << 10) | >>>>>>> - (32 << 0)); >>>>>>> + MXR_CSC_CT( 0.184, 0.614, 0.063) | >>>>>>> + MXR_CM_COEFF_RGB_FULL); >>>>>>> mixer_reg_write(res, MXR_CM_COEFF_CB, >>>>>>> - (972 << 20) | (851 << 10) | (225 << 0)); >>>>>>> + MXR_CSC_CT(-0.102, -0.338, 0.440)); >>>>>>> mixer_reg_write(res, MXR_CM_COEFF_CR, >>>>>>> - (225 << 20) | (820 << 10) | (1004 << 0)); >>>>>>> + MXR_CSC_CT( 0.440, -0.399, -0.040)); >>>>>>> break; >>>>>>> } >>>>>>> >>>>>>> @@ -715,10 +732,10 @@ static void mixer_win_reset(struct mixer_context *ctx) >>>>>>> /* reset default layer priority */ >>>>>>> mixer_reg_write(res, MXR_LAYER_CFG, 0); >>>>>>> >>>>>>> - /* setting background color */ >>>>>>> - mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); >>>>>>> - mixer_reg_write(res, MXR_BG_COLOR1, 0x008080); >>>>>>> - mixer_reg_write(res, MXR_BG_COLOR2, 0x008080); >>>>>>> + /* set all background colors to RGB (0,0,0) */ >>>>>>> + mixer_reg_write(res, MXR_BG_COLOR0, MXR_YCBCR_VAL(0, 128, 128)); >>>>>>> + mixer_reg_write(res, MXR_BG_COLOR1, MXR_YCBCR_VAL(0, 128, 128)); >>>>>>> + mixer_reg_write(res, MXR_BG_COLOR2, MXR_YCBCR_VAL(0, 128, 128)); >>>>>>> >>>>>>> if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) { >>>>>>> /* configuration of Video Processor Registers */ >>>>>>> diff --git a/drivers/gpu/drm/exynos/regs-mixer.h b/drivers/gpu/drm/exynos/regs-mixer.h >>>>>>> index 7f22df5..c311f57 100644 >>>>>>> --- a/drivers/gpu/drm/exynos/regs-mixer.h >>>>>>> +++ b/drivers/gpu/drm/exynos/regs-mixer.h >>>>>>> @@ -140,11 +140,11 @@ >>>>>>> #define MXR_INT_EN_VSYNC (1 << 11) >>>>>>> #define MXR_INT_EN_ALL (0x0f << 8) >>>>>>> >>>>>>> -/* bit for MXR_INT_STATUS */ >>>>>>> +/* bits for MXR_INT_STATUS */ >>>>>>> #define MXR_INT_CLEAR_VSYNC (1 << 11) >>>>>>> #define MXR_INT_STATUS_VSYNC (1 << 0) >>>>>>> >>>>>>> -/* bit for MXR_LAYER_CFG */ >>>>>>> +/* bits for MXR_LAYER_CFG */ >>>>>>> #define MXR_LAYER_CFG_GRP1_VAL(x) MXR_MASK_VAL(x, 11, 8) >>>>>>> #define MXR_LAYER_CFG_GRP1_MASK MXR_LAYER_CFG_GRP1_VAL(~0) >>>>>>> #define MXR_LAYER_CFG_GRP0_VAL(x) MXR_MASK_VAL(x, 7, 4) >>>>>>> @@ -152,5 +152,8 @@ >>>>>>> #define MXR_LAYER_CFG_VP_VAL(x) MXR_MASK_VAL(x, 3, 0) >>>>>>> #define MXR_LAYER_CFG_VP_MASK MXR_LAYER_CFG_VP_VAL(~0) >>>>>>> >>>>>>> +/* bits for MXR_CM_COEFF_Y */ >>>>>>> +#define MXR_CM_COEFF_RGB_FULL (1 << 30) >>>>>>> + >>>>>>> #endif /* SAMSUNG_REGS_MIXER_H */ >>>>>>> >>>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> dri-devel mailing list >>>>>> dri-devel@lists.freedesktop.org >>>>>> https://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 >>>> >>>> >>>> >>> -- >>> 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 >>> >> >> >> >>
2017년 04월 16일 20:50에 Tobias Jakobi 이(가) 쓴 글: > Hello Inki, > > Inki Dae wrote: > <snip> >> Tobias, why did you post the patch set 1/2 and 2/2 before? >> If these patches can be applied independently - you found out later - then you need to clarify why you posted them separately. > > > after I posted v1 of the patchset, Andrzej responded with a Reviewed-By > for patch 1/2 and suggestions for patch 2/2. I then integrated the > suggestions and added the Reviewed-By tag, hence changing both patch 1/2 > and 2/2. I then posted v2 of the patchset, because _both_ patches > changed. Andrzej then pointed out that the values used in patch 2/2 > could be further simplied. I then posted a v3 of patch 2/2, because only > this one patch changed. > When I pinged you concerning the patches, I specifically did this with > v3 of 2/2, and v2 of 1/2. It was perfectly clear which patches I wanted > you to pick up. Sorry but last comment about this, I just wanted this patch to be re-posted with patch series for other people. Only attendees who reviewed would know v3 of this patch is one of v2 patches. However other people don't know that so if they want to check this patch then they would need to search relevant patch in their email box. Thanks, Inki Dae > I can understand when maintainers of high-volume subsystems sometimes > get patch versions mixed up. But drm-exynos only gets very few patches > each cycle. If you have problems dealing with your maintainer duties in > a timely manner, I would suggest to contact Daniel Vetter so that exynos > is added to drm-misc. I personally think exynos would benefit from such > a move. > > With best wishes, > Tobias > > > >> >> Thanks, >> Inki Dae >> >>> >>> >>>> >>>> I can merge the new one - really trivial - but I think it'd be better you to resend this patch series for other people. >>>> >>>> This patch set has been removed from my tree. Please resend (not v4) this patch set like below, >>>> [PATCH v3 0/2] drm/exynos: clean up mixer driver?? >>>> [PATCH v3 1/2] drm/exynos: mixer: simplify mixer_cfg_rgb_fmt() >>>> [PATCH v3 2/2] drm/exynos: mixer: document YCbCr magic numbers >>> Not necessary, just pick up the two patches which for which I already >>> pinged you. For you convenience, here are the two on dri-devel patchwork: >>> https://patchwork.kernel.org/patch/9617085/ >>> https://patchwork.kernel.org/patch/9617493/ >>> >>> - Tobias >>> >>> >>> >>>> >>>> Thanks, >>>> Inki Dae >>>> >>>>> >>>>> - Tobias >>>>> >>>>> >>>>>> Thanks, >>>>>> Inki Dae >>>>>> >>>>>>> With best wishes, >>>>>>> Tobias >>>>>>> >>>>>>> >>>>>>> Tobias Jakobi wrote: >>>>>>>> The output stage of the mixer uses YCbCr for the internal >>>>>>>> computations, which is the reason that some registers take >>>>>>>> YCbCr related data as input. In particular this applies >>>>>>>> to MXR_BG_COLOR{0,1,2} and MXR_CM_COEFF_{Y,CB,CR}. >>>>>>>> >>>>>>>> Document the formatting of the data which we write to >>>>>>>> these registers. >>>>>>>> >>>>>>>> While at it, unify wording of comments in the register header. >>>>>>>> >>>>>>>> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> >>>>>>>> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> >>>>>>>> --- >>>>>>>> Changes in v2: >>>>>>>> - use floating point values as input for the macros, as >>>>>>>> suggested by Andrzej >>>>>>>> - the floating point values have been tuned to exactly match >>>>>>>> the values that are currently used >>>>>>>> >>>>>>>> Changes in v3: >>>>>>>> - use only three digit values (pointed out by Andrzej) >>>>>>>> >>>>>>>> drivers/gpu/drm/exynos/exynos_mixer.c | 33 +++++++++++++++++++++++++-------- >>>>>>>> drivers/gpu/drm/exynos/regs-mixer.h | 7 +++++-- >>>>>>>> 2 files changed, 30 insertions(+), 10 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c >>>>>>>> index 41d0c36..9648dd5 100644 >>>>>>>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c >>>>>>>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c >>>>>>>> @@ -45,6 +45,22 @@ >>>>>>>> #define MIXER_WIN_NR 3 >>>>>>>> #define VP_DEFAULT_WIN 2 >>>>>>>> >>>>>>>> +/* >>>>>>>> + * Mixer color space conversion coefficient triplet. >>>>>>>> + * Used for CSC from RGB to YCbCr. >>>>>>>> + * Each coefficient is a 10-bit fixed point number with >>>>>>>> + * sign and no integer part, i.e. >>>>>>>> + * [0:8] = fractional part (representing a value y = x / 2^9) >>>>>>>> + * [9] = sign >>>>>>>> + * Negative values are encoded with two's complement. >>>>>>>> + */ >>>>>>>> +#define MXR_CSC_C(x) ((int)((x) * 512.0) & 0x3ff) >>>>>>>> +#define MXR_CSC_CT(a0, a1, a2) \ >>>>>>>> + ((MXR_CSC_C(a0) << 20) | (MXR_CSC_C(a1) << 10) | (MXR_CSC_C(a2) << 0)) >>>>>>>> + >>>>>>>> +/* YCbCr value, used for mixer background color configuration. */ >>>>>>>> +#define MXR_YCBCR_VAL(y, cb, cr) (((y) << 16) | ((cb) << 8) | ((cr) << 0)) >>>>>>>> + >>>>>>>> /* The pixelformats that are natively supported by the mixer. */ >>>>>>>> #define MXR_FORMAT_RGB565 4 >>>>>>>> #define MXR_FORMAT_ARGB1555 5 >>>>>>>> @@ -391,13 +407,14 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height) >>>>>>>> case 1080: >>>>>>>> default: >>>>>>>> val = MXR_CFG_RGB709_16_235; >>>>>>>> + /* Configure the BT.709 CSC matrix for full range RGB. */ >>>>>>>> mixer_reg_write(res, MXR_CM_COEFF_Y, >>>>>>>> - (1 << 30) | (94 << 20) | (314 << 10) | >>>>>>>> - (32 << 0)); >>>>>>>> + MXR_CSC_CT( 0.184, 0.614, 0.063) | >>>>>>>> + MXR_CM_COEFF_RGB_FULL); >>>>>>>> mixer_reg_write(res, MXR_CM_COEFF_CB, >>>>>>>> - (972 << 20) | (851 << 10) | (225 << 0)); >>>>>>>> + MXR_CSC_CT(-0.102, -0.338, 0.440)); >>>>>>>> mixer_reg_write(res, MXR_CM_COEFF_CR, >>>>>>>> - (225 << 20) | (820 << 10) | (1004 << 0)); >>>>>>>> + MXR_CSC_CT( 0.440, -0.399, -0.040)); >>>>>>>> break; >>>>>>>> } >>>>>>>> >>>>>>>> @@ -715,10 +732,10 @@ static void mixer_win_reset(struct mixer_context *ctx) >>>>>>>> /* reset default layer priority */ >>>>>>>> mixer_reg_write(res, MXR_LAYER_CFG, 0); >>>>>>>> >>>>>>>> - /* setting background color */ >>>>>>>> - mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); >>>>>>>> - mixer_reg_write(res, MXR_BG_COLOR1, 0x008080); >>>>>>>> - mixer_reg_write(res, MXR_BG_COLOR2, 0x008080); >>>>>>>> + /* set all background colors to RGB (0,0,0) */ >>>>>>>> + mixer_reg_write(res, MXR_BG_COLOR0, MXR_YCBCR_VAL(0, 128, 128)); >>>>>>>> + mixer_reg_write(res, MXR_BG_COLOR1, MXR_YCBCR_VAL(0, 128, 128)); >>>>>>>> + mixer_reg_write(res, MXR_BG_COLOR2, MXR_YCBCR_VAL(0, 128, 128)); >>>>>>>> >>>>>>>> if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) { >>>>>>>> /* configuration of Video Processor Registers */ >>>>>>>> diff --git a/drivers/gpu/drm/exynos/regs-mixer.h b/drivers/gpu/drm/exynos/regs-mixer.h >>>>>>>> index 7f22df5..c311f57 100644 >>>>>>>> --- a/drivers/gpu/drm/exynos/regs-mixer.h >>>>>>>> +++ b/drivers/gpu/drm/exynos/regs-mixer.h >>>>>>>> @@ -140,11 +140,11 @@ >>>>>>>> #define MXR_INT_EN_VSYNC (1 << 11) >>>>>>>> #define MXR_INT_EN_ALL (0x0f << 8) >>>>>>>> >>>>>>>> -/* bit for MXR_INT_STATUS */ >>>>>>>> +/* bits for MXR_INT_STATUS */ >>>>>>>> #define MXR_INT_CLEAR_VSYNC (1 << 11) >>>>>>>> #define MXR_INT_STATUS_VSYNC (1 << 0) >>>>>>>> >>>>>>>> -/* bit for MXR_LAYER_CFG */ >>>>>>>> +/* bits for MXR_LAYER_CFG */ >>>>>>>> #define MXR_LAYER_CFG_GRP1_VAL(x) MXR_MASK_VAL(x, 11, 8) >>>>>>>> #define MXR_LAYER_CFG_GRP1_MASK MXR_LAYER_CFG_GRP1_VAL(~0) >>>>>>>> #define MXR_LAYER_CFG_GRP0_VAL(x) MXR_MASK_VAL(x, 7, 4) >>>>>>>> @@ -152,5 +152,8 @@ >>>>>>>> #define MXR_LAYER_CFG_VP_VAL(x) MXR_MASK_VAL(x, 3, 0) >>>>>>>> #define MXR_LAYER_CFG_VP_MASK MXR_LAYER_CFG_VP_VAL(~0) >>>>>>>> >>>>>>>> +/* bits for MXR_CM_COEFF_Y */ >>>>>>>> +#define MXR_CM_COEFF_RGB_FULL (1 << 30) >>>>>>>> + >>>>>>>> #endif /* SAMSUNG_REGS_MIXER_H */ >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> dri-devel mailing list >>>>>>> dri-devel@lists.freedesktop.org >>>>>>> https://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 >>>>> >>>>> >>>>> >>>> -- >>>> 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 41d0c36..9648dd5 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -45,6 +45,22 @@ #define MIXER_WIN_NR 3 #define VP_DEFAULT_WIN 2 +/* + * Mixer color space conversion coefficient triplet. + * Used for CSC from RGB to YCbCr. + * Each coefficient is a 10-bit fixed point number with + * sign and no integer part, i.e. + * [0:8] = fractional part (representing a value y = x / 2^9) + * [9] = sign + * Negative values are encoded with two's complement. + */ +#define MXR_CSC_C(x) ((int)((x) * 512.0) & 0x3ff) +#define MXR_CSC_CT(a0, a1, a2) \ + ((MXR_CSC_C(a0) << 20) | (MXR_CSC_C(a1) << 10) | (MXR_CSC_C(a2) << 0)) + +/* YCbCr value, used for mixer background color configuration. */ +#define MXR_YCBCR_VAL(y, cb, cr) (((y) << 16) | ((cb) << 8) | ((cr) << 0)) + /* The pixelformats that are natively supported by the mixer. */ #define MXR_FORMAT_RGB565 4 #define MXR_FORMAT_ARGB1555 5 @@ -391,13 +407,14 @@ static void mixer_cfg_rgb_fmt(struct mixer_context *ctx, unsigned int height) case 1080: default: val = MXR_CFG_RGB709_16_235; + /* Configure the BT.709 CSC matrix for full range RGB. */ mixer_reg_write(res, MXR_CM_COEFF_Y, - (1 << 30) | (94 << 20) | (314 << 10) | - (32 << 0)); + MXR_CSC_CT( 0.184, 0.614, 0.063) | + MXR_CM_COEFF_RGB_FULL); mixer_reg_write(res, MXR_CM_COEFF_CB, - (972 << 20) | (851 << 10) | (225 << 0)); + MXR_CSC_CT(-0.102, -0.338, 0.440)); mixer_reg_write(res, MXR_CM_COEFF_CR, - (225 << 20) | (820 << 10) | (1004 << 0)); + MXR_CSC_CT( 0.440, -0.399, -0.040)); break; } @@ -715,10 +732,10 @@ static void mixer_win_reset(struct mixer_context *ctx) /* reset default layer priority */ mixer_reg_write(res, MXR_LAYER_CFG, 0); - /* setting background color */ - mixer_reg_write(res, MXR_BG_COLOR0, 0x008080); - mixer_reg_write(res, MXR_BG_COLOR1, 0x008080); - mixer_reg_write(res, MXR_BG_COLOR2, 0x008080); + /* set all background colors to RGB (0,0,0) */ + mixer_reg_write(res, MXR_BG_COLOR0, MXR_YCBCR_VAL(0, 128, 128)); + mixer_reg_write(res, MXR_BG_COLOR1, MXR_YCBCR_VAL(0, 128, 128)); + mixer_reg_write(res, MXR_BG_COLOR2, MXR_YCBCR_VAL(0, 128, 128)); if (test_bit(MXR_BIT_VP_ENABLED, &ctx->flags)) { /* configuration of Video Processor Registers */ diff --git a/drivers/gpu/drm/exynos/regs-mixer.h b/drivers/gpu/drm/exynos/regs-mixer.h index 7f22df5..c311f57 100644 --- a/drivers/gpu/drm/exynos/regs-mixer.h +++ b/drivers/gpu/drm/exynos/regs-mixer.h @@ -140,11 +140,11 @@ #define MXR_INT_EN_VSYNC (1 << 11) #define MXR_INT_EN_ALL (0x0f << 8) -/* bit for MXR_INT_STATUS */ +/* bits for MXR_INT_STATUS */ #define MXR_INT_CLEAR_VSYNC (1 << 11) #define MXR_INT_STATUS_VSYNC (1 << 0) -/* bit for MXR_LAYER_CFG */ +/* bits for MXR_LAYER_CFG */ #define MXR_LAYER_CFG_GRP1_VAL(x) MXR_MASK_VAL(x, 11, 8) #define MXR_LAYER_CFG_GRP1_MASK MXR_LAYER_CFG_GRP1_VAL(~0) #define MXR_LAYER_CFG_GRP0_VAL(x) MXR_MASK_VAL(x, 7, 4) @@ -152,5 +152,8 @@ #define MXR_LAYER_CFG_VP_VAL(x) MXR_MASK_VAL(x, 3, 0) #define MXR_LAYER_CFG_VP_MASK MXR_LAYER_CFG_VP_VAL(~0) +/* bits for MXR_CM_COEFF_Y */ +#define MXR_CM_COEFF_RGB_FULL (1 << 30) + #endif /* SAMSUNG_REGS_MIXER_H */
The output stage of the mixer uses YCbCr for the internal computations, which is the reason that some registers take YCbCr related data as input. In particular this applies to MXR_BG_COLOR{0,1,2} and MXR_CM_COEFF_{Y,CB,CR}. Document the formatting of the data which we write to these registers. While at it, unify wording of comments in the register header. Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de> --- Changes in v2: - use floating point values as input for the macros, as suggested by Andrzej - the floating point values have been tuned to exactly match the values that are currently used Changes in v3: - use only three digit values (pointed out by Andrzej) drivers/gpu/drm/exynos/exynos_mixer.c | 33 +++++++++++++++++++++++++-------- drivers/gpu/drm/exynos/regs-mixer.h | 7 +++++-- 2 files changed, 30 insertions(+), 10 deletions(-)