Currently VI layer code reported wrong formats for DE2 and DE3. First three patches are fixes. Next two patches do code refactoring to remove redundant information, which is already included elsewhere. Last two patches are more cosmetic.
Note: It can be argued if patch 2 is really a fix. Consider that if only patch 1 and 3 go into stable, wrong formats will be reported for DE3 VI layers.
Please take a look.
Best regards, Jernej
Jernej Skrabec (7): drm/sun4i: de2/de3: Remove unsupported VI layer formats drm/sun4i: Add separate DE3 VI layer formats drm/sun4i: Fix DE2 VI layer format support drm/sun4i: de2: rgb field in de2 format struct is redundant drm/sun4i: de2: csc_mode in de2 format struct is mostly redundant drm/sun4i: de2: Don't return de2_fmt_info struct drm/sun4i: Sort includes in VI and UI layer code
drivers/gpu/drm/sun4i/sun8i_mixer.c | 159 ++++++++++++------------- drivers/gpu/drm/sun4i/sun8i_mixer.h | 21 ++-- drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 14 ++- drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 106 ++++++++++++++--- 4 files changed, 183 insertions(+), 117 deletions(-)
YUV444 and YVU444 are planar formats, but HW format RGB888 is packed. This means that those two mappings were never correct. Remove them.
Fixes: 60a3dcf96aa8 ("drm/sun4i: Add DE2 definitions for YUV formats") Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net --- drivers/gpu/drm/sun4i/sun8i_mixer.c | 12 ------------ drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 2 -- 2 files changed, 14 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 7c24f8f832a5..3a78dbbceb8a 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -196,12 +196,6 @@ static const struct de2_fmt_info de2_formats[] = { .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, - { - .drm_fmt = DRM_FORMAT_YUV444, - .de2_fmt = SUN8I_MIXER_FBFMT_RGB888, - .rgb = true, - .csc = SUN8I_CSC_MODE_YUV2RGB, - }, { .drm_fmt = DRM_FORMAT_YUV422, .de2_fmt = SUN8I_MIXER_FBFMT_YUV422, @@ -220,12 +214,6 @@ static const struct de2_fmt_info de2_formats[] = { .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, - { - .drm_fmt = DRM_FORMAT_YVU444, - .de2_fmt = SUN8I_MIXER_FBFMT_RGB888, - .rgb = true, - .csc = SUN8I_CSC_MODE_YVU2RGB, - }, { .drm_fmt = DRM_FORMAT_YVU422, .de2_fmt = SUN8I_MIXER_FBFMT_YUV422, diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index 42d445d23773..6a244d6fafd9 100644 --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c @@ -431,11 +431,9 @@ static const u32 sun8i_vi_layer_formats[] = { DRM_FORMAT_YUV411, DRM_FORMAT_YUV420, DRM_FORMAT_YUV422, - DRM_FORMAT_YUV444, DRM_FORMAT_YVU411, DRM_FORMAT_YVU420, DRM_FORMAT_YVU422, - DRM_FORMAT_YVU444, };
struct sun8i_vi_layer *sun8i_vi_layer_init_one(struct drm_device *drm,
DE3 VI layers support alpha blending, but DE2 VI layers do not. Additionally, DE3 VI layers support 10-bit RGB and YUV formats.
Make a separate list for DE3.
Fixes: c50519e6db4d ("drm/sun4i: Add basic support for DE3") Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net --- drivers/gpu/drm/sun4i/sun8i_mixer.c | 36 ++++++++++++++++ drivers/gpu/drm/sun4i/sun8i_mixer.h | 11 +++++ drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 58 ++++++++++++++++++++++++-- 3 files changed, 102 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 3a78dbbceb8a..655445bfe64a 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -148,6 +148,30 @@ static const struct de2_fmt_info de2_formats[] = { .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, + { + .drm_fmt = DRM_FORMAT_ARGB2101010, + .de2_fmt = SUN8I_MIXER_FBFMT_ARGB2101010, + .rgb = true, + .csc = SUN8I_CSC_MODE_OFF, + }, + { + .drm_fmt = DRM_FORMAT_ABGR2101010, + .de2_fmt = SUN8I_MIXER_FBFMT_ABGR2101010, + .rgb = true, + .csc = SUN8I_CSC_MODE_OFF, + }, + { + .drm_fmt = DRM_FORMAT_RGBA1010102, + .de2_fmt = SUN8I_MIXER_FBFMT_RGBA1010102, + .rgb = true, + .csc = SUN8I_CSC_MODE_OFF, + }, + { + .drm_fmt = DRM_FORMAT_BGRA1010102, + .de2_fmt = SUN8I_MIXER_FBFMT_BGRA1010102, + .rgb = true, + .csc = SUN8I_CSC_MODE_OFF, + }, { .drm_fmt = DRM_FORMAT_UYVY, .de2_fmt = SUN8I_MIXER_FBFMT_UYVY, @@ -232,6 +256,18 @@ static const struct de2_fmt_info de2_formats[] = { .rgb = false, .csc = SUN8I_CSC_MODE_YVU2RGB, }, + { + .drm_fmt = DRM_FORMAT_P010, + .de2_fmt = SUN8I_MIXER_FBFMT_P010_YUV, + .rgb = false, + .csc = SUN8I_CSC_MODE_YUV2RGB, + }, + { + .drm_fmt = DRM_FORMAT_P210, + .de2_fmt = SUN8I_MIXER_FBFMT_P210_YUV, + .rgb = false, + .csc = SUN8I_CSC_MODE_YUV2RGB, + }, };
const struct de2_fmt_info *sun8i_mixer_format_info(u32 format) diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h index c6cc94057faf..345b28b0a80a 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h @@ -93,6 +93,10 @@ #define SUN8I_MIXER_FBFMT_ABGR1555 17 #define SUN8I_MIXER_FBFMT_RGBA5551 18 #define SUN8I_MIXER_FBFMT_BGRA5551 19 +#define SUN8I_MIXER_FBFMT_ARGB2101010 20 +#define SUN8I_MIXER_FBFMT_ABGR2101010 21 +#define SUN8I_MIXER_FBFMT_RGBA1010102 22 +#define SUN8I_MIXER_FBFMT_BGRA1010102 23
#define SUN8I_MIXER_FBFMT_YUYV 0 #define SUN8I_MIXER_FBFMT_UYVY 1 @@ -109,6 +113,13 @@ /* format 12 is semi-planar YUV411 UVUV */ /* format 13 is semi-planar YUV411 VUVU */ #define SUN8I_MIXER_FBFMT_YUV411 14 +/* format 15 doesn't exist */ +/* format 16 is P010 YVU */ +#define SUN8I_MIXER_FBFMT_P010_YUV 17 +/* format 18 is P210 YVU */ +#define SUN8I_MIXER_FBFMT_P210_YUV 19 +/* format 20 is packed YVU444 10-bit */ +/* format 21 is packed YUV444 10-bit */
/* * Sub-engines listed bellow are unused for now. The EN registers are here only diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index 6a244d6fafd9..6c0084a3c3d7 100644 --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c @@ -436,24 +436,76 @@ static const u32 sun8i_vi_layer_formats[] = { DRM_FORMAT_YVU422, };
+static const u32 sun8i_vi_layer_de3_formats[] = { + DRM_FORMAT_ABGR1555, + DRM_FORMAT_ABGR2101010, + DRM_FORMAT_ABGR4444, + DRM_FORMAT_ABGR8888, + DRM_FORMAT_ARGB1555, + DRM_FORMAT_ARGB2101010, + DRM_FORMAT_ARGB4444, + DRM_FORMAT_ARGB8888, + DRM_FORMAT_BGR565, + DRM_FORMAT_BGR888, + DRM_FORMAT_BGRA1010102, + DRM_FORMAT_BGRA5551, + DRM_FORMAT_BGRA4444, + DRM_FORMAT_BGRA8888, + DRM_FORMAT_BGRX8888, + DRM_FORMAT_RGB565, + DRM_FORMAT_RGB888, + DRM_FORMAT_RGBA1010102, + DRM_FORMAT_RGBA4444, + DRM_FORMAT_RGBA5551, + DRM_FORMAT_RGBA8888, + DRM_FORMAT_RGBX8888, + DRM_FORMAT_XBGR8888, + DRM_FORMAT_XRGB8888, + + DRM_FORMAT_NV16, + DRM_FORMAT_NV12, + DRM_FORMAT_NV21, + DRM_FORMAT_NV61, + DRM_FORMAT_P010, + DRM_FORMAT_P210, + DRM_FORMAT_UYVY, + DRM_FORMAT_VYUY, + DRM_FORMAT_YUYV, + DRM_FORMAT_YVYU, + DRM_FORMAT_YUV411, + DRM_FORMAT_YUV420, + DRM_FORMAT_YUV422, + DRM_FORMAT_YVU411, + DRM_FORMAT_YVU420, + DRM_FORMAT_YVU422, +}; + struct sun8i_vi_layer *sun8i_vi_layer_init_one(struct drm_device *drm, struct sun8i_mixer *mixer, int index) { u32 supported_encodings, supported_ranges; + unsigned int plane_cnt, format_count; struct sun8i_vi_layer *layer; - unsigned int plane_cnt; + const u32 *formats; int ret;
layer = devm_kzalloc(drm->dev, sizeof(*layer), GFP_KERNEL); if (!layer) return ERR_PTR(-ENOMEM);
+ if (mixer->cfg->is_de3) { + formats = sun8i_vi_layer_de3_formats; + format_count = ARRAY_SIZE(sun8i_vi_layer_de3_formats); + } else { + formats = sun8i_vi_layer_formats; + format_count = ARRAY_SIZE(sun8i_vi_layer_formats); + } + /* possible crtcs are set later */ ret = drm_universal_plane_init(drm, &layer->plane, 0, &sun8i_vi_layer_funcs, - sun8i_vi_layer_formats, - ARRAY_SIZE(sun8i_vi_layer_formats), + formats, format_count, NULL, DRM_PLANE_TYPE_OVERLAY, NULL); if (ret) { dev_err(drm->dev, "Couldn't initialize layer\n");
DE2 VI layer doesn't support blending which means alpha channel is ignored. Replace all formats with alpha with "don't care" (X) channel.
Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net --- drivers/gpu/drm/sun4i/sun8i_mixer.c | 56 ++++++++++++++++++++++++++ drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 22 +++++----- 2 files changed, 67 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 655445bfe64a..4a64f7ae437a 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -106,48 +106,104 @@ static const struct de2_fmt_info de2_formats[] = { .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, + { + /* for DE2 VI layer which ignores alpha */ + .drm_fmt = DRM_FORMAT_XRGB4444, + .de2_fmt = SUN8I_MIXER_FBFMT_ARGB4444, + .rgb = true, + .csc = SUN8I_CSC_MODE_OFF, + }, { .drm_fmt = DRM_FORMAT_ABGR4444, .de2_fmt = SUN8I_MIXER_FBFMT_ABGR4444, .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, + { + /* for DE2 VI layer which ignores alpha */ + .drm_fmt = DRM_FORMAT_XBGR4444, + .de2_fmt = SUN8I_MIXER_FBFMT_ABGR4444, + .rgb = true, + .csc = SUN8I_CSC_MODE_OFF, + }, { .drm_fmt = DRM_FORMAT_RGBA4444, .de2_fmt = SUN8I_MIXER_FBFMT_RGBA4444, .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, + { + /* for DE2 VI layer which ignores alpha */ + .drm_fmt = DRM_FORMAT_RGBX4444, + .de2_fmt = SUN8I_MIXER_FBFMT_RGBA4444, + .rgb = true, + .csc = SUN8I_CSC_MODE_OFF, + }, { .drm_fmt = DRM_FORMAT_BGRA4444, .de2_fmt = SUN8I_MIXER_FBFMT_BGRA4444, .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, + { + /* for DE2 VI layer which ignores alpha */ + .drm_fmt = DRM_FORMAT_BGRX4444, + .de2_fmt = SUN8I_MIXER_FBFMT_BGRA4444, + .rgb = true, + .csc = SUN8I_CSC_MODE_OFF, + }, { .drm_fmt = DRM_FORMAT_ARGB1555, .de2_fmt = SUN8I_MIXER_FBFMT_ARGB1555, .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, + { + /* for DE2 VI layer which ignores alpha */ + .drm_fmt = DRM_FORMAT_XRGB1555, + .de2_fmt = SUN8I_MIXER_FBFMT_ARGB1555, + .rgb = true, + .csc = SUN8I_CSC_MODE_OFF, + }, { .drm_fmt = DRM_FORMAT_ABGR1555, .de2_fmt = SUN8I_MIXER_FBFMT_ABGR1555, .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, + { + /* for DE2 VI layer which ignores alpha */ + .drm_fmt = DRM_FORMAT_XBGR1555, + .de2_fmt = SUN8I_MIXER_FBFMT_ABGR1555, + .rgb = true, + .csc = SUN8I_CSC_MODE_OFF, + }, { .drm_fmt = DRM_FORMAT_RGBA5551, .de2_fmt = SUN8I_MIXER_FBFMT_RGBA5551, .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, + { + /* for DE2 VI layer which ignores alpha */ + .drm_fmt = DRM_FORMAT_RGBX5551, + .de2_fmt = SUN8I_MIXER_FBFMT_RGBA5551, + .rgb = true, + .csc = SUN8I_CSC_MODE_OFF, + }, { .drm_fmt = DRM_FORMAT_BGRA5551, .de2_fmt = SUN8I_MIXER_FBFMT_BGRA5551, .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, + { + /* for DE2 VI layer which ignores alpha */ + .drm_fmt = DRM_FORMAT_BGRX5551, + .de2_fmt = SUN8I_MIXER_FBFMT_BGRA5551, + .rgb = true, + .csc = SUN8I_CSC_MODE_OFF, + }, { .drm_fmt = DRM_FORMAT_ARGB2101010, .de2_fmt = SUN8I_MIXER_FBFMT_ARGB2101010, diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index 6c0084a3c3d7..b8398ca18b0f 100644 --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c @@ -398,26 +398,26 @@ static const struct drm_plane_funcs sun8i_vi_layer_funcs = { };
/* - * While all RGB formats are supported, VI planes don't support - * alpha blending, so there is no point having formats with alpha - * channel if their opaque analog exist. + * While DE2 VI layer supports same RGB formats as UI layer, alpha + * channel is ignored. This structure lists all unique variants + * where alpha channel is replaced with "don't care" (X) channel. */ static const u32 sun8i_vi_layer_formats[] = { - DRM_FORMAT_ABGR1555, - DRM_FORMAT_ABGR4444, - DRM_FORMAT_ARGB1555, - DRM_FORMAT_ARGB4444, DRM_FORMAT_BGR565, DRM_FORMAT_BGR888, - DRM_FORMAT_BGRA5551, - DRM_FORMAT_BGRA4444, + DRM_FORMAT_BGRX4444, + DRM_FORMAT_BGRX5551, DRM_FORMAT_BGRX8888, DRM_FORMAT_RGB565, DRM_FORMAT_RGB888, - DRM_FORMAT_RGBA4444, - DRM_FORMAT_RGBA5551, + DRM_FORMAT_RGBX4444, + DRM_FORMAT_RGBX5551, DRM_FORMAT_RGBX8888, + DRM_FORMAT_XBGR1555, + DRM_FORMAT_XBGR4444, DRM_FORMAT_XBGR8888, + DRM_FORMAT_XRGB1555, + DRM_FORMAT_XRGB4444, DRM_FORMAT_XRGB8888,
DRM_FORMAT_NV16,
drm_format_info structure already contains information if format is RGB or YUV. Use that instead.
Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net --- drivers/gpu/drm/sun4i/sun8i_mixer.c | 48 -------------------------- drivers/gpu/drm/sun4i/sun8i_mixer.h | 1 - drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 6 ++-- drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 6 ++-- 4 files changed, 8 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 4a64f7ae437a..45e3cd9e718c 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -31,297 +31,249 @@ static const struct de2_fmt_info de2_formats[] = { { .drm_fmt = DRM_FORMAT_ARGB8888, .de2_fmt = SUN8I_MIXER_FBFMT_ARGB8888, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_ABGR8888, .de2_fmt = SUN8I_MIXER_FBFMT_ABGR8888, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_RGBA8888, .de2_fmt = SUN8I_MIXER_FBFMT_RGBA8888, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_BGRA8888, .de2_fmt = SUN8I_MIXER_FBFMT_BGRA8888, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_XRGB8888, .de2_fmt = SUN8I_MIXER_FBFMT_XRGB8888, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_XBGR8888, .de2_fmt = SUN8I_MIXER_FBFMT_XBGR8888, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_RGBX8888, .de2_fmt = SUN8I_MIXER_FBFMT_RGBX8888, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_BGRX8888, .de2_fmt = SUN8I_MIXER_FBFMT_BGRX8888, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_RGB888, .de2_fmt = SUN8I_MIXER_FBFMT_RGB888, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_BGR888, .de2_fmt = SUN8I_MIXER_FBFMT_BGR888, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_RGB565, .de2_fmt = SUN8I_MIXER_FBFMT_RGB565, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_BGR565, .de2_fmt = SUN8I_MIXER_FBFMT_BGR565, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_ARGB4444, .de2_fmt = SUN8I_MIXER_FBFMT_ARGB4444, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_XRGB4444, .de2_fmt = SUN8I_MIXER_FBFMT_ARGB4444, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_ABGR4444, .de2_fmt = SUN8I_MIXER_FBFMT_ABGR4444, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_XBGR4444, .de2_fmt = SUN8I_MIXER_FBFMT_ABGR4444, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_RGBA4444, .de2_fmt = SUN8I_MIXER_FBFMT_RGBA4444, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_RGBX4444, .de2_fmt = SUN8I_MIXER_FBFMT_RGBA4444, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_BGRA4444, .de2_fmt = SUN8I_MIXER_FBFMT_BGRA4444, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_BGRX4444, .de2_fmt = SUN8I_MIXER_FBFMT_BGRA4444, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_ARGB1555, .de2_fmt = SUN8I_MIXER_FBFMT_ARGB1555, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_XRGB1555, .de2_fmt = SUN8I_MIXER_FBFMT_ARGB1555, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_ABGR1555, .de2_fmt = SUN8I_MIXER_FBFMT_ABGR1555, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_XBGR1555, .de2_fmt = SUN8I_MIXER_FBFMT_ABGR1555, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_RGBA5551, .de2_fmt = SUN8I_MIXER_FBFMT_RGBA5551, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_RGBX5551, .de2_fmt = SUN8I_MIXER_FBFMT_RGBA5551, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_BGRA5551, .de2_fmt = SUN8I_MIXER_FBFMT_BGRA5551, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_BGRX5551, .de2_fmt = SUN8I_MIXER_FBFMT_BGRA5551, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_ARGB2101010, .de2_fmt = SUN8I_MIXER_FBFMT_ARGB2101010, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_ABGR2101010, .de2_fmt = SUN8I_MIXER_FBFMT_ABGR2101010, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_RGBA1010102, .de2_fmt = SUN8I_MIXER_FBFMT_RGBA1010102, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_BGRA1010102, .de2_fmt = SUN8I_MIXER_FBFMT_BGRA1010102, - .rgb = true, .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_UYVY, .de2_fmt = SUN8I_MIXER_FBFMT_UYVY, - .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_VYUY, .de2_fmt = SUN8I_MIXER_FBFMT_VYUY, - .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_YUYV, .de2_fmt = SUN8I_MIXER_FBFMT_YUYV, - .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_YVYU, .de2_fmt = SUN8I_MIXER_FBFMT_YVYU, - .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_NV16, .de2_fmt = SUN8I_MIXER_FBFMT_NV16, - .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_NV61, .de2_fmt = SUN8I_MIXER_FBFMT_NV61, - .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_NV12, .de2_fmt = SUN8I_MIXER_FBFMT_NV12, - .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_NV21, .de2_fmt = SUN8I_MIXER_FBFMT_NV21, - .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_YUV422, .de2_fmt = SUN8I_MIXER_FBFMT_YUV422, - .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_YUV420, .de2_fmt = SUN8I_MIXER_FBFMT_YUV420, - .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_YUV411, .de2_fmt = SUN8I_MIXER_FBFMT_YUV411, - .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_YVU422, .de2_fmt = SUN8I_MIXER_FBFMT_YUV422, - .rgb = false, .csc = SUN8I_CSC_MODE_YVU2RGB, }, { .drm_fmt = DRM_FORMAT_YVU420, .de2_fmt = SUN8I_MIXER_FBFMT_YUV420, - .rgb = false, .csc = SUN8I_CSC_MODE_YVU2RGB, }, { .drm_fmt = DRM_FORMAT_YVU411, .de2_fmt = SUN8I_MIXER_FBFMT_YUV411, - .rgb = false, .csc = SUN8I_CSC_MODE_YVU2RGB, }, { .drm_fmt = DRM_FORMAT_P010, .de2_fmt = SUN8I_MIXER_FBFMT_P010_YUV, - .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_P210, .de2_fmt = SUN8I_MIXER_FBFMT_P210_YUV, - .rgb = false, .csc = SUN8I_CSC_MODE_YUV2RGB, }, }; diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h index 345b28b0a80a..447ad0b01b00 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h @@ -147,7 +147,6 @@ struct de2_fmt_info { u32 drm_fmt; u32 de2_fmt; - bool rgb; enum sun8i_csc_mode csc; };
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index c87fd842918e..99ee19a00415 100644 --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c @@ -175,12 +175,14 @@ static int sun8i_ui_layer_update_formats(struct sun8i_mixer *mixer, int channel, { struct drm_plane_state *state = plane->state; const struct de2_fmt_info *fmt_info; + const struct drm_format_info *fmt; u32 val, ch_base;
ch_base = sun8i_channel_base(mixer, channel);
- fmt_info = sun8i_mixer_format_info(state->fb->format->format); - if (!fmt_info || !fmt_info->rgb) { + fmt = state->fb->format; + fmt_info = sun8i_mixer_format_info(fmt->format); + if (!fmt_info || fmt->is_yuv) { DRM_DEBUG_DRIVER("Invalid format\n"); return -EINVAL; } diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index b8398ca18b0f..a2eb4371026d 100644 --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c @@ -215,11 +215,13 @@ static int sun8i_vi_layer_update_formats(struct sun8i_mixer *mixer, int channel, { struct drm_plane_state *state = plane->state; const struct de2_fmt_info *fmt_info; + const struct drm_format_info *fmt; u32 val, ch_base;
ch_base = sun8i_channel_base(mixer, channel);
- fmt_info = sun8i_mixer_format_info(state->fb->format->format); + fmt = state->fb->format; + fmt_info = sun8i_mixer_format_info(fmt->format); if (!fmt_info) { DRM_DEBUG_DRIVER("Invalid format\n"); return -EINVAL; @@ -239,7 +241,7 @@ static int sun8i_vi_layer_update_formats(struct sun8i_mixer *mixer, int channel, sun8i_csc_enable_ccsc(mixer, channel, false); }
- if (fmt_info->rgb) + if (!fmt->is_yuv) val = SUN8I_MIXER_CHAN_VI_LAYER_ATTR_RGB_MODE; else val = 0;
For RGB formats CSC mode is always set to none and for YUV formats almost always set to YUV to RGB.
Add a helper function to deduce CSC mode from format.
Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net --- drivers/gpu/drm/sun4i/sun8i_mixer.c | 48 -------------------------- drivers/gpu/drm/sun4i/sun8i_mixer.h | 6 ++-- drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 24 +++++++++++-- 3 files changed, 23 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index 45e3cd9e718c..e078ec96de2d 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -31,250 +31,202 @@ static const struct de2_fmt_info de2_formats[] = { { .drm_fmt = DRM_FORMAT_ARGB8888, .de2_fmt = SUN8I_MIXER_FBFMT_ARGB8888, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_ABGR8888, .de2_fmt = SUN8I_MIXER_FBFMT_ABGR8888, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_RGBA8888, .de2_fmt = SUN8I_MIXER_FBFMT_RGBA8888, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_BGRA8888, .de2_fmt = SUN8I_MIXER_FBFMT_BGRA8888, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_XRGB8888, .de2_fmt = SUN8I_MIXER_FBFMT_XRGB8888, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_XBGR8888, .de2_fmt = SUN8I_MIXER_FBFMT_XBGR8888, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_RGBX8888, .de2_fmt = SUN8I_MIXER_FBFMT_RGBX8888, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_BGRX8888, .de2_fmt = SUN8I_MIXER_FBFMT_BGRX8888, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_RGB888, .de2_fmt = SUN8I_MIXER_FBFMT_RGB888, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_BGR888, .de2_fmt = SUN8I_MIXER_FBFMT_BGR888, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_RGB565, .de2_fmt = SUN8I_MIXER_FBFMT_RGB565, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_BGR565, .de2_fmt = SUN8I_MIXER_FBFMT_BGR565, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_ARGB4444, .de2_fmt = SUN8I_MIXER_FBFMT_ARGB4444, - .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_XRGB4444, .de2_fmt = SUN8I_MIXER_FBFMT_ARGB4444, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_ABGR4444, .de2_fmt = SUN8I_MIXER_FBFMT_ABGR4444, - .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_XBGR4444, .de2_fmt = SUN8I_MIXER_FBFMT_ABGR4444, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_RGBA4444, .de2_fmt = SUN8I_MIXER_FBFMT_RGBA4444, - .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_RGBX4444, .de2_fmt = SUN8I_MIXER_FBFMT_RGBA4444, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_BGRA4444, .de2_fmt = SUN8I_MIXER_FBFMT_BGRA4444, - .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_BGRX4444, .de2_fmt = SUN8I_MIXER_FBFMT_BGRA4444, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_ARGB1555, .de2_fmt = SUN8I_MIXER_FBFMT_ARGB1555, - .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_XRGB1555, .de2_fmt = SUN8I_MIXER_FBFMT_ARGB1555, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_ABGR1555, .de2_fmt = SUN8I_MIXER_FBFMT_ABGR1555, - .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_XBGR1555, .de2_fmt = SUN8I_MIXER_FBFMT_ABGR1555, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_RGBA5551, .de2_fmt = SUN8I_MIXER_FBFMT_RGBA5551, - .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_RGBX5551, .de2_fmt = SUN8I_MIXER_FBFMT_RGBA5551, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_BGRA5551, .de2_fmt = SUN8I_MIXER_FBFMT_BGRA5551, - .csc = SUN8I_CSC_MODE_OFF, }, { /* for DE2 VI layer which ignores alpha */ .drm_fmt = DRM_FORMAT_BGRX5551, .de2_fmt = SUN8I_MIXER_FBFMT_BGRA5551, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_ARGB2101010, .de2_fmt = SUN8I_MIXER_FBFMT_ARGB2101010, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_ABGR2101010, .de2_fmt = SUN8I_MIXER_FBFMT_ABGR2101010, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_RGBA1010102, .de2_fmt = SUN8I_MIXER_FBFMT_RGBA1010102, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_BGRA1010102, .de2_fmt = SUN8I_MIXER_FBFMT_BGRA1010102, - .csc = SUN8I_CSC_MODE_OFF, }, { .drm_fmt = DRM_FORMAT_UYVY, .de2_fmt = SUN8I_MIXER_FBFMT_UYVY, - .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_VYUY, .de2_fmt = SUN8I_MIXER_FBFMT_VYUY, - .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_YUYV, .de2_fmt = SUN8I_MIXER_FBFMT_YUYV, - .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_YVYU, .de2_fmt = SUN8I_MIXER_FBFMT_YVYU, - .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_NV16, .de2_fmt = SUN8I_MIXER_FBFMT_NV16, - .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_NV61, .de2_fmt = SUN8I_MIXER_FBFMT_NV61, - .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_NV12, .de2_fmt = SUN8I_MIXER_FBFMT_NV12, - .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_NV21, .de2_fmt = SUN8I_MIXER_FBFMT_NV21, - .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_YUV422, .de2_fmt = SUN8I_MIXER_FBFMT_YUV422, - .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_YUV420, .de2_fmt = SUN8I_MIXER_FBFMT_YUV420, - .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_YUV411, .de2_fmt = SUN8I_MIXER_FBFMT_YUV411, - .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_YVU422, .de2_fmt = SUN8I_MIXER_FBFMT_YUV422, - .csc = SUN8I_CSC_MODE_YVU2RGB, }, { .drm_fmt = DRM_FORMAT_YVU420, .de2_fmt = SUN8I_MIXER_FBFMT_YUV420, - .csc = SUN8I_CSC_MODE_YVU2RGB, }, { .drm_fmt = DRM_FORMAT_YVU411, .de2_fmt = SUN8I_MIXER_FBFMT_YUV411, - .csc = SUN8I_CSC_MODE_YVU2RGB, }, { .drm_fmt = DRM_FORMAT_P010, .de2_fmt = SUN8I_MIXER_FBFMT_P010_YUV, - .csc = SUN8I_CSC_MODE_YUV2RGB, }, { .drm_fmt = DRM_FORMAT_P210, .de2_fmt = SUN8I_MIXER_FBFMT_P210_YUV, - .csc = SUN8I_CSC_MODE_YUV2RGB, }, };
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h index 447ad0b01b00..0dd4a347fa06 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h @@ -10,7 +10,6 @@ #include <linux/regmap.h> #include <linux/reset.h>
-#include "sun8i_csc.h" #include "sunxi_engine.h"
#define SUN8I_MIXER_SIZE(w, h) (((h) - 1) << 16 | ((w) - 1)) @@ -145,9 +144,8 @@ #define SUN50I_MIXER_CDC1_EN 0xd8000
struct de2_fmt_info { - u32 drm_fmt; - u32 de2_fmt; - enum sun8i_csc_mode csc; + u32 drm_fmt; + u32 de2_fmt; };
/** diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index a2eb4371026d..d783c2bfc77e 100644 --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c @@ -12,6 +12,7 @@ #include <drm/drm_plane_helper.h> #include <drm/drm_probe_helper.h>
+#include "sun8i_csc.h" #include "sun8i_vi_layer.h" #include "sun8i_mixer.h" #include "sun8i_vi_scaler.h" @@ -210,13 +211,29 @@ static int sun8i_vi_layer_update_coord(struct sun8i_mixer *mixer, int channel, return 0; }
+static bool sun8i_vi_layer_get_csc_mode(const struct drm_format_info *format) +{ + if (!format->is_yuv) + return SUN8I_CSC_MODE_OFF; + + switch (format->format) { + case DRM_FORMAT_YVU411: + case DRM_FORMAT_YVU420: + case DRM_FORMAT_YVU422: + case DRM_FORMAT_YVU444: + return SUN8I_CSC_MODE_YVU2RGB; + default: + return SUN8I_CSC_MODE_YUV2RGB; + } +} + static int sun8i_vi_layer_update_formats(struct sun8i_mixer *mixer, int channel, int overlay, struct drm_plane *plane) { struct drm_plane_state *state = plane->state; const struct de2_fmt_info *fmt_info; const struct drm_format_info *fmt; - u32 val, ch_base; + u32 val, ch_base, csc_mode;
ch_base = sun8i_channel_base(mixer, channel);
@@ -232,8 +249,9 @@ static int sun8i_vi_layer_update_formats(struct sun8i_mixer *mixer, int channel, SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay), SUN8I_MIXER_CHAN_VI_LAYER_ATTR_FBFMT_MASK, val);
- if (fmt_info->csc != SUN8I_CSC_MODE_OFF) { - sun8i_csc_set_ccsc_coefficients(mixer, channel, fmt_info->csc, + csc_mode = sun8i_vi_layer_get_csc_mode(fmt); + if (csc_mode != SUN8I_CSC_MODE_OFF) { + sun8i_csc_set_ccsc_coefficients(mixer, channel, csc_mode, state->color_encoding, state->color_range); sun8i_csc_enable_ccsc(mixer, channel, true);
Now that de2_fmt_info contains only DRM <-> HW format mapping, it doesn't make sense to return pointer to structure when searching by DRM format. Rework that to return only HW format instead.
This doesn't make any functional change.
Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net --- drivers/gpu/drm/sun4i/sun8i_mixer.c | 15 +++++++++++---- drivers/gpu/drm/sun4i/sun8i_mixer.h | 7 +------ drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 10 +++++----- drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 12 ++++++------ 4 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index e078ec96de2d..56cc037fd312 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -27,6 +27,11 @@ #include "sun8i_vi_layer.h" #include "sunxi_engine.h"
+struct de2_fmt_info { + u32 drm_fmt; + u32 de2_fmt; +}; + static const struct de2_fmt_info de2_formats[] = { { .drm_fmt = DRM_FORMAT_ARGB8888, @@ -230,15 +235,17 @@ static const struct de2_fmt_info de2_formats[] = { }, };
-const struct de2_fmt_info *sun8i_mixer_format_info(u32 format) +int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw_format) { unsigned int i;
for (i = 0; i < ARRAY_SIZE(de2_formats); ++i) - if (de2_formats[i].drm_fmt == format) - return &de2_formats[i]; + if (de2_formats[i].drm_fmt == format) { + *hw_format = de2_formats[i].de2_fmt; + return 0; + }
- return NULL; + return -EINVAL; }
static void sun8i_mixer_commit(struct sunxi_engine *engine) diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/sun8i_mixer.h index 0dd4a347fa06..7576b523fdbb 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h @@ -143,11 +143,6 @@ #define SUN50I_MIXER_CDC0_EN 0xd0000 #define SUN50I_MIXER_CDC1_EN 0xd8000
-struct de2_fmt_info { - u32 drm_fmt; - u32 de2_fmt; -}; - /** * struct sun8i_mixer_cfg - mixer HW configuration * @vi_num: number of VI channels @@ -207,5 +202,5 @@ sun8i_channel_base(struct sun8i_mixer *mixer, int channel) return DE2_CH_BASE + channel * DE2_CH_SIZE; }
-const struct de2_fmt_info *sun8i_mixer_format_info(u32 format); +int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw_format); #endif /* _SUN8I_MIXER_H_ */ diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index 99ee19a00415..a64aaea1ba74 100644 --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c @@ -174,20 +174,20 @@ static int sun8i_ui_layer_update_formats(struct sun8i_mixer *mixer, int channel, int overlay, struct drm_plane *plane) { struct drm_plane_state *state = plane->state; - const struct de2_fmt_info *fmt_info; const struct drm_format_info *fmt; - u32 val, ch_base; + u32 val, ch_base, hw_fmt; + int ret;
ch_base = sun8i_channel_base(mixer, channel);
fmt = state->fb->format; - fmt_info = sun8i_mixer_format_info(fmt->format); - if (!fmt_info || fmt->is_yuv) { + ret = sun8i_mixer_drm_format_to_hw(fmt->format, &hw_fmt); + if (ret || fmt->is_yuv) { DRM_DEBUG_DRIVER("Invalid format\n"); return -EINVAL; }
- val = fmt_info->de2_fmt << SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_OFFSET; + val = hw_fmt << SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_OFFSET; regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_CHAN_UI_LAYER_ATTR(ch_base, overlay), SUN8I_MIXER_CHAN_UI_LAYER_ATTR_FBFMT_MASK, val); diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index d783c2bfc77e..b1e1ba2da663 100644 --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c @@ -231,20 +231,20 @@ static int sun8i_vi_layer_update_formats(struct sun8i_mixer *mixer, int channel, int overlay, struct drm_plane *plane) { struct drm_plane_state *state = plane->state; - const struct de2_fmt_info *fmt_info; + u32 val, ch_base, csc_mode, hw_fmt; const struct drm_format_info *fmt; - u32 val, ch_base, csc_mode; + int ret;
ch_base = sun8i_channel_base(mixer, channel);
fmt = state->fb->format; - fmt_info = sun8i_mixer_format_info(fmt->format); - if (!fmt_info) { + ret = sun8i_mixer_drm_format_to_hw(fmt->format, &hw_fmt); + if (ret) { DRM_DEBUG_DRIVER("Invalid format\n"); - return -EINVAL; + return ret; }
- val = fmt_info->de2_fmt << SUN8I_MIXER_CHAN_VI_LAYER_ATTR_FBFMT_OFFSET; + val = hw_fmt << SUN8I_MIXER_CHAN_VI_LAYER_ATTR_FBFMT_OFFSET; regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_CHAN_VI_LAYER_ATTR(ch_base, overlay), SUN8I_MIXER_CHAN_VI_LAYER_ATTR_FBFMT_MASK, val);
Hi,
On Mon, Feb 24, 2020 at 06:39:00PM +0100, Jernej Skrabec wrote:
Now that de2_fmt_info contains only DRM <-> HW format mapping, it doesn't make sense to return pointer to structure when searching by DRM format. Rework that to return only HW format instead.
This doesn't make any functional change.
Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
drivers/gpu/drm/sun4i/sun8i_mixer.c | 15 +++++++++++---- drivers/gpu/drm/sun4i/sun8i_mixer.h | 7 +------ drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 10 +++++----- drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 12 ++++++------ 4 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index e078ec96de2d..56cc037fd312 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -27,6 +27,11 @@ #include "sun8i_vi_layer.h" #include "sunxi_engine.h"
+struct de2_fmt_info {
- u32 drm_fmt;
- u32 de2_fmt;
+};
static const struct de2_fmt_info de2_formats[] = { { .drm_fmt = DRM_FORMAT_ARGB8888, @@ -230,15 +235,17 @@ static const struct de2_fmt_info de2_formats[] = { }, };
-const struct de2_fmt_info *sun8i_mixer_format_info(u32 format) +int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw_format) { unsigned int i;
for (i = 0; i < ARRAY_SIZE(de2_formats); ++i)
if (de2_formats[i].drm_fmt == format)return &de2_formats[i];
if (de2_formats[i].drm_fmt == format) {*hw_format = de2_formats[i].de2_fmt;return 0;}
- return NULL;
- return -EINVAL;
}
I'm not too sure about that one. It breaks the consistency with the other functions, and I don't really see a particular benefit to it?
The rest of the series is Acked-by: Maxime Ripard mripard@kernel.org
Maxime
On Tue, Feb 25, 2020 at 4:35 PM Maxime Ripard maxime@cerno.tech wrote:
Hi,
On Mon, Feb 24, 2020 at 06:39:00PM +0100, Jernej Skrabec wrote:
Now that de2_fmt_info contains only DRM <-> HW format mapping, it doesn't make sense to return pointer to structure when searching by DRM format. Rework that to return only HW format instead.
This doesn't make any functional change.
Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
drivers/gpu/drm/sun4i/sun8i_mixer.c | 15 +++++++++++---- drivers/gpu/drm/sun4i/sun8i_mixer.h | 7 +------ drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 10 +++++----- drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 12 ++++++------ 4 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index e078ec96de2d..56cc037fd312 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -27,6 +27,11 @@ #include "sun8i_vi_layer.h" #include "sunxi_engine.h"
+struct de2_fmt_info {
u32 drm_fmt;u32 de2_fmt;+};
static const struct de2_fmt_info de2_formats[] = { { .drm_fmt = DRM_FORMAT_ARGB8888, @@ -230,15 +235,17 @@ static const struct de2_fmt_info de2_formats[] = { }, };
-const struct de2_fmt_info *sun8i_mixer_format_info(u32 format) +int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw_format) { unsigned int i;
for (i = 0; i < ARRAY_SIZE(de2_formats); ++i)
if (de2_formats[i].drm_fmt == format)return &de2_formats[i];
if (de2_formats[i].drm_fmt == format) {*hw_format = de2_formats[i].de2_fmt;return 0;}
return NULL;
return -EINVAL;}
I'm not too sure about that one. It breaks the consistency with the other functions, and I don't really see a particular benefit to it?
I guess we could just define an "invalid" value, and have the function return that if can't find a match? I'm guessing 0x0 is valid, so maybe 0xffffffff or 0xdeadbeef ?
That would keep consistency with everything else all the while removing the level of indirection you wanted to.
ChenYu
The rest of the series is Acked-by: Maxime Ripard mripard@kernel.org
Maxime
Hi!
Dne torek, 25. februar 2020 ob 09:52:18 CET je Chen-Yu Tsai napisal(a):
On Tue, Feb 25, 2020 at 4:35 PM Maxime Ripard maxime@cerno.tech wrote:
Hi,
On Mon, Feb 24, 2020 at 06:39:00PM +0100, Jernej Skrabec wrote:
Now that de2_fmt_info contains only DRM <-> HW format mapping, it doesn't make sense to return pointer to structure when searching by DRM format. Rework that to return only HW format instead.
This doesn't make any functional change.
Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
drivers/gpu/drm/sun4i/sun8i_mixer.c | 15 +++++++++++---- drivers/gpu/drm/sun4i/sun8i_mixer.h | 7 +------ drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 10 +++++----- drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 12 ++++++------ 4 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index e078ec96de2d..56cc037fd312 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -27,6 +27,11 @@
#include "sun8i_vi_layer.h" #include "sunxi_engine.h"
+struct de2_fmt_info {
u32 drm_fmt;u32 de2_fmt;+};
static const struct de2_fmt_info de2_formats[] = {
{ .drm_fmt = DRM_FORMAT_ARGB8888,@@ -230,15 +235,17 @@ static const struct de2_fmt_info de2_formats[] = {
},};
-const struct de2_fmt_info *sun8i_mixer_format_info(u32 format) +int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw_format)
{
unsigned int i; for (i = 0; i < ARRAY_SIZE(de2_formats); ++i)
if (de2_formats[i].drm_fmt == format)return &de2_formats[i];
if (de2_formats[i].drm_fmt == format) {*hw_format = de2_formats[i].de2_fmt;return 0;}
return NULL;
return -EINVAL;}
I'm not too sure about that one. It breaks the consistency with the other functions, and I don't really see a particular benefit to it?
I don't have strong opinion about this patch. It can be dropped.
I guess we could just define an "invalid" value, and have the function return that if can't find a match? I'm guessing 0x0 is valid, so maybe 0xffffffff or 0xdeadbeef ?
That would keep consistency with everything else all the while removing the level of indirection you wanted to.
I modeled this after static int sun4i_backend_drm_format_to_layer(u32 format, u32 *mode); from sun4i_backend.c.
What consistency do you have in mind?
ChenYu
The rest of the series is Acked-by: Maxime Ripard mripard@kernel.org
Thanks!
Best regards, Jernej
Maxime
On Wed, Feb 26, 2020 at 2:50 AM Jernej Škrabec jernej.skrabec@siol.net wrote:
Hi!
Dne torek, 25. februar 2020 ob 09:52:18 CET je Chen-Yu Tsai napisal(a):
On Tue, Feb 25, 2020 at 4:35 PM Maxime Ripard maxime@cerno.tech wrote:
Hi,
On Mon, Feb 24, 2020 at 06:39:00PM +0100, Jernej Skrabec wrote:
Now that de2_fmt_info contains only DRM <-> HW format mapping, it doesn't make sense to return pointer to structure when searching by DRM format. Rework that to return only HW format instead.
This doesn't make any functional change.
Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
drivers/gpu/drm/sun4i/sun8i_mixer.c | 15 +++++++++++---- drivers/gpu/drm/sun4i/sun8i_mixer.h | 7 +------ drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 10 +++++----- drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 12 ++++++------ 4 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index e078ec96de2d..56cc037fd312 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -27,6 +27,11 @@
#include "sun8i_vi_layer.h" #include "sunxi_engine.h"
+struct de2_fmt_info {
u32 drm_fmt;u32 de2_fmt;+};
static const struct de2_fmt_info de2_formats[] = {
{ .drm_fmt = DRM_FORMAT_ARGB8888,@@ -230,15 +235,17 @@ static const struct de2_fmt_info de2_formats[] = {
},};
-const struct de2_fmt_info *sun8i_mixer_format_info(u32 format) +int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw_format)
{
unsigned int i; for (i = 0; i < ARRAY_SIZE(de2_formats); ++i)
if (de2_formats[i].drm_fmt == format)return &de2_formats[i];
if (de2_formats[i].drm_fmt == format) {*hw_format = de2_formats[i].de2_fmt;return 0;}
return NULL;
return -EINVAL;}
I'm not too sure about that one. It breaks the consistency with the other functions, and I don't really see a particular benefit to it?
I don't have strong opinion about this patch. It can be dropped.
I guess we could just define an "invalid" value, and have the function return that if can't find a match? I'm guessing 0x0 is valid, so maybe 0xffffffff or 0xdeadbeef ?
That would keep consistency with everything else all the while removing the level of indirection you wanted to.
I modeled this after static int sun4i_backend_drm_format_to_layer(u32 format, u32 *mode); from sun4i_backend.c.
What consistency do you have in mind?
Directly returning values (or error codes) instead of passing in a pointer for data to be returned. I assumed that was what Maxime was referring to.
ChenYu
ChenYu
The rest of the series is Acked-by: Maxime Ripard mripard@kernel.org
Thanks!
Best regards, Jernej
Maxime
On Tue, Feb 25, 2020 at 07:50:03PM +0100, Jernej Škrabec wrote:
Hi!
Dne torek, 25. februar 2020 ob 09:52:18 CET je Chen-Yu Tsai napisal(a):
On Tue, Feb 25, 2020 at 4:35 PM Maxime Ripard maxime@cerno.tech wrote:
Hi,
On Mon, Feb 24, 2020 at 06:39:00PM +0100, Jernej Skrabec wrote:
Now that de2_fmt_info contains only DRM <-> HW format mapping, it doesn't make sense to return pointer to structure when searching by DRM format. Rework that to return only HW format instead.
This doesn't make any functional change.
Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net
drivers/gpu/drm/sun4i/sun8i_mixer.c | 15 +++++++++++---- drivers/gpu/drm/sun4i/sun8i_mixer.h | 7 +------ drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 10 +++++----- drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 12 ++++++------ 4 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c index e078ec96de2d..56cc037fd312 100644 --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c @@ -27,6 +27,11 @@
#include "sun8i_vi_layer.h" #include "sunxi_engine.h"
+struct de2_fmt_info {
u32 drm_fmt;u32 de2_fmt;+};
static const struct de2_fmt_info de2_formats[] = {
{ .drm_fmt = DRM_FORMAT_ARGB8888,@@ -230,15 +235,17 @@ static const struct de2_fmt_info de2_formats[] = {
},};
-const struct de2_fmt_info *sun8i_mixer_format_info(u32 format) +int sun8i_mixer_drm_format_to_hw(u32 format, u32 *hw_format)
{
unsigned int i; for (i = 0; i < ARRAY_SIZE(de2_formats); ++i)
if (de2_formats[i].drm_fmt == format)return &de2_formats[i];
if (de2_formats[i].drm_fmt == format) {*hw_format = de2_formats[i].de2_fmt;return 0;}
return NULL;
return -EINVAL;}
I'm not too sure about that one. It breaks the consistency with the other functions, and I don't really see a particular benefit to it?
I don't have strong opinion about this patch. It can be dropped.
I guess we could just define an "invalid" value, and have the function return that if can't find a match? I'm guessing 0x0 is valid, so maybe 0xffffffff or 0xdeadbeef ?
That would keep consistency with everything else all the while removing the level of indirection you wanted to.
I modeled this after static int sun4i_backend_drm_format_to_layer(u32 format, u32 *mode); from sun4i_backend.c.
What consistency do you have in mind?
Well I guess if we're doing that elsewhere it's not really fair to ask you to change this then :)
Fine by me
Maxime
sun8i_mixer.h include is misplaced. Move it.
Signed-off-by: Jernej Skrabec jernej.skrabec@siol.net --- drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 2 +- drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c index a64aaea1ba74..54f937a7d5e7 100644 --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c @@ -19,8 +19,8 @@ #include <drm/drm_plane_helper.h> #include <drm/drm_probe_helper.h>
-#include "sun8i_ui_layer.h" #include "sun8i_mixer.h" +#include "sun8i_ui_layer.h" #include "sun8i_ui_scaler.h"
static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel, diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c index b1e1ba2da663..22c8c5375d0d 100644 --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c @@ -13,8 +13,8 @@ #include <drm/drm_probe_helper.h>
#include "sun8i_csc.h" -#include "sun8i_vi_layer.h" #include "sun8i_mixer.h" +#include "sun8i_vi_layer.h" #include "sun8i_vi_scaler.h"
static void sun8i_vi_layer_enable(struct sun8i_mixer *mixer, int channel,
dri-devel@lists.freedesktop.org
-
Chen-Yu Tsai -
Jernej Skrabec -
Jernej Škrabec
-
Maxime Ripard