Re: [PATCH RFC v2 11/37] drm/connector: hdmi: Add Infoframes generation
Dne sreda, 20. september 2023 ob 16:35:26 CEST je Maxime Ripard napisal(a):
> Infoframes in KMS is usually handled by a bunch of low-level helpers
> that require quite some boilerplate for drivers. This leads to
> discrepancies with how drivers generate them, and which are actually
> sent.
>
> Now that we have everything needed to generate them in the HDMI
> connector state, we can generate them in our common logic so that
> drivers can simply reuse what we precomputed.
>
> Signed-off-by: Maxime Ripard
> ---
> drivers/gpu/drm/Kconfig | 1 +
> drivers/gpu/drm/drm_atomic_state_helper.c | 327
> ++
> drivers/gpu/drm/drm_connector.c | 9 +
> include/drm/drm_atomic_state_helper.h | 6 +
> include/drm/drm_connector.h | 131
> 5 files changed, 474 insertions(+)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index ab9ef1c20349..10caf2dcce93 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -99,6 +99,7 @@ config DRM_KUNIT_TEST
> config DRM_KMS_HELPER
> tristate
> depends on DRM
> + select DRM_DISPLAY_HDMI_HELPER
> help
> CRTC helpers for KMS drivers.
>
> diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
> b/drivers/gpu/drm/drm_atomic_state_helper.c
> index 2f85422cccd4..5bbdd2f7d306 100644
> --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> @@ -38,6 +38,8 @@
> #include
> #include
>
> +#include
> +
> #include
> #include
>
> @@ -839,6 +841,142 @@ hdmi_compute_config(const struct drm_connector
> *connector,
> return -EINVAL;
> }
>
> +static int hdmi_generate_avi_infoframe(const struct drm_connector *connector,
> +struct drm_connector_state *state)
> +{
> + const struct drm_display_mode *mode =
> + connector_state_get_adjusted_mode(state);
> + struct drm_connector_hdmi_infoframe *infoframe =
> + &state->hdmi.infoframes.avi;
> + struct hdmi_avi_infoframe *frame =
> + &infoframe->data.avi;
> + bool is_lim_range =
> + drm_atomic_helper_connector_hdmi_is_full_range(connector,
> +state);
> + enum hdmi_quantization_range rgb_quant_range =
> + is_lim_range ? HDMI_QUANTIZATION_RANGE_FULL :
> HDMI_QUANTIZATION_RANGE_LIMITED;
While usage of is_lim_range is correct, its name is off. Replace lim with full.
Best regards,
Jernej
> + int ret;
> +
> + ret = drm_hdmi_avi_infoframe_from_display_mode(frame, connector, mode);
> + if (ret)
> + return ret;
> +
> + frame->colorspace = state->hdmi.output_format;
> +
> + drm_hdmi_avi_infoframe_quant_range(frame, connector, mode,
> rgb_quant_range);
> + drm_hdmi_avi_infoframe_colorimetry(frame, state);
> + drm_hdmi_avi_infoframe_bars(frame, state);
> +
> + infoframe->set = true;
> +
> + return 0;
> +}
> +
> +static int hdmi_generate_spd_infoframe(const struct drm_connector *connector,
> +struct drm_connector_state *state)
> +{
> + struct drm_connector_hdmi_infoframe *infoframe =
> + &state->hdmi.infoframes.spd;
> + struct hdmi_spd_infoframe *frame =
> + &infoframe->data.spd;
> + int ret;
> +
> + ret = hdmi_spd_infoframe_init(frame,
> + connector->hdmi.vendor,
> + connector->hdmi.product);
> + if (ret)
> + return ret;
> +
> + frame->sdi = HDMI_SPD_SDI_PC;
> +
> + infoframe->set = true;
> +
> + return 0;
> +}
> +
> +static int hdmi_generate_hdr_infoframe(const struct drm_connector *connector,
> +struct drm_connector_state *state)
> +{
> + struct drm_connector_hdmi_infoframe *infoframe =
> + &state->hdmi.infoframes.drm;
> + struct hdmi_drm_infoframe *frame =
> + &infoframe->data.drm;
> + int ret;
> +
> + if (connector->max_bpc < 10)
> + return 0;
> +
> + if (!state->hdr_output_metadata)
> + return 0;
> +
> + ret = drm_hdmi_infoframe_set_hdr_metadata(frame, state);
> + if (ret)
> + return ret;
> +
> + infoframe->set = true;
> +
> + return 0;
> +}
> +
> +static int hdmi_generate_vendor_infoframe(const struct drm_connector
> *connector,
> + struct drm_connector_state *state)
> +{
> + const struct drm_display_mode *mode =
> + connector_state_get_adjusted_mode(state);
> + struct drm_connector_hdmi_infoframe *infoframe =
> + &state->hdmi.infoframes.vendor;
> + struct hdmi_vendor_infoframe *frame =
> + &infoframe->data.vendor.hdmi;
> + int ret;
> +
> + ret = drm_hdmi_vendor_infoframe_from_display_mode(frame, connector,
> mode);
> +
[PATCH RFC v2 11/37] drm/connector: hdmi: Add Infoframes generation
Infoframes in KMS is usually handled by a bunch of low-level helpers
that require quite some boilerplate for drivers. This leads to
discrepancies with how drivers generate them, and which are actually
sent.
Now that we have everything needed to generate them in the HDMI
connector state, we can generate them in our common logic so that
drivers can simply reuse what we precomputed.
Signed-off-by: Maxime Ripard
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/drm_atomic_state_helper.c | 327 ++
drivers/gpu/drm/drm_connector.c | 9 +
include/drm/drm_atomic_state_helper.h | 6 +
include/drm/drm_connector.h | 131
5 files changed, 474 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index ab9ef1c20349..10caf2dcce93 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -99,6 +99,7 @@ config DRM_KUNIT_TEST
config DRM_KMS_HELPER
tristate
depends on DRM
+ select DRM_DISPLAY_HDMI_HELPER
help
CRTC helpers for KMS drivers.
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
b/drivers/gpu/drm/drm_atomic_state_helper.c
index 2f85422cccd4..5bbdd2f7d306 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -38,6 +38,8 @@
#include
#include
+#include
+
#include
#include
@@ -839,6 +841,142 @@ hdmi_compute_config(const struct drm_connector *connector,
return -EINVAL;
}
+static int hdmi_generate_avi_infoframe(const struct drm_connector *connector,
+ struct drm_connector_state *state)
+{
+ const struct drm_display_mode *mode =
+ connector_state_get_adjusted_mode(state);
+ struct drm_connector_hdmi_infoframe *infoframe =
+ &state->hdmi.infoframes.avi;
+ struct hdmi_avi_infoframe *frame =
+ &infoframe->data.avi;
+ bool is_lim_range =
+ drm_atomic_helper_connector_hdmi_is_full_range(connector,
+ state);
+ enum hdmi_quantization_range rgb_quant_range =
+ is_lim_range ? HDMI_QUANTIZATION_RANGE_FULL :
HDMI_QUANTIZATION_RANGE_LIMITED;
+ int ret;
+
+ ret = drm_hdmi_avi_infoframe_from_display_mode(frame, connector, mode);
+ if (ret)
+ return ret;
+
+ frame->colorspace = state->hdmi.output_format;
+
+ drm_hdmi_avi_infoframe_quant_range(frame, connector, mode,
rgb_quant_range);
+ drm_hdmi_avi_infoframe_colorimetry(frame, state);
+ drm_hdmi_avi_infoframe_bars(frame, state);
+
+ infoframe->set = true;
+
+ return 0;
+}
+
+static int hdmi_generate_spd_infoframe(const struct drm_connector *connector,
+ struct drm_connector_state *state)
+{
+ struct drm_connector_hdmi_infoframe *infoframe =
+ &state->hdmi.infoframes.spd;
+ struct hdmi_spd_infoframe *frame =
+ &infoframe->data.spd;
+ int ret;
+
+ ret = hdmi_spd_infoframe_init(frame,
+ connector->hdmi.vendor,
+ connector->hdmi.product);
+ if (ret)
+ return ret;
+
+ frame->sdi = HDMI_SPD_SDI_PC;
+
+ infoframe->set = true;
+
+ return 0;
+}
+
+static int hdmi_generate_hdr_infoframe(const struct drm_connector *connector,
+ struct drm_connector_state *state)
+{
+ struct drm_connector_hdmi_infoframe *infoframe =
+ &state->hdmi.infoframes.drm;
+ struct hdmi_drm_infoframe *frame =
+ &infoframe->data.drm;
+ int ret;
+
+ if (connector->max_bpc < 10)
+ return 0;
+
+ if (!state->hdr_output_metadata)
+ return 0;
+
+ ret = drm_hdmi_infoframe_set_hdr_metadata(frame, state);
+ if (ret)
+ return ret;
+
+ infoframe->set = true;
+
+ return 0;
+}
+
+static int hdmi_generate_vendor_infoframe(const struct drm_connector
*connector,
+ struct drm_connector_state *state)
+{
+ const struct drm_display_mode *mode =
+ connector_state_get_adjusted_mode(state);
+ struct drm_connector_hdmi_infoframe *infoframe =
+ &state->hdmi.infoframes.vendor;
+ struct hdmi_vendor_infoframe *frame =
+ &infoframe->data.vendor.hdmi;
+ int ret;
+
+ ret = drm_hdmi_vendor_infoframe_from_display_mode(frame, connector,
mode);
+ if (ret == -EINVAL)
+ return 0;
+ else
+ return ret;
+
+ infoframe->set = true;
+
+ return 0;
+}
+
+static int
+hdmi_generate_infoframes(const struct drm_connector *connector,
+struct drm_connector_state *state)
+{
+ const struct drm_display_info *info = &c
