* [PATCH] drm: make sure to check callbacks of ENCODER and CRTC helpers
@ 2015-08-16 4:12 Inki Dae
2015-08-25 8:26 ` Daniel Vetter
0 siblings, 1 reply; 2+ messages in thread
From: Inki Dae @ 2015-08-16 4:12 UTC (permalink / raw)
To: dri-devel
This patch fixes NULL pointer access issues to ENCODER
and CRTC drivers.
Vendor specific DRM KMS drivers may not set their helper callbacks
to ENCODER dn CRTC drivers. In this case, that will incur NULL
pointer access when modeset is tried.
So this patch makes sure to check if the callbacks are NULL or not.
Signed-off-by: Inki Dae <inki.dae@samsung.com>
---
drivers/gpu/drm/drm_crtc_helper.c | 44 ++++++++++++++++++++++++++-------------
1 file changed, 29 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index ef53475..aa1f42f 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -318,16 +318,22 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
}
encoder_funcs = encoder->helper_private;
- if (!(ret = encoder_funcs->mode_fixup(encoder, mode,
- adjusted_mode))) {
- DRM_DEBUG_KMS("Encoder fixup failed\n");
- goto done;
+ if (encoder_funcs->mode_fixup) {
+ ret = encoder_funcs->mode_fixup(encoder, mode,
+ adjusted_mode);
+ if (!ret) {
+ DRM_DEBUG_KMS("Encoder fixup failed\n");
+ goto done;
+ }
}
}
- if (!(ret = crtc_funcs->mode_fixup(crtc, mode, adjusted_mode))) {
- DRM_DEBUG_KMS("CRTC fixup failed\n");
- goto done;
+ if (crtc_funcs->mode_fixup) {
+ ret = crtc_funcs->mode_fixup(crtc, mode, adjusted_mode);
+ if (!ret) {
+ DRM_DEBUG_KMS("CRTC fixup failed\n");
+ goto done;
+ }
}
DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
@@ -343,21 +349,26 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
encoder_funcs = encoder->helper_private;
/* Disable the encoders as the first thing we do. */
- encoder_funcs->prepare(encoder);
+ if (encoder_funcs->prepare)
+ encoder_funcs->prepare(encoder);
drm_bridge_post_disable(encoder->bridge);
}
drm_crtc_prepare_encoders(dev);
- crtc_funcs->prepare(crtc);
+ if (crtc_funcs->prepare)
+ crtc_funcs->prepare(crtc);
/* Set up the DPLL and any encoders state that needs to adjust or depend
* on the DPLL.
*/
- ret = !crtc_funcs->mode_set(crtc, mode, adjusted_mode, x, y, old_fb);
- if (!ret)
- goto done;
+ if (crtc_funcs->mode_set) {
+ ret = !crtc_funcs->mode_set(crtc, mode, adjusted_mode, x, y,
+ old_fb);
+ if (!ret)
+ goto done;
+ }
drm_for_each_encoder(encoder, dev) {
@@ -368,13 +379,15 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
encoder->base.id, encoder->name,
mode->base.id, mode->name);
encoder_funcs = encoder->helper_private;
- encoder_funcs->mode_set(encoder, mode, adjusted_mode);
+ if (encoder_funcs->mode_set)
+ encoder_funcs->mode_set(encoder, mode, adjusted_mode);
drm_bridge_mode_set(encoder->bridge, mode, adjusted_mode);
}
/* Now enable the clocks, plane, pipe, and connectors that we set up. */
- crtc_funcs->commit(crtc);
+ if (crtc_funcs->commit)
+ crtc_funcs->commit(crtc);
drm_for_each_encoder(encoder, dev) {
@@ -384,7 +397,8 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
drm_bridge_pre_enable(encoder->bridge);
encoder_funcs = encoder->helper_private;
- encoder_funcs->commit(encoder);
+ if (encoder_funcs->commit)
+ encoder_funcs->commit(encoder);
drm_bridge_enable(encoder->bridge);
}
--
1.9.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] drm: make sure to check callbacks of ENCODER and CRTC helpers
2015-08-16 4:12 [PATCH] drm: make sure to check callbacks of ENCODER and CRTC helpers Inki Dae
@ 2015-08-25 8:26 ` Daniel Vetter
0 siblings, 0 replies; 2+ messages in thread
From: Daniel Vetter @ 2015-08-25 8:26 UTC (permalink / raw)
To: Inki Dae; +Cc: dri-devel
On Sun, Aug 16, 2015 at 01:12:14PM +0900, Inki Dae wrote:
> This patch fixes NULL pointer access issues to ENCODER
> and CRTC drivers.
>
> Vendor specific DRM KMS drivers may not set their helper callbacks
> to ENCODER dn CRTC drivers. In this case, that will incur NULL
> pointer access when modeset is tried.
>
> So this patch makes sure to check if the callbacks are NULL or not.
>
> Signed-off-by: Inki Dae <inki.dae@samsung.com>
Is there really a lot of value in tuning legacy crtc helpers? New drivers
should just use atomic (where we've made all these changes already),
drivers which are still transitioning can just carry around dummy
functions for now.
-Daniel
> ---
> drivers/gpu/drm/drm_crtc_helper.c | 44 ++++++++++++++++++++++++++-------------
> 1 file changed, 29 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index ef53475..aa1f42f 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -318,16 +318,22 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
> }
>
> encoder_funcs = encoder->helper_private;
> - if (!(ret = encoder_funcs->mode_fixup(encoder, mode,
> - adjusted_mode))) {
> - DRM_DEBUG_KMS("Encoder fixup failed\n");
> - goto done;
> + if (encoder_funcs->mode_fixup) {
> + ret = encoder_funcs->mode_fixup(encoder, mode,
> + adjusted_mode);
> + if (!ret) {
> + DRM_DEBUG_KMS("Encoder fixup failed\n");
> + goto done;
> + }
> }
> }
>
> - if (!(ret = crtc_funcs->mode_fixup(crtc, mode, adjusted_mode))) {
> - DRM_DEBUG_KMS("CRTC fixup failed\n");
> - goto done;
> + if (crtc_funcs->mode_fixup) {
> + ret = crtc_funcs->mode_fixup(crtc, mode, adjusted_mode);
> + if (!ret) {
> + DRM_DEBUG_KMS("CRTC fixup failed\n");
> + goto done;
> + }
> }
> DRM_DEBUG_KMS("[CRTC:%d]\n", crtc->base.id);
>
> @@ -343,21 +349,26 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
>
> encoder_funcs = encoder->helper_private;
> /* Disable the encoders as the first thing we do. */
> - encoder_funcs->prepare(encoder);
> + if (encoder_funcs->prepare)
> + encoder_funcs->prepare(encoder);
>
> drm_bridge_post_disable(encoder->bridge);
> }
>
> drm_crtc_prepare_encoders(dev);
>
> - crtc_funcs->prepare(crtc);
> + if (crtc_funcs->prepare)
> + crtc_funcs->prepare(crtc);
>
> /* Set up the DPLL and any encoders state that needs to adjust or depend
> * on the DPLL.
> */
> - ret = !crtc_funcs->mode_set(crtc, mode, adjusted_mode, x, y, old_fb);
> - if (!ret)
> - goto done;
> + if (crtc_funcs->mode_set) {
> + ret = !crtc_funcs->mode_set(crtc, mode, adjusted_mode, x, y,
> + old_fb);
> + if (!ret)
> + goto done;
> + }
>
> drm_for_each_encoder(encoder, dev) {
>
> @@ -368,13 +379,15 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
> encoder->base.id, encoder->name,
> mode->base.id, mode->name);
> encoder_funcs = encoder->helper_private;
> - encoder_funcs->mode_set(encoder, mode, adjusted_mode);
> + if (encoder_funcs->mode_set)
> + encoder_funcs->mode_set(encoder, mode, adjusted_mode);
>
> drm_bridge_mode_set(encoder->bridge, mode, adjusted_mode);
> }
>
> /* Now enable the clocks, plane, pipe, and connectors that we set up. */
> - crtc_funcs->commit(crtc);
> + if (crtc_funcs->commit)
> + crtc_funcs->commit(crtc);
>
> drm_for_each_encoder(encoder, dev) {
>
> @@ -384,7 +397,8 @@ bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
> drm_bridge_pre_enable(encoder->bridge);
>
> encoder_funcs = encoder->helper_private;
> - encoder_funcs->commit(encoder);
> + if (encoder_funcs->commit)
> + encoder_funcs->commit(encoder);
>
> drm_bridge_enable(encoder->bridge);
> }
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-08-25 8:26 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-16 4:12 [PATCH] drm: make sure to check callbacks of ENCODER and CRTC helpers Inki Dae
2015-08-25 8:26 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox