On Fri, Feb 24, 2017 at 5:45 PM, liuxinliang <z.liuxinliang@xxxxxxxxxxxxx> wrote: > > > On 2017/2/25 9:39, liuxinliang wrote: >> >> Hi John, >> >> The patch seems good to me, except one minus comment. >> Maybe change fb_lock to fbdev_lock would be better. >> >> Thanks, >> -xinliang >> >> On 2017/2/25 9:25, John Stultz wrote: >>> >>> In some cases I've been seeing a race where two framebuffers >>> would be initialized, as kirin_fbdev_output_poll_changed() >>> might get called quickly in succession, resulting in the >>> initialization happening twice. This could cause the system >>> to boot up with a blank screen. >>> >>> This patch adds a simple mutex to serialize it and seems to >>> avoid the race. >>> >>> Suggestions or feedback would be greatly appreciated! >>> >>> Cc: Xinliang Liu <z.liuxinliang@xxxxxxxxxxxxx> >>> Cc: Rongrong Zou <zourongrong@xxxxxxxxx> >>> Cc: Xinwei Kong <kong.kongxinwei@xxxxxxxxxxxxx> >>> Cc: Chen Feng <puck.chen@xxxxxxxxxxxxx> >>> Cc: David Airlie <airlied@xxxxxxxx> >>> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> >>> Cc: Sean Paul <seanpaul@xxxxxxxxxxxx> >>> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx >>> Signed-off-by: John Stultz <john.stultz@xxxxxxxxxx> >>> --- >>> drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 4 ++++ >>> drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h | 1 + >>> 2 files changed, 5 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c >>> b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c >>> index 7ec93ae..b83556a 100644 >>> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c >>> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c >>> @@ -55,6 +55,7 @@ static void kirin_fbdev_output_poll_changed(struct >>> drm_device *dev) >>> { >>> struct kirin_drm_private *priv = dev->dev_private; >>> + mutex_lock(&priv->fb_lock); >>> if (priv->fbdev) { >>> drm_fbdev_cma_hotplug_event(priv->fbdev); >>> } else { >>> @@ -63,6 +64,7 @@ static void kirin_fbdev_output_poll_changed(struct >>> drm_device *dev) >>> if (IS_ERR(priv->fbdev)) >>> priv->fbdev = NULL; >>> } >>> + mutex_unlock(&priv->fb_lock); >>> } >>> #endif >>> @@ -95,6 +97,8 @@ static int kirin_drm_kms_init(struct drm_device *dev) >>> if (!priv) >>> return -ENOMEM; >>> + mutex_init(&priv->fb_lock); > > And put this line in CONFIG_DRM_FBDEV_EMULATION Ok. Done! Thanks again for the review and feedback! -john _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel