* [PATCH] drm: remove DRM_FORMAT_NV12MT
@ 2015-02-03 15:48 Daniel Vetter
2015-02-03 16:17 ` Gustavo Padovan
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Daniel Vetter @ 2015-02-03 15:48 UTC (permalink / raw)
To: DRI Development
Cc: Daniel Vetter, Seung-Woo Kim, Kyungmin Park, Rob Clark,
Daniel Vetter
So this has been merged originally in
commit 83052d4d5cd518332440bb4ee63d68bb5f744e0f
Author: Seung-Woo Kim <sw0312.kim@samsung.com>
Date: Thu Dec 15 15:40:55 2011 +0900
drm: Add multi buffer plane pixel formats
which hasn't seen a lot of review really. The problem is that it's not
a real pixel format, but just a different way to lay out NV12 pixels
in macroblocks, i.e. a tiling format.
The new way of doing this is with the soon-to-be-merge fb modifiers.
This was brough up in some long irc discussion around the entire
topic, as an example of where things have gone wrong. Luckily we can
correct the mistake:
- The kms side support for NV12MT is all dead code because
format_check in drm_crtc.c never accepted NV12MT.
- The gem side for the gsc support doesn't look better: The code
forgets to set the pixel format and makes a big mess with the tiling
mode bits, inadvertedly setting them all.
Conclusion: This never really worked (at least not in upstream) and
hence we can savely correct our mistake here.
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Rob Clark <robclark@freedesktop.org>
Cc: Daniel Stone <daniel@fooishbar.org>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
drivers/gpu/drm/exynos/exynos_drm_fimc.c | 14 ++------------
drivers/gpu/drm/exynos/exynos_drm_gsc.c | 6 ------
drivers/gpu/drm/exynos/exynos_drm_plane.c | 1 -
drivers/gpu/drm/exynos/exynos_mixer.c | 2 --
include/uapi/drm/drm_fourcc.h | 3 ---
5 files changed, 2 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
index 835b6af00970..842d6b8dc3c4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
@@ -461,7 +461,6 @@ static int fimc_src_set_fmt_order(struct fimc_context *ctx, u32 fmt)
cfg |= EXYNOS_MSCTRL_C_INT_IN_3PLANE;
break;
case DRM_FORMAT_NV12:
- case DRM_FORMAT_NV12MT:
case DRM_FORMAT_NV16:
cfg |= (EXYNOS_MSCTRL_ORDER2P_LSB_CBCR |
EXYNOS_MSCTRL_C_INT_IN_2PLANE);
@@ -511,7 +510,6 @@ static int fimc_src_set_fmt(struct device *dev, u32 fmt)
case DRM_FORMAT_YVU420:
case DRM_FORMAT_NV12:
case DRM_FORMAT_NV21:
- case DRM_FORMAT_NV12MT:
cfg |= EXYNOS_MSCTRL_INFORMAT_YCBCR420;
break;
default:
@@ -524,10 +522,7 @@ static int fimc_src_set_fmt(struct device *dev, u32 fmt)
cfg = fimc_read(ctx, EXYNOS_CIDMAPARAM);
cfg &= ~EXYNOS_CIDMAPARAM_R_MODE_MASK;
- if (fmt == DRM_FORMAT_NV12MT)
- cfg |= EXYNOS_CIDMAPARAM_R_MODE_64X32;
- else
- cfg |= EXYNOS_CIDMAPARAM_R_MODE_LINEAR;
+ cfg |= EXYNOS_CIDMAPARAM_R_MODE_LINEAR;
fimc_write(ctx, cfg, EXYNOS_CIDMAPARAM);
@@ -812,7 +807,6 @@ static int fimc_dst_set_fmt_order(struct fimc_context *ctx, u32 fmt)
cfg |= EXYNOS_CIOCTRL_YCBCR_3PLANE;
break;
case DRM_FORMAT_NV12:
- case DRM_FORMAT_NV12MT:
case DRM_FORMAT_NV16:
cfg |= EXYNOS_CIOCTRL_ORDER2P_LSB_CBCR;
cfg |= EXYNOS_CIOCTRL_YCBCR_2PLANE;
@@ -867,7 +861,6 @@ static int fimc_dst_set_fmt(struct device *dev, u32 fmt)
case DRM_FORMAT_YUV420:
case DRM_FORMAT_YVU420:
case DRM_FORMAT_NV12:
- case DRM_FORMAT_NV12MT:
case DRM_FORMAT_NV21:
cfg |= EXYNOS_CITRGFMT_OUTFORMAT_YCBCR420;
break;
@@ -883,10 +876,7 @@ static int fimc_dst_set_fmt(struct device *dev, u32 fmt)
cfg = fimc_read(ctx, EXYNOS_CIDMAPARAM);
cfg &= ~EXYNOS_CIDMAPARAM_W_MODE_MASK;
- if (fmt == DRM_FORMAT_NV12MT)
- cfg |= EXYNOS_CIDMAPARAM_W_MODE_64X32;
- else
- cfg |= EXYNOS_CIDMAPARAM_W_MODE_LINEAR;
+ cfg |= EXYNOS_CIDMAPARAM_W_MODE_LINEAR;
fimc_write(ctx, cfg, EXYNOS_CIDMAPARAM);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
index 0261468c8019..8040ed2a831f 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
@@ -542,9 +542,6 @@ static int gsc_src_set_fmt(struct device *dev, u32 fmt)
cfg |= (GSC_IN_CHROMA_ORDER_CBCR |
GSC_IN_YUV420_2P);
break;
- case DRM_FORMAT_NV12MT:
- cfg |= (GSC_IN_TILE_C_16x8 | GSC_IN_TILE_MODE);
- break;
default:
dev_err(ippdrv->dev, "inavlid target yuv order 0x%x.\n", fmt);
return -EINVAL;
@@ -809,9 +806,6 @@ static int gsc_dst_set_fmt(struct device *dev, u32 fmt)
cfg |= (GSC_OUT_CHROMA_ORDER_CBCR |
GSC_OUT_YUV420_2P);
break;
- case DRM_FORMAT_NV12MT:
- cfg |= (GSC_OUT_TILE_C_16x8 | GSC_OUT_TILE_MODE);
- break;
default:
dev_err(ippdrv->dev, "inavlid target yuv order 0x%x.\n", fmt);
return -EINVAL;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index c7045a663763..92d75a4eabd7 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -30,7 +30,6 @@ static const uint32_t formats[] = {
DRM_FORMAT_XRGB8888,
DRM_FORMAT_ARGB8888,
DRM_FORMAT_NV12,
- DRM_FORMAT_NV12MT,
};
/*
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 820b76234ef4..5da28443342d 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -417,8 +417,6 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
win_data = &ctx->win_data[win];
switch (win_data->pixel_format) {
- case DRM_FORMAT_NV12MT:
- tiled_mode = true;
case DRM_FORMAT_NV12:
crcb_mode = false;
buf_num = 2;
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 646ae5f39f42..a284f11a8ef5 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -109,9 +109,6 @@
#define DRM_FORMAT_NV24 fourcc_code('N', 'V', '2', '4') /* non-subsampled Cr:Cb plane */
#define DRM_FORMAT_NV42 fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */
-/* special NV12 tiled format */
-#define DRM_FORMAT_NV12MT fourcc_code('T', 'M', '1', '2') /* 2x2 subsampled Cr:Cb plane 64x32 macroblocks */
-
/*
* 3 plane YCbCr
* index 0: Y plane, [7:0] Y
--
2.1.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] drm: remove DRM_FORMAT_NV12MT
2015-02-03 15:48 [PATCH] drm: remove DRM_FORMAT_NV12MT Daniel Vetter
@ 2015-02-03 16:17 ` Gustavo Padovan
2015-02-04 2:30 ` Joonyoung Shim
2015-02-04 4:55 ` Seung-Woo Kim
2 siblings, 0 replies; 5+ messages in thread
From: Gustavo Padovan @ 2015-02-03 16:17 UTC (permalink / raw)
To: Daniel Vetter
Cc: Daniel Vetter, Kyungmin Park, Seung-Woo Kim, Rob Clark,
DRI Development
2015-02-03 Daniel Vetter <daniel.vetter@ffwll.ch>:
> So this has been merged originally in
>
> commit 83052d4d5cd518332440bb4ee63d68bb5f744e0f
> Author: Seung-Woo Kim <sw0312.kim@samsung.com>
> Date: Thu Dec 15 15:40:55 2011 +0900
>
> drm: Add multi buffer plane pixel formats
>
> which hasn't seen a lot of review really. The problem is that it's not
> a real pixel format, but just a different way to lay out NV12 pixels
> in macroblocks, i.e. a tiling format.
>
> The new way of doing this is with the soon-to-be-merge fb modifiers.
>
> This was brough up in some long irc discussion around the entire
> topic, as an example of where things have gone wrong. Luckily we can
> correct the mistake:
> - The kms side support for NV12MT is all dead code because
> format_check in drm_crtc.c never accepted NV12MT.
> - The gem side for the gsc support doesn't look better: The code
> forgets to set the pixel format and makes a big mess with the tiling
> mode bits, inadvertedly setting them all.
>
> Conclusion: This never really worked (at least not in upstream) and
> hence we can savely correct our mistake here.
>
> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Rob Clark <robclark@freedesktop.org>
> Cc: Daniel Stone <daniel@fooishbar.org>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 14 ++------------
> drivers/gpu/drm/exynos/exynos_drm_gsc.c | 6 ------
> drivers/gpu/drm/exynos/exynos_drm_plane.c | 1 -
> drivers/gpu/drm/exynos/exynos_mixer.c | 2 --
> include/uapi/drm/drm_fourcc.h | 3 ---
> 5 files changed, 2 insertions(+), 24 deletions(-)
That seems good to me.
Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Gustavo
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: remove DRM_FORMAT_NV12MT
2015-02-03 15:48 [PATCH] drm: remove DRM_FORMAT_NV12MT Daniel Vetter
2015-02-03 16:17 ` Gustavo Padovan
@ 2015-02-04 2:30 ` Joonyoung Shim
2015-02-04 9:13 ` Daniel Vetter
2015-02-04 4:55 ` Seung-Woo Kim
2 siblings, 1 reply; 5+ messages in thread
From: Joonyoung Shim @ 2015-02-04 2:30 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: Daniel Vetter, Kyungmin Park, Seung-Woo Kim, Rob Clark
+Cc Inki,
Hi,
On 02/04/2015 12:48 AM, Daniel Vetter wrote:
> So this has been merged originally in
>
> commit 83052d4d5cd518332440bb4ee63d68bb5f744e0f
> Author: Seung-Woo Kim <sw0312.kim@samsung.com>
> Date: Thu Dec 15 15:40:55 2011 +0900
>
> drm: Add multi buffer plane pixel formats
>
> which hasn't seen a lot of review really. The problem is that it's not
> a real pixel format, but just a different way to lay out NV12 pixels
> in macroblocks, i.e. a tiling format.
>
> The new way of doing this is with the soon-to-be-merge fb modifiers.
>
> This was brough up in some long irc discussion around the entire
> topic, as an example of where things have gone wrong. Luckily we can
> correct the mistake:
> - The kms side support for NV12MT is all dead code because
> format_check in drm_crtc.c never accepted NV12MT.
So it was tried to support NV12MT in format_check. I really welcome the
new way.
Acked-by: Joonyoung Shim <jy0922.shim@samsung.com>
Thanks.
> - The gem side for the gsc support doesn't look better: The code
> forgets to set the pixel format and makes a big mess with the tiling
> mode bits, inadvertedly setting them all.
>
> Conclusion: This never really worked (at least not in upstream) and
> hence we can savely correct our mistake here.
>
> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Rob Clark <robclark@freedesktop.org>
> Cc: Daniel Stone <daniel@fooishbar.org>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 14 ++------------
> drivers/gpu/drm/exynos/exynos_drm_gsc.c | 6 ------
> drivers/gpu/drm/exynos/exynos_drm_plane.c | 1 -
> drivers/gpu/drm/exynos/exynos_mixer.c | 2 --
> include/uapi/drm/drm_fourcc.h | 3 ---
> 5 files changed, 2 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> index 835b6af00970..842d6b8dc3c4 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> @@ -461,7 +461,6 @@ static int fimc_src_set_fmt_order(struct fimc_context *ctx, u32 fmt)
> cfg |= EXYNOS_MSCTRL_C_INT_IN_3PLANE;
> break;
> case DRM_FORMAT_NV12:
> - case DRM_FORMAT_NV12MT:
> case DRM_FORMAT_NV16:
> cfg |= (EXYNOS_MSCTRL_ORDER2P_LSB_CBCR |
> EXYNOS_MSCTRL_C_INT_IN_2PLANE);
> @@ -511,7 +510,6 @@ static int fimc_src_set_fmt(struct device *dev, u32 fmt)
> case DRM_FORMAT_YVU420:
> case DRM_FORMAT_NV12:
> case DRM_FORMAT_NV21:
> - case DRM_FORMAT_NV12MT:
> cfg |= EXYNOS_MSCTRL_INFORMAT_YCBCR420;
> break;
> default:
> @@ -524,10 +522,7 @@ static int fimc_src_set_fmt(struct device *dev, u32 fmt)
> cfg = fimc_read(ctx, EXYNOS_CIDMAPARAM);
> cfg &= ~EXYNOS_CIDMAPARAM_R_MODE_MASK;
>
> - if (fmt == DRM_FORMAT_NV12MT)
> - cfg |= EXYNOS_CIDMAPARAM_R_MODE_64X32;
> - else
> - cfg |= EXYNOS_CIDMAPARAM_R_MODE_LINEAR;
> + cfg |= EXYNOS_CIDMAPARAM_R_MODE_LINEAR;
>
> fimc_write(ctx, cfg, EXYNOS_CIDMAPARAM);
>
> @@ -812,7 +807,6 @@ static int fimc_dst_set_fmt_order(struct fimc_context *ctx, u32 fmt)
> cfg |= EXYNOS_CIOCTRL_YCBCR_3PLANE;
> break;
> case DRM_FORMAT_NV12:
> - case DRM_FORMAT_NV12MT:
> case DRM_FORMAT_NV16:
> cfg |= EXYNOS_CIOCTRL_ORDER2P_LSB_CBCR;
> cfg |= EXYNOS_CIOCTRL_YCBCR_2PLANE;
> @@ -867,7 +861,6 @@ static int fimc_dst_set_fmt(struct device *dev, u32 fmt)
> case DRM_FORMAT_YUV420:
> case DRM_FORMAT_YVU420:
> case DRM_FORMAT_NV12:
> - case DRM_FORMAT_NV12MT:
> case DRM_FORMAT_NV21:
> cfg |= EXYNOS_CITRGFMT_OUTFORMAT_YCBCR420;
> break;
> @@ -883,10 +876,7 @@ static int fimc_dst_set_fmt(struct device *dev, u32 fmt)
> cfg = fimc_read(ctx, EXYNOS_CIDMAPARAM);
> cfg &= ~EXYNOS_CIDMAPARAM_W_MODE_MASK;
>
> - if (fmt == DRM_FORMAT_NV12MT)
> - cfg |= EXYNOS_CIDMAPARAM_W_MODE_64X32;
> - else
> - cfg |= EXYNOS_CIDMAPARAM_W_MODE_LINEAR;
> + cfg |= EXYNOS_CIDMAPARAM_W_MODE_LINEAR;
>
> fimc_write(ctx, cfg, EXYNOS_CIDMAPARAM);
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> index 0261468c8019..8040ed2a831f 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> @@ -542,9 +542,6 @@ static int gsc_src_set_fmt(struct device *dev, u32 fmt)
> cfg |= (GSC_IN_CHROMA_ORDER_CBCR |
> GSC_IN_YUV420_2P);
> break;
> - case DRM_FORMAT_NV12MT:
> - cfg |= (GSC_IN_TILE_C_16x8 | GSC_IN_TILE_MODE);
> - break;
> default:
> dev_err(ippdrv->dev, "inavlid target yuv order 0x%x.\n", fmt);
> return -EINVAL;
> @@ -809,9 +806,6 @@ static int gsc_dst_set_fmt(struct device *dev, u32 fmt)
> cfg |= (GSC_OUT_CHROMA_ORDER_CBCR |
> GSC_OUT_YUV420_2P);
> break;
> - case DRM_FORMAT_NV12MT:
> - cfg |= (GSC_OUT_TILE_C_16x8 | GSC_OUT_TILE_MODE);
> - break;
> default:
> dev_err(ippdrv->dev, "inavlid target yuv order 0x%x.\n", fmt);
> return -EINVAL;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> index c7045a663763..92d75a4eabd7 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> @@ -30,7 +30,6 @@ static const uint32_t formats[] = {
> DRM_FORMAT_XRGB8888,
> DRM_FORMAT_ARGB8888,
> DRM_FORMAT_NV12,
> - DRM_FORMAT_NV12MT,
> };
>
> /*
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 820b76234ef4..5da28443342d 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -417,8 +417,6 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
> win_data = &ctx->win_data[win];
>
> switch (win_data->pixel_format) {
> - case DRM_FORMAT_NV12MT:
> - tiled_mode = true;
> case DRM_FORMAT_NV12:
> crcb_mode = false;
> buf_num = 2;
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 646ae5f39f42..a284f11a8ef5 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -109,9 +109,6 @@
> #define DRM_FORMAT_NV24 fourcc_code('N', 'V', '2', '4') /* non-subsampled Cr:Cb plane */
> #define DRM_FORMAT_NV42 fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */
>
> -/* special NV12 tiled format */
> -#define DRM_FORMAT_NV12MT fourcc_code('T', 'M', '1', '2') /* 2x2 subsampled Cr:Cb plane 64x32 macroblocks */
> -
> /*
> * 3 plane YCbCr
> * index 0: Y plane, [7:0] Y
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm: remove DRM_FORMAT_NV12MT
2015-02-04 2:30 ` Joonyoung Shim
@ 2015-02-04 9:13 ` Daniel Vetter
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2015-02-04 9:13 UTC (permalink / raw)
To: Joonyoung Shim
Cc: Daniel Vetter, Seung-Woo Kim, Rob Clark, Kyungmin Park,
DRI Development, Daniel Vetter
On Wed, Feb 04, 2015 at 11:30:23AM +0900, Joonyoung Shim wrote:
> +Cc Inki,
>
> Hi,
>
> On 02/04/2015 12:48 AM, Daniel Vetter wrote:
> > So this has been merged originally in
> >
> > commit 83052d4d5cd518332440bb4ee63d68bb5f744e0f
> > Author: Seung-Woo Kim <sw0312.kim@samsung.com>
> > Date: Thu Dec 15 15:40:55 2011 +0900
> >
> > drm: Add multi buffer plane pixel formats
> >
> > which hasn't seen a lot of review really. The problem is that it's not
> > a real pixel format, but just a different way to lay out NV12 pixels
> > in macroblocks, i.e. a tiling format.
> >
> > The new way of doing this is with the soon-to-be-merge fb modifiers.
> >
> > This was brough up in some long irc discussion around the entire
> > topic, as an example of where things have gone wrong. Luckily we can
> > correct the mistake:
> > - The kms side support for NV12MT is all dead code because
> > format_check in drm_crtc.c never accepted NV12MT.
>
> So it was tried to support NV12MT in format_check. I really welcome the
> new way.
Yeah, the new way should also make sharing buffers between devices easier
since we'll have a fairly rigid standard about things. On irc we've
discussed that we probably need a VENDOR_MPEG modifier range so that we
can have cross-chip-vendor standardized modifiers for the various
macroblock tiling layouts seen in video streams (64x32, 16x16).
> Acked-by: Joonyoung Shim <jy0922.shim@samsung.com>
Thanks, I've picked the patch up into my drm-misc branch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: remove DRM_FORMAT_NV12MT
2015-02-03 15:48 [PATCH] drm: remove DRM_FORMAT_NV12MT Daniel Vetter
2015-02-03 16:17 ` Gustavo Padovan
2015-02-04 2:30 ` Joonyoung Shim
@ 2015-02-04 4:55 ` Seung-Woo Kim
2 siblings, 0 replies; 5+ messages in thread
From: Seung-Woo Kim @ 2015-02-04 4:55 UTC (permalink / raw)
To: Daniel Vetter
Cc: DRI Development, Seung-Woo Kim, Kyungmin Park, Rob Clark,
Daniel Vetter
Hello Daniel,
On 2015년 02월 04일 00:48, Daniel Vetter wrote:
> So this has been merged originally in
>
> commit 83052d4d5cd518332440bb4ee63d68bb5f744e0f
> Author: Seung-Woo Kim <sw0312.kim@samsung.com>
> Date: Thu Dec 15 15:40:55 2011 +0900
>
> drm: Add multi buffer plane pixel formats
>
> which hasn't seen a lot of review really. The problem is that it's not
> a real pixel format, but just a different way to lay out NV12 pixels
> in macroblocks, i.e. a tiling format.
>
> The new way of doing this is with the soon-to-be-merge fb modifiers.
>
> This was brough up in some long irc discussion around the entire
> topic, as an example of where things have gone wrong. Luckily we can
> correct the mistake:
> - The kms side support for NV12MT is all dead code because
> format_check in drm_crtc.c never accepted NV12MT.
Yes, you are right. I sent patch to add the format to check function,
but it was not accepted. Anyway fb_modifier patch supports tiled option,
so kms with NV12MT can be change with some fix with fb_modifier.
> - The gem side for the gsc support doesn't look better: The code
> forgets to set the pixel format and makes a big mess with the tiling
> mode bits, inadvertedly setting them all.
Case of NV12MT in fimc is worked because it just passes fourcc format
with set_property instead fb. So it cannot be directly replaced with
fb_modifier way, but property to set tiled flag can be added to handle
tiled format.
So I agree about the remove of the internal format.
>
> Conclusion: This never really worked (at least not in upstream) and
> hence we can savely correct our mistake here.
>
> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Rob Clark <robclark@freedesktop.org>
> Cc: Daniel Stone <daniel@fooishbar.org>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Acked-by: Seung-Woo Kim <sw0312.kim@samsung.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_fimc.c | 14 ++------------
> drivers/gpu/drm/exynos/exynos_drm_gsc.c | 6 ------
> drivers/gpu/drm/exynos/exynos_drm_plane.c | 1 -
> drivers/gpu/drm/exynos/exynos_mixer.c | 2 --
> include/uapi/drm/drm_fourcc.h | 3 ---
> 5 files changed, 2 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimc.c b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> index 835b6af00970..842d6b8dc3c4 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimc.c
> @@ -461,7 +461,6 @@ static int fimc_src_set_fmt_order(struct fimc_context *ctx, u32 fmt)
> cfg |= EXYNOS_MSCTRL_C_INT_IN_3PLANE;
> break;
> case DRM_FORMAT_NV12:
> - case DRM_FORMAT_NV12MT:
> case DRM_FORMAT_NV16:
> cfg |= (EXYNOS_MSCTRL_ORDER2P_LSB_CBCR |
> EXYNOS_MSCTRL_C_INT_IN_2PLANE);
> @@ -511,7 +510,6 @@ static int fimc_src_set_fmt(struct device *dev, u32 fmt)
> case DRM_FORMAT_YVU420:
> case DRM_FORMAT_NV12:
> case DRM_FORMAT_NV21:
> - case DRM_FORMAT_NV12MT:
> cfg |= EXYNOS_MSCTRL_INFORMAT_YCBCR420;
> break;
> default:
> @@ -524,10 +522,7 @@ static int fimc_src_set_fmt(struct device *dev, u32 fmt)
> cfg = fimc_read(ctx, EXYNOS_CIDMAPARAM);
> cfg &= ~EXYNOS_CIDMAPARAM_R_MODE_MASK;
>
> - if (fmt == DRM_FORMAT_NV12MT)
> - cfg |= EXYNOS_CIDMAPARAM_R_MODE_64X32;
> - else
> - cfg |= EXYNOS_CIDMAPARAM_R_MODE_LINEAR;
> + cfg |= EXYNOS_CIDMAPARAM_R_MODE_LINEAR;
>
> fimc_write(ctx, cfg, EXYNOS_CIDMAPARAM);
>
> @@ -812,7 +807,6 @@ static int fimc_dst_set_fmt_order(struct fimc_context *ctx, u32 fmt)
> cfg |= EXYNOS_CIOCTRL_YCBCR_3PLANE;
> break;
> case DRM_FORMAT_NV12:
> - case DRM_FORMAT_NV12MT:
> case DRM_FORMAT_NV16:
> cfg |= EXYNOS_CIOCTRL_ORDER2P_LSB_CBCR;
> cfg |= EXYNOS_CIOCTRL_YCBCR_2PLANE;
> @@ -867,7 +861,6 @@ static int fimc_dst_set_fmt(struct device *dev, u32 fmt)
> case DRM_FORMAT_YUV420:
> case DRM_FORMAT_YVU420:
> case DRM_FORMAT_NV12:
> - case DRM_FORMAT_NV12MT:
> case DRM_FORMAT_NV21:
> cfg |= EXYNOS_CITRGFMT_OUTFORMAT_YCBCR420;
> break;
> @@ -883,10 +876,7 @@ static int fimc_dst_set_fmt(struct device *dev, u32 fmt)
> cfg = fimc_read(ctx, EXYNOS_CIDMAPARAM);
> cfg &= ~EXYNOS_CIDMAPARAM_W_MODE_MASK;
>
> - if (fmt == DRM_FORMAT_NV12MT)
> - cfg |= EXYNOS_CIDMAPARAM_W_MODE_64X32;
> - else
> - cfg |= EXYNOS_CIDMAPARAM_W_MODE_LINEAR;
> + cfg |= EXYNOS_CIDMAPARAM_W_MODE_LINEAR;
>
> fimc_write(ctx, cfg, EXYNOS_CIDMAPARAM);
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> index 0261468c8019..8040ed2a831f 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c
> @@ -542,9 +542,6 @@ static int gsc_src_set_fmt(struct device *dev, u32 fmt)
> cfg |= (GSC_IN_CHROMA_ORDER_CBCR |
> GSC_IN_YUV420_2P);
> break;
> - case DRM_FORMAT_NV12MT:
> - cfg |= (GSC_IN_TILE_C_16x8 | GSC_IN_TILE_MODE);
> - break;
> default:
> dev_err(ippdrv->dev, "inavlid target yuv order 0x%x.\n", fmt);
> return -EINVAL;
> @@ -809,9 +806,6 @@ static int gsc_dst_set_fmt(struct device *dev, u32 fmt)
> cfg |= (GSC_OUT_CHROMA_ORDER_CBCR |
> GSC_OUT_YUV420_2P);
> break;
> - case DRM_FORMAT_NV12MT:
> - cfg |= (GSC_OUT_TILE_C_16x8 | GSC_OUT_TILE_MODE);
> - break;
> default:
> dev_err(ippdrv->dev, "inavlid target yuv order 0x%x.\n", fmt);
> return -EINVAL;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> index c7045a663763..92d75a4eabd7 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> @@ -30,7 +30,6 @@ static const uint32_t formats[] = {
> DRM_FORMAT_XRGB8888,
> DRM_FORMAT_ARGB8888,
> DRM_FORMAT_NV12,
> - DRM_FORMAT_NV12MT,
> };
>
> /*
> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
> index 820b76234ef4..5da28443342d 100644
> --- a/drivers/gpu/drm/exynos/exynos_mixer.c
> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c
> @@ -417,8 +417,6 @@ static void vp_video_buffer(struct mixer_context *ctx, int win)
> win_data = &ctx->win_data[win];
>
> switch (win_data->pixel_format) {
> - case DRM_FORMAT_NV12MT:
> - tiled_mode = true;
> case DRM_FORMAT_NV12:
> crcb_mode = false;
> buf_num = 2;
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 646ae5f39f42..a284f11a8ef5 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -109,9 +109,6 @@
> #define DRM_FORMAT_NV24 fourcc_code('N', 'V', '2', '4') /* non-subsampled Cr:Cb plane */
> #define DRM_FORMAT_NV42 fourcc_code('N', 'V', '4', '2') /* non-subsampled Cb:Cr plane */
>
> -/* special NV12 tiled format */
> -#define DRM_FORMAT_NV12MT fourcc_code('T', 'M', '1', '2') /* 2x2 subsampled Cr:Cb plane 64x32 macroblocks */
> -
> /*
> * 3 plane YCbCr
> * index 0: Y plane, [7:0] Y
>
--
Seung-Woo Kim
Samsung Software R&D Center
--
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-02-04 9:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-03 15:48 [PATCH] drm: remove DRM_FORMAT_NV12MT Daniel Vetter
2015-02-03 16:17 ` Gustavo Padovan
2015-02-04 2:30 ` Joonyoung Shim
2015-02-04 9:13 ` Daniel Vetter
2015-02-04 4:55 ` Seung-Woo Kim
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.