Claude review: drm/amd/display: Add support for 'link bpc' property

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

Patch Review

**Unchecked return value:**

```c
		drm_connector_attach_link_bpc_property(&aconnector->base, 16);
```

The return value is not checked. While the existing `drm_connector_attach_max_bpc_property()` call on the line above is also not checked (and that function also returns `int`), this is still a pattern to avoid. If property creation fails (e.g., memory allocation failure), it will silently continue with a NULL `link_bpc_property`. The `drm_connector_update_link_bpc_property` does have a NULL guard, so it won't crash, but the connector will silently lack the property. Consider checking the return value, or at least adding a `drm_warn` on failure.

**Double clamping:**

```c
static void amdgpu_dm_update_link_bpc(struct drm_connector_state *conn_state,
				      enum dc_color_depth depth)
{
	/* 6 bpc is an experimental internal format only, use 8 as minimum */
	conn_state->link_bpc = clamp(convert_dc_color_depth_into_bpc(depth), 8,
				     conn_state->max_bpc);
}
```

This clamps link_bpc at assignment time. Then `drm_connector_update_link_bpc_property()` (from patch 1) clamps again:

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

The double clamping is harmless but redundant. The core helper already handles clamping, so the driver could just do a straight assignment (only filtering out the 6bpc case with a `max()`). Either way it works, but it adds unnecessary complexity.

**Same clamp() type concern** applies here: `convert_dc_color_depth_into_bpc()` returns `int`, `8` is `int`, and `conn_state->max_bpc` is `u8` — mixed types in `clamp()`.

**`convert_dc_color_depth_into_bpc` can return 0:** If `display_color_depth` hits the `default` case, the function returns 0. The `clamp` will push it to 8, which is a reasonable fallback, but it might be worth a `drm_warn` in that scenario rather than silently assuming 8 bpc.

**Placement of the update call:** The `amdgpu_dm_update_link_bpc()` call is in `dm_update_crtc_state()`, which is called from `amdgpu_dm_atomic_check()`. This means the value is computed during the check phase and stored in the connector state, which is consistent with the HDMI helper approach. Good.

---
Generated by Claude Code Patch Reviewer