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.
--
WANG "xen0n" Xuerui
Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
