| Message ID | 1499825019-7503-1-git-send-email-mark.yao@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. > 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. > > -#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. > + > +#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/ > + v, reg.write_mask, relaxed); \ > + else \ > + dev_dbg(vop->dev, "Warning: not support "#name"\n"); \ > + } while (0) Also better as static inline, IMO. > + > +#define REG_SET(x, name, off, reg, v, relaxed) \ > + _REG_SET(x, name, off, reg, reg.mask, v, relaxed) s/reg.mask/~0/ > +#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. > > #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) > #define VOP_SCL_SET(x, win, name, v) \ > - REG_SET(x, win->base, win->phy->scl->name, v, RELAXED) > + REG_SET(x, name, win->base, win->phy->scl->name, v, true) > #define VOP_SCL_SET_EXT(x, win, name, v) \ > - 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); \ > } while (0) > #define VOP_INTR_GET_TYPE(vop, name, type) \ > vop_get_intr_type(vop, &vop->data->intr->name, type) > > +#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 > + * 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? > }; > > 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) \ > {.offset = off, \ > .mask = _mask, \ > .shift = s, \ > - .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
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) >> #define VOP_SCL_SET(x, win, name, v) \ >> - REG_SET(x, win->base, win->phy->scl->name, v, RELAXED) >> + REG_SET(x, name, win->base, win->phy->scl->name, v, true) >> #define VOP_SCL_SET_EXT(x, win, name, v) \ >> - 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); \ >> } while (0) >> #define VOP_INTR_GET_TYPE(vop, name, type) \ >> vop_get_intr_type(vop, &vop->data->intr->name, type) >> >> +#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) \ >> {.offset = off, \ >> .mask = _mask, \ >> .shift = s, \ >> - .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
On Thu, Jul 13, 2017 at 09:45:12AM +0800, Mark yao wrote: > 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) > > > #define VOP_SCL_SET(x, win, name, v) \ > > > - REG_SET(x, win->base, win->phy->scl->name, v, RELAXED) > > > + REG_SET(x, name, win->base, win->phy->scl->name, v, true) > > > #define VOP_SCL_SET_EXT(x, win, name, v) \ > > > - 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); \ > > > } while (0) > > > #define VOP_INTR_GET_TYPE(vop, name, type) \ > > > vop_get_intr_type(vop, &vop->data->intr->name, type) > > > +#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. > Ehhh, let's not get too fancy. If you want to save space, you can use uint8_t and uint16_t. Packing like this makes it look like you're sending the struct elsewhere, which is just confusing. Sean > > > > > }; > > > 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) \ > > > {.offset = off, \ > > > .mask = _mask, \ > > > .shift = s, \ > > > - .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 > > > -- > Ｍark Yao > >
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)) -#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) + +#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, \ + v, reg.write_mask, relaxed); \ + else \ + dev_dbg(vop->dev, "Warning: not support "#name"\n"); \ + } while (0) + +#define REG_SET(x, name, off, reg, v, relaxed) \ + _REG_SET(x, name, off, reg, reg.mask, v, relaxed) +#define REG_SET_MASK(x, name, off, reg, mask, v, relaxed) \ + _REG_SET(x, name, off, reg, reg.mask & mask, v, relaxed) #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) #define VOP_SCL_SET(x, win, name, v) \ - REG_SET(x, win->base, win->phy->scl->name, v, RELAXED) + REG_SET(x, name, win->base, win->phy->scl->name, v, true) #define VOP_SCL_SET_EXT(x, win, name, v) \ - 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); \ } while (0) #define VOP_INTR_GET_TYPE(vop, name, type) \ vop_get_intr_type(vop, &vop->data->intr->name, type) +#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 + * 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; }; 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) \ {.offset = off, \ .mask = _mask, \ .shift = s, \ - .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,
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(-)