On Tue, 2 Feb 2021 at 04:46, Jeremy Kerr jk@ozlabs.org wrote:
Hi Joel,
There are minor differences in the values for the threshold value and the scan line size between families of ASPEED SoC. Additionally the SCU register for the output control differs between families.
This adds device tree matching to parameterise these values, allowing us to add support for the AST2400 now, and in the future the AST2600.
Looks good to me. Two super minor things:
--- a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c @@ -60,7 +60,8 @@ static void aspeed_gfx_enable_controller(struct aspeed_gfx *priv) u32 ctrl2 = readl(priv->base + CRT_CTRL2);
/* SCU2C: set DAC source for display output to Graphics CRT (GFX) */
regmap_update_bits(priv->scu, 0x2c, BIT(16), BIT(16));
regmap_update_bits(priv->scu, priv->dac_reg, BIT(16), BIT(16));The comment references SCU2C; but you've implied that this will change...
@@ -228,7 +258,7 @@ static ssize_t dac_mux_store(struct device *dev, struct device_attribute *attr, if (val > 3) return -EINVAL;
rc = regmap_update_bits(priv->scu, ASPEED_SCU_MISC_CTRL, 0x30000, val << 16);
rc = regmap_update_bits(priv->scu, priv->dac_reg, 0x30000, val << 16); if (rc < 0) return 0;@@ -241,7 +271,7 @@ static ssize_t dac_mux_show(struct device *dev, struct device_attribute *attr, c u32 reg; int rc;
rc = regmap_read(priv->scu, ASPEED_SCU_MISC_CTRL, ®);
rc = regmap_read(priv->scu, priv->dac_reg, ®); if (rc) return rc;You've removed the only uses of ASPEED_SCU_MISC_CTRL here, maybe drop the #define too?
Good idea. I'll implement both of your suggestions.