Hi Maxime
On Thu, 10 Dec 2020 at 14:23, Maxime Ripard maxime@cerno.tech wrote:
When run with a higher bpc than 8, the clock of the HDMI controller needs to be adjusted. Let's create a connector state that will be used at atomic_check and atomic_enable to compute and store the clock rate associated to the state.
Acked-by: Thomas Zimmermann tzimmermann@suse.de Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech
drivers/gpu/drm/vc4/vc4_hdmi.c | 33 ++++++++++++++++++++++++++++++--- drivers/gpu/drm/vc4/vc4_hdmi.h | 10 ++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 61039cc89d9d..8978df3f0ca4 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -170,10 +170,37 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
static void vc4_hdmi_connector_reset(struct drm_connector *connector) {
drm_atomic_helper_connector_reset(connector);
struct vc4_hdmi_connector_state *old_state =conn_state_to_vc4_hdmi_conn_state(connector->state);struct vc4_hdmi_connector_state *new_state =kzalloc(sizeof(*conn_state), GFP_KERNEL); if (connector->state)
drm_atomic_helper_connector_tv_reset(connector);
__drm_atomic_helper_connector_destroy_state(connector->state);kfree(old_state);if (!new_state)return;
This has changed since v5 that I added my R-B to.
So we no longer call __drm_atomic_helper_connector_reset should the kzalloc fail. Doesn't that mean connector->state is still set to old_state? Except we've called kfree on that and it's therefore invalid.
Calling __drm_atomic_helper_connector_reset unconditionally is fine as it checks for the new pointer being NULL before dereferencing, but always assigns it to connector->state. Calling drm_atomic_helper_connector_tv_reset when connector->state = NULL isn't safe.
__drm_atomic_helper_connector_reset(connector, &new_state->base);
Put the if (!new_state) return; here? Patch 9/9 can set the max_bpc and max_bpc_requested field here too.
Dave
drm_atomic_helper_connector_tv_reset(connector);+}
+static struct drm_connector_state * +vc4_hdmi_connector_duplicate_state(struct drm_connector *connector) +{
struct drm_connector_state *conn_state = connector->state;struct vc4_hdmi_connector_state *vc4_state = conn_state_to_vc4_hdmi_conn_state(conn_state);struct vc4_hdmi_connector_state *new_state;new_state = kzalloc(sizeof(*new_state), GFP_KERNEL);if (!new_state)return NULL;__drm_atomic_helper_connector_duplicate_state(connector, &new_state->base);return &new_state->base;}
static const struct drm_connector_funcs vc4_hdmi_connector_funcs = { @@ -181,7 +208,7 @@ static const struct drm_connector_funcs vc4_hdmi_connector_funcs = { .fill_modes = drm_helper_probe_single_connector_modes, .destroy = vc4_hdmi_connector_destroy, .reset = vc4_hdmi_connector_reset,
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
.atomic_duplicate_state = vc4_hdmi_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,};
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h index 0526a9cf608a..2cf5362052e2 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.h +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h @@ -180,6 +180,16 @@ encoder_to_vc4_hdmi(struct drm_encoder *encoder) return container_of(_encoder, struct vc4_hdmi, encoder); }
+struct vc4_hdmi_connector_state {
struct drm_connector_state base;+};
+static inline struct vc4_hdmi_connector_state * +conn_state_to_vc4_hdmi_conn_state(struct drm_connector_state *conn_state) +{
return container_of(conn_state, struct vc4_hdmi_connector_state, base);+}
void vc4_hdmi_phy_init(struct vc4_hdmi *vc4_hdmi, struct drm_display_mode *mode); void vc4_hdmi_phy_disable(struct vc4_hdmi *vc4_hdmi); -- 2.28.0