On 2017年07月13日 01:53, Sean Paul wrote:
On Wed, Jul 12, 2017 at 10:03:38AM +0800, Mark Yao wrote:
Please add a commit message describing *what* and *why* you are making the change.
Got it, I will fix it at next version.
Thanks.
Signed-off-by: Mark Yao mark.yao@rock-chips.com
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 66 +++++++++++++++++++++-------- drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 18 ++++++-- drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 20 ++++++--- 3 files changed, 77 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index 7a5f809..a9180fd 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -42,33 +42,60 @@ #include "rockchip_drm_psr.h" #include "rockchip_drm_vop.h"
-#define __REG_SET_RELAXED(x, off, mask, shift, v, write_mask) \
vop_mask_write(x, off, mask, shift, v, write_mask, true)+#define VOP_REG_SUPPORT(vop, reg) \
(!reg.major || (reg.major == VOP_MAJOR(vop->data->version) && \reg.begin_minor <= VOP_MINOR(vop->data->version) && \reg.end_minor >= VOP_MINOR(vop->data->version) && \reg.mask))This would be better suited as a static inline function. As would many of the macros below.
Got it, will fix it at next version.
-#define __REG_SET_NORMAL(x, off, mask, shift, v, write_mask) \
vop_mask_write(x, off, mask, shift, v, write_mask, false)+#define VOP_WIN_SUPPORT(vop, win, name) \
VOP_REG_SUPPORT(vop, win->phy->name)-#define REG_SET(x, base, reg, v, mode) \
__REG_SET_##mode(x, base + reg.offset, \reg.mask, reg.shift, v, reg.write_mask)-#define REG_SET_MASK(x, base, reg, mask, v, mode) \
__REG_SET_##mode(x, base + reg.offset, \mask, reg.shift, v, reg.write_mask)+#define VOP_CTRL_SUPPORT(vop, name) \
VOP_REG_SUPPORT(vop, vop->data->ctrl->name)+#define VOP_INTR_SUPPORT(vop, name) \
VOP_REG_SUPPORT(vop, vop->data->intr->name)+#define __REG_SET(x, off, mask, shift, v, write_mask, relaxed) \
vop_mask_write(x, off, mask, shift, v, write_mask, relaxed)There's really no point to this, just call vop_mask_write directly.
Got it, will fix it at next version.
+#define _REG_SET(vop, name, off, reg, mask, v, relaxed) \
- do { \
if (VOP_REG_SUPPORT(vop, reg)) \__REG_SET(vop, off + reg.offset, mask, reg.shift, \s/mask/reg.mask & mask/
Got it, will fix it at next version.
v, reg.write_mask, relaxed); \else \dev_dbg(vop->dev, "Warning: not support "#name"\n"); \- } while (0)
Also better as static inline, IMO.
Good idea, I will try it.
+#define REG_SET(x, name, off, reg, v, relaxed) \
_REG_SET(x, name, off, reg, reg.mask, v, relaxed)s/reg.mask/~0/
Got it, will fix it at next version.
+#define REG_SET_MASK(x, name, off, reg, mask, v, relaxed) \
_REG_SET(x, name, off, reg, reg.mask & mask, v, relaxed)s/reg.mask &//
Also, these can become static inline functions as well.
Got it, will fix it at next version.
#define VOP_WIN_SET(x, win, name, v) \
REG_SET(x, win->base, win->phy->name, v, RELAXED)
REG_SET(x, name, win->base, win->phy->name, v, true)+#define VOP_WIN_SET_EXT(x, win, ext, name, v) \
REG_SET(x, name, 0, win->ext->name, v, true)
REG_SET(x, win->base, win->phy->scl->name, v, RELAXED)
REG_SET(x, name, win->base, win->phy->scl->name, v, true)
REG_SET(x, win->base, win->phy->scl->ext->name, v, RELAXED)
REG_SET(x, name, win->base, win->phy->scl->ext->name, v, true)- #define VOP_CTRL_SET(x, name, v) \
REG_SET(x, 0, (x)->data->ctrl->name, v, NORMAL)
REG_SET(x, name, 0, (x)->data->ctrl->name, v, false)#define VOP_INTR_GET(vop, name) \ vop_read_reg(vop, 0, &vop->data->ctrl->name)
-#define VOP_INTR_SET(vop, name, mask, v) \
REG_SET_MASK(vop, 0, vop->data->intr->name, mask, v, NORMAL)+#define VOP_INTR_SET(vop, name, v) \
REG_SET(vop, name, 0, vop->data->intr->name, \v, false)+#define VOP_INTR_SET_MASK(vop, name, mask, v) \
REG_SET_MASK(vop, name, 0, vop->data->intr->name, \mask, v, false)- #define VOP_INTR_SET_TYPE(vop, name, type, v) \ do { \ int i, reg = 0, mask = 0; \
@@ -78,13 +105,16 @@ mask |= 1 << i; \ } \ } \
VOP_INTR_SET(vop, name, mask, reg); \
VOP_INTR_SET_MASK(vop, name, mask, reg); \+#define VOP_CTRL_GET(x, name) \
vop_read_reg(x, 0, &vop->data->ctrl->name)- #define VOP_WIN_GET(x, win, name) \
vop_read_reg(x, win->base, &win->phy->name)
vop_read_reg(x, win->offset, win->phy->name)#define VOP_WIN_GET_YRGBADDR(vop, win) \ vop_readl(vop, win->base + win->phy->yrgb_mst.offset)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h index 084d3b2..e4de890 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h @@ -15,6 +15,14 @@ #ifndef _ROCKCHIP_DRM_VOP_H #define _ROCKCHIP_DRM_VOP_H
+/*
- major: IP major vertion, used for IP structure
s/vertion/version
Got it, will fix it at next version. Thanks.
- minor: big feature change under same structure
- */
+#define VOP_VERSION(major, minor) ((major) << 8 | (minor)) +#define VOP_MAJOR(version) ((version) >> 8) +#define VOP_MINOR(version) ((version) & 0xff)
- enum vop_data_format { VOP_FMT_ARGB8888 = 0, VOP_FMT_RGB888,
@@ -25,10 +33,13 @@ enum vop_data_format { };
struct vop_reg {
- uint32_t offset;
- uint32_t shift; uint32_t mask;
- bool write_mask;
- uint32_t offset:12;
- uint32_t shift:5;
- uint32_t begin_minor:4;
- uint32_t end_minor:4;
- uint32_t major:3;
- uint32_t write_mask:1;
Why are you packing this?
make struct vop_reg not too big, jus u64 size, struct vop_reg use a lot on register define, packing it would reduce register table size.
};
struct vop_ctrl { @@ -134,6 +145,7 @@ struct vop_win_data { };
struct vop_data {
- uint32_t version; const struct vop_ctrl *ctrl; const struct vop_intr *intr; const struct vop_win_data *win;
diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c index 00e9d79..7744603 100644 --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c @@ -20,17 +20,25 @@ #include "rockchip_drm_vop.h" #include "rockchip_vop_reg.h"
-#define VOP_REG(off, _mask, s) \ +#define VOP_REG_VER_MASK(off, _mask, s, _write_mask, _major, \
_begin_minor, _end_minor) \
.write_mask = false,}
.write_mask = _write_mask, \.major = _major, \.begin_minor = _begin_minor, \.end_minor = _end_minor,}+#define VOP_REG_VER(off, _mask, s, _major, _begin_minor, _end_minor) \
VOP_REG_VER_MASK(off, _mask, s, false, \_major, _begin_minor, _end_minor)+#define VOP_REG(off, _mask, s) \
VOP_REG_VER(off, _mask, s, 0, 0, -1)#define VOP_REG_MASK(off, _mask, s) \
{.offset = off, \.mask = _mask, \.shift = s, \.write_mask = true,}
VOP_REG_VER_MASK(off, _mask, s, true, 0, 0, -1)static const uint32_t formats_win_full[] = { DRM_FORMAT_XRGB8888,
-- 1.9.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel