[PATCH] drm/exynos: init vblank with real number of crtcs
On 2014? 10? 08? 00:28, Inki Dae wrote: > > Sorry for late. > > On 2014? 10? 07? 21:27, Andrzej Hajda wrote: >> Hi Inki, >> >> Gently ping. >> >> Andrzej >> >> >> On 09/19/2014 02:57 PM, Andrzej Hajda wrote: >>> Initialization of vblank with MAX_CRTC caused attempts >>> to disabling vblanks for non-existing crtcs in case >>> drm used fewer crtcs. The patch fixes it. > > Nice catch~ This patch also resolves unbind issue. When exynos driver is > unbound, disable_vblank callback is called as dev->num_crtcs so if the > number of probed crtc drivers is different from the one of enabled crtc Oops, s/the one of enabled crtc drivers/MAX_CRTC > drivers then it could attempt to disabling vblank for non-existing crtc, > which in turn, null pointer access occurs. > > Thanks, > Inki Dae > >>> >>> Signed-off-by: Andrzej Hajda >>> --- >>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 18 +- >>> 1 file changed, 9 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c >>> b/drivers/gpu/drm/exynos/exynos_drm_drv.c >>> index 9b00e4e..dc4affd 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c >>> @@ -94,10 +94,6 @@ static int exynos_drm_load(struct drm_device *dev, >>> unsigned long flags) >>> /* init kms poll for handling hpd */ >>> drm_kms_helper_poll_init(dev); >>> >>> - ret = drm_vblank_init(dev, MAX_CRTC); >>> - if (ret) >>> - goto err_mode_config_cleanup; >>> - >>> /* setup possible_clones. */ >>> exynos_drm_encoder_setup(dev); >>> >>> @@ -106,22 +102,26 @@ static int exynos_drm_load(struct drm_device *dev, >>> unsigned long flags) >>> /* Try to bind all sub drivers. */ >>> ret = component_bind_all(dev->dev, dev); >>> if (ret) >>> - goto err_cleanup_vblank; >>> + goto err_mode_config_cleanup; >>> + >>> + ret = drm_vblank_init(dev, dev->mode_config.num_crtc); >>> + if (ret) >>> + goto err_unbind_all; >>> >>> /* Probe non kms sub drivers and virtual display driver. */ >>> ret = exynos_drm_device_subdrv_probe(dev); >>> if (ret) >>> - goto err_unbind_all; >>> + goto err_cleanup_vblank; >>> >>> /* force connectors detection */ >>> drm_helper_hpd_irq_event(dev); >>> >>> return 0; >>> >>> -err_unbind_all: >>> - component_unbind_all(dev->dev, dev); >>> err_cleanup_vblank: >>> drm_vblank_cleanup(dev); >>> +err_unbind_all: >>> + component_unbind_all(dev->dev, dev); >>> err_mode_config_cleanup: >>> drm_mode_config_cleanup(dev); >>> drm_release_iommu_mapping(dev); >>> @@ -138,8 +138,8 @@ static int exynos_drm_unload(struct drm_device *dev) >>> exynos_drm_fbdev_fini(dev); >>> drm_kms_helper_poll_fini(dev); >>> >>> - component_unbind_all(dev->dev, dev); >>> drm_vblank_cleanup(dev); >>> + component_unbind_all(dev->dev, dev); >>> drm_mode_config_cleanup(dev); >>> drm_release_iommu_mapping(dev); >>> >>> >> >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majordomo at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
[PATCH] drm/exynos: init vblank with real number of crtcs
Sorry for late. On 2014? 10? 07? 21:27, Andrzej Hajda wrote: > Hi Inki, > > Gently ping. > > Andrzej > > > On 09/19/2014 02:57 PM, Andrzej Hajda wrote: >> Initialization of vblank with MAX_CRTC caused attempts >> to disabling vblanks for non-existing crtcs in case >> drm used fewer crtcs. The patch fixes it. Nice catch~ This patch also resolves unbind issue. When exynos driver is unbound, disable_vblank callback is called as dev->num_crtcs so if the number of probed crtc drivers is different from the one of enabled crtc drivers then it could attempt to disabling vblank for non-existing crtc, which in turn, null pointer access occurs. Thanks, Inki Dae >> >> Signed-off-by: Andrzej Hajda >> --- >> drivers/gpu/drm/exynos/exynos_drm_drv.c | 18 +- >> 1 file changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c >> b/drivers/gpu/drm/exynos/exynos_drm_drv.c >> index 9b00e4e..dc4affd 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c >> @@ -94,10 +94,6 @@ static int exynos_drm_load(struct drm_device *dev, >> unsigned long flags) >> /* init kms poll for handling hpd */ >> drm_kms_helper_poll_init(dev); >> >> -ret = drm_vblank_init(dev, MAX_CRTC); >> -if (ret) >> -goto err_mode_config_cleanup; >> - >> /* setup possible_clones. */ >> exynos_drm_encoder_setup(dev); >> >> @@ -106,22 +102,26 @@ static int exynos_drm_load(struct drm_device *dev, >> unsigned long flags) >> /* Try to bind all sub drivers. */ >> ret = component_bind_all(dev->dev, dev); >> if (ret) >> -goto err_cleanup_vblank; >> +goto err_mode_config_cleanup; >> + >> +ret = drm_vblank_init(dev, dev->mode_config.num_crtc); >> +if (ret) >> +goto err_unbind_all; >> >> /* Probe non kms sub drivers and virtual display driver. */ >> ret = exynos_drm_device_subdrv_probe(dev); >> if (ret) >> -goto err_unbind_all; >> +goto err_cleanup_vblank; >> >> /* force connectors detection */ >> drm_helper_hpd_irq_event(dev); >> >> return 0; >> >> -err_unbind_all: >> -component_unbind_all(dev->dev, dev); >> err_cleanup_vblank: >> drm_vblank_cleanup(dev); >> +err_unbind_all: >> +component_unbind_all(dev->dev, dev); >> err_mode_config_cleanup: >> drm_mode_config_cleanup(dev); >> drm_release_iommu_mapping(dev); >> @@ -138,8 +138,8 @@ static int exynos_drm_unload(struct drm_device *dev) >> exynos_drm_fbdev_fini(dev); >> drm_kms_helper_poll_fini(dev); >> >> -component_unbind_all(dev->dev, dev); >> drm_vblank_cleanup(dev); >> +component_unbind_all(dev->dev, dev); >> drm_mode_config_cleanup(dev); >> drm_release_iommu_mapping(dev); >> >> > >
[PATCH] drm/exynos: init vblank with real number of crtcs
Hi Inki, Gently ping. Andrzej On 09/19/2014 02:57 PM, Andrzej Hajda wrote: > Initialization of vblank with MAX_CRTC caused attempts > to disabling vblanks for non-existing crtcs in case > drm used fewer crtcs. The patch fixes it. > > Signed-off-by: Andrzej Hajda > --- > drivers/gpu/drm/exynos/exynos_drm_drv.c | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c > b/drivers/gpu/drm/exynos/exynos_drm_drv.c > index 9b00e4e..dc4affd 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > @@ -94,10 +94,6 @@ static int exynos_drm_load(struct drm_device *dev, > unsigned long flags) > /* init kms poll for handling hpd */ > drm_kms_helper_poll_init(dev); > > - ret = drm_vblank_init(dev, MAX_CRTC); > - if (ret) > - goto err_mode_config_cleanup; > - > /* setup possible_clones. */ > exynos_drm_encoder_setup(dev); > > @@ -106,22 +102,26 @@ static int exynos_drm_load(struct drm_device *dev, > unsigned long flags) > /* Try to bind all sub drivers. */ > ret = component_bind_all(dev->dev, dev); > if (ret) > - goto err_cleanup_vblank; > + goto err_mode_config_cleanup; > + > + ret = drm_vblank_init(dev, dev->mode_config.num_crtc); > + if (ret) > + goto err_unbind_all; > > /* Probe non kms sub drivers and virtual display driver. */ > ret = exynos_drm_device_subdrv_probe(dev); > if (ret) > - goto err_unbind_all; > + goto err_cleanup_vblank; > > /* force connectors detection */ > drm_helper_hpd_irq_event(dev); > > return 0; > > -err_unbind_all: > - component_unbind_all(dev->dev, dev); > err_cleanup_vblank: > drm_vblank_cleanup(dev); > +err_unbind_all: > + component_unbind_all(dev->dev, dev); > err_mode_config_cleanup: > drm_mode_config_cleanup(dev); > drm_release_iommu_mapping(dev); > @@ -138,8 +138,8 @@ static int exynos_drm_unload(struct drm_device *dev) > exynos_drm_fbdev_fini(dev); > drm_kms_helper_poll_fini(dev); > > - component_unbind_all(dev->dev, dev); > drm_vblank_cleanup(dev); > + component_unbind_all(dev->dev, dev); > drm_mode_config_cleanup(dev); > drm_release_iommu_mapping(dev); > >
[PATCH] drm/exynos: init vblank with real number of crtcs
On Fri, Sep 19, 2014 at 02:57:20PM +0200, Andrzej Hajda wrote: > Initialization of vblank with MAX_CRTC caused attempts > to disabling vblanks for non-existing crtcs in case > drm used fewer crtcs. The patch fixes it. > > Signed-off-by: Andrzej Hajda > --- > drivers/gpu/drm/exynos/exynos_drm_drv.c | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c > b/drivers/gpu/drm/exynos/exynos_drm_drv.c > index 9b00e4e..dc4affd 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c > @@ -94,10 +94,6 @@ static int exynos_drm_load(struct drm_device *dev, > unsigned long flags) > /* init kms poll for handling hpd */ > drm_kms_helper_poll_init(dev); > > - ret = drm_vblank_init(dev, MAX_CRTC); > - if (ret) > - goto err_mode_config_cleanup; > - > /* setup possible_clones. */ > exynos_drm_encoder_setup(dev); > > @@ -106,22 +102,26 @@ static int exynos_drm_load(struct drm_device *dev, > unsigned long flags) > /* Try to bind all sub drivers. */ > ret = component_bind_all(dev->dev, dev); > if (ret) > - goto err_cleanup_vblank; > + goto err_mode_config_cleanup; > + > + ret = drm_vblank_init(dev, dev->mode_config.num_crtc); Hm, I wonder whether we should have a drm_mode_vblank_init which dtrt here for kms drivers? Suggestions for a better name welcome ;-) -Daniel > + if (ret) > + goto err_unbind_all; > > /* Probe non kms sub drivers and virtual display driver. */ > ret = exynos_drm_device_subdrv_probe(dev); > if (ret) > - goto err_unbind_all; > + goto err_cleanup_vblank; > > /* force connectors detection */ > drm_helper_hpd_irq_event(dev); > > return 0; > > -err_unbind_all: > - component_unbind_all(dev->dev, dev); > err_cleanup_vblank: > drm_vblank_cleanup(dev); > +err_unbind_all: > + component_unbind_all(dev->dev, dev); > err_mode_config_cleanup: > drm_mode_config_cleanup(dev); > drm_release_iommu_mapping(dev); > @@ -138,8 +138,8 @@ static int exynos_drm_unload(struct drm_device *dev) > exynos_drm_fbdev_fini(dev); > drm_kms_helper_poll_fini(dev); > > - component_unbind_all(dev->dev, dev); > drm_vblank_cleanup(dev); > + component_unbind_all(dev->dev, dev); > drm_mode_config_cleanup(dev); > drm_release_iommu_mapping(dev); > > -- > 1.9.1 > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm/exynos: init vblank with real number of crtcs
Initialization of vblank with MAX_CRTC caused attempts to disabling vblanks for non-existing crtcs in case drm used fewer crtcs. The patch fixes it. Signed-off-by: Andrzej Hajda --- drivers/gpu/drm/exynos/exynos_drm_drv.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c index 9b00e4e..dc4affd 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c @@ -94,10 +94,6 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags) /* init kms poll for handling hpd */ drm_kms_helper_poll_init(dev); - ret = drm_vblank_init(dev, MAX_CRTC); - if (ret) - goto err_mode_config_cleanup; - /* setup possible_clones. */ exynos_drm_encoder_setup(dev); @@ -106,22 +102,26 @@ static int exynos_drm_load(struct drm_device *dev, unsigned long flags) /* Try to bind all sub drivers. */ ret = component_bind_all(dev->dev, dev); if (ret) - goto err_cleanup_vblank; + goto err_mode_config_cleanup; + + ret = drm_vblank_init(dev, dev->mode_config.num_crtc); + if (ret) + goto err_unbind_all; /* Probe non kms sub drivers and virtual display driver. */ ret = exynos_drm_device_subdrv_probe(dev); if (ret) - goto err_unbind_all; + goto err_cleanup_vblank; /* force connectors detection */ drm_helper_hpd_irq_event(dev); return 0; -err_unbind_all: - component_unbind_all(dev->dev, dev); err_cleanup_vblank: drm_vblank_cleanup(dev); +err_unbind_all: + component_unbind_all(dev->dev, dev); err_mode_config_cleanup: drm_mode_config_cleanup(dev); drm_release_iommu_mapping(dev); @@ -138,8 +138,8 @@ static int exynos_drm_unload(struct drm_device *dev) exynos_drm_fbdev_fini(dev); drm_kms_helper_poll_fini(dev); - component_unbind_all(dev->dev, dev); drm_vblank_cleanup(dev); + component_unbind_all(dev->dev, dev); drm_mode_config_cleanup(dev); drm_release_iommu_mapping(dev); -- 1.9.1