| Message ID | 20241201-drm-connector-eld-mutex-v1-1-ba56a6545c03@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/connector: add eld_mutex to protect connector->eld | expand |
On Sun, 01 Dec 2024, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > The connector->eld is accessed by the .get_eld() callback. This access > can collide with the drm_edid_to_eld() updating the data at the same > time. Add drm_connector.eld_mutex to protect the data from concurrenct > access. Individual drivers are not updated (to reduce possible issues > while applying the patch), maintainers are to find a best suitable way > to lock that mutex while accessing the ELD data. Missed clear_eld(). BR, Jani. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/drm_connector.c | 1 + > drivers/gpu/drm/drm_edid.c | 4 ++++ > include/drm/drm_connector.h | 5 ++++- > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index fc35f47e2849ed6786d6223ac9c69e1c359fc648..bbdaaf7022b62d84594a29f1b60144920903a99a 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -277,6 +277,7 @@ static int __drm_connector_init(struct drm_device *dev, > INIT_LIST_HEAD(&connector->probed_modes); > INIT_LIST_HEAD(&connector->modes); > mutex_init(&connector->mutex); > + mutex_init(&connector->eld_mutex); > mutex_init(&connector->edid_override_mutex); > mutex_init(&connector->hdmi.infoframes.lock); > connector->edid_blob_ptr = NULL; > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 855beafb76ffbecf5c08d58e2f54bfb76f30b930..f0ca77bd42fd518d505d0db78557e0c8d83efb29 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -5657,6 +5657,8 @@ static void drm_edid_to_eld(struct drm_connector *connector, > if (!drm_edid) > return; > > + mutex_lock(&connector->eld_mutex); > + > mnl = get_monitor_name(drm_edid, &eld[DRM_ELD_MONITOR_NAME_STRING]); > drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] ELD monitor %s\n", > connector->base.id, connector->name, > @@ -5717,6 +5719,8 @@ static void drm_edid_to_eld(struct drm_connector *connector, > drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] ELD size %d, SAD count %d\n", > connector->base.id, connector->name, > drm_eld_size(eld), total_sad_count); > + > + mutex_unlock(&connector->eld_mutex); > } > > static int _drm_edid_to_sad(const struct drm_edid *drm_edid, > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index e3fa43291f449d70f3b92a00985336c4f2237bc6..1e2b25e204cb523d61d30f5409faa059bf2b86eb 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -2001,8 +2001,11 @@ struct drm_connector { > struct drm_encoder *encoder; > > #define MAX_ELD_BYTES 128 > - /** @eld: EDID-like data, if present */ > + /** @eld: EDID-like data, if present, protected by @eld_mutex */ > uint8_t eld[MAX_ELD_BYTES]; > + /** @eld_mutex: protection for concurrenct access to @eld */ > + struct mutex eld_mutex; > + > /** @latency_present: AV delay info from ELD, if found */ > bool latency_present[2]; > /**
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index fc35f47e2849ed6786d6223ac9c69e1c359fc648..bbdaaf7022b62d84594a29f1b60144920903a99a 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -277,6 +277,7 @@ static int __drm_connector_init(struct drm_device *dev, INIT_LIST_HEAD(&connector->probed_modes); INIT_LIST_HEAD(&connector->modes); mutex_init(&connector->mutex); + mutex_init(&connector->eld_mutex); mutex_init(&connector->edid_override_mutex); mutex_init(&connector->hdmi.infoframes.lock); connector->edid_blob_ptr = NULL; diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 855beafb76ffbecf5c08d58e2f54bfb76f30b930..f0ca77bd42fd518d505d0db78557e0c8d83efb29 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -5657,6 +5657,8 @@ static void drm_edid_to_eld(struct drm_connector *connector, if (!drm_edid) return; + mutex_lock(&connector->eld_mutex); + mnl = get_monitor_name(drm_edid, &eld[DRM_ELD_MONITOR_NAME_STRING]); drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] ELD monitor %s\n", connector->base.id, connector->name, @@ -5717,6 +5719,8 @@ static void drm_edid_to_eld(struct drm_connector *connector, drm_dbg_kms(connector->dev, "[CONNECTOR:%d:%s] ELD size %d, SAD count %d\n", connector->base.id, connector->name, drm_eld_size(eld), total_sad_count); + + mutex_unlock(&connector->eld_mutex); } static int _drm_edid_to_sad(const struct drm_edid *drm_edid, diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index e3fa43291f449d70f3b92a00985336c4f2237bc6..1e2b25e204cb523d61d30f5409faa059bf2b86eb 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -2001,8 +2001,11 @@ struct drm_connector { struct drm_encoder *encoder; #define MAX_ELD_BYTES 128 - /** @eld: EDID-like data, if present */ + /** @eld: EDID-like data, if present, protected by @eld_mutex */ uint8_t eld[MAX_ELD_BYTES]; + /** @eld_mutex: protection for concurrenct access to @eld */ + struct mutex eld_mutex; + /** @latency_present: AV delay info from ELD, if found */ bool latency_present[2]; /**
The connector->eld is accessed by the .get_eld() callback. This access can collide with the drm_edid_to_eld() updating the data at the same time. Add drm_connector.eld_mutex to protect the data from concurrenct access. Individual drivers are not updated (to reduce possible issues while applying the patch), maintainers are to find a best suitable way to lock that mutex while accessing the ELD data. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/drm_connector.c | 1 + drivers/gpu/drm/drm_edid.c | 4 ++++ include/drm/drm_connector.h | 5 ++++- 3 files changed, 9 insertions(+), 1 deletion(-)