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

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

Patch Review

**Timing of property update in `commit_tail`** (medium severity):

```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);
}
```

This fires the uevent before `funcs->atomic_commit_tail()` runs, meaning userspace is notified of a change before hardware is actually programmed. For drivers with custom `atomic_commit_tail` (like amdgpu), this is especially problematic. The update should be placed *after* the hw commit, e.g. after `drm_atomic_helper_commit_hw_done()`. However, the comment at line 2071 warns that `new_crtc_state` can't be accessed after `commit_hw_done()` — but `connector_state` should still be safe. Consider moving this loop after the `atomic_commit_tail` call.

**Double clamping** (minor):

```c
void
drm_connector_update_link_bpc_property(struct drm_connector *connector,
                                       struct drm_connector_state *state)
{
    u8 bpc = clamp(state->link_bpc, 8, state->max_bpc);
```

The clamping is done both here and in the amdgpu helper (`amdgpu_dm_update_link_bpc`). For HDMI connectors, `output_bpc` is assigned directly. The defensive clamping is fine but worth noting that drivers are expected to provide sane values, and the double-clamp in patch 2 is redundant.

**Hardcoded minimum of 8** (minor):

```c
if (max_bpc < 8 || max_bpc > U8_MAX)
    return -EINVAL;

prop = drm_property_create_range(dev, DRM_MODE_PROP_IMMUTABLE,
                                 "link bpc", 8, max_bpc);
```

The 8 bpc lower bound is hardcoded. Some display links (eDP panels, for instance) legitimately operate at 6 bpc. If the intent is to make this generic for future non-HDMI connectors, consider either making the minimum a parameter or documenting why 8 is the universal floor.

**HDMI helper sets link_bpc only when output changes** (potential issue):

```c
if (old_conn_state->hdmi.broadcast_rgb != new_conn_state->hdmi.broadcast_rgb ||
    old_conn_state->hdmi.output_bpc != new_conn_state->hdmi.output_bpc ||
    old_conn_state->hdmi.output_format != new_conn_state->hdmi.output_format) {
        ...
        new_conn_state->link_bpc = new_conn_state->hdmi.output_bpc;
```

`link_bpc` is only set when one of the HDMI output parameters changed. On the very first modeset (when `old_conn_state->link_bpc` is 0 from `kzalloc` and `new_conn_state->link_bpc` is also 0 from `duplicate_state` if it's the initial state), if `output_bpc` hasn't changed between old and new, `link_bpc` will remain 0 and the property won't be updated. This seems like it could lead to the property reporting `max_bpc` (the initial attached value) while the actual link_bpc state is 0 until the first mode change triggers the conditional.

**Property documentation** looks good and is well-placed in the standard connector properties section.

---
Generated by Claude Code Patch Reviewer