[PATCH v3 3/3] drm: zte: add overlay plane support

2017-01-09 Thread Shawn Guo
On Thu, Jan 05, 2017 at 02:26:30AM -0500, Sean Paul wrote:
> > +static u32 zx_vl_get_fmt(uint32_t format)
> > +{
> > +   u32 val = 0;
> > +
> > +   switch (format) {
> > +   case DRM_FORMAT_NV12:
> > +   val = VL_FMT_YUV420;
> > +   break;
> > +   case DRM_FORMAT_YUV420:
> > +   val = VL_YUV420_PLANAR | VL_FMT_YUV420;
> > +   break;
> > +   case DRM_FORMAT_YUYV:
> > +   val = VL_YUV422_YUYV | VL_FMT_YUV422;
> > +   break;
> > +   case DRM_FORMAT_YVYU:
> > +   val = VL_YUV422_YVYU | VL_FMT_YUV422;
> > +   break;
> > +   case DRM_FORMAT_UYVY:
> > +   val = VL_YUV422_UYVY | VL_FMT_YUV422;
> > +   break;
> > +   case DRM_FORMAT_VYUY:
> > +   val = VL_YUV422_VYUY | VL_FMT_YUV422;
> > +   break;
> > +   case DRM_FORMAT_YUV444:
> > +   val = VL_FMT_YUV444_8BIT;
> 
> Minor nit: You could have eliminated val and just returned directly
> from all of the cases. Seems like there are a few other functions this
> is also true for.

Okay.  I will change zx_vl_get_fmt() and zx_vl_rsz_get_fmt()
accordingly.

> 
> > +   break;
> > +   default:
> > +   WARN_ONCE(1, "invalid pixel format %d\n", format);
> > +   }
> > +
> > +   return val;
> > +}



> > +static void zx_vl_rsz_setup(struct zx_plane *zplane, uint32_t format,
> > +   u32 src_w, u32 src_h, u32 dst_w, u32 dst_h)
> > +{
> > +   void __iomem *rsz = zplane->rsz;
> > +   u32 src_chroma_w = src_w;
> > +   u32 src_chroma_h = src_h;
> > +   u32 fmt;
> > +
> > +   /* Set up source and destination resolution */
> > +   zx_writel(rsz + RSZ_SRC_CFG, RSZ_VER(src_h - 1) | RSZ_HOR(src_w - 
> > 1));
> > +   zx_writel(rsz + RSZ_DEST_CFG, RSZ_VER(dst_h - 1) | RSZ_HOR(dst_w - 
> > 1));
> > +
> > +   /* Configure data format for VL RSZ */
> > +   fmt = zx_vl_rsz_get_fmt(format);
> > +   zx_writel_mask(rsz + RSZ_VL_CTRL_CFG, RSZ_VL_FMT_MASK, fmt);
> > +
> > +   /* Calculate Chroma heigth and width */
> 
> s/heigth/height/

Thanks for spotting it.

> 
> > +   if (fmt == RSZ_VL_FMT_YCBCR420) {
> > +   src_chroma_w = src_w >> 1;
> > +   src_chroma_h = src_h >> 1;
> > +   } else if (fmt == RSZ_VL_FMT_YCBCR422) {
> > +   src_chroma_w = src_w >> 1;
> > +   }
> > +
> > +   /* Set up Luma and Chroma step registers */
> > +   zx_writel(rsz + RSZ_VL_LUMA_HOR, rsz_step_value(src_w, dst_w));
> > +   zx_writel(rsz + RSZ_VL_LUMA_VER, rsz_step_value(src_h, dst_h));
> > +   zx_writel(rsz + RSZ_VL_CHROMA_HOR, rsz_step_value(src_chroma_w, 
> > dst_w));
> > +   zx_writel(rsz + RSZ_VL_CHROMA_VER, rsz_step_value(src_chroma_h, 
> > dst_h));
> > +
> > +   zx_vl_rsz_set_update(zplane);
> > +}



> > diff --git a/drivers/gpu/drm/zte/zx_vou.c b/drivers/gpu/drm/zte/zx_vou.c
> > index 3fb4fc04e693..e832c2ec3156 100644
> > --- a/drivers/gpu/drm/zte/zx_vou.c
> > +++ b/drivers/gpu/drm/zte/zx_vou.c
> > @@ -84,6 +84,8 @@ struct zx_crtc_bits {
> >  struct zx_crtc {
> > struct drm_crtc crtc;
> > struct drm_plane *primary;
> > +   struct drm_plane *overlay_active[VL_NUM];
> > +   unsigned int overlay_active_num;
> 
> I don't think this belongs here. You can instead add an active (or
> enabled) bool to the zx_plane struct and keep track of it via
> atomic_plane_update/disable. This allows you to call
> zx_plane_set_update unconditionally in the vou irq handler and check
> active/enabled in zx_plane_set_update.

It's a truly great suggestion.  How did I not think of it :)  The v4 is
coming for that.

Thanks a lot for the review effort, Sean.

Shawn


[PATCH v3 3/3] drm: zte: add overlay plane support

2017-01-05 Thread Sean Paul
On Wed, Dec 28, 2016 at 9:37 PM, Shawn Guo  wrote:
> From: Shawn Guo 
>
> It enables VOU VL (Video Layer) to support overlay plane with scaling
> function.  VL0 has some quirks on scaling support.  We choose to skip it
> and only adds VL1 and VL2 into DRM core for now.
>
> Function zx_plane_atomic_disable() gets moved around with no changes to
> save a forward declaration.
>
> Signed-off-by: Shawn Guo 
> ---
>  drivers/gpu/drm/zte/zx_plane.c  | 311 
> +---
>  drivers/gpu/drm/zte/zx_plane_regs.h |  51 ++
>  drivers/gpu/drm/zte/zx_vou.c|  80 +-
>  drivers/gpu/drm/zte/zx_vou_regs.h   |  18 +++
>  4 files changed, 431 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
> index 5445eebf830f..c5ac42647735 100644
> --- a/drivers/gpu/drm/zte/zx_plane.c
> +++ b/drivers/gpu/drm/zte/zx_plane.c
> @@ -30,6 +30,275 @@
> DRM_FORMAT_ARGB,
>  };
>
> +static const uint32_t vl_formats[] = {
> +   DRM_FORMAT_NV12,/* Semi-planar YUV420 */
> +   DRM_FORMAT_YUV420,  /* Planar YUV420 */
> +   DRM_FORMAT_YUYV,/* Packed YUV422 */
> +   DRM_FORMAT_YVYU,
> +   DRM_FORMAT_UYVY,
> +   DRM_FORMAT_VYUY,
> +   DRM_FORMAT_YUV444,  /* YUV444 8bit */
> +   /*
> +* TODO: add formats below that HW supports:
> +*  - YUV420 P010
> +*  - YUV420 Hantro
> +*  - YUV444 10bit
> +*/
> +};
> +
> +#define FRAC_16_16(mult, div)(((mult) << 16) / (div))
> +
> +static int zx_vl_plane_atomic_check(struct drm_plane *plane,
> +   struct drm_plane_state *plane_state)
> +{
> +   struct drm_framebuffer *fb = plane_state->fb;
> +   struct drm_crtc *crtc = plane_state->crtc;
> +   struct drm_crtc_state *crtc_state;
> +   struct drm_rect clip;
> +   int min_scale = FRAC_16_16(1, 8);
> +   int max_scale = FRAC_16_16(8, 1);
> +
> +   if (!crtc || !fb)
> +   return 0;
> +
> +   crtc_state = drm_atomic_get_existing_crtc_state(plane_state->state,
> +   crtc);
> +   if (WARN_ON(!crtc_state))
> +   return -EINVAL;
> +
> +   /* nothing to check when disabling or disabled */
> +   if (!crtc_state->enable)
> +   return 0;
> +
> +   /* plane must be enabled */
> +   if (!plane_state->crtc)
> +   return -EINVAL;
> +
> +   clip.x1 = 0;
> +   clip.y1 = 0;
> +   clip.x2 = crtc_state->adjusted_mode.hdisplay;
> +   clip.y2 = crtc_state->adjusted_mode.vdisplay;
> +
> +   return drm_plane_helper_check_state(plane_state, ,
> +   min_scale, max_scale,
> +   true, true);
> +}
> +
> +static u32 zx_vl_get_fmt(uint32_t format)
> +{
> +   u32 val = 0;
> +
> +   switch (format) {
> +   case DRM_FORMAT_NV12:
> +   val = VL_FMT_YUV420;
> +   break;
> +   case DRM_FORMAT_YUV420:
> +   val = VL_YUV420_PLANAR | VL_FMT_YUV420;
> +   break;
> +   case DRM_FORMAT_YUYV:
> +   val = VL_YUV422_YUYV | VL_FMT_YUV422;
> +   break;
> +   case DRM_FORMAT_YVYU:
> +   val = VL_YUV422_YVYU | VL_FMT_YUV422;
> +   break;
> +   case DRM_FORMAT_UYVY:
> +   val = VL_YUV422_UYVY | VL_FMT_YUV422;
> +   break;
> +   case DRM_FORMAT_VYUY:
> +   val = VL_YUV422_VYUY | VL_FMT_YUV422;
> +   break;
> +   case DRM_FORMAT_YUV444:
> +   val = VL_FMT_YUV444_8BIT;

Minor nit: You could have eliminated val and just returned directly
from all of the cases. Seems like there are a few other functions this
is also true for.

> +   break;
> +   default:
> +   WARN_ONCE(1, "invalid pixel format %d\n", format);
> +   }
> +
> +   return val;
> +}
> +
> +static inline void zx_vl_set_update(struct zx_plane *zplane)
> +{
> +   void __iomem *layer = zplane->layer;
> +
> +   zx_writel_mask(layer + VL_CTRL0, VL_UPDATE, VL_UPDATE);
> +}
> +
> +static inline void zx_vl_rsz_set_update(struct zx_plane *zplane)
> +{
> +   zx_writel(zplane->rsz + RSZ_VL_ENABLE_CFG, 1);
> +}
> +
> +static u32 zx_vl_rsz_get_fmt(uint32_t format)
> +{
> +   u32 val = 0;
> +
> +   switch (format) {
> +   case DRM_FORMAT_NV12:
> +   case DRM_FORMAT_YUV420:
> +   val = RSZ_VL_FMT_YCBCR420;
> +   break;
> +   case DRM_FORMAT_YUYV:
> +   case DRM_FORMAT_YVYU:
> +   case DRM_FORMAT_UYVY:
> +   case DRM_FORMAT_VYUY:
> +   val = RSZ_VL_FMT_YCBCR422;
> +   break;
> +   case DRM_FORMAT_YUV444:
> +   val = RSZ_VL_FMT_YCBCR444;
> +   break;
> +   default:
> +   WARN_ONCE(1, "invalid pixel format %d\n", format);
> +   

[PATCH v3 3/3] drm: zte: add overlay plane support

2016-12-29 Thread Shawn Guo
From: Shawn Guo 

It enables VOU VL (Video Layer) to support overlay plane with scaling
function.  VL0 has some quirks on scaling support.  We choose to skip it
and only adds VL1 and VL2 into DRM core for now.

Function zx_plane_atomic_disable() gets moved around with no changes to
save a forward declaration.

Signed-off-by: Shawn Guo 
---
 drivers/gpu/drm/zte/zx_plane.c  | 311 +---
 drivers/gpu/drm/zte/zx_plane_regs.h |  51 ++
 drivers/gpu/drm/zte/zx_vou.c|  80 +-
 drivers/gpu/drm/zte/zx_vou_regs.h   |  18 +++
 4 files changed, 431 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/zte/zx_plane.c b/drivers/gpu/drm/zte/zx_plane.c
index 5445eebf830f..c5ac42647735 100644
--- a/drivers/gpu/drm/zte/zx_plane.c
+++ b/drivers/gpu/drm/zte/zx_plane.c
@@ -30,6 +30,275 @@
DRM_FORMAT_ARGB,
 };

+static const uint32_t vl_formats[] = {
+   DRM_FORMAT_NV12,/* Semi-planar YUV420 */
+   DRM_FORMAT_YUV420,  /* Planar YUV420 */
+   DRM_FORMAT_YUYV,/* Packed YUV422 */
+   DRM_FORMAT_YVYU,
+   DRM_FORMAT_UYVY,
+   DRM_FORMAT_VYUY,
+   DRM_FORMAT_YUV444,  /* YUV444 8bit */
+   /*
+* TODO: add formats below that HW supports:
+*  - YUV420 P010
+*  - YUV420 Hantro
+*  - YUV444 10bit
+*/
+};
+
+#define FRAC_16_16(mult, div)(((mult) << 16) / (div))
+
+static int zx_vl_plane_atomic_check(struct drm_plane *plane,
+   struct drm_plane_state *plane_state)
+{
+   struct drm_framebuffer *fb = plane_state->fb;
+   struct drm_crtc *crtc = plane_state->crtc;
+   struct drm_crtc_state *crtc_state;
+   struct drm_rect clip;
+   int min_scale = FRAC_16_16(1, 8);
+   int max_scale = FRAC_16_16(8, 1);
+
+   if (!crtc || !fb)
+   return 0;
+
+   crtc_state = drm_atomic_get_existing_crtc_state(plane_state->state,
+   crtc);
+   if (WARN_ON(!crtc_state))
+   return -EINVAL;
+
+   /* nothing to check when disabling or disabled */
+   if (!crtc_state->enable)
+   return 0;
+
+   /* plane must be enabled */
+   if (!plane_state->crtc)
+   return -EINVAL;
+
+   clip.x1 = 0;
+   clip.y1 = 0;
+   clip.x2 = crtc_state->adjusted_mode.hdisplay;
+   clip.y2 = crtc_state->adjusted_mode.vdisplay;
+
+   return drm_plane_helper_check_state(plane_state, ,
+   min_scale, max_scale,
+   true, true);
+}
+
+static u32 zx_vl_get_fmt(uint32_t format)
+{
+   u32 val = 0;
+
+   switch (format) {
+   case DRM_FORMAT_NV12:
+   val = VL_FMT_YUV420;
+   break;
+   case DRM_FORMAT_YUV420:
+   val = VL_YUV420_PLANAR | VL_FMT_YUV420;
+   break;
+   case DRM_FORMAT_YUYV:
+   val = VL_YUV422_YUYV | VL_FMT_YUV422;
+   break;
+   case DRM_FORMAT_YVYU:
+   val = VL_YUV422_YVYU | VL_FMT_YUV422;
+   break;
+   case DRM_FORMAT_UYVY:
+   val = VL_YUV422_UYVY | VL_FMT_YUV422;
+   break;
+   case DRM_FORMAT_VYUY:
+   val = VL_YUV422_VYUY | VL_FMT_YUV422;
+   break;
+   case DRM_FORMAT_YUV444:
+   val = VL_FMT_YUV444_8BIT;
+   break;
+   default:
+   WARN_ONCE(1, "invalid pixel format %d\n", format);
+   }
+
+   return val;
+}
+
+static inline void zx_vl_set_update(struct zx_plane *zplane)
+{
+   void __iomem *layer = zplane->layer;
+
+   zx_writel_mask(layer + VL_CTRL0, VL_UPDATE, VL_UPDATE);
+}
+
+static inline void zx_vl_rsz_set_update(struct zx_plane *zplane)
+{
+   zx_writel(zplane->rsz + RSZ_VL_ENABLE_CFG, 1);
+}
+
+static u32 zx_vl_rsz_get_fmt(uint32_t format)
+{
+   u32 val = 0;
+
+   switch (format) {
+   case DRM_FORMAT_NV12:
+   case DRM_FORMAT_YUV420:
+   val = RSZ_VL_FMT_YCBCR420;
+   break;
+   case DRM_FORMAT_YUYV:
+   case DRM_FORMAT_YVYU:
+   case DRM_FORMAT_UYVY:
+   case DRM_FORMAT_VYUY:
+   val = RSZ_VL_FMT_YCBCR422;
+   break;
+   case DRM_FORMAT_YUV444:
+   val = RSZ_VL_FMT_YCBCR444;
+   break;
+   default:
+   WARN_ONCE(1, "invalid pixel format %d\n", format);
+   }
+
+   return val;
+}
+
+static inline u32 rsz_step_value(u32 src, u32 dst)
+{
+   u32 val = 0;
+
+   if (src == dst)
+   val = 0;
+   else if (src < dst)
+   val = RSZ_PARA_STEP((src << 16) / dst);
+   else if (src > dst)
+   val = RSZ_DATA_STEP(src / dst) |
+ RSZ_PARA_STEP(((src << 16) / dst) & 0x);
+
+   return val;
+}
+
+static void zx_vl_rsz_setup(struct zx_plane *zplane, uint32_t