On 10/13/25 6:38 PM, Frank Li wrote:
Hello Frank,
+#define STATICCONTROL 0x8
+#define SHDTOKSEL_MASK GENMASK(6, 4)
+#define SHDTOKSEL(x) FIELD_PREP(SHDTOKSEL_MASK, (x))
+#define SHDLDSEL_MASK GENMASK(3, 1)
+#define SHDLDSEL(x) FIELD_PREP(SHDLDSEL_MASK, (x))
Can you keep bit fields as consistent order, from 31..0 or 0..31.
I sent a separate fix for that, so it can go in before these patches:
[PATCH] drm/imx: dc: Sort bits and bitfields in descending order
[...]
+static const struct dc_subdev_info dc_db_info[] = {
+ { .reg_start = 0x4b6a0000, .id = 0, },
+ { .reg_start = 0x4b720000, .id = 1, },
+};
Not sure why need map register address to id? Does graphic link or use
dt cells pass it as argument.
The display engine component (de) does use it to figure out which
matching domainblend component (db) to access , since there are multiple
instances of each.
Is there anything I should change ?
+static inline void dc_db_alphamaskmode_disable(struct dc_db *db)
+{
+ regmap_write_bits(db->reg_cfg, ALPHACONTROL, ALPHAMASKENABLE, 0);
+}
This helper function just write value to one register, not helper much
at all.
I agree, but it also makes dc_db_init() a bit more readable, so I don't
think inlining this is going to improve the driver much.
+static inline void dc_db_blendcontrol(struct dc_db *db)
+{
+ u32 val = PRIM_A_BLD_FUNC(DC_DOMAINBLEND_BLEND_ZERO) |
+ SEC_A_BLD_FUNC(DC_DOMAINBLEND_BLEND_ZERO) |
+ PRIM_C_BLD_FUNC(DC_DOMAINBLEND_BLEND_ZERO) |
+ SEC_C_BLD_FUNC(DC_DOMAINBLEND_BLEND_ONE);
+
+ regmap_write(db->reg_cfg, BLENDCONTROL, val);
+}
+
+void dc_db_init(struct dc_db *db)
+{
+ dc_db_enable_shden(db);
+ dc_db_shdtoksel(db, SW);
+ dc_db_shdldsel(db, SW);
+ dc_db_mode(db, DB_PRIMARY);
+ dc_db_alphamaskmode_disable(db);
+ dc_db_blendcontrol(db);
+}
+
...
+struct dc_db {
+ struct device *dev;
+ struct regmap *reg_cfg;
+ int id;
where actually use this id?
Please see dc_db_bind() .
I will be handling the rest of the feedback on this series piece by
piece, thank you for your input.
