Claude review: drm/connector: Add a 'link bpc' property

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Timing issue in commit_tail (major concern):**

The patch inserts the link_bpc property update and uevent between `drm_atomic_helper_wait_for_dependencies()` and the `atomic_commit_tail()` call:

```c
	drm_atomic_helper_wait_for_dependencies(state);

	for_each_oldnew_connector_in_state(state, connector, old_conn_state,
					   new_conn_state, i) {
		if (old_conn_state->link_bpc != new_conn_state->link_bpc)
			drm_connector_update_link_bpc_property(connector,
							       new_conn_state);
	}

	/* ... then atomic_commit_tail() actually programs hardware */
```

This fires the uevent *before* the hardware has been programmed. Userspace reacting to the uevent could query the property and see the new value while the hardware is still at the old state. The property update and uevent should be moved to *after* `atomic_commit_tail()` returns (but before `drm_atomic_helper_commit_cleanup_done()`). That would match the pattern used by `drm_hdcp_update_content_protection` where the property is updated after the HW state is known.

**clamp() type mismatch:**

```c
	u8 bpc = clamp(state->link_bpc, 8, state->max_bpc);
```

`state->link_bpc` is `u8`, `state->max_bpc` is `u8`, but the literal `8` is `int`. The kernel's `clamp()` macro has strict type checking and this will likely generate a compiler warning or error. Should be `clamp_t(u8, state->link_bpc, 8, state->max_bpc)` or use a `(u8)8` cast.

**Unconditional uevent on every link_bpc change:**

`drm_connector_update_link_bpc_property()` always fires a uevent via `drm_sysfs_connector_property_event()`. The caller in `commit_tail` already checks `old_conn_state->link_bpc != new_conn_state->link_bpc`, so the uevent is conditional on an actual change — that's good. However, the function itself doesn't gate on the property having actually changed from the connector object's perspective (via `drm_object_property_set_value`). If `drm_connector_update_link_bpc_property` is ever called from another path where the value hasn't changed, it would fire a spurious uevent. Consider adding a check inside the function, or document that callers must check for changes first.

**Missing `int i` declaration:**

The patch adds `for_each_oldnew_connector_in_state(state, connector, old_conn_state, new_conn_state, i)` in `commit_tail()` but the loop variable `i` is already declared on line 2042 as `unsigned int i` (shared with the self_refresh loop). This works, but only because the existing `i` is `unsigned int` which matches what the macro expects.

**Return value of `drm_connector_attach_link_bpc_property` returns int:**

This function returns `int` (error codes), while the existing `drm_connector_attach_max_bpc_property()` also returns `int`. This is fine, but callers need to check it — see patch 3 comments.

**Minor nit — double space in doc:**

```c
 * @max_bpc: specify the upper limit, matching  that of 'max bpc' property
```

Extra space before "that".

---
Generated by Claude Code Patch Reviewer