On Tue, 2 Feb 2021 at 04:46, Jeremy Kerr <jk@xxxxxxxxxx> 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. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
