Hi Mark,
Mark Yao <mark.yao <at> rock-chips.com> writes:
Some new vop register support mask, bit[16-31] is mask, bit[0-15] is value, the mask is correspond to the value.
Please see my comments inline.
Signed-off-by: Mark Yao <mark.yao <at> rock-chips.com>
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 45 ++++++++++++++-------
------
drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 1 + drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 9 +++++- 3 files changed, 32 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 28596e7..59f24cd 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
[snip]
-static inline void vop_mask_write_relaxed(struct vop *vop, uint32_t
offset,
uint32_t mask, uint32_t v)-{
- if (mask) {
- if (write_mask) {
v = (v << shift) | (mask << (shift + 16));
If the caller gives "v" too big, it might get over the bit 16 and affect the mask. IMHO it would be much safer to mask (v << shift) with 0xffff just in case.
- } else { uint32_t cached_val = vop->regsbak[offset >> 2];
cached_val = (cached_val & ~mask) | v;writel_relaxed(cached_val, vop->regs + offset);vop->regsbak[offset >> 2] = cached_val;
v = (cached_val & ~(mask << shift)) | (v << shift);
Should we also mask "v" with "mask" for better safety?
Best regards, Tomasz