On 2023/5/22 16:51, Sui Jingfeng wrote:
Hi,
On 2023/5/21 20:21, WANG Xuerui wrote:
<snip> + +static void lsdc_crtc0_soft_reset(struct lsdc_crtc *lcrtc) +{ + struct lsdc_device *ldev = lcrtc->ldev; + u32 val; + + val = lsdc_rreg32(ldev, LSDC_CRTC0_CFG_REG); + + val &= CFG_VALID_BITS_MASK; + + /* soft reset bit, active low */ + val &= ~CFG_RESET_N; + + val &= ~CFG_PIX_FMT_MASK; + + lsdc_wreg32(ldev, LSDC_CRTC0_CFG_REG, val); + + udelay(5); + + val |= CFG_RESET_N | LSDC_PF_XRGB8888 | CFG_OUTPUT_ENABLE; + + lsdc_wreg32(ldev, LSDC_CRTC0_CFG_REG, val); + + mdelay(20); +} + +static void lsdc_crtc1_soft_reset(struct lsdc_crtc *lcrtc) +{ + struct lsdc_device *ldev = lcrtc->ldev; + u32 val; + + val = lsdc_rreg32(ldev, LSDC_CRTC1_CFG_REG); + + val &= CFG_VALID_BITS_MASK; + + /* soft reset bit, active low */ + val &= ~CFG_RESET_N; + + val &= ~CFG_PIX_FMT_MASK; + + lsdc_wreg32(ldev, LSDC_CRTC1_CFG_REG, val); + + udelay(5); + + val |= CFG_RESET_N | LSDC_PF_XRGB8888 | CFG_OUTPUT_ENABLE; + + lsdc_wreg32(ldev, LSDC_CRTC1_CFG_REG, val); + + msleep(20);
So many magic sleeps without documentation?
It is just that you should wait the device for a while before it can reaction when doing the soft reset.
I think this is engineering...
As an engineer myself, I fully concur with this, but I mainly wanted some explanation as to "why 5 there? why 20 here? why 9 there?" -- where did all the discrete values come from, implied by HDL or found out by experimentations? Can these be extracted to properly named constants? Can some of the values get coalesced into one without harming functionality? Can some of them get shorter? -- questions like this.
<snip> >> + >> +/* All loongson display controller support scanout position hardware */ > > Commit message implies only 7A2000+ LSDC IPs have the "scanout > position recorders". Either that part or this code would need tweaking...
Both LS7A2000 and LS7A1000 have the scanout position recorders hardware.
Preciously, datasheet of LS7A1000 didn't told us if it support this feature.
I will adjust the commit message at next version, the code doesn't need change.
That's fine, the intent is always making the code more approachable and maintainable. Thanks.