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@xxxxxxxxxxx> >>>>>>> Signed-off-by: Tobias Jakobi <tjakobi@xxxxxxxxxxxxxxxxxxxxx> >>>>>>> --- >>>>>>> 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@xxxxxxxxxxxxxxxxxxxxx >>>>>> 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@xxxxxxxxxxxxxxx >>>>> 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@xxxxxxxxxxxxxxx >>>> 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@xxxxxxxxxxxxxxx >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >> >> >> >> _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
