Re: [PATCH] drm: remove DRM_FORMAT_NV12MT

From: Gustavo Padovan <gustavo@padovan.org>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Seung-Woo Kim <sw0312.kim@samsung.com>,
	Rob Clark <robclark@freedesktop.org>,
	DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] drm: remove DRM_FORMAT_NV12MT
Date: Tue, 3 Feb 2015 14:17:44 -0200	[thread overview]
Message-ID: <20150203161744.GC2122@joana> (raw)
In-Reply-To: <1422978525-13442-1-git-send-email-daniel.vetter@ffwll.ch>

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